Re: [PATCH v1 1/1] correct apply for files commited with CRLF

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 4, 2017, 17:57 UTC
Message-ID: <xmqqini3tdur.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <191dc9ea-6ad3-f754-59da-2075a6fd581e@web.de>

Torsten Bögershausen <tboegi@web.de> writes:

> Back to the fix, the read_old_data() from below works on the working tree,
> yes, but after convert_to_git().
> And that is why we need the index, to fix this very case.

But "git apply" (without "--cached" or "--index") is to work on the working tree file only. The target file of the patch that is going to be modified does not even have to be in the index, i.e. it is perfectly fine to:

 (1) create a file F, commit, then modify that file;

 (2) take two patches out of that repository you did (1);

 (3) go to a Git repository that does not yet know about file F;

 (4) run "git apply" to process the first patch you took in (2),
     which will create file F; then

 (5) run "git apply" to process the second patch on top, which will
     modify file F.

Step (4) will probably not cause too much issue, as the only thing we make sure is "Because the patch creates F, F does not exist in the directory", which would be the case.

Now, when trying to apply the second patch at step (5), we may need to adjust for the broken line-ending convention, but you do not have any entry in the index for F to rely on.

That is why I am saying convert_to_git() that checks the index content is a wrong thing to use in this codepath.

It is OK to use it for "git add" and friends, as the intent of the (I'd say "broken") "safe CRLF" mechanism is to take a hint from the "previous" state to see if CRLF in the "new" content should be munged, and the "previous" in the context of running "git add" _is_ what is in the index.

The "previous" in the context of running "git apply" (which does not look at the index) is _not_ what is in the index, on the other hand. Instead of "struct index_state *", convert_to_git() needs to be fixed to take something else that can be queried for the "previous" version to use as a hint, if "safe CRLF" wants to work correctly.