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

4 messages, 2 authors, 2016-06-15 · open the first message on its own page

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

From: Nathan Collins <hidden>
Date: 2016-06-15 23:00:57

Bug?
====

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. In
real life the 'diff.noprefix=true' is in my ~/.gitconfig, so this was
pretty confusing.

Here's an old bug that's kind of related:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=607044

I'm using Git 1.9.2.

Example
=======

Create a repo with a test commit:

  git init bug.git
  cd bug.git
  git add test
  git commit test -m Test

Revert the test commit in a contrived way (like
'git revert HEAD --no-commit; git reset'). This works:

  git -c diff.noprefix=false show | git -c diff.noprefix=false apply --reverse

And this works:

  git reset --hard
  git -c diff.noprefix=true show | git -c diff.noprefix=true apply -p0 --reverse

But this fails:

  git reset --hard
  git -c diff.noprefix=true show | git -c diff.noprefix=true apply --reverse

    fatal: git diff header lacks filename information when removing 1
leading pathname component (line 12)

Use Case
========

Partially reverting a commit:

http://git.661346.n2.nabble.com/Revert-a-single-commit-in-a-single-file-td6064050.html#a6064406

Cheers,

-nathan

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

From: Jonathan Nieder <hidden>
Date: 2016-06-15 23:00:57

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

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.

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.

[...]
git show | git apply --reverse
The following which only uses plumbing commands should work:

	git diff-tree -p HEAD^! |
	git apply --reverse

Thanks for some food for thought,
Jonathan

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

From: Nathan Collins <hidden>
Date: 2016-06-15 23:01:00

On Wed, Apr 30, 2014 at 7:40 PM, Jonathan Nieder [off-list ref] wrote:
Hi,

Nathan Collins wrote:
quoted
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.
quoted
                                                                  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
[...]
quoted
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

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

From: Jonathan Nieder <hidden>
Date: 2016-06-15 23:01:01

Nathan Collins wrote:
On Wed, Apr 30, 2014 at 7:40 PM, Jonathan Nieder [off-list ref] wrote:
quoted
Nathan Collins wrote:
quoted
quoted
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
This should do it:

	git rev-list HEAD |
	git diff-tree --no-commit-id -p -S<string> --stdin |
	git apply --reverse

More generally, when scripting plumbing commands tend to do the right
thing.

Will think more about the documentation and other parts (or if someone
else sends a patch before I can, I won't mind).

Thanks,
Jonathan
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help