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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Apr 30, 2014 at 7:40 PM, Jonathan Nieder <jrnieder@xxxxxxxxx> wrote:
> 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.

>>                                                                   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.

> 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.

> 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

> [...]
>> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]