New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
merge-ort: make informational messages from recursive merges clearer #1121
Conversation
This is another simple change with a long explanation... merge-recursive and merge-ort are both based on the same recursive idea: if there is more than one merge base, merge the merge bases (which may require first merging the merge bases of the merges bases, etc.). The depth of the inner merge is recorded via a variable called "call_depth", which we'll bring up again later. Naturally, the inner merges themselves can have conflicts and various messages generated about those files. merge-recursive immediately prints to stdout as it goes, at the risk of printing multiple conflict notices for the same path separated far apart from each other with many intervenining conflict notices for other paths between them. And this is true even if there are no inner merges involved. An example of this was given in [1] and apparently caused some confusion: CONFLICT (rename/add): Rename A->B in HEAD. B added in otherbranch ...dozens of conflicts for OTHER paths... CONFLICT (content): Merge conflicts in B In contrast, merge-ort collects messages and stores them by path so that it can print them grouped by path. Thus, the same case handled by merge-ort would have output of the form: CONFLICT (rename/add): Rename A->B in HEAD. B added in otherbranch CONFLICT (content): Merge conflicts in B ...dozens of conflicts for OTHER paths... This is generally helpful, but does make a separate bug more problematic. In particular, while merge-recursive might report the following for a recursive merge: Auto-merging dir.c Auto-merging midx.c CONFLICT (content): Merge conflict in midx.c Auto-merging diff.c Auto-merging dir.c CONFLICT (content): Merge conflict in dir.c merge-ort would instead report: Auto-merging diff.c Auto-merging dir.c Auto-merging dir.c CONFLICT (content): Merge conflict in dir.c Auto-merging midx.c CONFLICT (content): Merge conflict in midx.c The fact that messages for the same file are together is probably helpful in general, but with the indentation missing for the inner merge it unfortunately serves to confuse. This probably would lead users to wonder: * Why is Git reporting that "dir.c" is being merged twice? * If midx.c has conflicts, why do I not see any when I open up the file and why are no conflicts shown in the index? Fix this output confusion by changing the output to clearly differentiate the messages for outer merges from the ones for inner merges, changing the above output from merge-ort to: Auto-merging diff.c From inner merge: Auto-merging dir.c Auto-merging dir.c CONFLICT (content): Merge conflict in dir.c From inner merge: Auto-merging midx.c From inner merge: CONFLICT (content): Merge conflict in midx.c (Note: the number of spaces after the 'From inner merge:' is 2*call_depth). One other thing to note here, that I didn't notice until typing up this commit message, is that merge-recursive does not print any messages from the inner merges by default; the extra verbosity has to be requested. merge-ort currently has no verbosity controls and always prints these. We may also want to change that, but for now, just make the output clearer with these extra markings and indentation. [1] https://lore.kernel.org/git/CAGyf7-He4in8JWUh9dpAwvoPkQz9hr8nCBpxOxhZEd8+jtqTpg@mail.gmail.com/ Signed-off-by: Elijah Newren <newren@gmail.com>
931b3b8
to
0725e27
Compare
/submit |
Submitted as pull.1121.git.1645079923090.gitgitgadget@gmail.com To fetch this version into
To fetch this version to local tag
|
On the Git mailing list, Junio C Hamano wrote (reply to this):
|
This branch is now known as |
This patch series was integrated into seen via git@9743003. |
This patch series was integrated into seen via git@457db35. |
There was a status update in the "New Topics" section about the branch Messages "ort" merge backend prepares while dealing with conflicted paths were unnecessarily confusing since it did not differentiate inner merges and outer merges. Will merge to 'next'. source: <pull.1121.git.1645079923090.gitgitgadget@gmail.com> |
This patch series was integrated into seen via git@f477cb8. |
This patch series was integrated into next via git@367dd32. |
This patch series was integrated into seen via git@b2720f3. |
This patch series was integrated into seen via git@353f145. |
There was a status update in the "Cooking" section about the branch Messages "ort" merge backend prepares while dealing with conflicted paths were unnecessarily confusing since it did not differentiate inner merges and outer merges. Will merge to 'master'. source: <pull.1121.git.1645079923090.gitgitgadget@gmail.com> |
This patch series was integrated into seen via git@b3db182. |
This patch series was integrated into master via git@b3db182. |
Closed via b3db182. |
On the Git mailing list, Junio C Hamano wrote (reply to this):
|
On the Git mailing list, Elijah Newren wrote (reply to this):
|
User |
On the Git mailing list, Elijah Newren wrote (reply to this):
|
On the Git mailing list, Junio C Hamano wrote (reply to this):
|
Sorry for any confusion caused by this bug (though I'm surprised no one else reported it).
cc: Elijah Newren newren@gmail.com