{"thread":{"id":"7505","subject":"[PATCH] rerere should not repeat the earlier hunks in later ones","startedAt":"2007-04-03T23:28:46Z","lastAt":"2007-04-05T13:03:49Z","messageCount":7,"participants":["Junio C Hamano","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"38574","messageId":"7v1wj1ujf5.fsf@assigned-by-dhcp.cox.net","threadId":"7505","inReplyTo":null,"subject":"[PATCH] rerere should not repeat the earlier hunks in later ones","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-03T23:28:46Z","receivedAt":"2007-04-03T23:28:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"When a file has more then one conflicting hunks, it repeated the\ncontents of previous hunks in output for later ones.\n\nSigned-off-by: Junio C Hamano <junkio@cox.net>\n---\n\ndiff --git a/builtin-rerere.c b/builtin-rerere.c\nindex b8867ab..b463c07 100644\n--- a/builtin-rerere.c\n+++ b/builtin-rerere.c\n@@ -78,6 +78,13 @@ static void append_line(struct buffer *buffer, const char *line)\n \tbuffer->nr += len;\n }\n \n+static void clear_buffer(struct buffer *buffer)\n+{\n+\tfree(buffer->ptr);\n+\tbuffer->ptr = NULL;\n+\tbuffer->nr = buffer->alloc = 0;\n+}\n+\n static int handle_file(const char *path,\n \t unsigned char *sha1, const char *output)\n {\n@@ -131,6 +138,8 @@ static int handle_file(const char *path,\n \t\t\t\tSHA1_Update(&ctx, two->ptr, two->nr);\n \t\t\t\tSHA1_Update(&ctx, \"\\0\", 1);\n \t\t\t}\n+\t\t\tclear_buffer(one);\n+\t\t\tclear_buffer(two);\n \t\t} else if (hunk == 1)\n \t\t\tappend_line(one, buf);\n \t\telse if (hunk == 2)\ndiff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh\nindex e081b32..8b611bb 100755\n--- a/t/t4200-rerere.sh\n+++ b/t/t4200-rerere.sh\n@@ -34,7 +34,8 @@ EOF\n git commit -q -a -m first\n \n git checkout -b second master\n-git show first:a1 | sed 's/To die, t/To die! T/' > a1\n+git show first:a1 |\n+sed -e 's/To die, t/To die! T/' -e 's/life;$/life./' > a1\n git commit -q -a -m second\n \n # activate rerere\n@@ -42,19 +43,26 @@ mkdir .git/rr-cache\n \n test_expect_failure 'conflicting merge' 'git pull . first'\n \n-sha1=4f58849a60b4f969a2848966b6d02893b783e8fb\n+sha1=$(sed -e 's/\\t.*//' .git/rr-cache/MERGE_RR)\n rr=.git/rr-cache/$sha1\n test_expect_success 'recorded preimage' \"grep ======= $rr/preimage\"\n \n test_expect_success 'no postimage or thisimage yet' \\\n \t\"test ! -f $rr/postimage -a ! -f $rr/thisimage\"\n \n+test_expect_success 'preimage have right number of lines' '\n+\n+\tcnt=$(sed -ne \"/^<<<<<<</,/^>>>>>>>/p\" $rr/preimage | wc -l) &&\n+\ttest \"$cnt\" = 10\n+\n+'\n+\n git show first:a1 > a1\n \n cat > expect << EOF\n --- a/a1\n +++ b/a1\n-@@ -6,11 +6,7 @@\n+@@ -6,17 +6,9 @@\n  The heart-ache and the thousand natural shocks\n  That flesh is heir to, 'tis a consummation\n  Devoutly to be wish'd.\n@@ -66,8 +74,13 @@ cat > expect << EOF\n  To sleep: perchance to dream: ay, there's the rub;\n  For in that sleep of death what dreams may come\n  When we have shuffled off this mortal coil,\n+ Must give us pause: there's the respect\n+-<<<<<<<\n+-That makes calamity of so long life.\n+-=======\n+ That makes calamity of so long life;\n+->>>>>>>\n EOF\n-\n git rerere diff > out\n \n test_expect_success 'rerere diff' 'git diff expect out'\n"},{"id":"38626","messageId":"Pine.LNX.4.63.0704041746580.4045@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"7505","inReplyTo":"7v1wj1ujf5.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] rerere should not repeat the earlier hunks in later ones","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-04-04T15:51:21Z","receivedAt":"2007-04-04T15:51:21Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 3 Apr 2007, Junio C Hamano wrote:\n\n> When a file has more then one conflicting hunks, it repeated the\n> contents of previous hunks in output for later ones.\n\n/me wonders why our spell-checker has not replied yet: isn't it \"than\" \ninstead of \"then\"?\n\n> @@ -131,6 +138,8 @@ static int handle_file(const char *path,\n>  \t\t\t\tSHA1_Update(&ctx, two->ptr, two->nr);\n>  \t\t\t\tSHA1_Update(&ctx, \"\\0\", 1);\n>  \t\t\t}\n> +\t\t\tclear_buffer(one);\n> +\t\t\tclear_buffer(two);\n>  \t\t} else if (hunk == 1)\n>  \t\t\tappend_line(one, buf);\n>  \t\telse if (hunk == 2)\n\nOuch. Well spotted, thanks.\n\nI wonder if we should also do\n\ndiff --git a/builtin-rerere.c b/builtin-rerere.c\nindex b8867ab..4eae27b 100644\n--- a/builtin-rerere.c\n+++ b/builtin-rerere.c\n@@ -112,7 +112,8 @@ static int handle_file(const char *path,\n \t\telse if (!prefixcmp(buf, \">>>>>>> \")) {\n \t\t\thunk_no++;\n \t\t\thunk = 0;\n-\t\t\tif (memcmp(one->ptr, two->ptr, one->nr < two->nr ?\n+\t\t\tif (one->nr > two->nr || memcmp(one->ptr, two->ptr,\n+\t\t\t\t\t\tone->nr < two->nr ?\n \t\t\t\t\t\tone->nr : two->nr) > 0) {\n \t\t\t\tstruct buffer *swap = one;\n \t\t\t\tone = two;\n\nin case that one conflicting region is prefix of the other one.\n\nCiao,\nDscho\n"},{"id":"38641","messageId":"7v8xd83sfn.fsf@assigned-by-dhcp.cox.net","threadId":"7505","inReplyTo":"Pine.LNX.4.63.0704041746580.4045@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] rerere should not repeat the earlier hunks in later ones","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-04T18:28:28Z","receivedAt":"2007-04-04T18:28:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> I wonder if we should also do\n>\n> diff --git a/builtin-rerere.c b/builtin-rerere.c\n> index b8867ab..4eae27b 100644\n> --- a/builtin-rerere.c\n> +++ b/builtin-rerere.c\n> @@ -112,7 +112,8 @@ static int handle_file(const char *path,\n>  \t\telse if (!prefixcmp(buf, \">>>>>>> \")) {\n>  \t\t\thunk_no++;\n>  \t\t\thunk = 0;\n> -\t\t\tif (memcmp(one->ptr, two->ptr, one->nr < two->nr ?\n> +\t\t\tif (one->nr > two->nr || memcmp(one->ptr, two->ptr,\n> +\t\t\t\t\t\tone->nr < two->nr ?\n>  \t\t\t\t\t\tone->nr : two->nr) > 0) {\n>  \t\t\t\tstruct buffer *swap = one;\n>  \t\t\t\tone = two;\n>\n> in case that one conflicting region is prefix of the other one.\n\nIf one is not a prefix of two but simply longer what does that\ncode do?\n"},{"id":"38656","messageId":"7vircb2680.fsf@assigned-by-dhcp.cox.net","threadId":"7505","inReplyTo":"7v8xd83sfn.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH] rerere: make sorting really stable.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-04T21:13:35Z","receivedAt":"2007-04-04T21:13:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The earlier code does not swap hunks when the beginning of the\nfirst side is identical to the whole of the second side.  In\nsuch a case, the first one should sort later.\n\nSigned-off-by: Junio C Hamano <junkio@cox.net>\n---\n builtin-rerere.c  |    7 +++++--\n t/t4200-rerere.sh |    9 +++++----\n 2 files changed, 10 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin-rerere.c b/builtin-rerere.c\nindex b463c07..8c2c8bd 100644\n--- a/builtin-rerere.c\n+++ b/builtin-rerere.c\n@@ -117,10 +117,13 @@ static int handle_file(const char *path,\n \t\telse if (!prefixcmp(buf, \"=======\"))\n \t\t\thunk = 2;\n \t\telse if (!prefixcmp(buf, \">>>>>>> \")) {\n+\t\t\tint one_is_longer = (one->nr > two->nr);\n+\t\t\tint common_len = one_is_longer ? two->nr : one->nr;\n+\t\t\tint cmp = memcmp(one->ptr, two->ptr, common_len);\n+\n \t\t\thunk_no++;\n \t\t\thunk = 0;\n-\t\t\tif (memcmp(one->ptr, two->ptr, one->nr < two->nr ?\n-\t\t\t\t\t\tone->nr : two->nr) > 0) {\n+\t\t\tif ((cmp > 0) || ((cmp == 0) && one_is_longer)) {\n \t\t\t\tstruct buffer *swap = one;\n \t\t\t\tone = two;\n \t\t\t\ttwo = swap;\ndiff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh\nindex bc878d7..6ba63d7 100755\n--- a/t/t4200-rerere.sh\n+++ b/t/t4200-rerere.sh\n@@ -35,7 +35,8 @@ git commit -q -a -m first\n \n git checkout -b second master\n git show first:a1 |\n-sed -e 's/To die, t/To die! T/' -e 's/life;$/life./' > a1\n+sed -e 's/To die, t/To die! T/' > a1\n+echo \"* END *\" >>a1\n git commit -q -a -m second\n \n # activate rerere\n@@ -53,7 +54,7 @@ test_expect_success 'no postimage or thisimage yet' \\\n test_expect_success 'preimage has right number of lines' '\n \n \tcnt=$(sed -ne \"/^<<<<<<</,/^>>>>>>>/p\" $rr/preimage | wc -l) &&\n-\ttest $cnt = 10\n+\ttest $cnt = 9\n \n '\n \n@@ -75,10 +76,10 @@ cat > expect << EOF\n  For in that sleep of death what dreams may come\n  When we have shuffled off this mortal coil,\n  Must give us pause: there's the respect\n+ That makes calamity of so long life;\n -<<<<<<<\n--That makes calamity of so long life.\n -=======\n- That makes calamity of so long life;\n+-* END *\n ->>>>>>>\n EOF\n git rerere diff > out\n-- \n1.5.1.31.ge421f\n"},{"id":"38658","messageId":"Pine.LNX.4.63.0704042359470.4045@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"7505","inReplyTo":"7v8xd83sfn.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] rerere should not repeat the earlier hunks in later ones","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-04-04T22:00:40Z","receivedAt":"2007-04-04T22:00:40Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 4 Apr 2007, Junio C Hamano wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n> > I wonder if we should also do\n> >\n> > diff --git a/builtin-rerere.c b/builtin-rerere.c\n> > index b8867ab..4eae27b 100644\n> > --- a/builtin-rerere.c\n> > +++ b/builtin-rerere.c\n> > @@ -112,7 +112,8 @@ static int handle_file(const char *path,\n> >  \t\telse if (!prefixcmp(buf, \">>>>>>> \")) {\n> >  \t\t\thunk_no++;\n> >  \t\t\thunk = 0;\n> > -\t\t\tif (memcmp(one->ptr, two->ptr, one->nr < two->nr ?\n> > +\t\t\tif (one->nr > two->nr || memcmp(one->ptr, two->ptr,\n> > +\t\t\t\t\t\tone->nr < two->nr ?\n> >  \t\t\t\t\t\tone->nr : two->nr) > 0) {\n> >  \t\t\t\tstruct buffer *swap = one;\n> >  \t\t\t\tone = two;\n> >\n> > in case that one conflicting region is prefix of the other one.\n> \n> If one is not a prefix of two but simply longer what does that\n> code do?\n\nYou're right. With the eager merging algorithm, it is no longer possible \nthat one side is a strict prefix of the other.\n\nSo please forget about my comment.\n\nCiao,\nDscho\n"},{"id":"38664","messageId":"7vmz1nycjp.fsf@assigned-by-dhcp.cox.net","threadId":"7505","inReplyTo":"Pine.LNX.4.63.0704042359470.4045@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] rerere should not repeat the earlier hunks in later ones","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-04T22:55:54Z","receivedAt":"2007-04-04T22:55:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n>> > @@ -112,7 +112,8 @@ static int handle_file(const char *path,\n>> >  \t\telse if (!prefixcmp(buf, \">>>>>>> \")) {\n>> >  \t\t\thunk_no++;\n>> >  \t\t\thunk = 0;\n>> > -\t\t\tif (memcmp(one->ptr, two->ptr, one->nr < two->nr ?\n>> > +\t\t\tif (one->nr > two->nr || memcmp(one->ptr, two->ptr,\n>> > +\t\t\t\t\t\tone->nr < two->nr ?\n>> >  \t\t\t\t\t\tone->nr : two->nr) > 0) {\n>> >  \t\t\t\tstruct buffer *swap = one;\n>> >  \t\t\t\tone = two;\n>> >\n>> > in case that one conflicting region is prefix of the other one.\n>> \n>> If one is not a prefix of two but simply longer what does that\n>> code do?\n>\n> You're right. With the eager merging algorithm, it is no longer possible \n> that one side is a strict prefix of the other.\n>\n> So please forget about my comment.\n\nNow you confused me even more...\n\nIsn't there a case where one is full of text and two is empty?\n"},{"id":"38701","messageId":"Pine.LNX.4.63.0704051502540.4045@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"7505","inReplyTo":"7vmz1nycjp.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] rerere should not repeat the earlier hunks in later ones","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-04-05T13:03:49Z","receivedAt":"2007-04-05T13:03:49Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 4 Apr 2007, Junio C Hamano wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n> >> > @@ -112,7 +112,8 @@ static int handle_file(const char *path,\n> >> >  \t\telse if (!prefixcmp(buf, \">>>>>>> \")) {\n> >> >  \t\t\thunk_no++;\n> >> >  \t\t\thunk = 0;\n> >> > -\t\t\tif (memcmp(one->ptr, two->ptr, one->nr < two->nr ?\n> >> > +\t\t\tif (one->nr > two->nr || memcmp(one->ptr, two->ptr,\n> >> > +\t\t\t\t\t\tone->nr < two->nr ?\n> >> >  \t\t\t\t\t\tone->nr : two->nr) > 0) {\n> >> >  \t\t\t\tstruct buffer *swap = one;\n> >> >  \t\t\t\tone = two;\n> >> >\n> >> > in case that one conflicting region is prefix of the other one.\n> >> \n> >> If one is not a prefix of two but simply longer what does that\n> >> code do?\n> >\n> > You're right. With the eager merging algorithm, it is no longer possible \n> > that one side is a strict prefix of the other.\n> >\n> > So please forget about my comment.\n> \n> Now you confused me even more...\n> \n> Isn't there a case where one is full of text and two is empty?\n\nIf one is empty, yes, there can be a conflict. I only did not think of the \nempty string as a prefix to everything, but you are right.\n\nCiao,\nDscho\n"}]}