Subject: Re: [PATCH v3 02/11] merge-one-file: rewrite in C

From: Junio C Hamano <[email-masked]>
Date: Wed, 21 Oct 2020 13:30:41 -0700

Alban Gruin <[email-masked]> writes:

>>> +	int ret, i, dest;
>>> +	ssize_t written;
>>> +	mmbuffer_t result = {NULL, 0};
>>> +	mmfile_t mmfs[3];
>>> +	struct ll_merge_options merge_opts = {0};
>>> +	struct cache_entry *ce;
>>> +
>>> +	if (our_mode == S_IFLNK || their_mode == S_IFLNK)
>>> +		return error(_("%s: Not merging symbolic link changes."), path);
>>> +	else if (our_mode == S_IFGITLINK || their_mode == S_IFGITLINK)
>>> +		return error(_("%s: Not merging conflicting submodule changes."), path);
>>> +
>>> +	read_mmblob(mmfs + 1, our_blob);
>>> +	read_mmblob(mmfs + 2, their_blob);
>>> +
>>> +	if (orig_blob) {
>>> +		printf(_("Auto-merging %s\n"), path);
>>> +		read_mmblob(mmfs + 0, orig_blob);
>>> +	} else {
>>> +		printf(_("Added %s in both, but differently.\n"), path);
>>> +		read_mmblob(mmfs + 0, &null_oid);
>>> +	}
>>> +
>>> +	merge_opts.xdl_opts = XDL_MERGE_ZEALOUS_ALNUM;
>>> +	ret = ll_merge(&result, path,
>>> +		       mmfs + 0, "orig",
>>> +		       mmfs + 1, "our",
>>> +		       mmfs + 2, "their",
>>> +		       istate, &merge_opts);
>> 
>> Is it correct to call into ll_merge() here?  The original used to
>> call "git merge-file" which called into xdl_merge().  Calling into
>> ll_merge() means the path is used to look up the attributes and use
>> the custom merge driver, which I am not offhand sure is what we want
>> to see at this low level (and if it turns out to be a good idea, we
>> definitely should explain the change of semantics in the proposed
>> log message for this commit).

I am still not sure if it is correct to call ll_merge() and not the
xdl_merge() from here.  We need to highlight this change in the log
message, if we were still going to do this.

Thanks.