From: Phillip Wood <phillip.wood123@gmail.com>
Date: Wed, 07 Oct 2026 13:38:58 GMT
Subject: Re: [PATCH v2 2/2] merge: remember conflict labels
Message-ID: <42d3f663-6eff-4311-9bdb-d437a757bff5@gmail.com>
In-Reply-To: <xmqqh5iyg7bx.fsf@gitster.g>

On 06/10/2026 16:46, Junio C Hamano wrote:
> Phillip Wood <phillip.wood123@gmail.com> writes:
> >> @@ -4969,6 +4973,13 @@ void merge_switch_to_result(struct merge_options *opt,
>>   			return;
>>   		}
>>   		trace2_region_leave("merge", "write_auto_merge", opt->repo);
>> +
>> +		trace2_region_enter("merge", "write_merge_labels", opt->repo);
>> +		opt->priv = result->priv;
>> +		write_merge_labels(opt->repo, opt->priv->labels[0], opt->priv->labels[1],
>> +				   opt->priv->labels[2]);
>> +		opt->priv = NULL;
>> +		trace2_region_leave("merge", "write_merge_labels", opt->repo);
> > A "if (result->clean >= 0 && update_worktree_and_index)" condition
> guards this code path, so we unconditionall call
> write_merge_labels(), whether the result is clean or with conflict.
> Am I reading the code correctly?

Yes, I think so. Ouch! how did I miss that?

Will fix, thanks

Phillip
>> @@ -5234,6 +5245,14 @@ static void move_opt_priv_to_result_priv(struct merge_options *opt,
>>   	 * to move it.
>>   	 */
>>   	assert(opt->priv && !result->priv);
>> +	if (!result->clean) {
>> +		opt->priv->labels[0] =
>> +			mem_pool_strdup(&opt->priv->pool, opt->ancestor);
>> +		opt->priv->labels[1] =
>> +			mem_pool_strdup(&opt->priv->pool, opt->branch1);
>> +		opt->priv->labels[2] =
>> +			mem_pool_strdup(&opt->priv->pool, opt->branch2);
>> +	}
> > But we only populate the labels[] when conflicted.  What would we
> write when we do not have conflicts?
> >> +int write_merge_labels(struct repository *r, const char *base,
>> +			  const char *ours, const char *theirs)
>> +{
>> +	FILE *f = fopen_or_warn(git_path_merge_labels(r), "w");
>> +
>> +	if (!f)
>> +		return -1;
>> +
>> +	fprintf(f, "%s\n%s\n%s\n", base, ours, theirs);
>> +	if (fclose(f))
>> +		return error_errno("could not write '%s'",
>> +				   git_path_merge_labels(r));
>> +
>> +	return 0;
>> +}
> > Would the answer be "(null)\n(null)\n(null)\n"?