All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Torsten Bögershausen" <tboegi@web.de>
Cc: git@vger.kernel.org, asottile@umich.edu
Subject: Re: [PATCH v1 1/1] correct apply for files commited with CRLF
Date: Fri, 04 Aug 2017 10:57:48 -0700	[thread overview]
Message-ID: <xmqqini3tdur.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <191dc9ea-6ad3-f754-59da-2075a6fd581e@web.de> ("Torsten =?utf-8?Q?B=C3=B6gershausen=22's?= message of "Fri, 4 Aug 2017 19:31:42 +0200")

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.

  reply	other threads:[~2017-08-04 17:57 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-01 18:24 core.autocrlf=true causes `git apply` to fail on patch generated with `git diff-index HEAD --patch` Anthony Sottile
2017-08-01 20:47 ` Torsten Bögershausen
2017-08-01 20:58   ` Anthony Sottile
2017-08-02 15:44     ` Torsten Bögershausen
2017-08-02 20:42       ` [PATCH v1 1/1] correct apply for files commited with CRLF tboegi
2017-08-02 21:13         ` Junio C Hamano
2017-08-04 17:31           ` Torsten Bögershausen
2017-08-04 17:57             ` Junio C Hamano [this message]
2017-08-04 19:26               ` Junio C Hamano
2017-08-02 20:58       ` core.autocrlf=true causes `git apply` to fail on patch generated with `git diff-index HEAD --patch` Junio C Hamano
2017-08-12  5:45         ` Torsten Bögershausen
2017-08-12  5:53           ` Torsten Bögershausen
2017-08-12 14:56         ` [PATCH/RFC] convert: Add SAFE_CRLF_KEEP_CRLF tboegi
2017-08-12 14:56         ` [PATCH/RFC] File commited with CRLF should roundtrip diff and apply tboegi
2017-08-14 17:37           ` Junio C Hamano
2017-08-17  6:06             ` [PATCH v2 1/2] convert: Add SAFE_CRLF_KEEP_CRLF tboegi
2017-08-17  6:06             ` [PATCH v2 2/2] File commited with CRLF should roundtrip diff and apply tboegi
2017-08-17  6:37               ` Junio C Hamano
2017-08-17 21:43             ` [PATCH v3 1/2] convert: Add SAFE_CRLF_KEEP_CRLF tboegi
2017-08-17 21:43             ` [PATCH v3 2/2] File commited with CRLF should roundtrip diff and apply tboegi
2017-08-17 22:29               ` Junio C Hamano
2017-08-17 22:35               ` Junio C Hamano
2017-08-19 11:27             ` [PATCH v4 1/2] convert: Add SAFE_CRLF_KEEP_CRLF tboegi
2017-08-19 11:28             ` [PATCH v4 2/2] File commited with CRLF should roundtrip diff and apply tboegi
2017-08-13  8:51         ` [PATCH/RFC 1/2] convert: Add SAFE_CRLF_KEEP_CRLF tboegi
2017-08-13  8:51         ` [PATCH/RFC 2/2] File commited with CRLF should roundtrip diff and apply tboegi
2017-08-16 18:28           ` Junio C Hamano
2017-08-16 18:28           ` [PATCH/FIXUP 3/2] apply: remove unused member apply_state.flags Junio C Hamano
2017-08-16 18:29           ` [PATCH/FIXUP 4/2] apply: only pay attention to CRLF in the preimage Junio C Hamano
2017-08-16 18:30           ` [PATCH/FIXUP 5/2] apply: localize the CRLF business to read_old_data() Junio C Hamano
2017-08-16 18:34           ` [PATCH/FIXUP 6/2] apply: clarify read_old_data() is about no-index case Junio C Hamano
2017-08-17  6:24             ` Torsten Bögershausen
2017-08-17  7:06               ` Junio C Hamano
2017-08-17  7:12               ` Junio C Hamano
2017-08-17  8:24                 ` Torsten Bögershausen
2017-08-17 17:07                 ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqini3tdur.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=asottile@umich.edu \
    --cc=git@vger.kernel.org \
    --cc=tboegi@web.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.