Re: Better suggestions when git-am(1) fails

From: Jeff King <peff@peff.net>
Date: Mar 10, 2023, 09:39 UTC
Message-ID: <ZAr6vIOe3WbTIohE@coredump.intra.peff.net>
In-Reply-To: <xmqqedpxq4if.fsf@gitster.g>

On Thu, Mar 09, 2023 at 08:22:00AM -0800, Junio C Hamano wrote:

Show 11 quoted lines

> > The reason is probably that they have set diff.noprefix in their config,
> > and git-format-patch respects that. Which is arguably a bug.
> 
> FWIW, I've always considered it a feature to help projects that
> prefer their patches in -p0 form.  Of course, Git optimized itself
> for the usecase we consider the optimum, i.e. using a/ and b/ prefix
> on the diff generation side, while stripping them with -p1 on the
> applying side.
> 
> I wonder apply.plevel or am.plevel would be a good way to help them
> further?

I doubt they would help, because they imply a constant project workflow. We have seen several reports of "sometimes I get a patch without a prefix, and it doesn't apply. What's going on?". But I don't think anybody asked for "my project doesn't use prefixes, and I am tired of typing -p0".

The more interesting case to me is that the receiver _isn't_ using Git. They are using "patch" or similar, and they expect senders to send them patches without prefixes. And there, diff.noprefix is doing what they want. But I have to wonder if these hypothetical maintainers exist:

  1. I feel like "-p1" was pretty standard even before Git. You'd
     extract two copies of the tarball, one into "foo-1.2.3" and one
     into "foo-1.2.3.orig", and then "diff -Nru" between them to send a
     patch.

  2. It feels weird that a maintainer who isn't using Git would expect a
     lot of contributions from folks who are. And even weirder, that
     they would insist that all of the folks sending patches set
     diff.noprefix.

So I won't say it's not possible (especially in some closed community). But I'm skeptical.

All that said, if "apply" and "am" could automatically figure out and handle "-p0" patches, that would be a useful way to help people. I'm just hesitant because it probably involves some heuristics. E.g., we get "foo/bar", realize that "bar" doesn't exist, but "foo/bar" does. Except that fails if a project does have "bar". And so on.

Show 6 quoted lines

> I am not sure making format-patch _ignore_ diff.src/dst_prefix is a
> good approach.  If we were wiser, we may not have introduced the
> diff.noprefix option, made sure diff.src/dstprefix to be always a
> single level, and kept -p<n> on the application side as an escape
> hatch only to deal with non-Git generated patches.  The opportunity
> to simplify the world that way however we missed 15 years ago X-<.

Yeah, I am as always a little concerned that one person's fix is another one's regression. But it really just seems to that on balance people set diff.noprefix with no thought at all to how it would affect format-patch (in fact, I'd guess 99% of Git users do not use format-patch at all). And then they are surprised (or worse, the receiver is surprised) when it doesn't work.

-Peff