Re: [PATCH 2/2] merge: remember conflict labels

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Junio

On 30/09/2026 17:42, Junio C Hamano wrote:
> Phillip Wood <phillip.wood123@gmail.com> writes:
> 
>> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>>
>> When recreating merge conflicts with "git checkout -m <path>" the
>> original conflict labels are lost. For commands like "git merge" and
>> "git cherry-pick" we could use the presence of the related root
>> ref (MERGE_HEAD and CHERRY_PICK_HEAD respectively) to recreate the
>> labels. However, if the conflicts are from "git stash pop" or "git
>> checkout -m <branch>", then there is no ref to deduce the labels from. To
>> ensure the labels are always available, the merge machinery is updated to
>> write ".git/MERGE_LABELS" when it updates the worktree and
>> there are conflicts. The labels are then read from that file by "git
>> checkout -m <path>" when recreating the conflicts.
>>
>> As "git checkout -m <branch>" calls remove_branch_state() which
>> ordinarily removes the labels file, we need to pass a flag down
>> to optionally prevent that so that the labels are available for any
>> subsequent "git checkout -m <path>". Note that merge_switch_to_result()
>> we assign "result->priv" to "opt->priv" and later clear "opt->priv" in
>> order to get a pointer to the private struct as result->priv is void*.
>>
>> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
>> ---
>>   branch.c           | 11 ++++++--
>>   branch.h           |  1 +
>>   builtin/checkout.c | 24 +++++++++++++++---
>>   builtin/commit.c   |  1 +
>>   merge-ort.c        | 19 ++++++++++++++
>>   merge.c            | 63 ++++++++++++++++++++++++++++++++++++++++++++++
>>   merge.h            |  4 +++
>>   path.c             |  1 +
>>   path.h             |  1 +
>>   repository.c       |  1 +
>>   repository.h       |  1 +
>>   sequencer.c        |  1 +
>>   t/t7201-co.sh      | 21 ++++++++++++++++
>>   13 files changed, 143 insertions(+), 6 deletions(-)
> 
> Where do we talk about MERGE_HEAD and CHERRY_PICK_HEAD in the
> current documentation set?  Do we want to mention MERGE_LABELS
> alongside them?

We talk about those in gitrevisions, the "refs" section of gitglossary 
and in the merge documentation. As this is not a ref I don't think it 
fits with MERGE_HEAD, it is more like MERGE_MSG, or MERGE_MODE. The 
merge man page mentions MERGE_MSG in passing but never explicitly says 
what it contains and MERGE_MODE is undocumented as far as I can see. We 
would perhaps benefit from documenting the common files like 
COMMIT_EDITMSG, MERGE_MSG, SQUASH_MSG, MERGE_HEAD, FETCH_HEAD and 
MERGE_LABELS somewhere in gitrepository briefly explaining what they 
contain and how they are used as a separate series.

>> +static int parse_merge_label_line(const char **p, char **line)
>> +{
>> +	const char *eol = strchr(*p, '\n');
>> +
>> +	if (!eol)
>> +		return -1;
>> +
>> +	*line = xmemdupz(*p, eol - *p);
>> +	*p = eol + 1;
>> +
>> +	return 0;
>> +}
> 
> 
> OK, this reads one line at a time from the file contents already
> fully read by strbuf_read_file(), as seen below.
> 
> Which means that the CRLF fprintf() may have written in
> write_merge_labels() will come back to this function, and our 'ours'
> may become 'ours\015' after stripping only the LF at the end?

That's a good point, I've changed it to use strbuf_getline() instead.

> Thanks for working on these patches.

Thanks for reviewing them, I'll send a re-roll in a couple of days

Phillip