{"thread":{"id":"19273","subject":"Re: [BUG] fatal error during merge","startedAt":"2009-05-10T16:33:36Z","lastAt":"2009-05-11T08:34:55Z","messageCount":5,"participants":["Alex Riesen","Johannes Schindelin","Anders Melchiorsen"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"113467","messageId":"20090510163336.GA27241@blimp.localdomain","threadId":"19273","inReplyTo":null,"subject":"Re: [BUG] fatal error during merge","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2009-05-10T16:33:36Z","receivedAt":"2009-05-10T16:33:36Z","isPatch":false,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"I still have the patch below (rebased) in my tree.\nWas the problem fixed somehow differently?\n\nAlex Riesen, Fri, Nov 14, 2008 00:09:32 +0100:\n> SZEDER Gábor, Thu, Nov 13, 2008 19:09:31 +0100:\n> > On Thu, Nov 13, 2008 at 06:06:52PM +0100, Anders Melchiorsen wrote:\n> > > SZEDER Gábor wrote:\n> > > > It doesn't matter.  The test script errors out at the merge, and not\n> > > > at the checkout.  Furthermore, it doesn't matter, whether HEAD~,\n> > > > HEAD~, or HEAD^ is checked out, the results are the same.\n> > > \n> > > Just to be sure, I tried reverting the commit that you bisected -- and my\n> > > test case still fails.\n> > \n> > Well, oddly enough, your second test case behaves somewhat differently\n> > than the first one, at least as far as bisect is concerned.  Bisect\n> > nails down the second test case to 0d5e6c97 (Ignore merged status of\n> > the file-level merge, 2007-04-26; put Alex on Cc).  Reverting this\n> > commit on master makes both of your test cases pass.\n> \n> Well, the case is a bit unfair: all files have the same SHA-1!\n> \n> Whatever, the code pointed by the commit you bisected does look like a\n> problem: it does not update the index after refusing to rewrite the\n> worktree file (because its SHA-1 matches the SHA-1 of the data it\n> would be rewritten with. So updating the file would be a no-op, just\n> wasted effort). Instead of reverting the commit, I suggest the\n> attached patch. It is a long time ago since I looked at the code\n> (and it is a mess, which I'm feeling a bit ashamed of), so another\n> lot of reviewing eyeglasses is definitely in order.\n> \n\nFrom f8eb1a64251b3d4ce080c5aaa7240b209a1b5257 Mon Sep 17 00:00:00 2001\nFrom: Alex Riesen <raa.lkml@gmail.com>\nDate: Thu, 13 Nov 2008 23:55:04 +0100\nSubject: [PATCH] Update index after refusing to rewrite files unchanged during merge\n\nOtherwise the path can stay marked as unresolved in the index,\ncausing the merge to fail.\n\nSigned-off-by: Alex Riesen <raa.lkml@gmail.com>\n---\n merge-recursive.c |    5 +++--\n 1 files changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex a3721ef..d5c43d1 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -980,14 +980,15 @@ static int process_renames(struct merge_options *o,\n \n \t\t\t\tif (mfi.clean &&\n \t\t\t\t    sha_eq(mfi.sha, ren1->pair->two->sha1) &&\n-\t\t\t\t    mfi.mode == ren1->pair->two->mode)\n+\t\t\t\t    mfi.mode == ren1->pair->two->mode) {\n \t\t\t\t\t/*\n \t\t\t\t\t * This messaged is part of\n \t\t\t\t\t * t6022 test. If you change\n \t\t\t\t\t * it update the test too.\n \t\t\t\t\t */\n \t\t\t\t\toutput(o, 3, \"Skipped %s (merged same as existing)\", ren1_dst);\n-\t\t\t\telse {\n+\t\t\t\t\tadd_cacheinfo(mfi.mode, mfi.sha, ren1_dst, 0, 0, ADD_CACHE_OK_TO_ADD);\n+\t\t\t\t} else {\n \t\t\t\t\tif (mfi.merge || !mfi.clean)\n \t\t\t\t\t\toutput(o, 1, \"Renaming %s => %s\", ren1_src, ren1_dst);\n \t\t\t\t\tif (mfi.merge)\n-- \n1.6.3.28.ga852b\n"},{"id":"113500","messageId":"alpine.DEB.1.00.0905110106370.27348@pacific.mpi-cbg.de","threadId":"19273","inReplyTo":"20090510163336.GA27241@blimp.localdomain","subject":"Re: [BUG] fatal error during merge","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-05-10T23:10:24Z","receivedAt":"2009-05-10T23:10:24Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 10 May 2009, Alex Riesen wrote:\n\n> diff --git a/merge-recursive.c b/merge-recursive.c\n> index a3721ef..d5c43d1 100644\n> --- a/merge-recursive.c\n> +++ b/merge-recursive.c\n> @@ -980,14 +980,15 @@ static int process_renames(struct merge_options *o,\n>  \n>  \t\t\t\tif (mfi.clean &&\n>  \t\t\t\t    sha_eq(mfi.sha, ren1->pair->two->sha1) &&\n> -\t\t\t\t    mfi.mode == ren1->pair->two->mode)\n> +\t\t\t\t    mfi.mode == ren1->pair->two->mode) {\n>  \t\t\t\t\t/*\n>  \t\t\t\t\t * This messaged is part of\n>  \t\t\t\t\t * t6022 test. If you change\n>  \t\t\t\t\t * it update the test too.\n>  \t\t\t\t\t */\n>  \t\t\t\t\toutput(o, 3, \"Skipped %s (merged same as existing)\", ren1_dst);\n> -\t\t\t\telse {\n> +\t\t\t\t\tadd_cacheinfo(mfi.mode, mfi.sha, ren1_dst, 0, 0, ADD_CACHE_OK_TO_ADD);\n> +\t\t\t\t} else {\n>  \t\t\t\t\tif (mfi.merge || !mfi.clean)\n\nIf I read the message right, the file revision is supposed not to be \nchanged from HEAD.  Is unpack_trees() invalidating the \"cleanness\" of that \nfile?  (I would really love to have a better idea what's going on than \nwhat I get from both the commit message and the patch before giving my \nACK.)\n\nCiao,\nDscho\n"},{"id":"113521","messageId":"41870.bFoQE3daRhY=.1242027423.squirrel@webmail.hotelhot.dk","threadId":"19273","inReplyTo":"20090510163336.GA27241@blimp.localdomain","subject":"Re: [BUG] fatal error during merge","fromName":"Anders Melchiorsen","fromEmail":"mail@cup.kalibalik.dk","sentAt":"2009-05-11T07:37:03Z","receivedAt":"2009-05-11T07:37:03Z","isPatch":false,"sender":{"key":"mail@cup.kalibalik.dk","avatar":null},"body":"On Sun, 10 May 2009 18:33:36 +0200, Alex Riesen <raa.lkml@gmail.com> wrote:\n\n> I still have the patch below (rebased) in my tree.\n> Was the problem fixed somehow differently?\n\n> Subject: [PATCH] Update index after refusing to rewrite files unchanged\n> during merge\n\nI tested recently, and it does not appear to be fixed yet.\n\nHowever, your patch was not enough to fix my test case completely,\nso I am unsure whether it makes sense to apply it as a partial fix.\n\nThe test is here:\n\n   http://article.gmane.org/gmane.comp.version-control.git/116999\n\n\nAnders.\n"},{"id":"113522","messageId":"81b0412b0905110039l280b69acje4f9704c81028bcc@mail.gmail.com","threadId":"19273","inReplyTo":"alpine.DEB.1.00.0905110106370.27348@pacific.mpi-cbg.de","subject":"Re: [BUG] fatal error during merge","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2009-05-11T07:39:35Z","receivedAt":"2009-05-11T07:39:35Z","isPatch":false,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"2009/5/11 Johannes Schindelin <Johannes.Schindelin@gmx.de>:\n>\n> If I read the message right, the file revision is supposed not to be\n> changed from HEAD.  Is unpack_trees() invalidating the \"cleanness\" of that\n> file?\n\nI think it is the D/F (or F/D?) conflict. A file in one branch is renamed into\na directory. The script in the original post still works (err... fails).\n\n>  (I would really love to have a better idea what's going on than\n> what I get from both the commit message and the patch before giving my\n> ACK.)\n\nAh, scrap that. The patch is no good, and does not fix the original problem\nat all. In fact, it makes it even worse - hides the problem by removing conflict\ninformation from the index and _deletes_ the problematic file.\nThat's why it wasn't included - the brokenness was noticed.\nPity that then I run out of time, too.\n\nThe script to reproduce (note GIT_EXEC_PATH!):\n\n#!/bin/sh\n\nrm -rf merge-rename-fail\nmkdir merge-rename-fail || exit\ncd merge-rename-fail || exit\nexport GIT_MERGE_VERBOSITY=5\nexport GIT_EXEC_PATH=$HOME/projects/git\n$GIT_EXEC_PATH/git init\nmkdir before\necho FILE >before/one\necho FILE >after\n$GIT_EXEC_PATH/git add .\n$GIT_EXEC_PATH/git commit -mfirst\n\nrm -f after\n$GIT_EXEC_PATH/git mv before after\n$GIT_EXEC_PATH/git commit -mmove\n\n$GIT_EXEC_PATH/git checkout -b para HEAD^\necho COMPLETELY ANOTHER FILE >another\n$GIT_EXEC_PATH/git add .\n$GIT_EXEC_PATH/git commit -mpara\n\necho '***\n*** MERGE ***\n***'\necho export GIT_EXEC_PATH=$GIT_EXEC_PATH\necho $GIT_EXEC_PATH/git merge master\n"},{"id":"113523","messageId":"alpine.DEB.1.00.0905111033400.27348@pacific.mpi-cbg.de","threadId":"19273","inReplyTo":"41870.bFoQE3daRhY=.1242027423.squirrel@webmail.hotelhot.dk","subject":"Re: [BUG] fatal error during merge","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-05-11T08:34:55Z","receivedAt":"2009-05-11T08:34:55Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 11 May 2009, Anders Melchiorsen wrote:\n\n> On Sun, 10 May 2009 18:33:36 +0200, Alex Riesen <raa.lkml@gmail.com> wrote:\n> \n> > I still have the patch below (rebased) in my tree.\n> > Was the problem fixed somehow differently?\n> \n> > Subject: [PATCH] Update index after refusing to rewrite files unchanged\n> > during merge\n> \n> I tested recently, and it does not appear to be fixed yet.\n> \n> However, your patch was not enough to fix my test case completely,\n> so I am unsure whether it makes sense to apply it as a partial fix.\n> \n> The test is here:\n> \n>    http://article.gmane.org/gmane.comp.version-control.git/116999\n\nMaybe you can turn this into a patch adding a test (with \ntest_expect_failure to mark it as a bug)?  This would make debugging a lot \neasier, as a non-installed Git could be tested.\n\nCiao,\nDscho\n"}]}