Re: [PATCH] xmerge.c: fix xdl_merge to conform with the manual

From: Anton Trunov <anton.a.trunov@gmail.com>
Date: Mar 5, 2015, 09:50 UTC
Message-ID: <54F826DE.6010506@gmail.com>
In-Reply-To: <xmqqfv9k8rcs.fsf@gitster.dls.corp.google.com>

On 04/03/15 23:01, Junio C Hamano wrote:

Show 29 quoted lines

> Anton Trunov <anton.a.trunov@gmail.com> writes:
> 
>> For the code version before applying this patch the following scenario
>> will take place if "git merge -Xignore-all-space remote" gets executed.
>>
>> base file:
>> 1st line
>> 2nd line
>>
>> master file:
>>   1st line
>>   2nd line with substantial change
>>
>> remote file:
>>               1st line
>>               2nd line
>>
>> merge result file:
>>   1st line
>>   2nd line with substantial change
>>
>> So essentially it does what "git merge -s ours remote" does in case if
>> all their changes are trivial.
>> This seems like reasonable solution to me: we _are_ trying to ignore
>> whitespace changes and we are explicit about it.
> 
> Now, the above makes readers wonder what happens when you merged
> master into the remote.  I.e. in the opposite direction.
>

Interesting observation. I'd like to point out that the patch doesn't change the way this merge works.

But let us discuss it with the help of git log.

The function xdl_merge() was introduced to the code base in commit 857b933e04bc21ce02043c3107c148f8dcbb4a01.

As it was pointed out in 1cd12926cedb340d176db607e087495381032ce2 (t6023: merge-file fails to output anything for a degenerate merge) the merge-file command would just refuse to merge identical changes and produce output.

Then in 5719db91ce5915ee07c50f1afdc94fe34e91529f (Change xdl_merge to generate output even for null merges) this was fixed.

Let's return to the issue you've mentioned.
According to the manual the merge result file should be:
               1st line                     <- our line
   2nd line with substantial change         <- their line

The problem with the current code that it doesn't work line-wise when at least one side has only whitespace changes. It just completely throws away a side without any changes and takes the other side.

If we wanted to fix it then I'd suggest to completely remove the checks for null changes in xdl_merge() because xdl_do_merge() handles those.