{"thread":{"id":"36548","subject":"[PATCH] rerere: fix for merge.conflictstyle","startedAt":"2014-04-30T04:08:29Z","lastAt":"2014-04-30T14:04:58Z","messageCount":4,"participants":["Felipe Contreras","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"240266","messageId":"1398830909-24680-1-git-send-email-felipe.contreras@gmail.com","threadId":"36548","inReplyTo":null,"subject":"[PATCH] rerere: fix for merge.conflictstyle","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2014-04-30T04:08:29Z","receivedAt":"2014-04-30T04:08:29Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"If we use a different conflict style `git rerere forget` is not able to\nfind the matching conflict SHA-1 because the diff generated is actually\ndifferent from what `git merge` generated.\n\nThe fix is to call git_xmerge_config() so that git_xmerge_style is set\nproperly and the diffs match.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n builtin/rerere.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/builtin/rerere.c b/builtin/rerere.c\nindex 4e51add..98eb8c5 100644\n--- a/builtin/rerere.c\n+++ b/builtin/rerere.c\n@@ -60,6 +60,8 @@ int cmd_rerere(int argc, const char **argv, const char *prefix)\n \n \targc = parse_options(argc, argv, prefix, options, rerere_usage, 0);\n \n+\tgit_config(git_xmerge_config, NULL);\n+\n \tif (autoupdate == 1)\n \t\tflags = RERERE_AUTOUPDATE;\n \tif (autoupdate == 0)\n-- \n1.9.2+fc1.10.gc7d6c45.dirty\n"},{"id":"240268","messageId":"20140430060835.GA21698@sigill.intra.peff.net","threadId":"36548","inReplyTo":"1398830909-24680-1-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH] rerere: fix for merge.conflictstyle","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-04-30T06:08:35Z","receivedAt":"2014-04-30T06:08:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Apr 29, 2014 at 11:08:29PM -0500, Felipe Contreras wrote:\n\n> If we use a different conflict style `git rerere forget` is not able to\n> find the matching conflict SHA-1 because the diff generated is actually\n> different from what `git merge` generated.\n\nCan you show an example or test case?\n\nI could not reproduce the problem with a trivial case, and rerere\nspecifically tries to handle the differences between \"merge\" and \"diff3\"\nstyles by throwing away the base content between \"|\" and \"=\" lines.\nHowever, I wonder if it could still miss a match in some cases because\nthe \"merge\" style uses XDL_MERGE_ZEALOUS, whereas diff3 bumps it\ndown to XDL_MERGE_EAGER, which could lead to a slightly different\npreimage.\n\nIf so, I think this points to a slightly more pervasive problem in\n\"rerere\", then: data recorded under one merge level might not be usable\nlater (whether for \"rerere forget\" or for actually applying a\nresolution).\n\nThe level can change if:\n\n  1. you have run something like \"checkout --conflict=diff3\" (and rerere\n     reads in the working tree file, which it does for regular\n     resolution, but not for \"forget\").\n\n  2. you use \"git merge-file\", which uses XDL_MERGE_ZEALOUS_ALNUM\n\n  3. you record resolutions and then change merge.conflictstyle\n\nFor (1), this is hopefully rarely going to be an issue, since \"merge\"\napplies rerere itself before you get a chance to run checkout. So you\nwould have to manually run \"git rerere\" yourself (you might do that with\n\"rerere forget\", but \"forget\" always re-runs the merge from the index).\n\nFor (2), we can hopefully ignore it, as merge-file does not run rerere\n(and probably not many people use merge-file at all these days).\n\nFor (3), we can hopefully ignore this as rare; changing the variable\ninvalidates your cache, but only the hunks for which the ZEALOUS/EAGER\nlevel makes a difference.\n\nThere isn't currently a way to tweak the merge-level manually, which\nwould be the other obvious way to trigger the situation.\n\nWe already get around the merge/diff3 format by trying to normalize the\nmerge/diff3 hunks we see. It would be nice if we could normalize away\nthe merge-levels, too, but I don't think that is possible just from the\nconflict data. We'd have to actually re-run the low-level merge with\nknown settings. For git-merge, that would mean doubling the work when\n\"rerere\" is in use.  And it would mean that we could not run \"rerere\" on\na partially-resolved file.\n\nSo given all of the \"hopefully rare\" statements above and the complexity\nof the complete solution, it's probably not worth pursuing. In which\ncase your patch seems like the best we can do. At least it covers the\nmore common case when you have set merge.conflictstyle but _didn't_\nchange it since the postimage was recorded.\n\n-Peff\n"},{"id":"240273","messageId":"53609cee92532_4c3af672f0f4@nysa.notmuch","threadId":"36548","inReplyTo":"20140430060835.GA21698@sigill.intra.peff.net","subject":"Re: [PATCH] rerere: fix for merge.conflictstyle","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2014-04-30T06:49:18Z","receivedAt":"2014-04-30T06:49:18Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Jeff King wrote:\n> On Tue, Apr 29, 2014 at 11:08:29PM -0500, Felipe Contreras wrote:\n> \n> > If we use a different conflict style `git rerere forget` is not able to\n> > find the matching conflict SHA-1 because the diff generated is actually\n> > different from what `git merge` generated.\n> \n> Can you show an example or test case?\n\n% git clone https://github.com/felipec/git\n% git config merge.conflictstyle diff3\n% git checkout master\n% git merge --no-ff origin/fc/branch/nice-verbose\n% git merge --no-ff origin/fc/publish\n# fix conflicts as per -Xtheir strategy\n% git commit\n% git rerere forget builtin/branch.c\nerror: no remembered resolution for builtin/branch.c\n\n> I could not reproduce the problem with a trivial case, and rerere\n> specifically tries to handle the differences between \"merge\" and \"diff3\"\n> styles by throwing away the base content between \"|\" and \"=\" lines.\n> However, I wonder if it could still miss a match in some cases because\n> the \"merge\" style uses XDL_MERGE_ZEALOUS, whereas diff3 bumps it\n> down to XDL_MERGE_EAGER, which could lead to a slightly different\n> preimage.\n\nThat's probably it.\n\n-- \nFelipe Contreras\n"},{"id":"240304","messageId":"xmqqy4ymhq0l.fsf@gitster.dls.corp.google.com","threadId":"36548","inReplyTo":"20140430060835.GA21698@sigill.intra.peff.net","subject":"Re: [PATCH] rerere: fix for merge.conflictstyle","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-04-30T14:04:58Z","receivedAt":"2014-04-30T14:04:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> I could not reproduce the problem with a trivial case, and rerere\n> specifically tries to handle the differences between \"merge\" and \"diff3\"\n> styles by throwing away the base content between \"|\" and \"=\" lines.\n> However, I wonder if it could still miss a match in some cases because\n> the \"merge\" style uses XDL_MERGE_ZEALOUS, whereas diff3 bumps it\n> down to XDL_MERGE_EAGER, which could lead to a slightly different\n> preimage.\n>\n> If so, I think this points to a slightly more pervasive problem in\n> \"rerere\", then: data recorded under one merge level might not be usable\n> later (whether for \"rerere forget\" or for actually applying a\n> resolution).\n> ...\n>\n> So given all of the \"hopefully rare\" statements above and the complexity\n> of the complete solution, it's probably not worth pursuing. In which\n> case your patch seems like the best we can do. At least it covers the\n> more common case when you have set merge.conflictstyle but _didn't_\n> change it since the postimage was recorded.\n\nThanks for a fix, Felipe, and thanks for a more detailed discussion,\nPeff.  I think I saw the exact same symptom in \"forget\" the other\nday, but was too busy to pursue it at that moment.\n"}]}