{"thread":{"id":"25026","subject":"[PATCH] fast-export: ensure that a renamed file is printed after all references","startedAt":"2010-09-07T19:33:02Z","lastAt":"2010-09-08T19:26:14Z","messageCount":4,"participants":["Johannes Sixt","Elijah Newren"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"150242","messageId":"201009072133.02930.j6t@kdbg.org","threadId":"25026","inReplyTo":null,"subject":"[PATCH] fast-export: ensure that a renamed file is printed after all references","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2010-09-07T19:33:02Z","receivedAt":"2010-09-07T19:33:02Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"t9350 sets up a commit where a file is both copied and renamed. The output\nof fast-export for this commit should look like this:\n\n author ...\n committer ...\n from :19\n C \"file2\" \"file4\"\n R \"file2\" \"file5\"\n\nThe order of the two modification lines is derived from the result that\nthe diff machinery produces.\n\n060df62 (fast-export: Fix output order of D/F changes) inserted a qsort\ncall that modifies the order of the diff result. Unfortunately, qsort need\nnot be stable. Therefore, it is possible that the 'R' line appears before\nthe 'C' line and the resulting fast-import stream is incorrect.\n\nFix it by forcing that the rename entry is printed after all other\nmodification lines with the same file name.\n\nSigned-off-by: Johannes Sixt \n---\n builtin/fast-export.c | 10 +++++++++-\n 1 files changed, 9 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex 965e90e..007bba6 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -166,7 +166,15 @@ static int depth_first(const void *a_, const void *b_)\n \tcmp = memcmp(name_a, name_b, len);\n \tif (cmp)\n \t\treturn cmp;\n-\treturn (len_b - len_a);\n+\tcmp = len_b - len_a;\n+\tif (cmp)\n+\t\treturn cmp;\n+\t/*\n+\t * Move 'R'ename entries last so that all references of the file\n+\t * appear in the output before it is renamed (e.g., when a file\n+\t * was copied and renamed in the same commit).\n+\t */\n+\treturn (a->status == 'R') - (b->status == 'R');\n }\n \n static void show_filemodify(struct diff_queue_struct *q,\n-- \n1.7.1.402.gf1eeb\n"},{"id":"150277","messageId":"AANLkTim4iuDOG9U1epcafWarckaa6ZnxCOpfJW8KJW_m@mail.gmail.com","threadId":"25026","inReplyTo":"201009072133.02930.j6t@kdbg.org","subject":"Re: [PATCH] fast-export: ensure that a renamed file is printed after all references","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2010-09-08T05:46:34Z","receivedAt":"2010-09-08T05:46:34Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi,\n\nOn Tue, Sep 7, 2010 at 1:33 PM, Johannes Sixt wrote:\n> t9350 sets up a commit where a file is both copied and renamed. The output\n> of fast-export for this commit should look like this:\n>\n>  author ...\n>  committer ...\n>  from :19\n>  C \"file2\" \"file4\"\n>  R \"file2\" \"file5\"\n>\n> The order of the two modification lines is derived from the result that\n> the diff machinery produces.\n>\n> 060df62 (fast-export: Fix output order of D/F changes) inserted a qsort\n> call that modifies the order of the diff result. Unfortunately, qsort need\n> not be stable. Therefore, it is possible that the 'R' line appears before\n> the 'C' line and the resulting fast-import stream is incorrect.\n>\n> Fix it by forcing that the rename entry is printed after all other\n> modification lines with the same file name.\n\nPatch and description looks good to me...however, I was a little\nsurprised to see no testsuite addition or additional explanation. Are\nyou seeing some t9350 tests fail on some platform despite passing on\nlinux?\n\n\nElijah\n"},{"id":"150319","messageId":"201009082047.50083.j6t@kdbg.org","threadId":"25026","inReplyTo":"AANLkTim4iuDOG9U1epcafWarckaa6ZnxCOpfJW8KJW_m@mail.gmail.com","subject":"Re: [PATCH] fast-export: ensure that a renamed file is printed after all references","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2010-09-08T18:47:49Z","receivedAt":"2010-09-08T18:47:49Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"On Mittwoch, 8. September 2010, Elijah Newren wrote:\n> Patch and description looks good to me...however, I was a little\n> surprised to see no testsuite addition or additional explanation. Are\n> you seeing some t9350 tests fail on some platform despite passing on\n> linux?\n\nYes, I see the problem in t9350 on Windows. It is impossible to write a test \nfor it because the test would depend on the unspecified behavior of qsort.\n\nBTW, I don't know whether the diff machinery guarantees to produce 'C' and 'R' \nrecords in this order. If there is no such guarantee, then fast-export would \nhave been broken even before 060df62.\n\n-- Hannes\n"},{"id":"150320","messageId":"AANLkTimS96c1YFC+UUw0iRoPVT=COFhDRoSO1cNHYhTf@mail.gmail.com","threadId":"25026","inReplyTo":"201009082047.50083.j6t@kdbg.org","subject":"Re: [PATCH] fast-export: ensure that a renamed file is printed after all references","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2010-09-08T19:26:14Z","receivedAt":"2010-09-08T19:26:14Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Wed, Sep 8, 2010 at 12:47 PM, Johannes Sixt wrote:\n> On Mittwoch, 8. September 2010, Elijah Newren wrote:\n>> Patch and description looks good to me...however, I was a little\n>> surprised to see no testsuite addition or additional explanation.  Are\n>> you seeing some t9350 tests fail on some platform despite passing on\n>> linux?\n>\n> Yes, I see the problem in t9350 on Windows. It is impossible to write a test\n> for it because the test would depend on the unspecified behavior of qsort.\n\nMakes sense. Would it be worth briefly mentioning Windows in the\ncommit message?\n\nEither way,\nAcked-by: Elijah Newren \n\n> BTW, I don't know whether the diff machinery guarantees to produce 'C' and 'R'\n> records in this order. If there is no such guarantee, then fast-export would\n> have been broken even before 060df62.\n"}]}