{"thread":{"id":"14436","subject":"[PATCH/Test] Build in merge is broken","startedAt":"2008-07-13T08:13:55Z","lastAt":"2008-07-14T02:53:13Z","messageCount":7,"participants":["Sverre Hvammen Johansen","Miklos Vajna","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"83129","messageId":"loom.20080713T073129-112@post.gmane.org","threadId":"14436","inReplyTo":null,"subject":"[PATCH/Test] Build in merge is broken","fromName":"Sverre Hvammen Johansen","fromEmail":"hvammen+git@gmail.com","sentAt":"2008-07-13T08:13:55Z","receivedAt":"2008-07-13T08:13:55Z","isPatch":true,"sender":{"key":"hvammen+git@gmail.com","avatar":null},"body":"\nThis test case shows breakage of build in merge. This have been\nbisected to 1c7b76be Build in merge.\n\n---\nGreat that we now are introducing merge in C. Great job Miklos.\nHowever, it is broken as this patch shows. This have been\nbisected to 1c7b76be Build in merge.\n\nThe test case when run will record the parents that were asked for which is\nfine. However, only c2 is recorded as a parent in the commit object. Both\nc1 and c2 should have been recorded. The merge is otherwise working\ncorrectly.\n\n t/t7600-merge.sh | 11 +++++++++++\n 1 files changed, 11 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t7600-merge.sh b/t/t7600-merge.sh\nindex 16f4608..a96a497 100755\n--- a/t/t7600-merge.sh\n+++ b/t/t7600-merge.sh\n@@ -465,4 +465,15 @@ test_expect_success 'merge log message' '\n \n test_debug 'gitk --all'\n \n+test_expect_success 'merge c1 with c0, c2, c0, and c1' '\n+ git reset --hard c1 &&\n+ git config branch.master.mergeoptions \"\" &&\n+ test_tick &&\n+ git merge c0 c2 c0 c1 &&\n+ verify_merge file result.1-5 &&\n+ verify_parents $c1 $c2\n+'\n+\n+test_debug 'gitk --all'\n+\n test_done\n-- \n1.5.5.54.gc6550\n"},{"id":"83147","messageId":"20080713124100.GB10347@genesis.frugalware.org","threadId":"14436","inReplyTo":"loom.20080713T073129-112@post.gmane.org","subject":"Re: [PATCH/Test] Build in merge is broken","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-07-13T12:41:00Z","receivedAt":"2008-07-13T12:41:00Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"On Sun, Jul 13, 2008 at 08:13:55AM +0000, Sverre Hvammen Johansen wrote:\n> Great that we now are introducing merge in C. Great job Miklos.\n> However, it is broken as this patch shows. This have been\n> bisected to 1c7b76be Build in merge.\n> \n> The test case when run will record the parents that were asked for which is\n> fine. However, only c2 is recorded as a parent in the commit object. Both\n> c1 and c2 should have been recorded. The merge is otherwise working\n> correctly.\n\nThanks for the testcase, I'm on it. ;-)\n"},{"id":"83154","messageId":"20080713174659.GE10347@genesis.frugalware.org","threadId":"14436","inReplyTo":"20080713124100.GB10347@genesis.frugalware.org","subject":"Re: [PATCH/Test] Build in merge is broken","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-07-13T17:46:59Z","receivedAt":"2008-07-13T17:46:59Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"On Sun, Jul 13, 2008 at 02:41:00PM +0200, Miklos Vajna wrote:\n> > The test case when run will record the parents that were asked for which is\n> > fine. However, only c2 is recorded as a parent in the commit object. Both\n> > c1 and c2 should have been recorded. The merge is otherwise working\n> > correctly.\n> \n> Thanks for the testcase, I'm on it. ;-)\n\nSo far what I see is that the input for the reduce_heads() function is\n(c1, c0, c2, c0, c1). The expected output would be (c1, c2), but the\nactual output is c2. So I suspect the bug is not in builtin-merge.c\nitself but in reduce_heads().\n\nHmm..\n\nAdding Junio to Cc, who is the original author of reduce_heads().\n"},{"id":"83161","messageId":"20080713184300.GF10347@genesis.frugalware.org","threadId":"14436","inReplyTo":"20080713174659.GE10347@genesis.frugalware.org","subject":"Re: [PATCH/Test] Build in merge is broken","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-07-13T18:43:00Z","receivedAt":"2008-07-13T18:43:00Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"On Sun, Jul 13, 2008 at 07:46:59PM +0200, Miklos Vajna wrote:\n> So far what I see is that the input for the reduce_heads() function is\n> (c1, c0, c2, c0, c1). The expected output would be (c1, c2), but the\n> actual output is c2. So I suspect the bug is not in builtin-merge.c\n> itself but in reduce_heads().\n\nThis fixes the problem for me. Junio, does the fix looks correct to you\nas well?\n\nThanks.\n\ndiff --git a/commit.c b/commit.c\nindex d20b14e..03e73f3 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -747,7 +747,7 @@ struct commit_list *reduce_heads(struct commit_list *heads)\n \n \t\tnum_other = 0;\n \t\tfor (q = heads; q; q = q->next) {\n-\t\t\tif (p == q)\n+\t\t\tif (p->item == q->item)\n \t\t\t\tcontinue;\n \t\t\tother[num_other++] = q->item;\n \t\t}\n"},{"id":"83163","messageId":"7v3amdtx8x.fsf@gitster.siamese.dyndns.org","threadId":"14436","inReplyTo":"20080713184300.GF10347@genesis.frugalware.org","subject":"Re: [PATCH/Test] Build in merge is broken","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-13T19:11:42Z","receivedAt":"2008-07-13T19:11:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Miklos Vajna writes:\n\n> On Sun, Jul 13, 2008 at 07:46:59PM +0200, Miklos Vajna wrote:\n>> So far what I see is that the input for the reduce_heads() function is\n>> (c1, c0, c2, c0, c1). The expected output would be (c1, c2), but the\n>> actual output is c2. So I suspect the bug is not in builtin-merge.c\n>> itself but in reduce_heads().\n>\n> This fixes the problem for me. Junio, does the fix looks correct to you\n> as well?\n\nYou are correct, the \"item\"s are the highlander (i.e. \"there can be only\none\") objects but commit-list elements that hold pointers to them are not,\nso we need to dereference and compare.\n\nThanks.\n"},{"id":"83204","messageId":"402c10cd0807131915u6567cba9h361d26d3dc003739@mail.gmail.com","threadId":"14436","inReplyTo":"20080713184300.GF10347@genesis.frugalware.org","subject":"Re: [PATCH/Test] Build in merge is broken","fromName":"Sverre Hvammen Johansen","fromEmail":"hvammen@gmail.com","sentAt":"2008-07-14T02:15:00Z","receivedAt":"2008-07-14T02:15:00Z","isPatch":true,"sender":{"key":"hvammen@gmail.com","avatar":"https://gravatar.com/avatar/d1fc25ec327eea135af60fe7b56b2a2e21f711aa32167e7f505014ec88aa24bc?d=mp&s=160"},"body":"Test cases for build in merge.\n---\nAfter applying Miklos's fix there still are some breakages. I have squashed in\nanother test case. c1 is merged with c1 and c2. Three parents are\nrecorded in the\nmerge commit object; c1, c1, and c2.\n\n t/t7600-merge.sh | 22 ++++++++++++++++++++++\n 1 files changed, 22 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t7600-merge.sh b/t/t7600-merge.sh\nindex 16f4608..80cfee6 100755\n--- a/t/t7600-merge.sh\n+++ b/t/t7600-merge.sh\n@@ -465,4 +465,26 @@ test_expect_success 'merge log message' '\n\n test_debug 'gitk --all'\n\n+test_expect_success 'merge c1 with c0, c2, c0, and c1' '\n+ git reset --hard c1 &&\n+ git config branch.master.mergeoptions \"\" &&\n+ test_tick &&\n+ git merge c0 c2 c0 c1 &&\n+ verify_merge file result.1-5 &&\n+ verify_parents $c1 $c2\n+'\n+\n+test_debug 'gitk --all'\n+\n+test_expect_success 'merge c1 with c1 and c2' '\n+ git reset --hard c1 &&\n+ git config branch.master.mergeoptions \"\" &&\n+ test_tick &&\n+ git merge c1 c2 &&\n+ verify_merge file result.1-5 &&\n+ verify_parents $c1 $c2\n+'\n+\n+test_debug 'gitk --all'\n+\n test_done\n-- \n1.5.5.54.gc6550\n\n-- \nSverre Hvammen Johansen\n"},{"id":"83207","messageId":"7v8ww5mb1i.fsf@gitster.siamese.dyndns.org","threadId":"14436","inReplyTo":"402c10cd0807131915u6567cba9h361d26d3dc003739@mail.gmail.com","subject":"Re: [PATCH/Test] Build in merge is broken","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-14T02:53:13Z","receivedAt":"2008-07-14T02:53:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Sverre Hvammen Johansen\" writes:\n\n> Test cases for build in merge.\n> ---\n\nThanks.\n\nObviously this is not for application but to help Miklos and others to\nhelp fixing the remaining issues.\n\nTonight's pu won't have this but that is only because I am currently deep\nin today's integration session already.\n"}]}