Re: [BUG?] Patches created with 'diff.noprefix=true' don't 'git apply'.

From: Nathan Collins <nathan.collins@gmail.com>
Date: May 6, 2014, 01:33 UTC
Message-ID: <CAO8RVveV4SgXmJzWpYnyB3rnQALQkrrLNtLL5Ej5Y9Mjjekmmw@mail.gmail.com>
In-Reply-To: <20140501024042.GX9218@google.com>

On Wed, Apr 30, 2014 at 7:40 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:

Show 13 quoted lines

> Hi,
>
> Nathan Collins wrote:
>
>> Patches created with 'diff.noprefix=true' don't 'git apply' without
>> specifying '-p0'.
>>
>> I'm not sure this is a bug -- the 'man git-apply' just says "Reads the
>> supplied diff output (i.e. "a patch") and applies it to files" -- but
>> I would expect patches I create locally to apply cleanly locally.
>
> Sounds like a documentation bug, at least.  Any ideas for clearer
> wording?

Hmmm. Maybe a warning that the patch is expected to be in '-p1' format, and that setting 'diff.noprefix=true' makes some commands generate '-p0' patches? But I worry this would just confuse / distract the people that don't have 'diff.noprefix=true' set, which I expect is the majority of users. Better I think would be for 'git apply' to be smarter, as you suggest below.

Show 10 quoted lines

>>                                                                   In
>> real life the 'diff.noprefix=true' is in my ~/.gitconfig, so this was
>> pretty confusing.
>
> I personally think setting diff.noprefix is not very sane (it also
> breaks "patch -p1"), and I suppose I should have been louder about
> that when it was introduced.
>
> Can you say more about the workflow you use that requires
> diff.noprefix?  Maybe we can make other changes to improve it, too.

I have 'diff.noprefix=true' set so I can copy and paste paths from the 'git diff' output easily. I like to create small, logically independent commits, usually comprising a subset of my current changes. So, I do 'git diff' in one terminal, and then 'git add <path>' or 'git add --patch <path>' in another terminal to build up a commit (I suppose this is the work flow that 'git add --interactive' is designed for ...), where I get '<path>' from the diff by copying and pasting. With 'diff.noprefix=true', I can copy with double left click and paste with middle click; with 'diff.noprefix=false', to copy I instead have to carefully highlight the non-prefix part of the path in the diff, which is less convenient.

Show 5 quoted lines

> At first glance I don't suspect making diff.noprefix imply -p0 for
> "git am" would be great, since that would generate the the opposite
> problem when applying patches from the outside world.  But maybe we
> need better autodetection and maybe noprefix is a good signal about
> when to use it.

Autodetecting the lack or presence of the 'a/' and 'b/' prefixes seems like a great solution to me: externally user friendly and easy to implement internally.

Show 6 quoted lines

> Another complication is that unlike 'git diff', 'git apply' is
> plumbing that is meant to be useful and reliable for scripts.  And
> unlike most plumbing, there is no higher-level command with similar
> functionality for which we can experiment more freely with the UI.
> Adding a new command to fix that might be a good direction toward
> handling noprefix patches better.

Related to 'git apply' being a scriptable plumbing command: naively I would expect there to be a "scripting mode" for Git commands which ignored the local configuration entirely (e.g. ~/.gitconfig). I've wanted this a few times and was surprised I could find no very sane way to achieve it. In fact, here's the corresponding question I posted on Stack Overflow while I was composing my original email (I wanted to be sure that 'diff.noprefix=true' was the only relevant part of my ~/.gitconfig, so I wanted disable my ~/.gitconfig entirely):

http://stackoverflow.com/questions/23400449/how-to-make-git-temporarily-ignore-gitconfig

Show 7 quoted lines

> [...]
>> git show | git apply --reverse
>
> The following which only uses plumbing commands should work:
>
>         git diff-tree -p HEAD^! |
>         git apply --reverse

Nice! However, I don't now how to generalize this solution to other (probably insane) use cases, e.g.

  git log -S<string> --patch | git apply --reverse

(Context: http://stackoverflow.com/a/23401018/470844).

> Thanks for some food for thought,
> Jonathan

Thanks for your reply. I didn't see it until today because a GMail filter ate it :P

-nathan