From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Alejandro Colomar <alx.manpages@gmail.com>,
Git Mailing List <git@vger.kernel.org>
Subject: Re: Better suggestions when git-am(1) fails
Date: Fri, 10 Mar 2023 04:39:08 -0500 [thread overview]
Message-ID: <ZAr6vIOe3WbTIohE@coredump.intra.peff.net> (raw)
In-Reply-To: <xmqqedpxq4if.fsf@gitster.g>
On Thu, Mar 09, 2023 at 08:22:00AM -0800, Junio C Hamano wrote:
> > 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.
> 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
next prev parent reply other threads:[~2023-03-10 9:40 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-08 20:15 Better suggestions when git-am(1) fails Alejandro Colomar
2023-03-09 3:17 ` Jeff King
2023-03-09 6:06 ` Jeff King
2023-03-09 6:07 ` [PATCH 1/5] diff: factor out src/dst prefix setup Jeff King
2023-03-09 10:50 ` Alejandro Colomar
2023-03-09 6:07 ` [PATCH 2/5] t4013: add tests for diff prefix options Jeff King
2023-03-09 6:09 ` [PATCH 3/5] diff: add --default-prefix option Jeff King
2023-03-09 10:51 ` Alejandro Colomar
2023-03-09 16:31 ` Junio C Hamano
2023-03-10 9:44 ` Jeff King
2023-03-10 17:04 ` Junio C Hamano
2023-03-13 16:43 ` Jeff King
2023-03-13 17:17 ` Junio C Hamano
2023-03-13 17:31 ` Junio C Hamano
2023-03-13 19:54 ` Jeff King
2023-03-09 6:11 ` [PATCH 4/5] format-patch: do not respect diff.noprefix Jeff King
2023-03-09 10:53 ` Alejandro Colomar
2023-03-09 16:41 ` Junio C Hamano
2023-03-10 9:49 ` Jeff King
2023-03-09 6:12 ` [PATCH 5/5] format-patch: add format.noprefix option Jeff King
2023-03-09 17:00 ` Junio C Hamano
2023-03-10 9:51 ` Jeff King
2023-03-09 10:58 ` Better suggestions when git-am(1) fails Alejandro Colomar
2023-03-09 21:53 ` Junio C Hamano
2023-03-10 9:54 ` Jeff King
2023-03-09 16:22 ` Junio C Hamano
2023-03-10 9:39 ` Jeff King [this message]
2023-03-10 16:28 ` Junio C Hamano
2023-03-13 16:37 ` Jeff King
2023-03-13 17:10 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZAr6vIOe3WbTIohE@coredump.intra.peff.net \
--to=peff@peff.net \
--cc=alx.manpages@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).