{"thread":{"id":"59361","subject":"Better suggestions when git-am(1) fails","startedAt":"2023-03-08T20:16:37Z","lastAt":"2023-03-13T19:54:16Z","messageCount":30,"participants":["Alejandro Colomar","Jeff King","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"473218","messageId":"897c200c-afb3-ceb4-bf44-9af651f5feb4@gmail.com","threadId":"59361","inReplyTo":null,"subject":"Better suggestions when git-am(1) fails","fromName":"Alejandro Colomar","fromEmail":"alx.manpages@gmail.com","sentAt":"2023-03-08T20:15:53Z","receivedAt":"2023-03-08T20:16:37Z","isPatch":false,"sender":{"key":"alx.manpages@gmail.com","avatar":null},"body":"Hi,\n\nI had the following error already a few times, when some contributors,\nfor some reason unknown to me, remove the leading path components from\nthe patch. Now I know that the fix is to use -p0, but the first times\nit wasn't obvious. And still I forget about -p0 sometimes and it's\nhard to find in the manual pages. I think it would be good to suggest\nusing it when such an error appears.\n\n\n$ git am -s patches/\\[PATCH\\ 1_2\\]\\ CONTRIBUTING\\:\\ Fix\\ typo\\,\\ there\\ is\\ one\\ active\\ maintainer\\ -\\ Rodrigo\\ Campos\\ \\\\ -\\ 2023-03-08\\ 1622.eml\nApplying: CONTRIBUTING: Fix typo, there is one active maintainer\nerror: git diff header lacks filename information when removing 1 leading pathname component (line 9)\nPatch failed at 0001 CONTRIBUTING: Fix typo, there is one active maintainer\nhint: Use 'git am --show-current-patch=diff' to see the failed patch\nWhen you have resolved this problem, run \"git am --continue\".\nIf you prefer to skip this patch, run \"git am --skip\" instead.\nTo restore the original branch and stop patching, run \"git am --abort\".\nalx@asus5775:~/src/linux/man-pages/man-pages/main$ man git-am\nalx@asus5775:~/src/linux/man-pages/man-pages/main$ git am --show-current-patch=diff\n---\n CONTRIBUTING | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git CONTRIBUTING CONTRIBUTING\nindex 3b4408108..3bb671eca 100644\n--- CONTRIBUTING\n+++ CONTRIBUTING\n@@ -8,7 +8,7 @@ Description\n Mailing list\n The main discussions regarding development of the project, patches,\n bugs, news, doubts, etc. happen on the mailing list. To send an email\n- to the project, send it to both maintainers and CC the mailing list:\n+ to the project, send it to Alejandro and CC the mailing list:\n \n To: Alejandro Colomar \n Cc: \n-- \n2.39.2\n\n\ngit(1) could recommend using `-p` to sort this out. It could go\nfurther and check which level would be needed for the patch to apply,\nbut at the very least it could tell the user which option it wants\nto look for in the documentation.\n\nDoes this make sense to you? I usually find git-am error messages\nquite uninformative.\n\n\nCheers,\n\nAlex\n\n\n-- \n\nGPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5\n"},{"id":"473251","messageId":"ZAlPtxZ/0Z28r5tF@coredump.intra.peff.net","threadId":"59361","inReplyTo":"897c200c-afb3-ceb4-bf44-9af651f5feb4@gmail.com","subject":"Re: Better suggestions when git-am(1) fails","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-03-09T03:17:11Z","receivedAt":"2023-03-09T03:17:16Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Mar 08, 2023 at 09:15:53PM +0100, Alejandro Colomar wrote:\n\n> I had the following error already a few times, when some contributors,\n> for some reason unknown to me, remove the leading path components from\n> the patch.\n\nThe reason is probably that they have set diff.noprefix in their config,\nand git-format-patch respects that. Which is arguably a bug. There's a\nlittle discussion in this message, along with references to some\nprevious discussions:\n\n https://lore.kernel.org/git/ZAWnDUkgO5clf6qu@coredump.intra.peff.net/\n\n> Now I know that the fix is to use -p0, but the first times it wasn't\n> obvious. And still I forget about -p0 sometimes and it's hard to find\n> in the manual pages. I think it would be good to suggest using it\n> when such an error appears.\n\nI agree it may be reasonable to have \"git am\" be more helpful on the\nreceiving side. Hopefully if format-patch is changed then you wouldn't\nsee the situation as often, but it could still happen.\n\n-Peff\n"},{"id":"473254","messageId":"ZAl3bHB9zxjLITgf@coredump.intra.peff.net","threadId":"59361","inReplyTo":"ZAlPtxZ/0Z28r5tF@coredump.intra.peff.net","subject":"Re: Better suggestions when git-am(1) fails","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-03-09T06:06:36Z","receivedAt":"2023-03-09T06:06:42Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Mar 08, 2023 at 10:17:11PM -0500, Jeff King wrote:\n\n> On Wed, Mar 08, 2023 at 09:15:53PM +0100, Alejandro Colomar wrote:\n> \n> > I had the following error already a few times, when some contributors,\n> > for some reason unknown to me, remove the leading path components from\n> > the patch.\n> \n> The reason is probably that they have set diff.noprefix in their config,\n> and git-format-patch respects that. Which is arguably a bug. There's a\n> little discussion in this message, along with references to some\n> previous discussions:\n> \n> https://lore.kernel.org/git/ZAWnDUkgO5clf6qu@coredump.intra.peff.net/\n\nSo here's a patch series which I think should help with the sending\nside. Most of it is just filling in gaps in the code and tests for\ncurrent features. Patch 4 is the actual change. Patch 5 adds an\nequivalent option just for format-patch. I'm not convinced anybody\nreally wants it (which is why I split it out), but it's probably worth\ndoing just in case.\n\n [1/5]: diff: factor out src/dst prefix setup\n [2/5]: t4013: add tests for diff prefix options\n [3/5]: diff: add --default-prefix option\n [4/5]: format-patch: do not respect diff.noprefix\n [5/5]: format-patch: add format.noprefix option\n\n Documentation/config/format.txt | 7 ++++++\n Documentation/diff-options.txt | 5 ++++\n builtin/log.c | 17 +++++++++++++\n diff.c | 33 ++++++++++++++++++++++----\n diff.h | 2 ++\n t/t4013-diff-various.sh | 42 +++++++++++++++++++++++++++++++++\n t/t4014-format-patch.sh | 16 +++++++++++++\n 7 files changed, 117 insertions(+), 5 deletions(-)\n\n-Peff\n"},{"id":"473255","messageId":"ZAl3iqcSL4PODx01@coredump.intra.peff.net","threadId":"59361","inReplyTo":"ZAl3bHB9zxjLITgf@coredump.intra.peff.net","subject":"[PATCH 1/5] diff: factor out src/dst prefix setup","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-03-09T06:07:06Z","receivedAt":"2023-03-09T06:07:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We directly manipulate diffopt's a_prefix and b_prefix to set up either\nthe default \"a/foo\" prefix or the \"--no-prefix\" variant. Although this\nis only a few lines, it's worth pulling these into their own functions.\nThat lets us avoid one repetition already in this patch, but will also\ngive us a cleaner interface for callers which want to tweak this\nsetting.\n\nSigned-off-by: Jeff King \n---\n diff.c | 19 ++++++++++++++-----\n diff.h | 2 ++\n 2 files changed, 16 insertions(+), 5 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 469e18aed20..750d1b1a6c3 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -3374,6 +3374,17 @@ void diff_set_mnemonic_prefix(struct diff_options *options, const char *a, const\n \t\toptions->b_prefix = b;\n }\n \n+void diff_set_noprefix(struct diff_options *options)\n+{\n+\toptions->a_prefix = options->b_prefix = \"\";\n+}\n+\n+void diff_set_default_prefix(struct diff_options *options)\n+{\n+\toptions->a_prefix = \"a/\";\n+\toptions->b_prefix = \"b/\";\n+}\n+\n struct userdiff_driver *get_textconv(struct repository *r,\n \t\t\t\t struct diff_filespec *one)\n {\n@@ -4674,10 +4685,9 @@ void repo_diff_setup(struct repository *r, struct diff_options *options)\n \t\toptions->flags.ignore_untracked_in_submodules = 1;\n \n \tif (diff_no_prefix) {\n-\t\toptions->a_prefix = options->b_prefix = \"\";\n+\t\tdiff_set_noprefix(options);\n \t} else if (!diff_mnemonic_prefix) {\n-\t\toptions->a_prefix = \"a/\";\n-\t\toptions->b_prefix = \"b/\";\n+\t\tdiff_set_default_prefix(options);\n \t}\n \n \toptions->color_moved = diff_color_moved_default;\n@@ -5261,8 +5271,7 @@ static int diff_opt_no_prefix(const struct option *opt,\n \n \tBUG_ON_OPT_NEG(unset);\n \tBUG_ON_OPT_ARG(optarg);\n-\toptions->a_prefix = \"\";\n-\toptions->b_prefix = \"\";\n+\tdiff_set_noprefix(options);\n \treturn 0;\n }\n \ndiff --git a/diff.h b/diff.h\nindex 8d770b1d579..2af10bc5851 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -497,6 +497,8 @@ void diff_tree_combined(const struct object_id *oid, const struct oid_array *par\n void diff_tree_combined_merge(const struct commit *commit, struct rev_info *rev);\n \n void diff_set_mnemonic_prefix(struct diff_options *options, const char *a, const char *b);\n+void diff_set_noprefix(struct diff_options *options);\n+void diff_set_default_prefix(struct diff_options *options);\n \n int diff_can_quit_early(struct diff_options *);\n \n-- \n2.40.0.rc2.537.g928a61c97db\n\n"},{"id":"473256","messageId":"ZAl3sZufzTb2FRP9@coredump.intra.peff.net","threadId":"59361","inReplyTo":"ZAl3bHB9zxjLITgf@coredump.intra.peff.net","subject":"[PATCH 2/5] t4013: add tests for diff prefix options","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-03-09T06:07:45Z","receivedAt":"2023-03-09T06:07:55Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We don't have any specific test coverage of diff's various prefix\noptions. We do incidentally invoke them in a few places, but it's worth\nhaving a more thorough set of tests that covers all of the effects we\nexpect to see, and that the options kick in at the appropriate times.\n\nThis will be especially useful as the next patch adds more options.\n\nSigned-off-by: Jeff King \n---\n t/t4013-diff-various.sh | 32 ++++++++++++++++++++++++++++++++\n 1 file changed, 32 insertions(+)\n\ndiff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh\nindex dfcf3a0aaae..0bc69579898 100755\n--- a/t/t4013-diff-various.sh\n+++ b/t/t4013-diff-various.sh\n@@ -616,4 +616,36 @@ test_expect_success 'diff -I: detect malformed regex' '\n \ttest_i18ngrep \"invalid regex given to -I: \" error\n '\n \n+# check_prefix \n+# check only lines with paths to avoid dependency on exact oid/contents\n+check_prefix () {\n+\tgrep -E '^(diff|---|\\+\\+\\+) ' \"$1\" >actual.paths &&\n+\tcat >expect <<-EOF &&\n+\tdiff --git $2 $3\n+\t--- $2\n+\t+++ $3\n+\tEOF\n+\ttest_cmp expect actual.paths\n+}\n+\n+test_expect_success 'diff-files does not respect diff.noprefix' '\n+\tgit -c diff.noprefix diff-files -p >actual &&\n+\tcheck_prefix actual a/file0 b/file0\n+'\n+\n+test_expect_success 'diff-files respects --no-prefix' '\n+\tgit diff-files -p --no-prefix >actual &&\n+\tcheck_prefix actual file0 file0\n+'\n+\n+test_expect_success 'diff respects diff.noprefix' '\n+\tgit -c diff.noprefix diff >actual &&\n+\tcheck_prefix actual file0 file0\n+'\n+\n+test_expect_success 'diff respects diff.mnemonicprefix' '\n+\tgit -c diff.mnemonicprefix diff >actual &&\n+\tcheck_prefix actual i/file0 w/file0\n+'\n+\n test_done\n-- \n2.40.0.rc2.537.g928a61c97db\n\n"},{"id":"473257","messageId":"ZAl4MkWVV8fr+3fO@coredump.intra.peff.net","threadId":"59361","inReplyTo":"ZAl3bHB9zxjLITgf@coredump.intra.peff.net","subject":"[PATCH 3/5] diff: add --default-prefix option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-03-09T06:09:54Z","receivedAt":"2023-03-09T06:09:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"You can change the output of prefixes with diff.noprefix and\ndiff.mnemonicprefix, but there's no easy way to override them from the\ncommand-line. We do have \"--no-prefix\", but there's no way to get back\nto the default prefix. So let's add an option to do that.\n\nSigned-off-by: Jeff King \n---\nThis isn't strictly necessary for the series, but it seemed like a gap.\nYou can always do:\n\n git -c diff.noprefix=false -c diff.mnemonicprefix=false ...\n\nbut that's rather a mouthful.\n\nNote that there isn't a command-line equivalent for mnemonicprefix,\neither. I don't think it's worth adding unless somebody really wants it.\n\n Documentation/diff-options.txt | 5 +++++\n diff.c | 14 ++++++++++++++\n t/t4013-diff-various.sh | 10 ++++++++++\n 3 files changed, 29 insertions(+)\n\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex 7d73e976d99..08ab86189a7 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -852,6 +852,11 @@ endif::git-format-patch[]\n --no-prefix::\n \tDo not show any source or destination prefix.\n \n+--default-prefix::\n+\tUse the default source and destination prefixes (\"a/\" and \"b/\").\n+\tThis is usually the default already, but may be used to override\n+\tconfig such as `diff.noprefix`.\n+\n --line-prefix=::\n \tPrepend an additional prefix to every line of output.\n \ndiff --git a/diff.c b/diff.c\nindex 750d1b1a6c3..b322e319ff3 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -5275,6 +5275,17 @@ static int diff_opt_no_prefix(const struct option *opt,\n \treturn 0;\n }\n \n+static int diff_opt_default_prefix(const struct option *opt,\n+\t\t\t\t const char *optarg, int unset)\n+{\n+\tstruct diff_options *options = opt->value;\n+\n+\tBUG_ON_OPT_NEG(unset);\n+\tBUG_ON_OPT_ARG(optarg);\n+\tdiff_set_default_prefix(options);\n+\treturn 0;\n+}\n+\n static enum parse_opt_result diff_opt_output(struct parse_opt_ctx_t *ctx,\n \t\t\t\t\t const struct option *opt,\n \t\t\t\t\t const char *arg, int unset)\n@@ -5564,6 +5575,9 @@ struct option *add_diff_options(const struct option *opts,\n \t\tOPT_CALLBACK_F(0, \"no-prefix\", options, NULL,\n \t\t\t N_(\"do not show any source or destination prefix\"),\n \t\t\t PARSE_OPT_NONEG | PARSE_OPT_NOARG, diff_opt_no_prefix),\n+\t\tOPT_CALLBACK_F(0, \"default-prefix\", options, NULL,\n+\t\t\t N_(\"use default prefixes a/ and b/\"),\n+\t\t\t PARSE_OPT_NONEG | PARSE_OPT_NOARG, diff_opt_default_prefix),\n \t\tOPT_INTEGER_F(0, \"inter-hunk-context\", &options->interhunkcontext,\n \t\t\t N_(\"show context between diff hunks up to the specified number of lines\"),\n \t\t\t PARSE_OPT_NONEG),\ndiff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh\nindex 0bc69579898..5de1d190759 100755\n--- a/t/t4013-diff-various.sh\n+++ b/t/t4013-diff-various.sh\n@@ -643,9 +643,19 @@ test_expect_success 'diff respects diff.noprefix' '\n \tcheck_prefix actual file0 file0\n '\n \n+test_expect_success 'diff --default-prefix overrides diff.noprefix' '\n+\tgit -c diff.noprefix diff --default-prefix >actual &&\n+\tcheck_prefix actual a/file0 b/file0\n+'\n+\n test_expect_success 'diff respects diff.mnemonicprefix' '\n \tgit -c diff.mnemonicprefix diff >actual &&\n \tcheck_prefix actual i/file0 w/file0\n '\n \n+test_expect_success 'diff --default-prefix overrides diff.mnemonicprefix' '\n+\tgit -c diff.mnemonicprefix diff --default-prefix >actual &&\n+\tcheck_prefix actual a/file0 b/file0\n+'\n+\n test_done\n-- \n2.40.0.rc2.537.g928a61c97db\n\n"},{"id":"473258","messageId":"ZAl4pZV08a6Bgoip@coredump.intra.peff.net","threadId":"59361","inReplyTo":"ZAl3bHB9zxjLITgf@coredump.intra.peff.net","subject":"[PATCH 4/5] format-patch: do not respect diff.noprefix","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-03-09T06:11:49Z","receivedAt":"2023-03-09T06:11:58Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The output of format-patch respects diff.noprefix, but this usually ends\nup being a hassle for people receiving the patch, as they have to\nmanually specify \"-p0\" in order to apply it.\n\nI don't think there was any specific intention for it to behave this\nway. The noprefix option is handled by git_diff_ui_config(), and\nformat-patch exists in a gray area between plumbing and porcelain.\nPeople do look at the output, and we'd expect it to colorize things,\nrespect their choice of algorithm, and so on. But this particular option\ncreates problems for the receiver (in theory so does diff.mnemonicprefix,\nbut since we are always formatting commits, the mnemonic prefixes will\nalways be \"a/\" and \"b/\").\n\nSo let's disable it. The slight downsides are:\n\n - people who have set diff.noprefix presumably like to see their\n patches without prefixes. If they use format-patch to review their\n series, they'll see prefixes. On the other hand, it is probably a\n good idea for them to look at what will actually get sent out.\n\n We could try to play games here with \"is stdout a tty\", as we do for\n color. But that's not a completely reliable signal, and it's\n probably not worth the trouble. If you want to see the patch with\n the usual bells and whistles, then you are better off using \"git\n log\" or \"git show\".\n\n - if a project really does have a workflow that likes prefix-less\n patches, and the receiver is prepared to use \"-p0\", then the sender\n now has to manually say \"--no-prefix\" for each format-patch\n invocation. That doesn't seem _too_ terrible given that the receiver\n has to manually say \"-p0\" for each git-am invocation.\n\nSigned-off-by: Jeff King \n---\n builtin/log.c | 9 +++++++++\n t/t4014-format-patch.sh | 5 +++++\n 2 files changed, 14 insertions(+)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex a70fba198f9..eaf511aab86 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1085,6 +1085,15 @@ static int git_format_config(const char *var, const char *value, void *cb)\n \t\treturn 0;\n \t}\n \n+\t/*\n+\t * ignore some porcelain config which would otherwise be parsed by\n+\t * git_diff_ui_config(), via git_log_config(); we can't just avoid\n+\t * diff_ui_config completely, because we do care about some ui options\n+\t * like color.\n+\t */\n+\tif (!strcmp(var, \"diff.noprefix\"))\n+\t\treturn 0;\n+\n \treturn git_log_config(var, value, cb);\n }\n \ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex f3313b8c58f..f5a41fd47ed 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -2386,4 +2386,9 @@ test_expect_success 'interdiff: solo-patch' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'format-patch does not respect diff.noprefix' '\n+\tgit -c diff.noprefix format-patch -1 --stdout >actual &&\n+\tgrep \"^--- a/blorp\" actual\n+'\n+\n test_done\n-- \n2.40.0.rc2.537.g928a61c97db\n\n"},{"id":"473259","messageId":"ZAl41V7n77ej844x@coredump.intra.peff.net","threadId":"59361","inReplyTo":"ZAl3bHB9zxjLITgf@coredump.intra.peff.net","subject":"[PATCH 5/5] format-patch: add format.noprefix option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-03-09T06:12:37Z","receivedAt":"2023-03-09T06:12:44Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The previous commit dropped support for diff.noprefix in format-patch.\nWhile this will do the right thing in most cases (where sending patches\nwithout a prefix was an accidental side effect of the sender preferring\nto see their local patches without prefixes), it left no good option for\na project or workflow where you really do want to send patches without\nprefixes. You'd be stuck using \"--no-prefix\" for every invocation.\n\nSo let's add a config option specific to format-patch that enables this\nbehavior. That gives people who have such a workflow a way to get what\nthey want, but makes it hard to accidentally trigger it.\n\nA more backwards-compatible way of doing the transition would be to have\nformat.noprefix default to diff.noprefix when it's not set. But that\ndoesn't really help the \"accidental\" problem; people would have to\nmanually set format.noprefix=false. And it's unlikely that anybody\nreally wants format.noprefix=true in the first place. I'm adding it here\nmostly as an escape hatch, not because anybody has expressed any\ninterest in it.\n\nSigned-off-by: Jeff King \n---\n Documentation/config/format.txt | 7 +++++++\n builtin/log.c | 8 ++++++++\n t/t4014-format-patch.sh | 11 +++++++++++\n 3 files changed, 26 insertions(+)\n\ndiff --git a/Documentation/config/format.txt b/Documentation/config/format.txt\nindex 73678d88a1d..8cf6f00d936 100644\n--- a/Documentation/config/format.txt\n+++ b/Documentation/config/format.txt\n@@ -144,3 +144,10 @@ will only show notes from `refs/notes/bar`.\n format.mboxrd::\n \tA boolean value which enables the robust \"mboxrd\" format when\n \t`--stdout` is in use to escape \"^>+From \" lines.\n+\n+format.noprefix::\n+\tIf set, do not show any source or destination prefix in patches.\n+\tThis is equivalent to the `diff.noprefix` option used by `git\n+\tdiff` (but which is not respected by `format-patch`). Note that\n+\tby setting this, the receiver of any patches you generate will\n+\thave to apply them using the `-p0` option.\ndiff --git a/builtin/log.c b/builtin/log.c\nindex eaf511aab86..b1f59062f40 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -56,6 +56,7 @@ static int stdout_mboxrd;\n static const char *fmt_patch_subject_prefix = \"PATCH\";\n static int fmt_patch_name_max = FORMAT_PATCH_NAME_MAX_DEFAULT;\n static const char *fmt_pretty;\n+static int format_no_prefix;\n \n static const char * const builtin_log_usage[] = {\n \tN_(\"git log [] [] [[--] ...]\"),\n@@ -1084,6 +1085,10 @@ static int git_format_config(const char *var, const char *value, void *cb)\n \t\tstdout_mboxrd = git_config_bool(var, value);\n \t\treturn 0;\n \t}\n+\tif (!strcmp(var, \"format.noprefix\")) {\n+\t\tformat_no_prefix = 1;\n+\t\treturn 0;\n+\t}\n \n \t/*\n \t * ignore some porcelain config which would otherwise be parsed by\n@@ -2002,6 +2007,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \ts_r_opt.def = \"HEAD\";\n \ts_r_opt.revarg_opt = REVARG_COMMITTISH;\n \n+\tif (format_no_prefix)\n+\t\tdiff_set_noprefix(&rev.diffopt);\n+\n \tif (default_attach) {\n \t\trev.mime_boundary = default_attach;\n \t\trev.no_inline = 1;\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex f5a41fd47ed..2711fd09ca0 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -2391,4 +2391,15 @@ test_expect_success 'format-patch does not respect diff.noprefix' '\n \tgrep \"^--- a/blorp\" actual\n '\n \n+test_expect_success 'format-patch respects format.noprefix' '\n+\tgit -c format.noprefix format-patch -1 --stdout >actual &&\n+\tgrep \"^--- blorp\" actual\n+'\n+\n+test_expect_success 'format-patch --default-prefix overrides format.noprefix' '\n+\tgit -c format.noprefix \\\n+\t\tformat-patch -1 --default-prefix --stdout >actual &&\n+\tgrep \"^--- a/blorp\" actual\n+'\n+\n test_done\n-- \n2.40.0.rc2.537.g928a61c97db\n"},{"id":"473273","messageId":"62c59063-e168-57b0-43d5-be7f007c2d37@gmail.com","threadId":"59361","inReplyTo":"ZAl3iqcSL4PODx01@coredump.intra.peff.net","subject":"Re: [PATCH 1/5] diff: factor out src/dst prefix setup","fromName":"Alejandro Colomar","fromEmail":"alx.manpages@gmail.com","sentAt":"2023-03-09T10:50:37Z","receivedAt":"2023-03-09T10:50:52Z","isPatch":true,"sender":{"key":"alx.manpages@gmail.com","avatar":null},"body":"\n\nOn 3/9/23 07:07, Jeff King wrote:\n> We directly manipulate diffopt's a_prefix and b_prefix to set up either\n> the default \"a/foo\" prefix or the \"--no-prefix\" variant. Although this\n> is only a few lines, it's worth pulling these into their own functions.\n> That lets us avoid one repetition already in this patch, but will also\n> give us a cleaner interface for callers which want to tweak this\n> setting.\n> \n> Signed-off-by: Jeff King \n\nAcked-by: Alejandro Colomar \n\n> ---\n> diff.c | 19 ++++++++++++++-----\n> diff.h | 2 ++\n> 2 files changed, 16 insertions(+), 5 deletions(-)\n> \n> diff --git a/diff.c b/diff.c\n> index 469e18aed20..750d1b1a6c3 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -3374,6 +3374,17 @@ void diff_set_mnemonic_prefix(struct diff_options *options, const char *a, const\n> \t\toptions->b_prefix = b;\n> }\n> \n> +void diff_set_noprefix(struct diff_options *options)\n> +{\n> +\toptions->a_prefix = options->b_prefix = \"\";\n> +}\n> +\n> +void diff_set_default_prefix(struct diff_options *options)\n> +{\n> +\toptions->a_prefix = \"a/\";\n> +\toptions->b_prefix = \"b/\";\n> +}\n> +\n> struct userdiff_driver *get_textconv(struct repository *r,\n> \t\t\t\t struct diff_filespec *one)\n> {\n> @@ -4674,10 +4685,9 @@ void repo_diff_setup(struct repository *r, struct diff_options *options)\n> \t\toptions->flags.ignore_untracked_in_submodules = 1;\n> \n> \tif (diff_no_prefix) {\n> -\t\toptions->a_prefix = options->b_prefix = \"\";\n> +\t\tdiff_set_noprefix(options);\n> \t} else if (!diff_mnemonic_prefix) {\n> -\t\toptions->a_prefix = \"a/\";\n> -\t\toptions->b_prefix = \"b/\";\n> +\t\tdiff_set_default_prefix(options);\n> \t}\n> \n> \toptions->color_moved = diff_color_moved_default;\n> @@ -5261,8 +5271,7 @@ static int diff_opt_no_prefix(const struct option *opt,\n> \n> \tBUG_ON_OPT_NEG(unset);\n> \tBUG_ON_OPT_ARG(optarg);\n> -\toptions->a_prefix = \"\";\n> -\toptions->b_prefix = \"\";\n> +\tdiff_set_noprefix(options);\n> \treturn 0;\n> }\n> \n> diff --git a/diff.h b/diff.h\n> index 8d770b1d579..2af10bc5851 100644\n> --- a/diff.h\n> +++ b/diff.h\n> @@ -497,6 +497,8 @@ void diff_tree_combined(const struct object_id *oid, const struct oid_array *par\n> void diff_tree_combined_merge(const struct commit *commit, struct rev_info *rev);\n> \n> void diff_set_mnemonic_prefix(struct diff_options *options, const char *a, const char *b);\n> +void diff_set_noprefix(struct diff_options *options);\n> +void diff_set_default_prefix(struct diff_options *options);\n> \n> int diff_can_quit_early(struct diff_options *);\n> \n\n-- \n\nGPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5\n"},{"id":"473274","messageId":"33be045b-0ac7-ba88-bec7-d805a90580d6@gmail.com","threadId":"59361","inReplyTo":"ZAl4MkWVV8fr+3fO@coredump.intra.peff.net","subject":"Re: [PATCH 3/5] diff: add --default-prefix option","fromName":"Alejandro Colomar","fromEmail":"alx.manpages@gmail.com","sentAt":"2023-03-09T10:51:05Z","receivedAt":"2023-03-09T10:51:14Z","isPatch":true,"sender":{"key":"alx.manpages@gmail.com","avatar":null},"body":"\n\nOn 3/9/23 07:09, Jeff King wrote:\n> You can change the output of prefixes with diff.noprefix and\n> diff.mnemonicprefix, but there's no easy way to override them from the\n> command-line. We do have \"--no-prefix\", but there's no way to get back\n> to the default prefix. So let's add an option to do that.\n> \n> Signed-off-by: Jeff King \n\nAcked-by: Alejandro Colomar \n\n> ---\n> This isn't strictly necessary for the series, but it seemed like a gap.\n> You can always do:\n> \n> git -c diff.noprefix=false -c diff.mnemonicprefix=false ...\n> \n> but that's rather a mouthful.\n> \n> Note that there isn't a command-line equivalent for mnemonicprefix,\n> either. I don't think it's worth adding unless somebody really wants it.\n> \n> Documentation/diff-options.txt | 5 +++++\n> diff.c | 14 ++++++++++++++\n> t/t4013-diff-various.sh | 10 ++++++++++\n> 3 files changed, 29 insertions(+)\n> \n> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\n> index 7d73e976d99..08ab86189a7 100644\n> --- a/Documentation/diff-options.txt\n> +++ b/Documentation/diff-options.txt\n> @@ -852,6 +852,11 @@ endif::git-format-patch[]\n> --no-prefix::\n> \tDo not show any source or destination prefix.\n> \n> +--default-prefix::\n> +\tUse the default source and destination prefixes (\"a/\" and \"b/\").\n> +\tThis is usually the default already, but may be used to override\n> +\tconfig such as `diff.noprefix`.\n> +\n> --line-prefix=::\n> \tPrepend an additional prefix to every line of output.\n> \n> diff --git a/diff.c b/diff.c\n> index 750d1b1a6c3..b322e319ff3 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -5275,6 +5275,17 @@ static int diff_opt_no_prefix(const struct option *opt,\n> \treturn 0;\n> }\n> \n> +static int diff_opt_default_prefix(const struct option *opt,\n> +\t\t\t\t const char *optarg, int unset)\n> +{\n> +\tstruct diff_options *options = opt->value;\n> +\n> +\tBUG_ON_OPT_NEG(unset);\n> +\tBUG_ON_OPT_ARG(optarg);\n> +\tdiff_set_default_prefix(options);\n> +\treturn 0;\n> +}\n> +\n> static enum parse_opt_result diff_opt_output(struct parse_opt_ctx_t *ctx,\n> \t\t\t\t\t const struct option *opt,\n> \t\t\t\t\t const char *arg, int unset)\n> @@ -5564,6 +5575,9 @@ struct option *add_diff_options(const struct option *opts,\n> \t\tOPT_CALLBACK_F(0, \"no-prefix\", options, NULL,\n> \t\t\t N_(\"do not show any source or destination prefix\"),\n> \t\t\t PARSE_OPT_NONEG | PARSE_OPT_NOARG, diff_opt_no_prefix),\n> +\t\tOPT_CALLBACK_F(0, \"default-prefix\", options, NULL,\n> +\t\t\t N_(\"use default prefixes a/ and b/\"),\n> +\t\t\t PARSE_OPT_NONEG | PARSE_OPT_NOARG, diff_opt_default_prefix),\n> \t\tOPT_INTEGER_F(0, \"inter-hunk-context\", &options->interhunkcontext,\n> \t\t\t N_(\"show context between diff hunks up to the specified number of lines\"),\n> \t\t\t PARSE_OPT_NONEG),\n> diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh\n> index 0bc69579898..5de1d190759 100755\n> --- a/t/t4013-diff-various.sh\n> +++ b/t/t4013-diff-various.sh\n> @@ -643,9 +643,19 @@ test_expect_success 'diff respects diff.noprefix' '\n> \tcheck_prefix actual file0 file0\n> '\n> \n> +test_expect_success 'diff --default-prefix overrides diff.noprefix' '\n> +\tgit -c diff.noprefix diff --default-prefix >actual &&\n> +\tcheck_prefix actual a/file0 b/file0\n> +'\n> +\n> test_expect_success 'diff respects diff.mnemonicprefix' '\n> \tgit -c diff.mnemonicprefix diff >actual &&\n> \tcheck_prefix actual i/file0 w/file0\n> '\n> \n> +test_expect_success 'diff --default-prefix overrides diff.mnemonicprefix' '\n> +\tgit -c diff.mnemonicprefix diff --default-prefix >actual &&\n> +\tcheck_prefix actual a/file0 b/file0\n> +'\n> +\n> test_done\n\n-- \n\nGPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5\n"},{"id":"473275","messageId":"48706417-c184-fdd8-e3f4-edfd582b4358@gmail.com","threadId":"59361","inReplyTo":"ZAl4pZV08a6Bgoip@coredump.intra.peff.net","subject":"Re: [PATCH 4/5] format-patch: do not respect diff.noprefix","fromName":"Alejandro Colomar","fromEmail":"alx.manpages@gmail.com","sentAt":"2023-03-09T10:53:00Z","receivedAt":"2023-03-09T10:53:31Z","isPatch":true,"sender":{"key":"alx.manpages@gmail.com","avatar":null},"body":"\n\nOn 3/9/23 07:11, Jeff King wrote:\n> The output of format-patch respects diff.noprefix, but this usually ends\n> up being a hassle for people receiving the patch, as they have to\n> manually specify \"-p0\" in order to apply it.\n> \n> I don't think there was any specific intention for it to behave this\n> way. The noprefix option is handled by git_diff_ui_config(), and\n> format-patch exists in a gray area between plumbing and porcelain.\n> People do look at the output, and we'd expect it to colorize things,\n> respect their choice of algorithm, and so on. But this particular option\n> creates problems for the receiver (in theory so does diff.mnemonicprefix,\n> but since we are always formatting commits, the mnemonic prefixes will\n> always be \"a/\" and \"b/\").\n> \n> So let's disable it. The slight downsides are:\n> \n> - people who have set diff.noprefix presumably like to see their\n> patches without prefixes. If they use format-patch to review their\n> series, they'll see prefixes. On the other hand, it is probably a\n> good idea for them to look at what will actually get sent out.\n> \n> We could try to play games here with \"is stdout a tty\", as we do for\n> color. But that's not a completely reliable signal, and it's\n> probably not worth the trouble. If you want to see the patch with\n> the usual bells and whistles, then you are better off using \"git\n> log\" or \"git show\".\n> \n> - if a project really does have a workflow that likes prefix-less\n> patches, and the receiver is prepared to use \"-p0\", then the sender\n> now has to manually say \"--no-prefix\" for each format-patch\n> invocation. That doesn't seem _too_ terrible given that the receiver\n> has to manually say \"-p0\" for each git-am invocation.\n> \n> Signed-off-by: Jeff King \n\nAcked-by: Alejandro Colomar \n\n> ---\n> builtin/log.c | 9 +++++++++\n> t/t4014-format-patch.sh | 5 +++++\n> 2 files changed, 14 insertions(+)\n> \n> diff --git a/builtin/log.c b/builtin/log.c\n> index a70fba198f9..eaf511aab86 100644\n> --- a/builtin/log.c\n> +++ b/builtin/log.c\n> @@ -1085,6 +1085,15 @@ static int git_format_config(const char *var, const char *value, void *cb)\n> \t\treturn 0;\n> \t}\n> \n> +\t/*\n> +\t * ignore some porcelain config which would otherwise be parsed by\n> +\t * git_diff_ui_config(), via git_log_config(); we can't just avoid\n> +\t * diff_ui_config completely, because we do care about some ui options\n> +\t * like color.\n> +\t */\n> +\tif (!strcmp(var, \"diff.noprefix\"))\n> +\t\treturn 0;\n> +\n> \treturn git_log_config(var, value, cb);\n> }\n> \n> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\n> index f3313b8c58f..f5a41fd47ed 100755\n> --- a/t/t4014-format-patch.sh\n> +++ b/t/t4014-format-patch.sh\n> @@ -2386,4 +2386,9 @@ test_expect_success 'interdiff: solo-patch' '\n> \ttest_cmp expect actual\n> '\n> \n> +test_expect_success 'format-patch does not respect diff.noprefix' '\n> +\tgit -c diff.noprefix format-patch -1 --stdout >actual &&\n> +\tgrep \"^--- a/blorp\" actual\n> +'\n> +\n> test_done\n\n-- \n\nGPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5\n"},{"id":"473276","messageId":"82d78a22-76d5-28fd-e87e-e90e9ddc36e1@gmail.com","threadId":"59361","inReplyTo":"ZAl3bHB9zxjLITgf@coredump.intra.peff.net","subject":"Re: Better suggestions when git-am(1) fails","fromName":"Alejandro Colomar","fromEmail":"alx.manpages@gmail.com","sentAt":"2023-03-09T10:58:00Z","receivedAt":"2023-03-09T11:02:05Z","isPatch":false,"sender":{"key":"alx.manpages@gmail.com","avatar":null},"body":"Hi Jeff,\n\nOn 3/9/23 07:06, Jeff King wrote:\n> On Wed, Mar 08, 2023 at 10:17:11PM -0500, Jeff King wrote:\n> \n>> On Wed, Mar 08, 2023 at 09:15:53PM +0100, Alejandro Colomar wrote:\n>>\n>>> I had the following error already a few times, when some contributors,\n>>> for some reason unknown to me, remove the leading path components from\n>>> the patch.\n>>\n>> The reason is probably that they have set diff.noprefix in their config,\n>> and git-format-patch respects that. Which is arguably a bug. There's a\n>> little discussion in this message, along with references to some\n>> previous discussions:\n>>\n>> https://lore.kernel.org/git/ZAWnDUkgO5clf6qu@coredump.intra.peff.net/\n> \n> So here's a patch series which I think should help with the sending\n> side. Most of it is just filling in gaps in the code and tests for\n> current features. Patch 4 is the actual change. Patch 5 adds an\n> equivalent option just for format-patch. I'm not convinced anybody\n> really wants it (which is why I split it out), but it's probably worth\n> doing just in case.\n\nThanks for the rapid patch set :)\n\n> \n> [1/5]: diff: factor out src/dst prefix setup\n> [2/5]: t4013: add tests for diff prefix options\n> [3/5]: diff: add --default-prefix option\n> [4/5]: format-patch: do not respect diff.noprefix\n> [5/5]: format-patch: add format.noprefix option\n\n1, 3, and 4 LGTM. I'm not used to your tests, so can't really check\nwhat 2 does without further reading, and I'm not sure 5 is useful.\n\nBTW, I'll probably report a few more things I don't like from\ngit-am(1)'s error reports, whenever I find them again. ;)\n\nCheers,\n\nAlex\n\n> \n> Documentation/config/format.txt | 7 ++++++\n> Documentation/diff-options.txt | 5 ++++\n> builtin/log.c | 17 +++++++++++++\n> diff.c | 33 ++++++++++++++++++++++----\n> diff.h | 2 ++\n> t/t4013-diff-various.sh | 42 +++++++++++++++++++++++++++++++++\n> t/t4014-format-patch.sh | 16 +++++++++++++\n> 7 files changed, 117 insertions(+), 5 deletions(-)\n> \n> -Peff\n\n-- \n\nGPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5\n"},{"id":"473290","messageId":"xmqqedpxq4if.fsf@gitster.g","threadId":"59361","inReplyTo":"ZAlPtxZ/0Z28r5tF@coredump.intra.peff.net","subject":"Re: Better suggestions when git-am(1) fails","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-03-09T16:22:00Z","receivedAt":"2023-03-09T16:31:14Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Wed, Mar 08, 2023 at 09:15:53PM +0100, Alejandro Colomar wrote:\n>\n>> I had the following error already a few times, when some contributors,\n>> for some reason unknown to me, remove the leading path components from\n>> the patch.\n>\n> The reason is probably that they have set diff.noprefix in their config,\n> and git-format-patch respects that. Which is arguably a bug.\n\nFWIW, I've always considered it a feature to help projects that\nprefer their patches in -p0 form. Of course, Git optimized itself\nfor the usecase we consider the optimum, i.e. using a/ and b/ prefix\non the diff generation side, while stripping them with -p1 on the\napplying side.\n\nI wonder apply.plevel or am.plevel would be a good way to help them\nfurther?\n\nI am not sure making format-patch _ignore_ diff.src/dst_prefix is a\ngood approach. If we were wiser, we may not have introduced the\ndiff.noprefix option, made sure diff.src/dstprefix to be always a\nsingle level, and kept -p on the application side as an escape\nhatch only to deal with non-Git generated patches. The opportunity\nto simplify the world that way however we missed 15 years ago X-<.\n"},{"id":"473291","messageId":"xmqq5yb9q42e.fsf@gitster.g","threadId":"59361","inReplyTo":"ZAl4MkWVV8fr+3fO@coredump.intra.peff.net","subject":"Re: [PATCH 3/5] diff: add --default-prefix option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-03-09T16:31:37Z","receivedAt":"2023-03-09T16:42:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> This isn't strictly necessary for the series, but it seemed like a gap.\n> You can always do:\n>\n> git -c diff.noprefix=false -c diff.mnemonicprefix=false ...\n>\n> but that's rather a mouthful.\n\nor \"git diff --src-prefix=a/ --dst-prefix=b/\"\n\n>\n> Note that there isn't a command-line equivalent for mnemonicprefix,\n> either. I don't think it's worth adding unless somebody really wants it.\n\nI don't either. We already have src-prefix and dst-prefix.\n\n> +--default-prefix::\n> +\tUse the default source and destination prefixes (\"a/\" and \"b/\").\n> +\tThis is usually the default already, but may be used to override\n> +\tconfig such as `diff.noprefix`.\n\nOK.\n\n> +static int diff_opt_default_prefix(const struct option *opt,\n> +\t\t\t\t const char *optarg, int unset)\n> +{\n> +\tstruct diff_options *options = opt->value;\n> +\n> +\tBUG_ON_OPT_NEG(unset);\n> +\tBUG_ON_OPT_ARG(optarg);\n\nOK. It is a bit unsatisfactory that we already said this does not\ntake negative form or any argument in the option[] array, and still\nhave to do this, but that is completely outside the topic of this\nseries.\n\n> +\tdiff_set_default_prefix(options);\n> +\treturn 0;\n> +}\n> +\n> static enum parse_opt_result diff_opt_output(struct parse_opt_ctx_t *ctx,\n> \t\t\t\t\t const struct option *opt,\n> \t\t\t\t\t const char *arg, int unset)\n> @@ -5564,6 +5575,9 @@ struct option *add_diff_options(const struct option *opts,\n> \t\tOPT_CALLBACK_F(0, \"no-prefix\", options, NULL,\n> \t\t\t N_(\"do not show any source or destination prefix\"),\n> \t\t\t PARSE_OPT_NONEG | PARSE_OPT_NOARG, diff_opt_no_prefix),\n> +\t\tOPT_CALLBACK_F(0, \"default-prefix\", options, NULL,\n> +\t\t\t N_(\"use default prefixes a/ and b/\"),\n> +\t\t\t PARSE_OPT_NONEG | PARSE_OPT_NOARG, diff_opt_default_prefix),\n> \t\tOPT_INTEGER_F(0, \"inter-hunk-context\", &options->interhunkcontext,\n> \t\t\t N_(\"show context between diff hunks up to the specified number of lines\"),\n> \t\t\t PARSE_OPT_NONEG),\n\nThanks.\n"},{"id":"473292","messageId":"xmqqy1o5op1i.fsf@gitster.g","threadId":"59361","inReplyTo":"ZAl4pZV08a6Bgoip@coredump.intra.peff.net","subject":"Re: [PATCH 4/5] format-patch: do not respect diff.noprefix","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-03-09T16:41:29Z","receivedAt":"2023-03-09T16:51:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> - if a project really does have a workflow that likes prefix-less\n> patches, and the receiver is prepared to use \"-p0\", then the sender\n> now has to manually say \"--no-prefix\" for each format-patch\n> invocation. That doesn't seem _too_ terrible given that the receiver\n> has to manually say \"-p0\" for each git-am invocation.\n\nIt does seem very terrible if any existing projects do use the\nworkflow, as their receivers need to change their workflow, though.\n\nBut we can declare that we do not care about such projects that do\nnot honor our -p1 worldview, and I have no objection to this change\nif we can have list consensus for us to go in that direction.\n\nColored patches, by the way, cannot be applied, so perhaps we should\ndisable ui_config altogether, on the other hand? I dunno.\n\nThanks, queued.\n"},{"id":"473293","messageId":"xmqq5yb9dfn2.fsf@gitster.g","threadId":"59361","inReplyTo":"ZAl41V7n77ej844x@coredump.intra.peff.net","subject":"Re: [PATCH 5/5] format-patch: add format.noprefix option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-03-09T17:00:01Z","receivedAt":"2023-03-09T17:05:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> A more backwards-compatible way of doing the transition would be to have\n> format.noprefix default to diff.noprefix when it's not set. But that\n> doesn't really help the \"accidental\" problem; people would have to\n> manually set format.noprefix=false. And it's unlikely that anybody\n> really wants format.noprefix=true in the first place. I'm adding it here\n> mostly as an escape hatch, not because anybody has expressed any\n> interest in it.\n\nI tend to agree that this is closing the barn door after the horse\nescaped for projects who did use diff.noprefix because it is their\npreference to exchange prefix-free patches.\n\nThe other direction we could go is to tie the default p-value\nspecified centrally for both producer and consumer. If the project\nwants no-prefix patches so much that the contributors use it in\ntheir own diff by setting diff.noprefix in their configuration, the\nproject would be perfectly happy if format-patch sent prefix-less\npatches honoring the configuration, and if apply took prefix-less\npatches honoring the configuration, all honoring diff.noprefix.\n\nBut that is the other extreme.\n\nI wonder if the consumer side should be made configurable for\ncompleteness, though.\n\nHere is how \"apply.pValue\" configuration variable would look like.\nProjects whose members want diff.noprefix set can standardise on\nusing that and then receiving end configured to match with this.\n\n---\n apply.c | 5 +++++\n cache.h | 1 +\n environment.c | 1 +\n 3 files changed, 7 insertions(+)\n\ndiff --git c/apply.c w/apply.c\nindex 5cc5479c9c..20645fc9af 100644\n--- c/apply.c\n+++ w/apply.c\n@@ -33,6 +33,7 @@ static void git_apply_config(void)\n {\n \tgit_config_get_string(\"apply.whitespace\", &apply_default_whitespace);\n \tgit_config_get_string(\"apply.ignorewhitespace\", &apply_default_ignorewhitespace);\n+\tgit_config_get_int(\"apply.pValue\", &apply_default_p_value);\n \tgit_config(git_xmerge_config, NULL);\n }\n \n@@ -112,6 +113,10 @@ int init_apply_state(struct apply_state *state,\n \t\treturn -1;\n \tif (apply_default_ignorewhitespace && parse_ignorewhitespace_option(state, apply_default_ignorewhitespace))\n \t\treturn -1;\n+\tif (0 <= apply_default_p_value) {\n+\t\tstate->p_value = apply_default_p_value;\n+\t\tstate->p_value_known = 1;\n+\t}\n \treturn 0;\n }\n \ndiff --git c/cache.h w/cache.h\nindex 12789903e8..05efa9dc52 100644\n--- c/cache.h\n+++ w/cache.h\n@@ -967,6 +967,7 @@ extern int assume_unchanged;\n extern int prefer_symlink_refs;\n extern int warn_ambiguous_refs;\n extern int warn_on_object_refname_ambiguity;\n+extern int apply_default_p_value;\n extern char *apply_default_whitespace;\n extern char *apply_default_ignorewhitespace;\n extern const char *git_attributes_file;\ndiff --git c/environment.c w/environment.c\nindex 1ee3686fd8..b82ad370b4 100644\n--- c/environment.c\n+++ w/environment.c\n@@ -38,6 +38,7 @@ const char *git_commit_encoding;\n const char *git_log_output_encoding;\n char *apply_default_whitespace;\n char *apply_default_ignorewhitespace;\n+int apply_default_p_value = -1; /* unspecified */\n const char *git_attributes_file;\n const char *git_hooks_path;\n int zlib_compression_level = Z_BEST_SPEED;\n\n\n\n"},{"id":"473319","messageId":"xmqqr0tx8ubw.fsf@gitster.g","threadId":"59361","inReplyTo":"ZAl3bHB9zxjLITgf@coredump.intra.peff.net","subject":"Re: Better suggestions when git-am(1) fails","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-03-09T21:53:55Z","receivedAt":"2023-03-09T21:54:04Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> So here's a patch series which I think should help with the sending\n> side. Most of it is just filling in gaps in the code and tests for\n> current features. Patch 4 is the actual change. Patch 5 adds an\n> equivalent option just for format-patch. I'm not convinced anybody\n> really wants it (which is why I split it out), but it's probably worth\n> doing just in case.\n>\n> [1/5]: diff: factor out src/dst prefix setup\n> [2/5]: t4013: add tests for diff prefix options\n> [3/5]: diff: add --default-prefix option\n> [4/5]: format-patch: do not respect diff.noprefix\n> [5/5]: format-patch: add format.noprefix option\n\nI've reviewed these five changes, and while I am not 100% sold to\nthe idea that we should force our -p1 worldview to those who choose\nto use diff.noprefix for whatever reason, I think these patches\ndescribe what they want to do and implement it in a very readable\nway.\n\nThanks. Queued.\n"},{"id":"473345","messageId":"ZAr6vIOe3WbTIohE@coredump.intra.peff.net","threadId":"59361","inReplyTo":"xmqqedpxq4if.fsf@gitster.g","subject":"Re: Better suggestions when git-am(1) fails","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-03-10T09:39:08Z","receivedAt":"2023-03-10T09:40:55Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 09, 2023 at 08:22:00AM -0800, Junio C Hamano wrote:\n\n> > The reason is probably that they have set diff.noprefix in their config,\n> > and git-format-patch respects that. Which is arguably a bug.\n> \n> FWIW, I've always considered it a feature to help projects that\n> prefer their patches in -p0 form. Of course, Git optimized itself\n> for the usecase we consider the optimum, i.e. using a/ and b/ prefix\n> on the diff generation side, while stripping them with -p1 on the\n> applying side.\n> \n> I wonder apply.plevel or am.plevel would be a good way to help them\n> further?\n\nI doubt they would help, because they imply a constant project workflow.\nWe have seen several reports of \"sometimes I get a patch without a\nprefix, and it doesn't apply. What's going on?\". But I don't think\nanybody asked for \"my project doesn't use prefixes, and I am tired of\ntyping -p0\".\n\nThe more interesting case to me is that the receiver _isn't_ using Git.\nThey are using \"patch\" or similar, and they expect senders to send them\npatches without prefixes. And there, diff.noprefix is doing what they\nwant. But I have to wonder if these hypothetical maintainers exist:\n\n 1. I feel like \"-p1\" was pretty standard even before Git. You'd\n extract two copies of the tarball, one into \"foo-1.2.3\" and one\n into \"foo-1.2.3.orig\", and then \"diff -Nru\" between them to send a\n patch.\n\n 2. It feels weird that a maintainer who isn't using Git would expect a\n lot of contributions from folks who are. And even weirder, that\n they would insist that all of the folks sending patches set\n diff.noprefix.\n\nSo I won't say it's not possible (especially in some closed community).\nBut I'm skeptical.\n\nAll that said, if \"apply\" and \"am\" could automatically figure out and\nhandle \"-p0\" patches, that would be a useful way to help people. I'm\njust hesitant because it probably involves some heuristics. E.g., we get\n\"foo/bar\", realize that \"bar\" doesn't exist, but \"foo/bar\" does. Except\nthat fails if a project does have \"bar\". And so on.\n\n> I am not sure making format-patch _ignore_ diff.src/dst_prefix is a\n> good approach. If we were wiser, we may not have introduced the\n> diff.noprefix option, made sure diff.src/dstprefix to be always a\n> single level, and kept -p on the application side as an escape\n> hatch only to deal with non-Git generated patches. The opportunity\n> to simplify the world that way however we missed 15 years ago X-<.\n\nYeah, I am as always a little concerned that one person's fix is another\none's regression. But it really just seems to that on balance people set\ndiff.noprefix with no thought at all to how it would affect format-patch\n(in fact, I'd guess 99% of Git users do not use format-patch at all).\nAnd then they are surprised (or worse, the receiver is surprised) when\nit doesn't work.\n\n-Peff\n"},{"id":"473346","messageId":"ZAr7+zW+pkOXoIfL@coredump.intra.peff.net","threadId":"59361","inReplyTo":"xmqq5yb9q42e.fsf@gitster.g","subject":"Re: [PATCH 3/5] diff: add --default-prefix option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-03-10T09:44:27Z","receivedAt":"2023-03-10T09:45:17Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 09, 2023 at 08:31:37AM -0800, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > This isn't strictly necessary for the series, but it seemed like a gap.\n> > You can always do:\n> >\n> > git -c diff.noprefix=false -c diff.mnemonicprefix=false ...\n> >\n> > but that's rather a mouthful.\n> \n> or \"git diff --src-prefix=a/ --dst-prefix=b/\"\n\nDoh. How did I write this whole patch series without remembering the\nexistence of those options?\n\nWhile it is not _quite_ the same thing to say \"use prefixes a/ and b/\"\nversus \"countermand any config and use the default\", it is close enough\nthat I am tempted to say this patch should be scrapped. I mostly just\nwanted to have a way to counter format.noprefix, if we are going to\nendorse it as a concept (whether by adding it, or saying \"no, respecting\ndiff.noprefix is not a bug\").\n\n(If we do scrap it, I'd probably fold the extra tests into the previous\ncommit, but using --src-prefix, etc).\n\n> > +static int diff_opt_default_prefix(const struct option *opt,\n> > +\t\t\t\t const char *optarg, int unset)\n> > +{\n> > +\tstruct diff_options *options = opt->value;\n> > +\n> > +\tBUG_ON_OPT_NEG(unset);\n> > +\tBUG_ON_OPT_ARG(optarg);\n> \n> OK. It is a bit unsatisfactory that we already said this does not\n> take negative form or any argument in the option[] array, and still\n> have to do this, but that is completely outside the topic of this\n> series.\n\nWe don't strictly have to do it. It's a cross-check that the correct\nflags were set in the options struct, and serves as documentation both\nfor the human and the compiler (via -Wunused-parameter) that yes, it\nreally is correct to take \"unset\" and not look at it. We could just as\neasily mark unset with \"UNUSED\", but I consider the extra run-time check\na bonus.\n\nI do admit that in a one-off callback like this, it is not accomplishing\nmuch. It's much more useful for generic ones like parse_opt_commit(),\nthat may be triggered from many places. I do wish there was a better way\nto make sure they matched at compile-time, but I can't think of one.\n\n-Peff\n"},{"id":"473347","messageId":"ZAr9PbevIMASG5b+@coredump.intra.peff.net","threadId":"59361","inReplyTo":"xmqqy1o5op1i.fsf@gitster.g","subject":"Re: [PATCH 4/5] format-patch: do not respect diff.noprefix","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-03-10T09:49:49Z","receivedAt":"2023-03-10T09:50:41Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 09, 2023 at 08:41:29AM -0800, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > - if a project really does have a workflow that likes prefix-less\n> > patches, and the receiver is prepared to use \"-p0\", then the sender\n> > now has to manually say \"--no-prefix\" for each format-patch\n> > invocation. That doesn't seem _too_ terrible given that the receiver\n> > has to manually say \"-p0\" for each git-am invocation.\n> \n> It does seem very terrible if any existing projects do use the\n> workflow, as their receivers need to change their workflow, though.\n\nI think the escape hatch there is patch 5, where the sender just sets\nthe new variable to say \"no, really, I actually want to send patches\nwithout a prefix\".\n\nI had originally thought to squash them together to help explain that\nbetter, but I wasn't 100% sure we'd want format.noprefix.\n\n> But we can declare that we do not care about such projects that do\n> not honor our -p1 worldview, and I have no objection to this change\n> if we can have list consensus for us to go in that direction.\n\nYeah, I would very much like to hear from others on the list, especially\nanybody who does have a \"-p0\" workflow.\n\n> Colored patches, by the way, cannot be applied, so perhaps we should\n> disable ui_config altogether, on the other hand? I dunno.\n\nYeah, color is a bit weird there. We auto-disable it when the patch\nisn't going to stdout or a pager, so it's mostly a non-issue. I think\nmore interesting cases are ones like diff.algorithm, diff.context, etc,\nwhere they don't break the diff, but we don't quite consider them\nvanilla enough for plumbing.\n\nI do wonder about diff.relative, which may or may not cause confusion on\nthe receiving end, depending on what you're trying to achieve (are you\nsending a patch for somebody else's git repo, or did you make a git repo\nyourself and want to send a diff of some subset).\n\nAlso diff.submodule, but the implications of submodule-via-format-patch\nare too scary for me to even contemplate. ;)\n\n-Peff\n"},{"id":"473348","messageId":"ZAr9pcP8Ixuqwt51@coredump.intra.peff.net","threadId":"59361","inReplyTo":"xmqq5yb9dfn2.fsf@gitster.g","subject":"Re: [PATCH 5/5] format-patch: add format.noprefix option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-03-10T09:51:33Z","receivedAt":"2023-03-10T09:51:38Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 09, 2023 at 09:00:01AM -0800, Junio C Hamano wrote:\n\n> But that is the other extreme.\n> \n> I wonder if the consumer side should be made configurable for\n> completeness, though.\n> \n> Here is how \"apply.pValue\" configuration variable would look like.\n> Projects whose members want diff.noprefix set can standardise on\n> using that and then receiving end configured to match with this.\n\nI don't mind this at all for the sake of completeness, and your patch\nlooks reasonable to me. I mostly just wouldn't bother if there's not a\ndemonstrated need (and again, this is something people could be asking\nfor _now_, because of the way diff.noprefix works, and nobody has done\nso).\n\n-Peff\n"},{"id":"473349","messageId":"ZAr+ZF0kCMEdaDo2@coredump.intra.peff.net","threadId":"59361","inReplyTo":"xmqqr0tx8ubw.fsf@gitster.g","subject":"Re: Better suggestions when git-am(1) fails","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-03-10T09:54:44Z","receivedAt":"2023-03-10T09:54:49Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 09, 2023 at 01:53:55PM -0800, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > So here's a patch series which I think should help with the sending\n> > side. Most of it is just filling in gaps in the code and tests for\n> > current features. Patch 4 is the actual change. Patch 5 adds an\n> > equivalent option just for format-patch. I'm not convinced anybody\n> > really wants it (which is why I split it out), but it's probably worth\n> > doing just in case.\n> >\n> > [1/5]: diff: factor out src/dst prefix setup\n> > [2/5]: t4013: add tests for diff prefix options\n> > [3/5]: diff: add --default-prefix option\n> > [4/5]: format-patch: do not respect diff.noprefix\n> > [5/5]: format-patch: add format.noprefix option\n> \n> I've reviewed these five changes, and while I am not 100% sold to\n> the idea that we should force our -p1 worldview to those who choose\n> to use diff.noprefix for whatever reason, I think these patches\n> describe what they want to do and implement it in a very readable\n> way.\n> \n> Thanks. Queued.\n\nThanks for looking at them. Let's see if we get any other comments on\nthe direction, and then I may re-roll. Even if we don't do 4 or 5, I\nthink the extra tests are worth adding. Either way I'd probably drop 3\n(in favor of --src-prefix) and squash its tests into 2. Patch 1 isn't\nworthwhile if we don't do 3-5, since we wouldn't be adding any new\ncallers of the helpers.\n\nIf we do proceed, I'd suggest trying to cook in 'next' for a long time\nto get comment. Though I think both you and I are pessimistic that we\nget a wide variety of user testing that way.\n\n-Peff\n"},{"id":"473358","messageId":"xmqqh6us7epk.fsf@gitster.g","threadId":"59361","inReplyTo":"ZAr6vIOe3WbTIohE@coredump.intra.peff.net","subject":"Re: Better suggestions when git-am(1) fails","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-03-10T16:28:55Z","receivedAt":"2023-03-10T16:32:13Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> 1. I feel like \"-p1\" was pretty standard even before Git. You'd\n> extract two copies of the tarball, one into \"foo-1.2.3\" and one\n> into \"foo-1.2.3.orig\", and then \"diff -Nru\" between them to send a\n> patch.\n\nI would too, but then we wouldn't have accepted the request to add\n.noprefix configuration; I do not recall where it came from.\n\n> 2. It feels weird that a maintainer who isn't using Git would expect a\n> lot of contributions from folks who are. And even weirder, that\n> they would insist that all of the folks sending patches set\n> diff.noprefix.\n>\n> So I won't say it's not possible (especially in some closed community).\n> But I'm skeptical.\n\nThe scenario I would find more likely is a project established long\nbefore we were popular wants to keep using -p0 even after switching\nto use Git.\n\n> All that said, if \"apply\" and \"am\" could automatically figure out\n> and handle \"-p0\" patches, that would be a useful way to help\n> people. I'm just hesitant because it probably involves some heuristics.\n\nI am not all that interested in that direction, for exactly the same\nreason as I are heditant. Such a tool that outsmarts users will\neventually bite them.\n\n> Yeah, I am as always a little concerned that one person's fix is another\n> one's regression. But it really just seems to that on balance people set\n> diff.noprefix with no thought at all to how it would affect format-patch\n> (in fact, I'd guess 99% of Git users do not use format-patch at all).\n> And then they are surprised (or worse, the receiver is surprised) when\n> it doesn't work.\n\nFor these 99% users, if format-patch paid attention to their\ndiff.noprefix and used -p0, the world would become even more\ninteresting place. I am not sure this particular cure is an\noverall win. And as you mentioned elsewhere, a change that is\ndeliberately designed to be breaking like this does not become\nmuch safer by cooking in 'next', which is another sad thing.\n\n\n\n"},{"id":"473360","messageId":"xmqqcz5g7d2i.fsf@gitster.g","threadId":"59361","inReplyTo":"ZAr7+zW+pkOXoIfL@coredump.intra.peff.net","subject":"Re: [PATCH 3/5] diff: add --default-prefix option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-03-10T17:04:21Z","receivedAt":"2023-03-10T17:06:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> While it is not _quite_ the same thing to say \"use prefixes a/ and b/\"\n> versus \"countermand any config and use the default\", it is close enough\n> that I am tempted to say this patch should be scrapped. I mostly just\n> wanted to have a way to counter format.noprefix, if we are going to\n> endorse it as a concept (whether by adding it, or saying \"no, respecting\n> diff.noprefix is not a bug\").\n>\n> (If we do scrap it, I'd probably fold the extra tests into the previous\n> commit, but using --src-prefix, etc).\n\nI would very much like to keep this one; if we can find a shorter\nname that would be even sweeter.\n\nI am wondering if we can keep the current behaviour instead and send\na message: \"if you do not want your everyday diff not to have\nprefixes, fine, go set diff.noprefix, but if you do not like that\nformat-patch also gives a no-prefix patches with that configuration,\nor at times you may want your 'git show' to show the standard\nprefix, you can countermand your diff.noprefix configuration\".\n\n"},{"id":"473459","messageId":"ZA9RYncZaqoA0mCw@coredump.intra.peff.net","threadId":"59361","inReplyTo":"xmqqh6us7epk.fsf@gitster.g","subject":"Re: Better suggestions when git-am(1) fails","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-03-13T16:37:54Z","receivedAt":"2023-03-13T16:38:38Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Mar 10, 2023 at 08:28:55AM -0800, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > 1. I feel like \"-p1\" was pretty standard even before Git. You'd\n> > extract two copies of the tarball, one into \"foo-1.2.3\" and one\n> > into \"foo-1.2.3.orig\", and then \"diff -Nru\" between them to send a\n> > patch.\n> \n> I would too, but then we wouldn't have accepted the request to add\n> .noprefix configuration; I do not recall where it came from.\n\nI always thought it was an aesthetic thing for humans viewing diffs (and\nlikewise mnemonicprefix). The original thread doesn't give much\nmotivation, though:\n\n https://lore.kernel.org/git/1272852221-14927-1-git-send-email-eli@cloudera.com/\n\nThat is not really important as what the option has grown to be used\nfor, of course, but it's another data point (or lack thereof in this\ncase).\n\n-Peff\n"},{"id":"473460","messageId":"ZA9SmZaUyrgbH2fb@coredump.intra.peff.net","threadId":"59361","inReplyTo":"xmqqcz5g7d2i.fsf@gitster.g","subject":"Re: [PATCH 3/5] diff: add --default-prefix option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-03-13T16:43:05Z","receivedAt":"2023-03-13T16:43:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Mar 10, 2023 at 09:04:21AM -0800, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > While it is not _quite_ the same thing to say \"use prefixes a/ and b/\"\n> > versus \"countermand any config and use the default\", it is close enough\n> > that I am tempted to say this patch should be scrapped. I mostly just\n> > wanted to have a way to counter format.noprefix, if we are going to\n> > endorse it as a concept (whether by adding it, or saying \"no, respecting\n> > diff.noprefix is not a bug\").\n> >\n> > (If we do scrap it, I'd probably fold the extra tests into the previous\n> > commit, but using --src-prefix, etc).\n> \n> I would very much like to keep this one; if we can find a shorter\n> name that would be even sweeter.\n\nOK. I don't mind keeping it, if you think it's useful (and certainly it\ndoesn't hurt). I couldn't come up with a better name, but suggestions\nare welcome.\n\nBy the way, we might also want something like this:\n\ndiff --git a/builtin/rebase.c b/builtin/rebase.c\nindex dd31d5ab91e..5b7b908b66b 100644\n--- a/builtin/rebase.c\n+++ b/builtin/rebase.c\n@@ -661,7 +661,7 @@ static int run_am(struct rebase_options *opts)\n \tformat_patch.git_cmd = 1;\n \tstrvec_pushl(&format_patch.args, \"format-patch\", \"-k\", \"--stdout\",\n \t\t \"--full-index\", \"--cherry-pick\", \"--right-only\",\n-\t\t \"--src-prefix=a/\", \"--dst-prefix=b/\", \"--no-renames\",\n+\t\t \"--default-prefix\", \"--no-renames\",\n \t\t \"--no-cover-letter\", \"--pretty=mboxrd\", \"--topo-order\",\n \t\t \"--no-base\", NULL);\n \tif (opts->git_format_patch_opt.len)\n\nwhich uses --src-prefix to (you may have guessed it!) counteract\ndiff.noprefix in the user's config. (It would still be necessary even\nwith my series because the user might have set format.patch).\n\n> I am wondering if we can keep the current behaviour instead and send\n> a message: \"if you do not want your everyday diff not to have\n> prefixes, fine, go set diff.noprefix, but if you do not like that\n> format-patch also gives a no-prefix patches with that configuration,\n> or at times you may want your 'git show' to show the standard\n> prefix, you can countermand your diff.noprefix configuration\".\n\nSure, but how do we send that message? I guess if we leave diff.noprefix\nas it is and add a new format.patch (which preempts diff.noprefix only\nfor format-patch), then people will still accidentally send patches\nwithout prefixes, but at least there is an \"out\" for the maintainer\nreceiving them to say \"don't do that; please set format.patch\".\n\nI was hoping to avoid having the accident happen in the first place, but\nif we're not willing to change how diff.noprefix works now, then I think\nthat's our runner-up.\n\nIt would be pretty easy to rework the series (drop patch 4, and tweak\npatch 5 to be a yes/no/unset tri-state, plus a new test to check the\nfallback behavior).\n\n-Peff\n"},{"id":"473461","messageId":"xmqqpm9cwp9h.fsf@gitster.g","threadId":"59361","inReplyTo":"ZA9RYncZaqoA0mCw@coredump.intra.peff.net","subject":"Re: Better suggestions when git-am(1) fails","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-03-13T17:10:50Z","receivedAt":"2023-03-13T17:12:25Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Fri, Mar 10, 2023 at 08:28:55AM -0800, Junio C Hamano wrote:\n>\n>> Jeff King writes:\n>> \n>> > 1. I feel like \"-p1\" was pretty standard even before Git. You'd\n>> > extract two copies of the tarball, one into \"foo-1.2.3\" and one\n>> > into \"foo-1.2.3.orig\", and then \"diff -Nru\" between them to send a\n>> > patch.\n>> \n>> I would too, but then we wouldn't have accepted the request to add\n>> .noprefix configuration; I do not recall where it came from.\n>\n> I always thought it was an aesthetic thing for humans viewing diffs (and\n> likewise mnemonicprefix). The original thread doesn't give much\n> motivation, though:\n>\n> https://lore.kernel.org/git/1272852221-14927-1-git-send-email-eli@cloudera.com/\n\nInteresting.\n\nComparison with mnemonicprefix is a bit unfair, as it does not break\nother tools, though ;-).\n"},{"id":"473462","messageId":"xmqqjzzkwoya.fsf@gitster.g","threadId":"59361","inReplyTo":"ZA9SmZaUyrgbH2fb@coredump.intra.peff.net","subject":"Re: [PATCH 3/5] diff: add --default-prefix option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-03-13T17:17:33Z","receivedAt":"2023-03-13T17:19:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Sure, but how do we send that message? I guess if we leave diff.noprefix\n> as it is and add a new format.patch (which preempts diff.noprefix only\n> for format-patch), then people will still accidentally send patches\n> without prefixes, but at least there is an \"out\" for the maintainer\n> receiving them to say \"don't do that; please set format.patch\".\n\nI actually was hoping that it would be enough if the message were\n\"please unset diff.noprefix---in this project the convention is\nto use -p1 patches, so get used to seeing a/ and b/ prefixes\".\n\nEven if a project wants -p0, the same approach would almost work,\nbut it would need apply.pValue support to help the receiving end.\n\nBut as we already concluded, let's cook the current 5-patch series\nin 'next' and see what happens.\n\nThanks.\n"},{"id":"473463","messageId":"xmqqpm9cv9rb.fsf@gitster.g","threadId":"59361","inReplyTo":"ZA9SmZaUyrgbH2fb@coredump.intra.peff.net","subject":"Re: [PATCH 3/5] diff: add --default-prefix option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-03-13T17:31:04Z","receivedAt":"2023-03-13T17:31:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> By the way, we might also want something like this:\n>\n> diff --git a/builtin/rebase.c b/builtin/rebase.c\n> index dd31d5ab91e..5b7b908b66b 100644\n> --- a/builtin/rebase.c\n> +++ b/builtin/rebase.c\n> @@ -661,7 +661,7 @@ static int run_am(struct rebase_options *opts)\n> \tformat_patch.git_cmd = 1;\n> \tstrvec_pushl(&format_patch.args, \"format-patch\", \"-k\", \"--stdout\",\n> \t\t \"--full-index\", \"--cherry-pick\", \"--right-only\",\n> -\t\t \"--src-prefix=a/\", \"--dst-prefix=b/\", \"--no-renames\",\n> +\t\t \"--default-prefix\", \"--no-renames\",\n> \t\t \"--no-cover-letter\", \"--pretty=mboxrd\", \"--topo-order\",\n> \t\t \"--no-base\", NULL);\n> \tif (opts->git_format_patch_opt.len)\n>\n> which uses --src-prefix to (you may have guessed it!) counteract\n> diff.noprefix in the user's config. (It would still be necessary even\n> with my series because the user might have set format.patch).\n\nOh, you grepped around ;-)? Good find.\n\nOf course, nothing breaks without the above one-liner so it is\nsomewhere between a \"Meh\" and a \"clean-up we should do before we\nforget\".\n\n"},{"id":"473474","messageId":"ZA9/Y6HN7y5dAy0T@coredump.intra.peff.net","threadId":"59361","inReplyTo":"xmqqpm9cv9rb.fsf@gitster.g","subject":"Re: [PATCH 3/5] diff: add --default-prefix option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-03-13T19:54:11Z","receivedAt":"2023-03-13T19:54:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 13, 2023 at 10:31:04AM -0700, Junio C Hamano wrote:\n\n> > -\t\t \"--src-prefix=a/\", \"--dst-prefix=b/\", \"--no-renames\",\n> > +\t\t \"--default-prefix\", \"--no-renames\",\n> [...]\n> Of course, nothing breaks without the above one-liner so it is\n> somewhere between a \"Meh\" and a \"clean-up we should do before we\n> forget\".\n\nSo here it is as a patch, if you want to throw it on top of\njk/format-patch-ignore-noprefix.\n\nI have to admit that after writing the relatively weak argument in the\ncommit message, it does feel a bit like churn. So I am also OK to just\nleave it as-is.\n\n-- >8 --\nSubject: rebase: prefer --default-prefix to --{src,dst}-prefix for format-patch\n\nWhen git-rebase invokes format-patch, it wants to make sure we use the\nnormal prefixes, and are not confused by diff.noprefix or similar. When\nthis was added in 5b220a6876f (Add --src/dst-prefix to git-formt-patch\nin git-rebase.sh, 2010-09-09), we only had --src-prefix and --dst-prefix\nto do so, which requires re-specifying the prefixes we expect to see.\nThese days we can say what we want more directly: just use the defaults.\n\nThis is a minor cleanup that should have no behavior change, but\nhopefully the result expresses more clearly what the code is trying to\naccomplish.\n\nSigned-off-by: Jeff King \n---\n builtin/rebase.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/rebase.c b/builtin/rebase.c\nindex 6635f10d529..a47dfd45efd 100644\n--- a/builtin/rebase.c\n+++ b/builtin/rebase.c\n@@ -660,7 +660,7 @@ static int run_am(struct rebase_options *opts)\n \tformat_patch.git_cmd = 1;\n \tstrvec_pushl(&format_patch.args, \"format-patch\", \"-k\", \"--stdout\",\n \t\t \"--full-index\", \"--cherry-pick\", \"--right-only\",\n-\t\t \"--src-prefix=a/\", \"--dst-prefix=b/\", \"--no-renames\",\n+\t\t \"--default-prefix\", \"--no-renames\",\n \t\t \"--no-cover-letter\", \"--pretty=mboxrd\", \"--topo-order\",\n \t\t \"--no-base\", NULL);\n \tif (opts->git_format_patch_opt.len)\n-- \n2.40.0.572.gc7ee6f1a071\n\n"}]}