Re: [PATCH] rerere: fix for merge.conflictstyle
On Tue, Apr 29, 2014 at 11:08:29PM -0500, Felipe Contreras wrote: If we use a different conflict style `git rerere forget` is not able to find the matching conflict SHA-1 because the diff generated is actually different from what `git merge` generated. Can you show an example or test case? I could not reproduce the problem with a trivial case, and rerere specifically tries to handle the differences between merge and diff3 styles by throwing away the base content between | and = lines. However, I wonder if it could still miss a match in some cases because the merge style uses XDL_MERGE_ZEALOUS, whereas diff3 bumps it down to XDL_MERGE_EAGER, which could lead to a slightly different preimage. If so, I think this points to a slightly more pervasive problem in rerere, then: data recorded under one merge level might not be usable later (whether for rerere forget or for actually applying a resolution). The level can change if: 1. you have run something like checkout --conflict=diff3 (and rerere reads in the working tree file, which it does for regular resolution, but not for forget). 2. you use git merge-file, which uses XDL_MERGE_ZEALOUS_ALNUM 3. you record resolutions and then change merge.conflictstyle For (1), this is hopefully rarely going to be an issue, since merge applies rerere itself before you get a chance to run checkout. So you would have to manually run git rerere yourself (you might do that with rerere forget, but forget always re-runs the merge from the index). For (2), we can hopefully ignore it, as merge-file does not run rerere (and probably not many people use merge-file at all these days). For (3), we can hopefully ignore this as rare; changing the variable invalidates your cache, but only the hunks for which the ZEALOUS/EAGER level makes a difference. There isn't currently a way to tweak the merge-level manually, which would be the other obvious way to trigger the situation. We already get around the merge/diff3 format by trying to normalize the merge/diff3 hunks we see. It would be nice if we could normalize away the merge-levels, too, but I don't think that is possible just from the conflict data. We'd have to actually re-run the low-level merge with known settings. For git-merge, that would mean doubling the work when rerere is in use. And it would mean that we could not run rerere on a partially-resolved file. So given all of the hopefully rare statements above and the complexity of the complete solution, it's probably not worth pursuing. In which case your patch seems like the best we can do. At least it covers the more common case when you have set merge.conflictstyle but _didn't_ change it since the postimage was recorded. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rerere: fix for merge.conflictstyle
Jeff King wrote: On Tue, Apr 29, 2014 at 11:08:29PM -0500, Felipe Contreras wrote: If we use a different conflict style `git rerere forget` is not able to find the matching conflict SHA-1 because the diff generated is actually different from what `git merge` generated. Can you show an example or test case? % git clone https://github.com/felipec/git % git config merge.conflictstyle diff3 % git checkout master % git merge --no-ff origin/fc/branch/nice-verbose % git merge --no-ff origin/fc/publish # fix conflicts as per -Xtheir strategy % git commit % git rerere forget builtin/branch.c error: no remembered resolution for builtin/branch.c I could not reproduce the problem with a trivial case, and rerere specifically tries to handle the differences between merge and diff3 styles by throwing away the base content between | and = lines. However, I wonder if it could still miss a match in some cases because the merge style uses XDL_MERGE_ZEALOUS, whereas diff3 bumps it down to XDL_MERGE_EAGER, which could lead to a slightly different preimage. That's probably it. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rerere: fix for merge.conflictstyle
Jeff King p...@peff.net writes: I could not reproduce the problem with a trivial case, and rerere specifically tries to handle the differences between merge and diff3 styles by throwing away the base content between | and = lines. However, I wonder if it could still miss a match in some cases because the merge style uses XDL_MERGE_ZEALOUS, whereas diff3 bumps it down to XDL_MERGE_EAGER, which could lead to a slightly different preimage. If so, I think this points to a slightly more pervasive problem in rerere, then: data recorded under one merge level might not be usable later (whether for rerere forget or for actually applying a resolution). ... So given all of the hopefully rare statements above and the complexity of the complete solution, it's probably not worth pursuing. In which case your patch seems like the best we can do. At least it covers the more common case when you have set merge.conflictstyle but _didn't_ change it since the postimage was recorded. Thanks for a fix, Felipe, and thanks for a more detailed discussion, Peff. I think I saw the exact same symptom in forget the other day, but was too busy to pursue it at that moment. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] rerere: fix for merge.conflictstyle
If we use a different conflict style `git rerere forget` is not able to find the matching conflict SHA-1 because the diff generated is actually different from what `git merge` generated. The fix is to call git_xmerge_config() so that git_xmerge_style is set properly and the diffs match. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- builtin/rerere.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/builtin/rerere.c b/builtin/rerere.c index 4e51add..98eb8c5 100644 --- a/builtin/rerere.c +++ b/builtin/rerere.c @@ -60,6 +60,8 @@ int cmd_rerere(int argc, const char **argv, const char *prefix) argc = parse_options(argc, argv, prefix, options, rerere_usage, 0); + git_config(git_xmerge_config, NULL); + if (autoupdate == 1) flags = RERERE_AUTOUPDATE; if (autoupdate == 0) -- 1.9.2+fc1.10.gc7d6c45.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
