Re: [PATCH v1 0/1] t6038-merge-text-auto.sh

[Junio C Hamano <gitster@pobox.com>]

tboegi@web.de writes:

> This is a little bit of a hen-and-egg problem:
> The problem comes up after the "unified auto handling".
> In theory, it should have been since before:
> get_sha1_from_index() says:
>
>  * We might be in the middle of a merge, in which
>  * case we would read stage #2 (ours).
>
> This seams wrong, as in the merge we sometimes need to
> look at "theirs".

The two comment you quoted is absolutely the right thing to do.
"In a merge, we sometimes need to look at 'theirs'" is like saying
"When we are dong 'git add', we need to look at what is in the
working tree".  It is total red herring.

Step back and think why we even look at what in the index in the
first place; it is to decide if we want or do not want to disable
the automatic CRLF -> LF conversion.  And think again the reason why
do we look at the index.

There may be a line with CRLF line endings in the new contents,
whether it came from a merge, cherry-pick, patch application, or
plain-simple "git add" from the working tree.  Auto-CRLF usually
says "We want CRLF turned into LF".  But the user misconfigured and
for this path the user might not want the conversion take place, in
which case you would disable the conversion.  Where do you take that
hint "the user might have misconfigured?" from?  By looking at what
the user _started_ her update from.  If the state before this "we
need to replace the blob in the index with a new contents, so we
need to hash the new contents to come up with the updated blob"
started contains CRLF already, that may be a hint--if we apply the
CRLF->LF conversion on the original, even if the "new contents" were
identical to what she already had, we would end up changing the blob
with her current configuration.  Hence we disable.

Isn't that the reasoning behind that "safe auto-crlf" thing?

The new contents getting integrated into her current state may have
CRLF, and if a merge or a cherry-pick leaves conflicts, they may be
stored in stage #3.  But paying attention to that to decide if we
want to disable Auto-CRLF conversion is simply wrong; you should
look at the CRLF in stage #3 as purely something that might need to
be converted, not something that affects the decision if it needs to
be converted, just like you view CRLF in a working tree file when
you do "git add"..

Imagine that you started from a history where somebody recorded a
text file with CRLF in the blob, unconverted.  Later the project
decided to express their text with LF to support cross-platform
development better, and sets up the Auto-CRLF.  Your user is working
near the tip of that history after the eol correction happened.  Now
she gets a pull-request of a branch that forked from an old point in
the history, before the eol correction and full of CRLF.  Yes, to
integrate the change being proposed, she needs to look at "theirs";
that's the whole point of a "merge".  Why should she revert the eol
correction her history has by getting fooled by the fact that the
update was based on a part of the history before the eol correction?