Re: [PATCH/FIXUP 6/2] apply: clarify read_old_data() is about no-index case

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 17, 2017, 17:07 UTC
Message-ID: <xmqq7ey2i0ly.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <xmqqbmneis63.fsf@gitster.mtv.corp.google.com>

Junio C Hamano <gitster@pobox.com> writes:

Show 14 quoted lines

> Torsten Bögershausen <tboegi@web.de> writes:
>
>> Unless we re-define the meaning of "NULL" into "don't do CRLF conversions,
>> like SAFE_CRLF_KEEP_CRLF does.
>
> My preference is not to use NULL as any hint.  Instead, the "flag"
> parameter we already pass to convert_to_git(), just like the updated
> read_old_data() uses SAFE_CRLF_KEEP_CRLF to tell it that it should
> not disturb existing CRLF without looking at the istate, should be
> used to tell convert_to_git() to do the opposite, but do so without
> looking at the istate.
>
> Perhaps SAFE_CRLF_FALSE should be such a flag.  Or perhaps we need
> to invent another flag.  I dunno.

I grepped for SAFE_CRLF_FALSE and found only two callers that explicitly pass it down the callchain, both of which essentially says "if we are writing the object out, use core.safecrlf, but if we are merely hashing, do SAFE_CRLF_FALSE thing".

I think their use case is quite similar to the codepath we are discussing---they have a data that come from the outside world, and they know the index entry that happens to be at the path has nothing to do with the data they are asking convert_to_git() to massage (i.e. it is _wrong_ if the contents of the blob that happens to be in the index at the path affected the outcome of the conversion).

So I think the fix to convert_to_git() can just use SAFE_CRLF_FALSE as such an instruction that tells the function not do the "safe crlf" thing, which looks at the contents in the index and decide if the CRLFs in the contents being converted should be turned into LFs. And because the function is told not to look at the index, it should be made safe to pass istate=NULL. There does not seem to be a need to invent another flag.

Thanks.