{"thread":{"id":"45791","subject":"[PATCH 1/2] fast-export: deletion action first","startedAt":"2017-04-25T00:12:30Z","lastAt":"2017-05-04T21:45:17Z","messageCount":9,"participants":["Miguel Torroja","Jeff King","Junio C Hamano","miguel torroja"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"317743","messageId":"1493079137-1838-1-git-send-email-miguel.torroja@gmail.com","threadId":"45791","inReplyTo":null,"subject":"[PATCH 1/2] fast-export: deletion action first","fromName":"Miguel Torroja","fromEmail":"miguel.torroja@gmail.com","sentAt":"2017-04-25T00:12:16Z","receivedAt":"2017-04-25T00:12:30Z","isPatch":true,"sender":{"key":"miguel.torroja@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5366212?v=4"},"body":"The delete operations of the fast-export output should precede any addition\nbelonging to the same commit, Addition and deletion with the same name\nentry could happen in case of file to directory and viceversa.\n\nThe fast-export sorting was added in 060df62 (fast-export: Fix output\norder of D/F changes). That change was made in order to fix the case of\ndirectory to file in the same commit, but it broke the reverse case\n(File to directory).\n\nSigned-off-by: Miguel Torroja \n---\n builtin/fast-export.c | 25 +++++++++----------------\n 1 file changed, 9 insertions(+), 16 deletions(-)\n\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex e022063..a3ab7da 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -260,26 +260,19 @@ static void export_blob(const struct object_id *oid)\n \t\tfree(buf);\n }\n \n-static int depth_first(const void *a_, const void *b_)\n+/*\n+ * Compares two diff types to order based on output priorities.\n+ */\n+static int diff_type_cmp(const void *a_, const void *b_)\n {\n \tconst struct diff_filepair *a = *((const struct diff_filepair **)a_);\n \tconst struct diff_filepair *b = *((const struct diff_filepair **)b_);\n-\tconst char *name_a, *name_b;\n-\tint len_a, len_b, len;\n \tint cmp;\n \n-\tname_a = a->one ? a->one->path : a->two->path;\n-\tname_b = b->one ? b->one->path : b->two->path;\n-\n-\tlen_a = strlen(name_a);\n-\tlen_b = strlen(name_b);\n-\tlen = (len_a < len_b) ? len_a : len_b;\n-\n-\t/* strcmp will sort 'd' before 'd/e', we want 'd/e' before 'd' */\n-\tcmp = memcmp(name_a, name_b, len);\n-\tif (cmp)\n-\t\treturn cmp;\n-\tcmp = len_b - len_a;\n+\t/*\n+\t * Move Delete entries first so that an addition is always reported after\n+\t */\n+\tcmp = (b->status == DIFF_STATUS_DELETED) - (a->status == DIFF_STATUS_DELETED);\n \tif (cmp)\n \t\treturn cmp;\n \t/*\n@@ -347,7 +340,7 @@ static void show_filemodify(struct diff_queue_struct *q,\n \t * Handle files below a directory first, in case they are all deleted\n \t * and the directory changes to a file or symlink.\n \t */\n-\tQSORT(q->queue, q->nr, depth_first);\n+\tQSORT(q->queue, q->nr, diff_type_cmp);\n \n \tfor (i = 0; i < q->nr; i++) {\n \t\tstruct diff_filespec *ospec = q->queue[i]->one;\n-- \n2.1.4\n\n"},{"id":"317744","messageId":"1493079137-1838-2-git-send-email-miguel.torroja@gmail.com","threadId":"45791","inReplyTo":"1493079137-1838-1-git-send-email-miguel.torroja@gmail.com","subject":"[PATCH 2/2] fast-export: DIFF_STATUS_RENAMED instead of 'R'","fromName":"Miguel Torroja","fromEmail":"miguel.torroja@gmail.com","sentAt":"2017-04-25T00:12:17Z","receivedAt":"2017-04-25T00:12:32Z","isPatch":true,"sender":{"key":"miguel.torroja@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5366212?v=4"},"body":"Minor change to be consistent with the rest of the fast-export code.\nDIFF_STATUS_RENAMED is defined as 'R'.\n\nSigned-off-by: Miguel Torroja \n---\n builtin/fast-export.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex a3ab7da..4d39324 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -280,7 +280,7 @@ static int diff_type_cmp(const void *a_, const void *b_)\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+\treturn (a->status == DIFF_STATUS_RENAMED) - (b->status == DIFF_STATUS_RENAMED);\n }\n \n static void print_path_1(const char *path)\n-- \n2.1.4\n\n"},{"id":"317764","messageId":"20170425032927.74btvfcexbdq4rmz@sigill.intra.peff.net","threadId":"45791","inReplyTo":"1493079137-1838-1-git-send-email-miguel.torroja@gmail.com","subject":"Re: [PATCH 1/2] fast-export: deletion action first","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-04-25T03:29:27Z","receivedAt":"2017-04-25T03:29:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Apr 25, 2017 at 02:12:16AM +0200, Miguel Torroja wrote:\n\n> The delete operations of the fast-export output should precede any addition\n> belonging to the same commit, Addition and deletion with the same name\n> entry could happen in case of file to directory and viceversa.\n> \n> The fast-export sorting was added in 060df62 (fast-export: Fix output\n> order of D/F changes). That change was made in order to fix the case of\n> directory to file in the same commit, but it broke the reverse case\n> (File to directory).\n\nThat explanation makes sense.\n\n> builtin/fast-export.c | 25 +++++++++----------------\n> 1 file changed, 9 insertions(+), 16 deletions(-)\n\nPerhaps we would want a test for the case you are fixing (to be sure it\nis not re-broken), as well as confirming that we have not re-broken the\noriginal case (it looks like 060df62 added a test, so we may be OK with\nthat).\n\n> +/*\n> + * Compares two diff types to order based on output priorities.\n> + */\n> +static int diff_type_cmp(const void *a_, const void *b_)\n> [...]\n> +\t/*\n> +\t * Move Delete entries first so that an addition is always reported after\n> +\t */\n> +\tcmp = (b->status == DIFF_STATUS_DELETED) - (a->status == DIFF_STATUS_DELETED);\n> \tif (cmp)\n> \t\treturn cmp;\n> \t/*\n\nSo we sort deletions first. And the bit that the context doesn't quite\nshow here is that we then compare renames and push them to the end.\nEverything else will compare equal.\n\nIs qsort() guaranteed to be stable? If not, then we'll get the majority\nof entries in a non-deterministic order. Should we fallback to strcmp()\nso that within a given class, the entries are sorted by name?\n\n-Peff\n"},{"id":"317769","messageId":"xmqqfugxw1us.fsf@gitster.mtv.corp.google.com","threadId":"45791","inReplyTo":"20170425032927.74btvfcexbdq4rmz@sigill.intra.peff.net","subject":"Re: [PATCH 1/2] fast-export: deletion action first","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-04-25T04:24:59Z","receivedAt":"2017-04-25T04:25:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Perhaps we would want a test for the case you are fixing (to be sure it\n> is not re-broken), as well as confirming that we have not re-broken the\n> original case (it looks like 060df62 added a test, so we may be OK with\n> that).\n\nGood suggestion.\n\n>\n>> +/*\n>> + * Compares two diff types to order based on output priorities.\n>> + */\n>> +static int diff_type_cmp(const void *a_, const void *b_)\n>> [...]\n>> +\t/*\n>> +\t * Move Delete entries first so that an addition is always reported after\n>> +\t */\n>> +\tcmp = (b->status == DIFF_STATUS_DELETED) - (a->status == DIFF_STATUS_DELETED);\n>> \tif (cmp)\n>> \t\treturn cmp;\n>> \t/*\n>\n> So we sort deletions first. And the bit that the context doesn't quite\n> show here is that we then compare renames and push them to the end.\n> Everything else will compare equal.\n\nWait--we also allow renames? Rename is like delete in the context\nof discussing d/f conflicts, in that it tells us that the source\npath will be missing in the end result. If you rename a file \"d\" to\n\"e\", then there is a room for you to create a directory \"d\" to store\na file \"d/f\" in. Shouldn't it participate also in this \"delete\nbefore add to avoid d/f conflict\" logic?\n\n> Is qsort() guaranteed to be stable? If not, then we'll get the majority\n> of entries in a non-deterministic order. Should we fallback to strcmp()\n> so that within a given class, the entries are sorted by name?\n\nAgain, very good point, especially with the existing comment in the\ncomparison function that explains why renames are shown last.\n\n"},{"id":"317773","messageId":"20170425044641.sx5uoql4oiug6iq7@sigill.intra.peff.net","threadId":"45791","inReplyTo":"xmqqfugxw1us.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 1/2] fast-export: deletion action first","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-04-25T04:46:41Z","receivedAt":"2017-04-25T04:46:48Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Apr 24, 2017 at 09:24:59PM -0700, Junio C Hamano wrote:\n\n> > So we sort deletions first. And the bit that the context doesn't quite\n> > show here is that we then compare renames and push them to the end.\n> > Everything else will compare equal.\n> \n> Wait--we also allow renames? Rename is like delete in the context\n> of discussing d/f conflicts, in that it tells us that the source\n> path will be missing in the end result. If you rename a file \"d\" to\n> \"e\", then there is a room for you to create a directory \"d\" to store\n> a file \"d/f\" in. Shouldn't it participate also in this \"delete\n> before add to avoid d/f conflict\" logic?\n\nHrm. Yeah, I agree that case is problematic. But putting the renames\nearly creates the opposite problem. If you delete \"d/f\" to make way for\na rename to a file \"d\", then that deletion has to come first.\n\nSo naively you might think that pure deletions come first, then renames.\nBut I think you could have dependencies within the renames. For\ninstance:\n\n git init\n mkdir a b c\n seq 1 1000 >a/f\n seq 1001 2000 >b/f\n seq 2001 3000 >c/f\n git add .\n git commit -m base\n\n git mv a tmp\n git mv b/f a; rmdir b\n git mv c/f b; rmdir c\n git mv tmp/f c; rmdir tmp\n\nThere's no correct order there; it's a cycle.\n\nSo I suspect that any reader that accepts renames needs to be able to\nhandle the inputs in any order (I'd also suspect that many\nimplementations _don't_, but get by because people don't do silly things\nlike this in practice).\n\nAnyway. I don't think Miguel's patch needs to solve all of the lingering\nrename cases. But I am curious whether it makes some rename cases worse,\nbecause the depth-sorting was kicking in before and making them work.\n\n-Peff\n"},{"id":"317778","messageId":"xmqqy3upuk4o.fsf@gitster.mtv.corp.google.com","threadId":"45791","inReplyTo":"20170425044641.sx5uoql4oiug6iq7@sigill.intra.peff.net","subject":"Re: [PATCH 1/2] fast-export: deletion action first","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-04-25T05:33:11Z","receivedAt":"2017-04-25T05:33:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Anyway. I don't think Miguel's patch needs to solve all of the lingering\n> rename cases. But I am curious whether it makes some rename cases worse,\n> because the depth-sorting was kicking in before and making them work.\n\nI agree with you on both counts, and I care more about the second\nsentence, not just \"am curious\", but \"am worried\". I am not sure\nthat this patch is safe---it looked more like robbing peter to pay\npaul or the other way around. Fixing for one class of breakage\nwithout regressing is one thing and it is perfectly fine to leave\nsome already broken case broken with such a fix. Claiming to fix\none class and breaking other class that was happily working is quite\ndifferent, and that is where my \"Wait, we also allow renames?\" comes\nfrom.\n\n"},{"id":"317780","messageId":"20170425055817.codq2q3fd54uebfx@sigill.intra.peff.net","threadId":"45791","inReplyTo":"xmqqy3upuk4o.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 1/2] fast-export: deletion action first","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-04-25T05:58:17Z","receivedAt":"2017-04-25T05:58:26Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Apr 24, 2017 at 10:33:11PM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > Anyway. I don't think Miguel's patch needs to solve all of the lingering\n> > rename cases. But I am curious whether it makes some rename cases worse,\n> > because the depth-sorting was kicking in before and making them work.\n> \n> I agree with you on both counts, and I care more about the second\n> sentence, not just \"am curious\", but \"am worried\". I am not sure\n> that this patch is safe---it looked more like robbing peter to pay\n> paul or the other way around. Fixing for one class of breakage\n> without regressing is one thing and it is perfectly fine to leave\n> some already broken case broken with such a fix. Claiming to fix\n> one class and breaking other class that was happily working is quite\n> different, and that is where my \"Wait, we also allow renames?\" comes\n> from.\n\nYeah, I don't disagree. I am just curious first, then worried second. :)\n\nIf I had to choose, though, I'd rather see the order be reliable for the\nno-renames case. IOW, if we must rob one peter, I'd rather it be the\nrenames, which already have tons of corner cases (and which I do not\nthink can be plugged for a reader which depends on the order of the\nentries; the dependencies can be cycles).\n\nOf course if we can make it work correctly in all of the non-cyclical\ncases, all the better.\n\n-Peff\n"},{"id":"318800","messageId":"1493933779-25611-1-git-send-email-miguel.torroja@gmail.com","threadId":"45791","inReplyTo":"20170425055817.codq2q3fd54uebfx@sigill.intra.peff.net","subject":"[PATCH] fast-export: deletion action first","fromName":"Miguel Torroja","fromEmail":"miguel.torroja@gmail.com","sentAt":"2017-05-04T21:36:19Z","receivedAt":"2017-05-04T21:36:31Z","isPatch":true,"sender":{"key":"miguel.torroja@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5366212?v=4"},"body":"The delete operations of the fast-export output should precede any addition\nbelonging to the same commit, Addition and deletion with the same name\nentry could happen in case of file to directory and viceversa.\n\nAs an equal comparison doesn't have any deterministic final order,\nit's better to keep original diff order input when there is no prefer order\n( that's done comparing pointers)\n\nThe fast-export sorting was added in 060df62 (fast-export: Fix output\norder of D/F changes). That change was made in order to fix the case of\ndirectory to file in the same commit, but it broke the reverse case\n(File to directory).\n\nThe test \"file becomes directory\" has been added in order to exercise\nthe original motivation of the deletion reorder.\n\nSigned-off-by: Miguel Torroja \n---\n builtin/fast-export.c | 32 +++++++++++++++-----------------\n t/t9350-fast-export.sh | 25 +++++++++++++++++++++++++\n 2 files changed, 40 insertions(+), 17 deletions(-)\n\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex e022063..e82f654 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -260,26 +260,19 @@ static void export_blob(const struct object_id *oid)\n \t\tfree(buf);\n }\n \n-static int depth_first(const void *a_, const void *b_)\n+/*\n+ * Compares two diff types to order based on output priorities.\n+ */\n+static int diff_type_cmp(const void *a_, const void *b_)\n {\n \tconst struct diff_filepair *a = *((const struct diff_filepair **)a_);\n \tconst struct diff_filepair *b = *((const struct diff_filepair **)b_);\n-\tconst char *name_a, *name_b;\n-\tint len_a, len_b, len;\n \tint cmp;\n \n-\tname_a = a->one ? a->one->path : a->two->path;\n-\tname_b = b->one ? b->one->path : b->two->path;\n-\n-\tlen_a = strlen(name_a);\n-\tlen_b = strlen(name_b);\n-\tlen = (len_a < len_b) ? len_a : len_b;\n-\n-\t/* strcmp will sort 'd' before 'd/e', we want 'd/e' before 'd' */\n-\tcmp = memcmp(name_a, name_b, len);\n-\tif (cmp)\n-\t\treturn cmp;\n-\tcmp = len_b - len_a;\n+\t/*\n+\t * Move Delete entries first so that an addition is always reported after\n+\t */\n+\tcmp = (b->status == DIFF_STATUS_DELETED) - (a->status == DIFF_STATUS_DELETED);\n \tif (cmp)\n \t\treturn cmp;\n \t/*\n@@ -287,7 +280,12 @@ static int depth_first(const void *a_, const void *b_)\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+\tcmp = (a->status == DIFF_STATUS_RENAMED) - (b->status == DIFF_STATUS_RENAMED);\n+\tif (cmp)\n+\t\treturn cmp;\n+\n+\t/* For the remaining cases we keep the original ordering comparing the pointers */\n+\treturn (a-b);\n }\n \n static void print_path_1(const char *path)\n@@ -347,7 +345,7 @@ static void show_filemodify(struct diff_queue_struct *q,\n \t * Handle files below a directory first, in case they are all deleted\n \t * and the directory changes to a file or symlink.\n \t */\n-\tQSORT(q->queue, q->nr, depth_first);\n+\tQSORT(q->queue, q->nr, diff_type_cmp);\n \n \tfor (i = 0; i < q->nr; i++) {\n \t\tstruct diff_filespec *ospec = q->queue[i]->one;\ndiff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh\nindex b5149fd..d4f369a 100755\n--- a/t/t9350-fast-export.sh\n+++ b/t/t9350-fast-export.sh\n@@ -419,6 +419,31 @@ test_expect_success 'directory becomes symlink' '\n \t(cd result && git show master:foo)\n '\n \n+test_expect_success 'file becomes directory' '\n+\tgit init filetodir_orig &&\n+\tgit init --bare filetodir_replica.git &&\n+\t(\n+\t\tcd filetodir_orig &&\n+\t\techo foo > filethendir &&\n+\t\tgit add filethendir &&\n+\t\ttest_tick &&\n+\t\tgit commit -mfile &&\n+\t\tgit rm filethendir &&\n+\t\tmkdir filethendir &&\n+\t\techo bar > filethendir/a &&\n+\t\tgit add filethendir/a &&\n+\t\ttest_tick &&\n+\t\tgit commit -mdir\n+\t) &&\n+\tgit --git-dir=filetodir_orig/.git fast-export master |\n+\t\tgit --git-dir=filetodir_replica.git/ fast-import &&\n+\t(\n+\t\tORIG=$(git --git-dir=filetodir_orig/.git rev-parse --verify master) &&\n+\t\tREPLICA=$(git --git-dir=filetodir_replica.git rev-parse --verify master) &&\n+\t\ttest $ORIG = $REPLICA\n+\t)\n+'\n+\n test_expect_success 'fast-export quotes pathnames' '\n \tgit init crazy-paths &&\n \t(cd crazy-paths &&\n-- \n2.1.4\n\n"},{"id":"318803","messageId":"CAKYtbVZTnNG8Y-kZKQun9aYdwq9bR2_FX9L3gm+25qj6i7XyAQ@mail.gmail.com","threadId":"45791","inReplyTo":"1493933779-25611-1-git-send-email-miguel.torroja@gmail.com","subject":"Re: [PATCH] fast-export: deletion action first","fromName":"miguel torroja","fromEmail":"miguel.torroja@gmail.com","sentAt":"2017-05-04T21:45:11Z","receivedAt":"2017-05-04T21:45:17Z","isPatch":true,"sender":{"key":"miguel.torroja@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5366212?v=4"},"body":"The previous patch reorders the delete operations in fast-export\n(preceding any other one), keeps renaming as last operations to\nprocess (as original source code) and for any other operation it keeps\nthe same order as \"diff\"\n\nThe non deterministic reordering was one of the concerns when I first\nsent the patch.\nThe behavior for the other corner cases pointed out by Jeff\n(delete/rename dir/file ) are not tackled in this patch and the final\nresult is unknown.\n\n\nOn Thu, May 4, 2017 at 9:36 PM, Miguel Torroja wrote:\n>\n> The delete operations of the fast-export output should precede any addition\n> belonging to the same commit, Addition and deletion with the same name\n> entry could happen in case of file to directory and viceversa.\n>\n> As an equal comparison doesn't have any deterministic final order,\n> it's better to keep original diff order input when there is no prefer order\n> ( that's done comparing pointers)\n>\n> The fast-export sorting was added in 060df62 (fast-export: Fix output\n> order of D/F changes). That change was made in order to fix the case of\n> directory to file in the same commit, but it broke the reverse case\n> (File to directory).\n>\n> The test \"file becomes directory\" has been added in order to exercise\n> the original motivation of the deletion reorder.\n>\n> Signed-off-by: Miguel Torroja \n> ---\n> builtin/fast-export.c | 32 +++++++++++++++-----------------\n> t/t9350-fast-export.sh | 25 +++++++++++++++++++++++++\n> 2 files changed, 40 insertions(+), 17 deletions(-)\n>\n> diff --git a/builtin/fast-export.c b/builtin/fast-export.c\n> index e022063..e82f654 100644\n> --- a/builtin/fast-export.c\n> +++ b/builtin/fast-export.c\n> @@ -260,26 +260,19 @@ static void export_blob(const struct object_id *oid)\n> free(buf);\n> }\n>\n> -static int depth_first(const void *a_, const void *b_)\n> +/*\n> + * Compares two diff types to order based on output priorities.\n> + */\n> +static int diff_type_cmp(const void *a_, const void *b_)\n> {\n> const struct diff_filepair *a = *((const struct diff_filepair **)a_);\n> const struct diff_filepair *b = *((const struct diff_filepair **)b_);\n> - const char *name_a, *name_b;\n> - int len_a, len_b, len;\n> int cmp;\n>\n> - name_a = a->one ? a->one->path : a->two->path;\n> - name_b = b->one ? b->one->path : b->two->path;\n> -\n> - len_a = strlen(name_a);\n> - len_b = strlen(name_b);\n> - len = (len_a < len_b) ? len_a : len_b;\n> -\n> - /* strcmp will sort 'd' before 'd/e', we want 'd/e' before 'd' */\n> - cmp = memcmp(name_a, name_b, len);\n> - if (cmp)\n> - return cmp;\n> - cmp = len_b - len_a;\n> + /*\n> + * Move Delete entries first so that an addition is always reported after\n> + */\n> + cmp = (b->status == DIFF_STATUS_DELETED) - (a->status == DIFF_STATUS_DELETED);\n> if (cmp)\n> return cmp;\n> /*\n> @@ -287,7 +280,12 @@ static int depth_first(const void *a_, const void *b_)\n> * appear in the output before it is renamed (e.g., when a file\n> * was copied and renamed in the same commit).\n> */\n> - return (a->status == 'R') - (b->status == 'R');\n> + cmp = (a->status == DIFF_STATUS_RENAMED) - (b->status == DIFF_STATUS_RENAMED);\n> + if (cmp)\n> + return cmp;\n> +\n> + /* For the remaining cases we keep the original ordering comparing the pointers */\n> + return (a-b);\n> }\n>\n> static void print_path_1(const char *path)\n> @@ -347,7 +345,7 @@ static void show_filemodify(struct diff_queue_struct *q,\n> * Handle files below a directory first, in case they are all deleted\n> * and the directory changes to a file or symlink.\n> */\n> - QSORT(q->queue, q->nr, depth_first);\n> + QSORT(q->queue, q->nr, diff_type_cmp);\n>\n> for (i = 0; i < q->nr; i++) {\n> struct diff_filespec *ospec = q->queue[i]->one;\n> diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh\n> index b5149fd..d4f369a 100755\n> --- a/t/t9350-fast-export.sh\n> +++ b/t/t9350-fast-export.sh\n> @@ -419,6 +419,31 @@ test_expect_success 'directory becomes symlink' '\n> (cd result && git show master:foo)\n> '\n>\n> +test_expect_success 'file becomes directory' '\n> + git init filetodir_orig &&\n> + git init --bare filetodir_replica.git &&\n> + (\n> + cd filetodir_orig &&\n> + echo foo > filethendir &&\n> + git add filethendir &&\n> + test_tick &&\n> + git commit -mfile &&\n> + git rm filethendir &&\n> + mkdir filethendir &&\n> + echo bar > filethendir/a &&\n> + git add filethendir/a &&\n> + test_tick &&\n> + git commit -mdir\n> + ) &&\n> + git --git-dir=filetodir_orig/.git fast-export master |\n> + git --git-dir=filetodir_replica.git/ fast-import &&\n> + (\n> + ORIG=$(git --git-dir=filetodir_orig/.git rev-parse --verify master) &&\n> + REPLICA=$(git --git-dir=filetodir_replica.git rev-parse --verify master) &&\n> + test $ORIG = $REPLICA\n> + )\n> +'\n> +\n> test_expect_success 'fast-export quotes pathnames' '\n> git init crazy-paths &&\n> (cd crazy-paths &&\n> --\n> 2.1.4\n>\n"}]}