{"thread":{"id":"8943","subject":"rerere fails to record resolution if file doesn't exist in merge base","startedAt":"2007-07-09T07:07:25Z","lastAt":"2007-07-09T15:32:09Z","messageCount":6,"participants":["Uwe Kleine-König","Junio C Hamano","Johannes Schindelin"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"46837","messageId":"20070709070725.GA4445@lala","threadId":"8943","inReplyTo":null,"subject":"rerere fails to record resolution if file doesn't exist in merge base","fromName":"Uwe Kleine-König","fromEmail":"ukleinek@informatik.uni-freiburg.de","sentAt":"2007-07-09T07:07:25Z","receivedAt":"2007-07-09T07:07:25Z","isPatch":false,"sender":{"key":"u.kleine-koenig@pengutronix.de","avatar":"https://gravatar.com/avatar/354b5e3ceb2806a2f1e1e382ac29ddbdad18288654da62b61eb13583a857eee7?d=mp&s=160"},"body":"Hello,\n\nSome time ago, I sent a test for that[1], but the patch doesn't apply\nanymore.\n\nAnyhow, the failure still exists, even though the original report was\nsent when rerere was still a perl script ...\n\n\tzeisberg@cassiopeia:/tmp/rerere$ git init\n\tInitialized empty Git repository in .git/\n\tzeisberg@cassiopeia:/tmp/rerere$ mkdir .git/rerere\n\tzeisberg@cassiopeia:/tmp/rerere$ echo just something to commit > file\n\tzeisberg@cassiopeia:/tmp/rerere$ git add file\n\tzeisberg@cassiopeia:/tmp/rerere$ git commit -m 1\n\tCreated initial commit 51384cb: 1\n\t 1 files changed, 1 insertions(+), 0 deletions(-)\n\t create mode 100644 file\n\tzeisberg@cassiopeia:/tmp/rerere$ git branch b\n\tzeisberg@cassiopeia:/tmp/rerere$ echo lala > lolo\n\tzeisberg@cassiopeia:/tmp/rerere$ git add lolo\n\tzeisberg@cassiopeia:/tmp/rerere$ git commit -m 2\n\tCreated commit 98b91cc: 2\n\t 1 files changed, 1 insertions(+), 0 deletions(-)\n\t create mode 100644 lolo\n\tzeisberg@cassiopeia:/tmp/rerere$ git checkout b\n\tSwitched to branch \"b\"\n\tzeisberg@cassiopeia:/tmp/rerere$ echo lali > lolo\n\tzeisberg@cassiopeia:/tmp/rerere$ git add lolo\n\tzeisberg@cassiopeia:/tmp/rerere$ git commit -m 2a\n\tCreated commit bb900f3: 2a\n\t 1 files changed, 1 insertions(+), 0 deletions(-)\n\t create mode 100644 lolo\n\tzeisberg@cassiopeia:/tmp/rerere$ git pull . master\n\tAuto-merged lolo\n\tCONFLICT (add/add): Merge conflict in lolo\n\tAutomatic merge failed; fix conflicts and then commit the result.\n\tzeisberg@cassiopeia:/tmp/rerere$ perl -n -i -e \"print if /^l/\" lolo\n\tzeisberg@cassiopeia:/tmp/rerere$ cat lolo\n\tlali\n\tlala\n\tzeisberg@cassiopeia:/tmp/rerere$ git rerere\n\tzeisberg@cassiopeia:/tmp/rerere$ \n\nIn my eyes the last command should have recorded the resolution for\nlolo, shouldn't it?\n\nBest regards\nUwe\n\n[1] http://article.gmane.org/gmane.comp.version-control.git/19267\n\n-- \nUwe Kleine-König\n\nhttp://www.google.com/search?q=1+stone%3D\n"},{"id":"46843","messageId":"7vps32ugu2.fsf@assigned-by-dhcp.cox.net","threadId":"8943","inReplyTo":"20070709070725.GA4445@lala","subject":"Re: rerere fails to record resolution if file doesn't exist in merge base","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-09T08:22:29Z","receivedAt":"2007-07-09T08:22:29Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Uwe Kleine-König  <ukleinek@informatik.uni-freiburg.de> writes:\n\n> Anyhow, the failure still exists, even though the original report was\n> sent when rerere was still a perl script ...\n>\n> \tzeisberg@cassiopeia:/tmp/rerere$ git init\n> \tInitialized empty Git repository in .git/\n> \tzeisberg@cassiopeia:/tmp/rerere$ mkdir .git/rerere\n\nThis should be creating .git/rr-cache, but what you said in your\nmessage is correct regardless.\n\n> In my eyes the last command should have recorded the resolution for\n> lolo, shouldn't it?\n\nI think two-file merge (ancestor did not have it, and you and he\nadded the path differently) is rare enough that it was dropped\noutside of the radar.  A fix would probably be a trivial change\nto builtin-rerere.c::find_conflict(), I think.  While it would\nstill be sane to insist that we do not do rerere for symlinks,\nand require to have stages #2 and #3, we should be able to drop\nthe requirement to have stage #1.  rerere does not use\ninformation from there anyway.\n\nNot even compile tested, but something like this should do.\n\n builtin-rerere.c |   17 +++++++----------\n 1 files changed, 7 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin-rerere.c b/builtin-rerere.c\nindex c25b3d5..6ffc43d 100644\n--- a/builtin-rerere.c\n+++ b/builtin-rerere.c\n@@ -168,19 +168,16 @@ static int find_conflict(struct path_list *conflict)\n \tint i;\n \tif (read_cache() < 0)\n \t\treturn error(\"Could not read index\");\n-\tfor (i = 0; i + 2 < active_nr; i++) {\n-\t\tstruct cache_entry *e1 = active_cache[i];\n-\t\tstruct cache_entry *e2 = active_cache[i+1];\n-\t\tstruct cache_entry *e3 = active_cache[i+2];\n-\t\tif (ce_stage(e1) == 1 &&\n-\t\t    ce_stage(e2) == 2 &&\n+\tfor (i = 0; i+1 < active_nr; i++) {\n+\t\tstruct cache_entry *e2 = active_cache[i];\n+\t\tstruct cache_entry *e3 = active_cache[i+1];\n+\t\tif (ce_stage(e2) == 2 &&\n \t\t    ce_stage(e3) == 3 &&\n-\t\t    ce_same_name(e1, e2) && ce_same_name(e1, e3) &&\n-\t\t    S_ISREG(ntohl(e1->ce_mode)) &&\n+\t\t    ce_same_name(e2, e3) &&\n \t\t    S_ISREG(ntohl(e2->ce_mode)) &&\n \t\t    S_ISREG(ntohl(e3->ce_mode))) {\n-\t\t\tpath_list_insert((const char *)e1->name, conflict);\n-\t\t\ti += 2;\n+\t\t\tpath_list_insert((const char *)e2->name, conflict);\n+\t\t\ti++; /* skip over both #2 and #3 */\n \t\t}\n \t}\n \treturn 0;\n"},{"id":"46853","messageId":"20070709113543.GA12633@informatik.uni-freiburg.de","threadId":"8943","inReplyTo":"7vps32ugu2.fsf@assigned-by-dhcp.cox.net","subject":"Re: rerere fails to record resolution if file doesn't exist in merge base","fromName":"Uwe Kleine-König","fromEmail":"ukleinek@informatik.uni-freiburg.de","sentAt":"2007-07-09T11:35:43Z","receivedAt":"2007-07-09T11:35:43Z","isPatch":false,"sender":{"key":"u.kleine-koenig@pengutronix.de","avatar":"https://gravatar.com/avatar/354b5e3ceb2806a2f1e1e382ac29ddbdad18288654da62b61eb13583a857eee7?d=mp&s=160"},"body":"Hello Junio,\n\nJunio C Hamano wrote:\n> Uwe Kleine-König  <ukleinek@informatik.uni-freiburg.de> writes:\n> \n> > Anyhow, the failure still exists, even though the original report was\n> > sent when rerere was still a perl script ...\n> >\n> > \tzeisberg@cassiopeia:/tmp/rerere$ git init\n> > \tInitialized empty Git repository in .git/\n> > \tzeisberg@cassiopeia:/tmp/rerere$ mkdir .git/rerere\n> \n> This should be creating .git/rr-cache, but what you said in your\n> message is correct regardless.\nYou're right.  copy-n-paste error.\n \n> > In my eyes the last command should have recorded the resolution for\n> > lolo, shouldn't it?\n> \n> I think two-file merge (ancestor did not have it, and you and he\n> added the path differently) is rare enough that it was dropped\n> outside of the radar.  A fix would probably be a trivial change\n> to builtin-rerere.c::find_conflict(), I think.  While it would\n> still be sane to insist that we do not do rerere for symlinks,\n> and require to have stages #2 and #3, we should be able to drop\n> the requirement to have stage #1.  rerere does not use\n> information from there anyway.\n> \n> Not even compile tested, but something like this should do.\nIt compiles, but seems not to work.  I will try to make up a test.\n\nBest regards\nUwe\n\n-- \nUwe Kleine-König\n\nhttp://www.google.com/search?q=5+choose+3\n"},{"id":"46864","messageId":"Pine.LNX.4.64.0707091438540.5546@racer.site","threadId":"8943","inReplyTo":"20070709113543.GA12633@informatik.uni-freiburg.de","subject":"Re: rerere fails to record resolution if file doesn't exist in merge base","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-09T13:40:14Z","receivedAt":"2007-07-09T13:40:14Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 9 Jul 2007, Uwe Kleine-K?nig wrote:\n\n> Junio C Hamano wrote:\n>\n> > Not even compile tested, but something like this should do.\n> It compiles, but seems not to work.  I will try to make up a test.\n\nI compiled it, and it works as expected.  For safety, git-rerere leaves \nthe index unmerged, so that you can verify that you wanted this \nresolution.\n\nWill resubmit with a test case.\n\nCiao,\nDscho\n"},{"id":"46866","messageId":"Pine.LNX.4.64.0707091445450.5546@racer.site","threadId":"8943","inReplyTo":"7vps32ugu2.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH] rerere: record resolution even if file is not in merge base","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-09T13:47:24Z","receivedAt":"2007-07-09T13:47:24Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nTwo-file merges were rare enough that they were dropped outside of the \nradar.  This fix is a trivial change to builtin-rerere.c::find_conflict().  \nIt is still sane to insist that we do not do rerere for symlinks, and \nrequire to have stages #2 and #3, but we can drop the requirement to have \nstage #1. rerere does not use information from there anyway.\n\nThis fix is from Junio, together with two tests to verify that it works\nas expected.\n\nNoticed by Uwe Kleine-König.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n\n\tHeh, this time I steal your patch and take all the credit ;-)\n\n builtin-rerere.c  |   17 +++++++----------\n t/t4200-rerere.sh |   44 +++++++++++++++++++++++++++++++++++++++-----\n 2 files changed, 46 insertions(+), 15 deletions(-)\n\ndiff --git a/builtin-rerere.c b/builtin-rerere.c\nindex c25b3d5..6ffc43d 100644\n--- a/builtin-rerere.c\n+++ b/builtin-rerere.c\n@@ -168,19 +168,16 @@ static int find_conflict(struct path_list *conflict)\n \tint i;\n \tif (read_cache() < 0)\n \t\treturn error(\"Could not read index\");\n-\tfor (i = 0; i + 2 < active_nr; i++) {\n-\t\tstruct cache_entry *e1 = active_cache[i];\n-\t\tstruct cache_entry *e2 = active_cache[i+1];\n-\t\tstruct cache_entry *e3 = active_cache[i+2];\n-\t\tif (ce_stage(e1) == 1 &&\n-\t\t    ce_stage(e2) == 2 &&\n+\tfor (i = 0; i+1 < active_nr; i++) {\n+\t\tstruct cache_entry *e2 = active_cache[i];\n+\t\tstruct cache_entry *e3 = active_cache[i+1];\n+\t\tif (ce_stage(e2) == 2 &&\n \t\t    ce_stage(e3) == 3 &&\n-\t\t    ce_same_name(e1, e2) && ce_same_name(e1, e3) &&\n-\t\t    S_ISREG(ntohl(e1->ce_mode)) &&\n+\t\t    ce_same_name(e2, e3) &&\n \t\t    S_ISREG(ntohl(e2->ce_mode)) &&\n \t\t    S_ISREG(ntohl(e3->ce_mode))) {\n-\t\t\tpath_list_insert((const char *)e1->name, conflict);\n-\t\t\ti += 2;\n+\t\t\tpath_list_insert((const char *)e2->name, conflict);\n+\t\t\ti++; /* skip over both #2 and #3 */\n \t\t}\n \t}\n \treturn 0;\ndiff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh\nindex 6f55ba0..cfcdb69 100755\n--- a/t/t4200-rerere.sh\n+++ b/t/t4200-rerere.sh\n@@ -116,11 +116,12 @@ test_expect_success 'commit succeeds' \\\n \n test_expect_success 'recorded postimage' \"test -f $rr/postimage\"\n \n-git checkout -b third master\n-git show second^:a1 | sed 's/To die: t/To die! T/' > a1\n-git commit -q -a -m third\n-\n-test_expect_failure 'another conflicting merge' 'git pull . first'\n+test_expect_success 'another conflicting merge' '\n+\tgit checkout -b third master &&\n+\tgit show second^:a1 | sed \"s/To die: t/To die! T/\" > a1 &&\n+\tgit commit -q -a -m third &&\n+\t! git pull . first\n+'\n \n git show first:a1 | sed 's/To die: t/To die! T/' > expect\n test_expect_success 'rerere kicked in' \"! grep ======= a1\"\n@@ -164,4 +165,37 @@ test_expect_success 'garbage collection (part2)' 'git rerere gc'\n test_expect_success 'old records rest in peace' \\\n \t\"test ! -f $rr/preimage && test ! -f $rr2/preimage\"\n \n+test_expect_success 'file2 added differently in two branches' '\n+\tgit reset --hard &&\n+\tgit checkout -b fourth &&\n+\techo Hallo > file2 &&\n+\tgit add file2 &&\n+\tgit commit -m version1 &&\n+\tgit checkout third &&\n+\techo Bello > file2 &&\n+\tgit add file2 &&\n+\tgit commit -m version2 &&\n+\t! git merge fourth &&\n+\tsha1=$(sed -e \"s/\t.*//\" .git/rr-cache/MERGE_RR) &&\n+\trr=.git/rr-cache/$sha1 &&\n+\techo Cello > file2 &&\n+\tgit add file2 &&\n+\tgit commit -m resolution\n+'\n+\n+test_expect_success 'resolution was recorded properly' '\n+\tgit reset --hard HEAD~2 &&\n+\tgit checkout -b fifth &&\n+\techo Hallo > file3 &&\n+\tgit add file3 &&\n+\tgit commit -m version1 &&\n+\tgit checkout third &&\n+\techo Bello > file3 &&\n+\tgit add file3 &&\n+\tgit commit -m version2 &&\n+\t! git merge fifth &&\n+\tgit diff-files -q &&\n+\ttest Cello = \"$(cat file3)\"\n+'\n+\n test_done\n-- \n1.5.3.rc0.2769.gd9be2\n\n"},{"id":"46874","messageId":"20070709153209.GA19665@informatik.uni-freiburg.de","threadId":"8943","inReplyTo":"Pine.LNX.4.64.0707091445450.5546@racer.site","subject":"Re: [PATCH] rerere: record resolution even if file is not in merge base","fromName":"Uwe Kleine-König","fromEmail":"ukleinek@informatik.uni-freiburg.de","sentAt":"2007-07-09T15:32:09Z","receivedAt":"2007-07-09T15:32:09Z","isPatch":true,"sender":{"key":"u.kleine-koenig@pengutronix.de","avatar":"https://gravatar.com/avatar/354b5e3ceb2806a2f1e1e382ac29ddbdad18288654da62b61eb13583a857eee7?d=mp&s=160"},"body":"Hello Johannes, hello Junio,\n\nJohannes Schindelin wrote:\n> Two-file merges were rare enough that they were dropped outside of the \n> radar.  This fix is a trivial change to builtin-rerere.c::find_conflict().  \n> It is still sane to insist that we do not do rerere for symlinks, and \n> require to have stages #2 and #3, but we can drop the requirement to have \n> stage #1. rerere does not use information from there anyway.\n> \n> This fix is from Junio, together with two tests to verify that it works\n> as expected.\n> \n> Noticed by Uwe Kleine-König.\nwhen I tested Junio's patch and wrote that it doesn't work, I was wrong.\nI just recompiled git-rerere and called that instead of the installed\nversion.  The problem there was, that merge didn't called the updated.\n\nSo:\n\nAcked-by: Uwe Kleine-König <ukleinek@informatik.uni-freiburg.de>\n\n-- \nUwe Kleine-König\n\nhttp://www.google.com/search?q=1+hertz+in+sec**-1\n"}]}