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

Junio C Hamano [off-list ref] writes:

Torsten Bögershausen [off-list ref] writes:

quoted

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.