Request for adding a "clean" merge strategy for a double-commit merge to deal with conflicts separately

Hi,

Even though the merge commit's message includes conflicted files by default, the *resolution* itself is lost, that is, it's hard or impossible to review how the author *resolved* said conflicts.

The proposal is that an option like `-X clean` would commit a clean merge and leave out any conflicting hunks in the tree for a follow-up commit to resolve conflicts.

That would be extremely helpful for a code reviewer to see how a possibly external contributor has dealt with upstream changes e.g. in a long-standing branch.

Any comment would be appreciated.

Thanks, Alireza

On Thu, Jul 16, 2020 at 09:45:24PM +0430, Alireza wrote:

No, the merge commit includes the resolution. You can compare with all parents, and even perform the same merge locally using your strategy and tooling of choice and compare the result with what the merge author committed as resolution.

I don't see any value above performing the same merge locally and comparing with the committed resolution. Can you please elaborate on the expected format of the merge that does add some value?

Thanks

Michal

Alireza <rezaxm@gmail.com> writes:

You need to clarify what "leave out" means to you. If you and a side branch started from the same place and you did one thing while the side branch did something else, you would get a conflict.

What would the "clean" (by your definition) result have in that block of contents that actually has a conflict? Do you mean to say "Pick our version and ignore theirs in the blocks where the changes conflict"? If so perhaps -Xours merge strategy option that the recursive backend offers is what you are looking for?

Say I'm merging from upstream with 100 changed files, but I only get two conflicts. If I manually resolve those, the changes I made in the process is actually lost in a large merge diff. What I'm trying to do is to separate those manual changes from anything else that could merge without conflict.

That's actually what I first tried. But when I use -Xours, I can't run `git merge <previous>` again to reproduce those conflicting hunks - because the resulting commit is deemed to be in sync with both parents. As a result, all the upstream changes are now overridden in this side branch.

On Thu, Jul 16, 2020 at 10:17 AM Alireza <rezaxm@gmail.com> wrote:

I disagree that they are "lost". Rather, git doesn't make them very easy to access: git log -p won't show you any output for a merge by default, and the only options that exist (-c, -cc) don't do what you need to see how conflicts were resolved. Thus, the only way to get them would be to check out the first parent of the merge, do a merge with the second parent, then do a "git diff -R $merge_commit". That's doable, it's just annoying.

If there were an option to allow git log for a merge to show the difference between what an automatic merge would do (complete with conflicts) and the end-state that was committed, then the resolution would become very accessible and the rest of your request would be moot. See https://bugs.chromium.org/p/git/issues/detail?id=12. I'm getting closer to having such a thing.

Elijah

Happy to see there's already some interest to make this easier. However, my main use case is not just to see those changes, ideally it should be a separate commit to be easily reviewed in a PR, for example, otherwise the reviewer must pull all changes locally, which doesn't sound like an improvement, IMO.

On Tue, Jul 21, 2020 at 10:17 AM Alireza <rezaxm@gmail.com> wrote:

Well, perhaps here is where we diverge. Providing the ability in core git from the command line to make it easy to review manual user changes within a merge commit would make it so that various PR-related tools could add that capability as well. In fact, gerrit already has such a capability and has for years, so it's not even necessary for git to have any changes to get this kind of ability.

In contrast, recording the automatic changes and the manual changes as separate commits is a short term workaround for a lack of a feature, but one that has some long term ramifications. In my opinion this workaround wrecks history and makes it ugly for the rest of forever. Yes, I have a strong opinion here, but it's actually a bit worse than that: if people like broken history, they can already record the merge as two commits -- so why do we need to add a separate option to facilitate breaking things? I don't like the idea of breaking things just because external tools don't yet have an ability we would like. I think the external tools should be fixed, and perhaps we could provide a base capability those tools can use to more easily implement such a feature.

Just my (strongly worded) $0.02, of course.

Elijah

Elijah Newren <newren@gmail.com> writes:

With "sometimes" sprinkled into appropriate places, you are 110% right.