Re: [PATCH v3 02/11] merge-one-file: rewrite in C
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Oct 21, 2020, 20:30 UTC
- Message-ID
- <xmqqeelr724u.fsf@gitster.c.googlers.com>
- In-Reply-To
- <e407ce78-8f93-3fb1-4ef2-ce8213f39df2@gmail.com>
Alban Gruin <alban.gruin@gmail.com> writes:
Show 37 quoted lines
>>> + 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.