{"thread":{"id":"41873","subject":"[PATCH] diffcore: fix iteration order of identical files during rename detection","startedAt":"2016-03-30T08:35:07Z","lastAt":"2016-03-30T20:51:44Z","messageCount":2,"participants":["SZEDER Gábor","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"282201","messageId":"1459326907-16179-1-git-send-email-szeder@ira.uka.de","threadId":"41873","inReplyTo":null,"subject":"[PATCH] diffcore: fix iteration order of identical files during rename detection","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2016-03-30T08:35:07Z","receivedAt":"2016-03-30T08:35:07Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"If the two paths 'dir/A/file' and 'dir/B/file' have identical content\nand the parent directory is renamed, e.g. 'git mv dir other-dir', then\ndiffcore reports the following exact renames:\n\n    renamed:    dir/B/file -> other-dir/A/file\n    renamed:    dir/A/file -> other-dir/B/file\n\nWhile technically not wrong, this is confusing not only for the user,\nbut also for git commands that make decisions based on rename\ninformation, e.g. 'git log --follow other-dir/A/file' follows\n'dir/B/file' past the rename.\n\nThis behavior is a side effect of commit v2.0.0-rc4~8^2~14\n(diffcore-rename.c: simplify finding exact renames, 2013-11-14): the\nhashmap storing sources returns entries from the same bucket, i.e.\nsources matching the current destination, in LIFO order.  Thus the\niteration first examines 'other-dir/A/file' and 'dir/B/file' and, upon\nfinding identical content and basename, reports an exact rename.\n\nOther hashmap users are apparently happy with the current iteration\norder over the entries of a bucket.  Changing the iteration order\nwould risk upsetting other hashmap users and would increase the memory\nfootprint of each bucket by a pointer to the tail element.\n\nFill the hashmap with source entries in reverse order to restore the\noriginal exact rename detection behavior.\n\nReported-by: Bill Okara <billokara@gmail.com>\nSigned-off-by: SZEDER Gábor <szeder@ira.uka.de>\n---\n\nResend of the patch, with a slightly updated commit message, included\nin\n\n  http://thread.gmane.org/gmane.comp.version-control.git/287281/focus=287570\n\nBeing embedded with scissors in an email without Junio among the\nrecipients on the day the first -rc was tagged...  no wonder it flew\nbelow the radar.\n\n diffcore-rename.c      |  6 ++++--\n t/t4001-diff-rename.sh | 11 +++++++++++\n 2 files changed, 15 insertions(+), 2 deletions(-)\n\ndiff --git a/diffcore-rename.c b/diffcore-rename.c\nindex 3b3c1ed535e7..7f03eb5a0404 100644\n--- a/diffcore-rename.c\n+++ b/diffcore-rename.c\n@@ -340,9 +340,11 @@ static int find_exact_renames(struct diff_options *options)\n \tint i, renames = 0;\n \tstruct hashmap file_table;\n \n-\t/* Add all sources to the hash table */\n+\t/* Add all sources to the hash table in reverse order, because\n+\t * later on they will be retrieved in LIFO order.\n+\t */\n \thashmap_init(&file_table, NULL, rename_src_nr);\n-\tfor (i = 0; i < rename_src_nr; i++)\n+\tfor (i = rename_src_nr-1; i >= 0; i--)\n \t\tinsert_file_table(&file_table, i, rename_src[i].p->one);\n \n \t/* Walk the destinations and find best source match */\ndiff --git a/t/t4001-diff-rename.sh b/t/t4001-diff-rename.sh\nindex 2f327b749588..ed90c6c6f984 100755\n--- a/t/t4001-diff-rename.sh\n+++ b/t/t4001-diff-rename.sh\n@@ -77,6 +77,17 @@ test_expect_success 'favour same basenames even with minor differences' '\n \tgit show HEAD:path1 | sed \"s/15/16/\" > subdir/path1 &&\n \tgit status | test_i18ngrep \"renamed: .*path1 -> subdir/path1\"'\n \n+test_expect_success 'two files with same basename and same content' '\n+\tgit reset --hard &&\n+\tmkdir -p dir/A dir/B &&\n+\tcp path1 dir/A/file &&\n+\tcp path1 dir/B/file &&\n+\tgit add dir &&\n+\tgit commit -m 2 &&\n+\tgit mv dir other-dir &&\n+\tgit status | test_i18ngrep \"renamed: .*dir/A/file -> other-dir/A/file\"\n+'\n+\n test_expect_success 'setup for many rename source candidates' '\n \tgit reset --hard &&\n \tfor i in 0 1 2 3 4 5 6 7 8 9;\n-- \n2.8.0.46.gb821760\n"},{"id":"282268","messageId":"xmqqtwjnsn8f.fsf@gitster.mtv.corp.google.com","threadId":"41873","inReplyTo":"1459326907-16179-1-git-send-email-szeder@ira.uka.de","subject":"Re: [PATCH] diffcore: fix iteration order of identical files during rename detection","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-30T20:51:44Z","receivedAt":"2016-03-30T20:51:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor <szeder@ira.uka.de> writes:\n\n> Fill the hashmap with source entries in reverse order to restore the\n> original exact rename detection behavior.\n\nThanks for digging out and fixing this unintended regression that\nhappened long time ago.  Will queue.\n\n>\n> Reported-by: Bill Okara <billokara@gmail.com>\n> Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>\n> ---\n>\n> Resend of the patch, with a slightly updated commit message, included\n> in\n>\n>   http://thread.gmane.org/gmane.comp.version-control.git/287281/focus=287570\n>\n> Being embedded with scissors in an email without Junio among the\n> recipients on the day the first -rc was tagged...  no wonder it flew\n> below the radar.\n>\n>  diffcore-rename.c      |  6 ++++--\n>  t/t4001-diff-rename.sh | 11 +++++++++++\n>  2 files changed, 15 insertions(+), 2 deletions(-)\n>\n> diff --git a/diffcore-rename.c b/diffcore-rename.c\n> index 3b3c1ed535e7..7f03eb5a0404 100644\n> --- a/diffcore-rename.c\n> +++ b/diffcore-rename.c\n> @@ -340,9 +340,11 @@ static int find_exact_renames(struct diff_options *options)\n>  \tint i, renames = 0;\n>  \tstruct hashmap file_table;\n>  \n> -\t/* Add all sources to the hash table */\n> +\t/* Add all sources to the hash table in reverse order, because\n> +\t * later on they will be retrieved in LIFO order.\n> +\t */\n>  \thashmap_init(&file_table, NULL, rename_src_nr);\n> -\tfor (i = 0; i < rename_src_nr; i++)\n> +\tfor (i = rename_src_nr-1; i >= 0; i--)\n>  \t\tinsert_file_table(&file_table, i, rename_src[i].p->one);\n>  \n>  \t/* Walk the destinations and find best source match */\n> diff --git a/t/t4001-diff-rename.sh b/t/t4001-diff-rename.sh\n> index 2f327b749588..ed90c6c6f984 100755\n> --- a/t/t4001-diff-rename.sh\n> +++ b/t/t4001-diff-rename.sh\n> @@ -77,6 +77,17 @@ test_expect_success 'favour same basenames even with minor differences' '\n>  \tgit show HEAD:path1 | sed \"s/15/16/\" > subdir/path1 &&\n>  \tgit status | test_i18ngrep \"renamed: .*path1 -> subdir/path1\"'\n>  \n> +test_expect_success 'two files with same basename and same content' '\n> +\tgit reset --hard &&\n> +\tmkdir -p dir/A dir/B &&\n> +\tcp path1 dir/A/file &&\n> +\tcp path1 dir/B/file &&\n> +\tgit add dir &&\n> +\tgit commit -m 2 &&\n> +\tgit mv dir other-dir &&\n> +\tgit status | test_i18ngrep \"renamed: .*dir/A/file -> other-dir/A/file\"\n> +'\n> +\n>  test_expect_success 'setup for many rename source candidates' '\n>  \tgit reset --hard &&\n>  \tfor i in 0 1 2 3 4 5 6 7 8 9;\n"}]}