[PATCH] rerere: fix for merge.conflictstyle

[PATCH] rerere: fix for merge.conflictstyle

[Felipe Contreras]

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.contreras@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

Re: [PATCH] rerere: fix for merge.conflictstyle

[Jeff King]

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

Re: [PATCH] rerere: fix for merge.conflictstyle

[Felipe Contreras]

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

Re: [PATCH] rerere: fix for merge.conflictstyle

[Junio C Hamano]

Jeff King <peff@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.