# [BUG] fatal error during merge 14 messages from 2008-11-13 to 2008-11-14. Participants: Anders Melchiorsen, Samuel Tardieu, SZEDER Gábor, Junio C Hamano, Alex Riesen. Thread: https://gitlist.dev/t/16301 ## Anders Melchiorsen, 2008-11-13 09:22 Subject: [BUG] fatal error during merge Message-ID: <53328.bFoQE3daRhY=.1226568134.squirrel@webmail.hotelhot.dk> URL: https://gitlist.dev/e/53328.bFoQE3daRhY%3D.1226568134.squirrel%40webmail.hotelhot.dk ``` It took a while to make a minimal test case for this, but I think that all the below steps are required. It ends up with this message: after/one: unmerged (e69de29bb2d1d6434b8b29ae775ad8c2e48c5391) fatal: git write-tree failed to write a tree and the file that was moved has disappeared from the working tree. I have tested the script with Git 1.6.0.2, but the real scenario that made this appear seems to also fail with master and next from git.git. Cheers, Anders. Test script: mkdir am-merge-fail cd am-merge-fail git init mkdir before touch before/one after git add -A git commit -minitial rm -f after git mv before after git commit -mmove git checkout -b parallel HEAD~ touch another git add -A git commit -mparallel git merge master ``` ## Samuel Tardieu, 2008-11-13 13:23 Subject: Re: [BUG] fatal error during merge Message-ID: <2008-11-13-14-23-19+trackit+sam@rfc1149.net> URL: https://gitlist.dev/e/2008-11-13-14-23-19%2Btrackit%2Bsam%40rfc1149.net In-Reply-To: <53328.bFoQE3daRhY=.1226568134.squirrel@webmail.hotelhot.dk> ``` >>>>> "Anders" == Anders Melchiorsen writes: Anders> I have tested the script with Git 1.6.0.2, but the real Anders> scenario that made this appear seems to also fail with master Anders> and next from git.git. I confirm that your test case also fails with the current "next". Sam -- Samuel Tardieu -- sam@rfc1149.net -- http://www.rfc1149.net/ ``` ## SZEDER Gábor, 2008-11-13 14:03 Subject: Re: [BUG] fatal error during merge Message-ID: <20081113140323.GA10267@neumann> URL: https://gitlist.dev/e/20081113140323.GA10267%40neumann In-Reply-To: <2008-11-13-14-23-19+trackit+sam@rfc1149.net> ``` Hi, On Thu, Nov 13, 2008 at 02:23:19PM +0100, Samuel Tardieu wrote: > >>>>> "Anders" == Anders Melchiorsen writes: > > Anders> I have tested the script with Git 1.6.0.2, but the real > Anders> scenario that made this appear seems to also fail with master > Anders> and next from git.git. > > I confirm that your test case also fails with the current "next". Yeah, and it can be bisected to commit 621ff675 (rev-parse: fix meaning of rev~ vs rev~0, 2008-03-14), which is from Linus, so put him on Cc. Best, Gábor ``` ## Anders Melchiorsen, 2008-11-13 14:25 Subject: Re: [BUG] fatal error during merge Message-ID: <44684.bFoQE3daRhY=.1226586339.squirrel@webmail.hotelhot.dk> URL: https://gitlist.dev/e/44684.bFoQE3daRhY%3D.1226586339.squirrel%40webmail.hotelhot.dk In-Reply-To: <20081113140323.GA10267@neumann> ``` SZEDER Gábor wrote: > > On Thu, Nov 13, 2008 at 02:23:19PM +0100, Samuel Tardieu wrote: >> >>>>> "Anders" == Anders Melchiorsen writes: >> >> Anders> I have tested the script with Git 1.6.0.2, but the real >> Anders> scenario that made this appear seems to also fail with master >> Anders> and next from git.git. >> >> I confirm that your test case also fails with the current "next". > > Yeah, and it can be bisected to commit 621ff675 (rev-parse: fix > meaning of rev~ vs rev~0, 2008-03-14), which is from Linus, so put him > on Cc. I guess that is due to using HEAD~ in the test script? If you are bisecting, here is a version that does not use the HEAD~ notation: mkdir am-merge-fail cd am-merge-fail git init mkdir before touch before/one after git add before after git commit -minitial git branch parallel rm -f after git mv before after git commit -mmove git checkout parallel touch other git add other git commit -mparallel git merge master ``` ## Samuel Tardieu, 2008-11-13 14:26 Subject: Re: [BUG] fatal error during merge Message-ID: <2008-11-13-15-26-33+trackit+sam@rfc1149.net> URL: https://gitlist.dev/e/2008-11-13-15-26-33%2Btrackit%2Bsam%40rfc1149.net In-Reply-To: <20081113140323.GA10267@neumann> ``` * SZEDER Gábor [2008-11-13 15:03:23 +0100] | On Thu, Nov 13, 2008 at 02:23:19PM +0100, Samuel Tardieu wrote: | > >>>>> "Anders" == Anders Melchiorsen writes: | > | > Anders> I have tested the script with Git 1.6.0.2, but the real | > Anders> scenario that made this appear seems to also fail with master | > Anders> and next from git.git. | > | > I confirm that your test case also fails with the current "next". | | Yeah, and it can be bisected to commit 621ff675 (rev-parse: fix | meaning of rev~ vs rev~0, 2008-03-14), which is from Linus, so put him | on Cc. I think your pinpointed a change of behaviour in "HEAD~", which is probably not the problem at hand. To find the real bug, you should update the test script so that it uses "HEAD~1" instead of "HEAD~". ``` ## SZEDER Gábor, 2008-11-13 14:53 Subject: Re: [BUG] fatal error during merge Message-ID: <20081113145325.GD29274@neumann> URL: https://gitlist.dev/e/20081113145325.GD29274%40neumann In-Reply-To: <2008-11-13-15-26-33+trackit+sam@rfc1149.net> ``` Hi, On Thu, Nov 13, 2008 at 03:26:33PM +0100, Samuel Tardieu wrote: > * SZEDER Gábor [2008-11-13 15:03:23 +0100] > | Yeah, and it can be bisected to commit 621ff675 (rev-parse: fix > | meaning of rev~ vs rev~0, 2008-03-14), which is from Linus, so put him > | on Cc. > > I think your pinpointed a change of behaviour in "HEAD~", which is > probably not the problem at hand. To find the real bug, you should > update the test script so that it uses "HEAD~1" instead of "HEAD~". It doesn't matter. The test script errors out at the merge, and not at the checkout. Furthermore, it doesn't matter, whether HEAD~, HEAD~, or HEAD^ is checked out, the results are the same. But yes, it's possible that not the bisected commit is the culprit, but something in or behind merge relies on the old undocumented and illogical behaviour. Gábor ``` ## Anders Melchiorsen, 2008-11-13 17:06 Subject: Re: [BUG] fatal error during merge Message-ID: <57814.N1gUGH5fRhE=.1226596012.squirrel@webmail.hotelhot.dk> URL: https://gitlist.dev/e/57814.N1gUGH5fRhE%3D.1226596012.squirrel%40webmail.hotelhot.dk In-Reply-To: <20081113145325.GD29274@neumann> ``` SZEDER Gábor wrote: > It doesn't matter. The test script errors out at the merge, and not > at the checkout. Furthermore, it doesn't matter, whether HEAD~, > HEAD~, or HEAD^ is checked out, the results are the same. Just to be sure, I tried reverting the commit that you bisected -- and my test case still fails. Anders. ``` ## SZEDER Gábor, 2008-11-13 18:09 Subject: Re: [BUG] fatal error during merge Message-ID: <20081113180931.GE29274@neumann> URL: https://gitlist.dev/e/20081113180931.GE29274%40neumann In-Reply-To: <57814.N1gUGH5fRhE=.1226596012.squirrel@webmail.hotelhot.dk> ``` On Thu, Nov 13, 2008 at 06:06:52PM +0100, Anders Melchiorsen wrote: > SZEDER Gábor wrote: > > It doesn't matter. The test script errors out at the merge, and not > > at the checkout. Furthermore, it doesn't matter, whether HEAD~, > > HEAD~, or HEAD^ is checked out, the results are the same. > > Just to be sure, I tried reverting the commit that you bisected -- and my > test case still fails. Well, oddly enough, your second test case behaves somewhat differently than the first one, at least as far as bisect is concerned. Bisect nails down the second test case to 0d5e6c97 (Ignore merged status of the file-level merge, 2007-04-26; put Alex on Cc). Reverting this commit on master makes both of your test cases pass. Gábor ``` ## Anders Melchiorsen, 2008-11-13 22:25 Subject: Re: [BUG] fatal error during merge Message-ID: <55583.N1gUGH5fRhE=.1226615119.squirrel@webmail.hotelhot.dk> URL: https://gitlist.dev/e/55583.N1gUGH5fRhE%3D.1226615119.squirrel%40webmail.hotelhot.dk In-Reply-To: <20081113180931.GE29274@neumann> ``` SZEDER Gábor wrote: > Well, oddly enough, your second test case behaves somewhat differently > than the first one, at least as far as bisect is concerned. Bisect > nails down the second test case to 0d5e6c97 (Ignore merged status of > the file-level merge, 2007-04-26; put Alex on Cc). Reverting this > commit on master makes both of your test cases pass. Hmm. With that one reverted, git prints out this error instead: error: failed to create path 'after/one': perhaps a D/F conflict? and still has the moved file disapper in the working tree. The merge is successful, though, and the moved file is in the committed tree. Anders. ``` ## Alex Riesen, 2008-11-13 23:09 Subject: Re: [BUG] fatal error during merge Message-ID: <20081113230932.GA8552@blimp.localdomain> URL: https://gitlist.dev/e/20081113230932.GA8552%40blimp.localdomain In-Reply-To: <20081113180931.GE29274@neumann> ``` SZEDER Gábor, Thu, Nov 13, 2008 19:09:31 +0100: > On Thu, Nov 13, 2008 at 06:06:52PM +0100, Anders Melchiorsen wrote: > > SZEDER Gábor wrote: > > > It doesn't matter. The test script errors out at the merge, and not > > > at the checkout. Furthermore, it doesn't matter, whether HEAD~, > > > HEAD~, or HEAD^ is checked out, the results are the same. > > > > Just to be sure, I tried reverting the commit that you bisected -- and my > > test case still fails. > > Well, oddly enough, your second test case behaves somewhat differently > than the first one, at least as far as bisect is concerned. Bisect > nails down the second test case to 0d5e6c97 (Ignore merged status of > the file-level merge, 2007-04-26; put Alex on Cc). Reverting this > commit on master makes both of your test cases pass. Well, the case is a bit unfair: all files have the same SHA-1! Whatever, the code pointed by the commit you bisected does look like a problem: it does not update the index after refusing to rewrite the worktree file (because its SHA-1 matches the SHA-1 of the data it would be rewritten with. So updating the file would be a no-op, just wasted effort). Instead of reverting the commit, I suggest the attached patch. It is a long time ago since I looked at the code (and it is a mess, which I'm feeling a bit ashamed of), so another lot of reviewing eyeglasses is definitely in order. From c395f4234ca5492206923e1821a316a777c651cd Mon Sep 17 00:00:00 2001 From: Alex Riesen Date: Thu, 13 Nov 2008 23:55:04 +0100 Subject: [PATCH] Update index after refusing to rewrite unchanged files Specifically, which were not changed during recursive merge. Otherwise the path can stay marked as unresolved in the index, causing the merge to fail. Signed-off-by: Alex Riesen diff --git a/merge-recursive.c b/merge-recursive.c index 7472d3e..28f9e12 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -944,14 +944,15 @@ static int process_renames(struct merge_options *o, if (mfi.clean && sha_eq(mfi.sha, ren1->pair->two->sha1) && - mfi.mode == ren1->pair->two->mode) + mfi.mode == ren1->pair->two->mode) { /* * This messaged is part of * t6022 test. If you change * it update the test too. */ output(o, 3, "Skipped %s (merged same as existing)", ren1_dst); - else { + add_cacheinfo(mfi.mode, mfi.sha, ren1_dst, 0, 0, ADD_CACHE_OK_TO_ADD); + } else { if (mfi.merge || !mfi.clean) output(o, 1, "Renaming %s => %s", ren1_src, ren1_dst); if (mfi.merge) ``` ## Anders Melchiorsen, 2008-11-13 23:34 Subject: Re: [BUG] fatal error during merge Message-ID: <48332.N1gUGH5fRhE=.1226619243.squirrel@webmail.hotelhot.dk> URL: https://gitlist.dev/e/48332.N1gUGH5fRhE%3D.1226619243.squirrel%40webmail.hotelhot.dk In-Reply-To: <20081113230932.GA8552@blimp.localdomain> ``` Alex Riesen wrote: > Well, the case is a bit unfair: all files have the same SHA-1! Here is an updated test, where all files have different SHA-1. It still fails. Your patch got rid of the errors and it commits the merged tree. But the working tree is not updated correctly, so the moved file disappears. Cheers, Anders. mkdir am-merge-fail cd am-merge-fail git init mkdir before yes | head -n100 >before/100 touch after git add before/100 after git commit -minitial git branch parallel git rm before/100 after mkdir after yes | head -n99 >after/99 git add after/99 git commit -mmove git checkout parallel echo "Just to cause a non-ff" >other git add other git commit -mparallel git merge master ``` ## Junio C Hamano, 2008-11-14 00:29 Subject: Re: [BUG] fatal error during merge Message-ID: <7vfxlvf8up.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vfxlvf8up.fsf%40gitster.siamese.dyndns.org In-Reply-To: <48332.N1gUGH5fRhE=.1226619243.squirrel@webmail.hotelhot.dk> ``` "Anders Melchiorsen" writes: > Your patch got rid of the errors and it commits the merged tree. But the > working tree is not updated correctly, so the moved file disappears. Isn't this a long known breakage of D/F conflict check logic in merge-recursive backend? ``` ## Alex Riesen, 2008-11-14 07:16 Subject: Re: [BUG] fatal error during merge Message-ID: <20081114071636.GA4013@blimp.localdomain> URL: https://gitlist.dev/e/20081114071636.GA4013%40blimp.localdomain In-Reply-To: <7vfxlvf8up.fsf@gitster.siamese.dyndns.org> ``` Junio C Hamano, Fri, Nov 14, 2008 01:29:34 +0100: > "Anders Melchiorsen" writes: > > > Your patch got rid of the errors and it commits the merged tree. But the > > working tree is not updated correctly, so the moved file disappears. > > Isn't this a long known breakage of D/F conflict check logic in > merge-recursive backend? Could be, but there is no conflicting entries in the merged branches. All file names are different and the D/F transition happens completely on one branch. And the patch isn't enough: the file Anders noticed missing is in the index (correctly) but somehow removed from worktree. Which is maybe because it _still_ was a rename: 99% similarity. Well, just "yes | tail -nNN" usually producess very similar data :) Still broken, of course... ``` ## Anders Melchiorsen, 2008-11-14 09:16 Subject: Re: [BUG] fatal error during merge Message-ID: <38773.bFoQE3daRhY=.1226654164.squirrel@webmail.hotelhot.dk> URL: https://gitlist.dev/e/38773.bFoQE3daRhY%3D.1226654164.squirrel%40webmail.hotelhot.dk In-Reply-To: <20081114071636.GA4013@blimp.localdomain> ``` Alex Riesen wrote: > And the patch isn't enough: the file Anders noticed missing is in the > index (correctly) but somehow removed from worktree. Which is maybe > because it _still_ was a rename: 99% similarity. Well, just > "yes | tail -nNN" usually producess very similar data :) Maybe reducing the example all the way down to "touch" lost the implicit information that, indeed, it has to be a rename for this to fail. Anders. ```