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

From: Nathan Collins <nathan.collins@gmail.com>
Date: May 6, 2014, 19:36 UTC
Message-ID: <CAO8RVvfHVBG6QZ5M1DJK4kOX8GMDrvEw4_L=LsEc4G6E0pP7ug@mail.gmail.com>
In-Reply-To: <xmqqa9aug4my.fsf@gitster.dls.corp.google.com>

On Tue, May 6, 2014 at 11:10 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 7 quoted lines

> Nathan Collins <nathan.collins@gmail.com> writes:
>
>> 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?
>
> "some"?  Do you have exceptions in mind?

As Jonathan pointed out in his first reply, 'git diff-tree' ignores the 'diff.noprefix=true' setting. Compare

  git -c diff.noprefix=true diff HEAD~

with

  git -c diff.noprefix=true diff-tree -p HEAD

(E.g.

   diff <(git -c diff.noprefix=true diff HEAD~) <(git -c
diff.noprefix=true diff-tree -p HEAD)

Show 5 quoted lines

>> But I worry this would just confuse / distract
>> the people that don't have 'diff.noprefix=true' set,
>
> Probably.  But that would suggest that the place to improve the doc
> is for diff.noprefix configuration variable, no?

I don't think that would actually help much in practice. The problem is that a person (like me) that set 'diff.noprefix=true' in their ~/.gitconfig months or years ago is unlikely to do 'man git-config' when 'git apply' fails. Having the warning in 'man git-apply' is better than (only) in 'man git-config', if making 'git apply' smarter is not an option.

Show 7 quoted lines

>> Better I think would be for 'git apply' to be
>> smarter, as you suggest below.
>
> As it is a plumbing command behind "add -p", "am", and friends, I
> would hate to see "git apply" pretend to be smarter than its users.
> When the user tells it to use -p0, it shouldn't guess, and when the
> user tells it to use -p1 by not giving any -p$n, it shouldn't guess.

Is there a non-plumbing command for applying patches not in mailboxes? I don't see how to replace '| git apply --reverse' with '| git am ???' here.

> As long as we make it clear "git apply" without any explicit -p$n
> means the user is telling it to do -p1 in its documentation, I think
> it would be fine.

OK, then how about a smarter error message? Right now I get

  git -c diff.noprefix=true diff HEAD~ | git -c diff.noprefix=true
apply --reverse
  error: Data/Function/Decorator/Memoizer/Unsafe.hs: No such file or directory

vs

  git -c diff.noprefix=true diff HEAD~ | patch --reverse
  can't find file to patch at input line 5
  Perhaps you should have used the -p or --strip option?
  [...]

But 'git apply' could be much more helpful than 'patch' even, since the presence or absence of the 'a/' and 'b/' prefixes in the patch, and the 'diff.noprefix' setting, give Git enough info to be very helpful to the user.

Cheers,

-nathan