{"thread":{"id":"42970","subject":"[PATCH 1/2] format-patch: Add a config option format.from to set the default for --from","startedAt":"2016-07-30T09:42:08Z","lastAt":"2016-08-08T04:52:44Z","messageCount":10,"participants":["Josh Triplett","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"292594","messageId":"20160730094156.etvrzqbhcpg3is2z@x","threadId":"42970","inReplyTo":"cover.8678faa71de50c8e78760b0bcb3d15ebeda207d5.1469871675.git-series.josh@joshtriplett.org","subject":"[PATCH 1/2] format-patch: Add a config option format.from to set the default for --from","fromName":"Josh Triplett","fromEmail":"josh@joshtriplett.org","sentAt":"2016-07-30T09:41:56Z","receivedAt":"2016-07-30T09:42:08Z","isPatch":true,"sender":{"key":"josh@joshtriplett.org","avatar":"https://avatars.githubusercontent.com/u/162737?v=4"},"body":"This helps users who would prefer format-patch to default to --from, and\nmakes it easier to change the default in the future.\n\nSigned-off-by: Josh Triplett \n---\n Documentation/config.txt | 10 +++++++-\n builtin/log.c | 13 ++++++++-\n contrib/completion/git-completion.bash | 1 +-\n t/t4014-format-patch.sh | 40 +++++++++++++++++++++++++++-\n 4 files changed, 63 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 8b1aee4..bd34774 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1253,6 +1253,16 @@ format.attach::\n \tvalue as the boundary. See the --attach option in\n \tlinkgit:git-format-patch[1].\n \n+format.from::\n+\tProvides the default value for the `--from` option to format-patch.\n+\tAccepts a boolean value, or a name and email address. If false,\n+\tformat-patch defaults to `--no-from`, using commit authors directly in\n+\tthe \"From:\" field of patch mails. If true, format-patch defaults to\n+\t`--from`, using your committer identity in the \"From:\" field of patch\n+\tmails and including a \"From:\" field in the body of the patch mail if\n+\tdifferent. If set to a non-boolean value, format-patch uses that\n+\tvalue instead of your committer identity. Defaults to false.\n+\n format.numbered::\n \tA boolean which can enable or disable sequence numbers in patch\n \tsubjects. It defaults to \"auto\" which enables it only if there\ndiff --git a/builtin/log.c b/builtin/log.c\nindex fd1652f..1f116be 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -719,6 +719,7 @@ static void add_header(const char *value)\n static int thread;\n static int do_signoff;\n static int base_auto;\n+static char *from;\n static const char *signature = git_version_string;\n static const char *signature_file;\n static int config_cover_letter;\n@@ -807,6 +808,17 @@ static int git_format_config(const char *var, const char *value, void *cb)\n \t\tbase_auto = git_config_bool(var, value);\n \t\treturn 0;\n \t}\n+\tif (!strcmp(var, \"format.from\")) {\n+\t\tint b = git_config_maybe_bool(var, value);\n+\t\tfree(from);\n+\t\tif (b < 0)\n+\t\t\tfrom = xstrdup(value);\n+\t\telse if (b)\n+\t\t\tfrom = xstrdup(git_committer_info(IDENT_NO_DATE));\n+\t\telse\n+\t\t\tfrom = NULL;\n+\t\treturn 0;\n+\t}\n \n \treturn git_log_config(var, value, cb);\n }\n@@ -1384,7 +1396,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tint quiet = 0;\n \tint reroll_count = -1;\n \tchar *branch_name = NULL;\n-\tchar *from = NULL;\n \tchar *base_commit = NULL;\n \tstruct base_tree_info bases;\n \ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 10f6d52..4393033 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -2181,6 +2181,7 @@ _git_config ()\n \t\tformat.attach\n \t\tformat.cc\n \t\tformat.coverLetter\n+\t\tformat.from\n \t\tformat.headers\n \t\tformat.numbered\n \t\tformat.pretty\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex 1206c48..b0579dd 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -229,6 +229,46 @@ check_patch () {\n \tgrep -e \"^Subject:\" \"$1\"\n }\n \n+test_expect_success 'format.from=false' '\n+\n+\tgit -c format.from=false format-patch --stdout master..side |\n+\tsed -e \"/^\\$/q\" >patch &&\n+\tcheck_patch patch &&\n+\t! grep \"^From: C O Mitter \\$\" patch\n+'\n+\n+test_expect_success 'format.from=true' '\n+\n+\tgit -c format.from=true format-patch --stdout master..side |\n+\tsed -e \"/^\\$/q\" >patch &&\n+\tcheck_patch patch &&\n+\tgrep \"^From: C O Mitter \\$\" patch\n+'\n+\n+test_expect_success 'format.from with address' '\n+\n+\tgit -c format.from=\"F R Om \" format-patch --stdout master..side |\n+\tsed -e \"/^\\$/q\" >patch &&\n+\tcheck_patch patch &&\n+\tgrep \"^From: F R Om \\$\" patch\n+'\n+\n+test_expect_success '--no-from overrides format.from' '\n+\n+\tgit -c format.from=\"F R Om \" format-patch --no-from --stdout master..side |\n+\tsed -e \"/^\\$/q\" >patch &&\n+\tcheck_patch patch &&\n+\t! grep \"^From: F R Om \\$\" patch\n+'\n+\n+test_expect_success '--from overrides format.from' '\n+\n+\tgit -c format.from=\"F R Om \" format-patch --from --stdout master..side |\n+\tsed -e \"/^\\$/q\" >patch &&\n+\tcheck_patch patch &&\n+\t! grep \"^From: F R Om \\$\" patch\n+'\n+\n test_expect_success '--no-to overrides config.to' '\n \n \tgit config --replace-all format.to \\\n-- \ngit-series 0.8.7\n"},{"id":"292595","messageId":"20160730094205.vmb2yt3ed6pox7it@x","threadId":"42970","inReplyTo":"cover.8678faa71de50c8e78760b0bcb3d15ebeda207d5.1469871675.git-series.josh@joshtriplett.org","subject":"[PATCH 2/2] format-patch: Default to --from","fromName":"Josh Triplett","fromEmail":"josh@joshtriplett.org","sentAt":"2016-07-30T09:42:05Z","receivedAt":"2016-07-30T09:42:18Z","isPatch":true,"sender":{"key":"josh@joshtriplett.org","avatar":"https://avatars.githubusercontent.com/u/162737?v=4"},"body":"This avoids spoofing mails when formatting commits not written by the\nuser.\n\nAdd tests for the new default, and fix tests whose expected output\ndepended on the old default.\n\nSigned-off-by: Josh Triplett \n---\n Documentation/config.txt | 2 +-\n builtin/log.c | 1 +\n t/t4014-format-patch.sh | 28 +++++++++++++++++++++++++---\n 3 files changed, 27 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex bd34774..2310877 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1261,7 +1261,7 @@ format.from::\n \t`--from`, using your committer identity in the \"From:\" field of patch\n \tmails and including a \"From:\" field in the body of the patch mail if\n \tdifferent. If set to a non-boolean value, format-patch uses that\n-\tvalue instead of your committer identity. Defaults to false.\n+\tvalue instead of your committer identity. Defaults to true.\n \n format.numbered::\n \tA boolean which can enable or disable sequence numbers in patch\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 1f116be..53b2f62 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1472,6 +1472,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \textra_hdr.strdup_strings = 1;\n \textra_to.strdup_strings = 1;\n \textra_cc.strdup_strings = 1;\n+\tfrom = xstrdup(git_committer_info(IDENT_NO_DATE));\n \tinit_log_defaults();\n \tgit_config(git_format_config, NULL);\n \tinit_revisions(&rev, prefix);\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex b0579dd..fa35cbe 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -652,7 +652,7 @@ EOF\n \n test_expect_success 'format-patch -p suppresses stat' '\n \n-\tgit format-patch -p -2 &&\n+\tgit format-patch --no-from -p -2 &&\n \tsed -e \"1,/^\\$/d\" -e \"/^+5/q\" < 0001-This-is-an-excessively-long-subject-line-for-a-messa.patch > output &&\n \ttest_cmp expect output\n \n@@ -973,7 +973,7 @@ check_author() {\n \techo content >>file &&\n \tgit add file &&\n \tGIT_AUTHOR_NAME=$1 git commit -m author-check &&\n-\tgit format-patch --stdout -1 >patch &&\n+\tgit format-patch --no-from --stdout -1 >patch &&\n \tsed -n \"/^From: /p; /^ /p; /^$/q\" actual &&\n \ttest_cmp expect actual\n }\n@@ -1089,6 +1089,18 @@ test_expect_success '--from=ident replaces author' '\n \ttest_cmp expect patch.head\n '\n \n+test_expect_success 'Default uses committer ident' '\n+\tgit format-patch -1 --stdout >patch &&\n+\tcat >expect <<-\\EOF &&\n+\tFrom: C O Mitter \n+\n+\tFrom: A U Thor \n+\n+\tEOF\n+\tsed -ne \"/^From:/p; /^$/p; /^---$/q\" patch.head &&\n+\ttest_cmp expect patch.head\n+'\n+\n test_expect_success '--from uses committer ident' '\n \tgit format-patch -1 --stdout --from >patch &&\n \tcat >expect <<-\\EOF &&\n@@ -1101,6 +1113,16 @@ test_expect_success '--from uses committer ident' '\n \ttest_cmp expect patch.head\n '\n \n+test_expect_success '--no-from suppresses default --from' '\n+\tgit format-patch -1 --stdout --no-from >patch &&\n+\tcat >expect <<-\\EOF &&\n+\tFrom: A U Thor \n+\n+\tEOF\n+\tsed -ne \"/^From:/p; /^$/p; /^---$/q\" patch.head &&\n+\ttest_cmp expect patch.head\n+'\n+\n test_expect_success '--from omits redundant in-body header' '\n \tgit format-patch -1 --stdout --from=\"A U Thor \" >patch &&\n \tcat >expect <<-\\EOF &&\n@@ -1129,7 +1151,7 @@ test_expect_success 'in-body headers trigger content encoding' '\n append_signoff()\n {\n \tC=$(git commit-tree HEAD^^{tree} -p HEAD) &&\n-\tgit format-patch --stdout --signoff $C^..$C >append_signoff.patch &&\n+\tgit format-patch --no-from --stdout --signoff $C^..$C >append_signoff.patch &&\n \tsed -n -e \"1,/^---$/p\" append_signoff.patch |\n \t\tegrep -n \"^Subject|Sign|^$\"\n }\n-- \ngit-series 0.8.7\n"},{"id":"292608","messageId":"20160730154034.thjrfu6zeprzwvxb@sigill.intra.peff.net","threadId":"42970","inReplyTo":"20160730094156.etvrzqbhcpg3is2z@x","subject":"Re: [PATCH 1/2] format-patch: Add a config option format.from to set the default for --from","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-07-30T15:40:34Z","receivedAt":"2016-07-30T15:40:43Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jul 30, 2016 at 02:41:56AM -0700, Josh Triplett wrote:\n\n> @@ -807,6 +808,17 @@ static int git_format_config(const char *var, const char *value, void *cb)\n> \t\tbase_auto = git_config_bool(var, value);\n> \t\treturn 0;\n> \t}\n> +\tif (!strcmp(var, \"format.from\")) {\n> +\t\tint b = git_config_maybe_bool(var, value);\n> +\t\tfree(from);\n> +\t\tif (b < 0)\n> +\t\t\tfrom = xstrdup(value);\n> +\t\telse if (b)\n> +\t\t\tfrom = xstrdup(git_committer_info(IDENT_NO_DATE));\n> +\t\telse\n> +\t\t\tfrom = NULL;\n> +\t\treturn 0;\n> +\t}\n\nThis \"free old, then handle tri-state\" mirrors the code in the parseopt\ncallback pretty closely. I wonder if they could share the logic (it is\nnot many lines, but we would want the logic to stay identical). I\nsuspect the helper function would end up with more boilerplate than it's\nworth, though, trying to handle the unset and default cases.\n\n> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\n> index 1206c48..b0579dd 100755\n> --- a/t/t4014-format-patch.sh\n> +++ b/t/t4014-format-patch.sh\n> @@ -229,6 +229,46 @@ check_patch () {\n> \tgrep -e \"^Subject:\" \"$1\"\n> }\n> \n> +test_expect_success 'format.from=false' '\n> +\n> +\tgit -c format.from=false format-patch --stdout master..side |\n> +\tsed -e \"/^\\$/q\" >patch &&\n> +\tcheck_patch patch &&\n> +\t! grep \"^From: C O Mitter \\$\" patch\n> +'\n\nThese tests follow a different style from the \"--from\" tests later in\nthe script (and your second patch does follow it, and puts its test\nclose there). Any reason not to have all of the \"from\" tests together,\nand using the same style?\n\n\nOverall, the whole thing looks cleanly done, and I don't mind it going\nin as-is. These are just two things I noticed while reading it over.\n\n-Peff\n"},{"id":"292655","messageId":"20160730181246.4aifnvqfeenddzdl@x","threadId":"42970","inReplyTo":"20160730154034.thjrfu6zeprzwvxb@sigill.intra.peff.net","subject":"Re: [PATCH 1/2] format-patch: Add a config option format.from to set the default for --from","fromName":"Josh Triplett","fromEmail":"josh@joshtriplett.org","sentAt":"2016-07-30T18:12:46Z","receivedAt":"2016-07-30T18:13:00Z","isPatch":true,"sender":{"key":"josh@joshtriplett.org","avatar":"https://avatars.githubusercontent.com/u/162737?v=4"},"body":"On Sat, Jul 30, 2016 at 11:40:34AM -0400, Jeff King wrote:\n> On Sat, Jul 30, 2016 at 02:41:56AM -0700, Josh Triplett wrote:\n> \n> > @@ -807,6 +808,17 @@ static int git_format_config(const char *var, const char *value, void *cb)\n> > \t\tbase_auto = git_config_bool(var, value);\n> > \t\treturn 0;\n> > \t}\n> > +\tif (!strcmp(var, \"format.from\")) {\n> > +\t\tint b = git_config_maybe_bool(var, value);\n> > +\t\tfree(from);\n> > +\t\tif (b < 0)\n> > +\t\t\tfrom = xstrdup(value);\n> > +\t\telse if (b)\n> > +\t\t\tfrom = xstrdup(git_committer_info(IDENT_NO_DATE));\n> > +\t\telse\n> > +\t\t\tfrom = NULL;\n> > +\t\treturn 0;\n> > +\t}\n> \n> This \"free old, then handle tri-state\" mirrors the code in the parseopt\n> callback pretty closely. I wonder if they could share the logic (it is\n> not many lines, but we would want the logic to stay identical). I\n> suspect the helper function would end up with more boilerplate than it's\n> worth, though, trying to handle the unset and default cases.\n\nI looked at trying to share that code for exactly that reason, but\ndidn't find a convenient way to share the two, because from_callback\nchecked two separate variables (unset and arg), while the logic above\nchecks one. So, while the *bodies* of the three-way if are duplicated,\nthe *conditions* aren't.\n\nHowever, if you'd like to avoid the duplication between the three\nvalues, I can do that with a set_from function that takes an enum and a\nnew value; it'll actually increase lines of code, but remove the\nduplication (as well as the second patch's third copy of setting from to\nthe committer info).\n\n> > diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\n> > index 1206c48..b0579dd 100755\n> > --- a/t/t4014-format-patch.sh\n> > +++ b/t/t4014-format-patch.sh\n> > @@ -229,6 +229,46 @@ check_patch () {\n> > \tgrep -e \"^Subject:\" \"$1\"\n> > }\n> > \n> > +test_expect_success 'format.from=false' '\n> > +\n> > +\tgit -c format.from=false format-patch --stdout master..side |\n> > +\tsed -e \"/^\\$/q\" >patch &&\n> > +\tcheck_patch patch &&\n> > +\t! grep \"^From: C O Mitter \\$\" patch\n> > +'\n> \n> These tests follow a different style from the \"--from\" tests later in\n> the script (and your second patch does follow it, and puts its test\n> close there). Any reason not to have all of the \"from\" tests together,\n> and using the same style?\n\nThe tests covered different things. The later --from tests made sure\nthat --from behaved as expected. These tests made sure that format.from\nand --from/--no-from interacted in the expected way, with the\ncommand-line options overriding the configuration. So, I put them next\nto the tests for other options like format.to and format.cc, which\ntested the same thing (overriding those with --no-to, --no-cc, etc).\n\n> Overall, the whole thing looks cleanly done, and I don't mind it going\n> in as-is. These are just two things I noticed while reading it over.\n\nI'll send a v2 with the code duplication fixed.\n\n- Josh Triplett\n"},{"id":"292658","messageId":"20160730191104.2ps5k7eji7aqgufg@x","threadId":"42970","inReplyTo":"20160730181246.4aifnvqfeenddzdl@x","subject":"[PATCH v2 0/2] format-patch: Transition the default to --from to avoid spoofed mails","fromName":"Josh Triplett","fromEmail":"josh@joshtriplett.org","sentAt":"2016-07-30T19:11:05Z","receivedAt":"2016-07-30T19:11:17Z","isPatch":true,"sender":{"key":"josh@joshtriplett.org","avatar":"https://avatars.githubusercontent.com/u/162737?v=4"},"body":"As discussed, this patch series allows transitioning the default\nbehavior of format-patch to --from, to avoid spoofing mails when\nformatting commits not written by the user.\n\nThe first patch introduces the format.from option to set the default\nvalue of format-patch --from. This patch doesn't change the default\nbehavior of format-patch, so it can go in without any transition.\n\nThe second patch changes the default to --from. If you'd like to delay\nthis patch for a release and mention the planned change in the release\nnotes, let me know and I'll provide text for the release notes; if you\ndon't think this needs a transition period, you can go ahead and apply\nthe second patch.\n\nv2: Unify the various places setting from into a single helper function.\n\nJosh Triplett (2):\n format-patch: Add a config option format.from to set the default for --from\n format-patch: Default to --from\n\n Documentation/config.txt | 10 ++++-\n builtin/log.c | 47 +++++++++++++++----\n contrib/completion/git-completion.bash | 1 +-\n t/t4014-format-patch.sh | 68 +++++++++++++++++++++++++--\n 4 files changed, 114 insertions(+), 12 deletions(-)\n\n-- \ngit-series 0.8.7\n"},{"id":"292757","messageId":"20160801173053.3ggwr6vnyywl3l4h@sigill.intra.peff.net","threadId":"42970","inReplyTo":"20160730181246.4aifnvqfeenddzdl@x","subject":"Re: [PATCH 1/2] format-patch: Add a config option format.from to set the default for --from","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-08-01T17:30:54Z","receivedAt":"2016-08-01T17:38:23Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jul 30, 2016 at 11:12:46AM -0700, Josh Triplett wrote:\n\n> > These tests follow a different style from the \"--from\" tests later in\n> > the script (and your second patch does follow it, and puts its test\n> > close there). Any reason not to have all of the \"from\" tests together,\n> > and using the same style?\n> \n> The tests covered different things. The later --from tests made sure\n> that --from behaved as expected. These tests made sure that format.from\n> and --from/--no-from interacted in the expected way, with the\n> command-line options overriding the configuration. So, I put them next\n> to the tests for other options like format.to and format.cc, which\n> tested the same thing (overriding those with --no-to, --no-cc, etc).\n\nOK. I would have grouped by \"things that influence this area of\nbehavior\", not by \"config versus command-line\". But I don't think either\nis wrong or right. And since you are the one writing the patch, \"how I\nwould have done it\" is not a compelling review comment.\n\n-Peff\n"},{"id":"292761","messageId":"20160801174724.m4ags4ag4ftwoafj@sigill.intra.peff.net","threadId":"42970","inReplyTo":"20160730191104.2ps5k7eji7aqgufg@x","subject":"Re: [PATCH v2 0/2] format-patch: Transition the default to --from to avoid spoofed mails","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-08-01T17:47:24Z","receivedAt":"2016-08-01T17:54:28Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jul 30, 2016 at 12:11:05PM -0700, Josh Triplett wrote:\n\n> Josh Triplett (2):\n> format-patch: Add a config option format.from to set the default for --from\n> format-patch: Default to --from\n\nBy the way, I notice that the threading between your patches and cover\nletter are broken. Since I see you are also working on a tool for\nhandling such things, I'd suspect the tool (or your workflow) has a bug.\n:)\n\nThe message-id of this message is:\n\n <20160730191104.2ps5k7eji7aqgufg@x>\n\nbut the patches have both \"References\" and \"In-Reply-To\" set to:\n\n \n\nI also see your MUA is mutt, and I think I saw you mention using \"mutt\n-H\" elsewhere. IIRC, when I started using a similar workflow years ago,\nI tried the same thing and had the same problem: \"-H\" treats the input\nfile as a template, not a message, and thus generates a new message-id.\n\nI switched to using mutt's internal \"resend-message\" function, which\ndoes a more literal re-send. I don't think I ever found a way to\nconvince mutt to do a resend from the command line.\n\n-Peff\n"},{"id":"292782","messageId":"20160801195815.7krmjszhzw32vtmo@x","threadId":"42970","inReplyTo":"20160801174724.m4ags4ag4ftwoafj@sigill.intra.peff.net","subject":"Re: [PATCH v2 0/2] format-patch: Transition the default to --from to avoid spoofed mails","fromName":"Josh Triplett","fromEmail":"josh@joshtriplett.org","sentAt":"2016-08-01T19:58:15Z","receivedAt":"2016-08-01T19:59:35Z","isPatch":true,"sender":{"key":"josh@joshtriplett.org","avatar":"https://avatars.githubusercontent.com/u/162737?v=4"},"body":"On Mon, Aug 01, 2016 at 01:47:24PM -0400, Jeff King wrote:\n> On Sat, Jul 30, 2016 at 12:11:05PM -0700, Josh Triplett wrote:\n> \n> > Josh Triplett (2):\n> > format-patch: Add a config option format.from to set the default for --from\n> > format-patch: Default to --from\n> \n> By the way, I notice that the threading between your patches and cover\n> letter are broken. Since I see you are also working on a tool for\n> handling such things, I'd suspect the tool (or your workflow) has a bug.\n> :)\n\nMy workflow, fortunately. :)\n\n> The message-id of this message is:\n> \n> <20160730191104.2ps5k7eji7aqgufg@x>\n> \n> but the patches have both \"References\" and \"In-Reply-To\" set to:\n> \n> \n> \n> I also see your MUA is mutt, and I think I saw you mention using \"mutt\n> -H\" elsewhere. IIRC, when I started using a similar workflow years ago,\n> I tried the same thing and had the same problem: \"-H\" treats the input\n> file as a template, not a message, and thus generates a new message-id.\n> \n> I switched to using mutt's internal \"resend-message\" function, which\n> does a more literal re-send. I don't think I ever found a way to\n> convince mutt to do a resend from the command line.\n\nI actually tried using mutt's resend function (alt-e) with an mbox; I\nchecked the Message-Id and In-Reply-To headers, and they looked correct.\nI've used mutt -H successfully before without breaking threads. The\nDebian mutt packages recently upgraded to the \"neomutt\" fork; I wonder\nif something broke recently?\n\nThanks for letting me know; I'll investigate and try to figure out the\nproblem.\n\n- Josh Triplett\n"},{"id":"292788","messageId":"xmqqpopsi61a.fsf@gitster.mtv.corp.google.com","threadId":"42970","inReplyTo":"20160730094156.etvrzqbhcpg3is2z@x","subject":"Re: [PATCH 1/2] format-patch: Add a config option format.from to set the default for --from","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-01T20:32:49Z","receivedAt":"2016-08-01T20:40:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Josh Triplett writes:\n\n> +static char *from;\n> static const char *signature = git_version_string;\n> static const char *signature_file;\n> static int config_cover_letter;\n> @@ -807,6 +808,17 @@ static int git_format_config(const char *var, const char *value, void *cb)\n> \t\tbase_auto = git_config_bool(var, value);\n> \t\treturn 0;\n> \t}\n> +\tif (!strcmp(var, \"format.from\")) {\n> +\t\tint b = git_config_maybe_bool(var, value);\n> +\t\tfree(from);\n> +\t\tif (b < 0)\n> +\t\t\tfrom = xstrdup(value);\n> +\t\telse if (b)\n> +\t\t\tfrom = xstrdup(git_committer_info(IDENT_NO_DATE));\n> +\t\telse\n> +\t\t\tfrom = NULL;\n> +\t\treturn 0;\n> +\t}\n\nOne potential issue I see here is that if we ever plan to switch the\ndefault, we may want to issue a warning message to users who do not\nhave any format.from configured when they do run the program on a\ncommit that will get a different result before and after the switch\nin a release of Git before that default switch happens. The message\nwould say something like \"you are formatting somebody else's commit.\nthe output will change in future versions of Git and show an explicit\nin-body From: header; if you want to keep the current behaviour, set\nformat.from configuration variable to false\".\n\nBut you cannot tell by looking at from that is NULL between two\ncases, it is NULL because we defaulted to it (in which case we do\nwant to warn), or the user set it explicitly to false (we do not\nwant to give the warning).\n\nSo \"... makes it easier to change the default in the future.\" in the\nlog message is a bit of exaggeration. The change in this patch is\nnot there yet.\n\n> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\n> index 1206c48..b0579dd 100755\n> --- a/t/t4014-format-patch.sh\n> +++ b/t/t4014-format-patch.sh\n> @@ -229,6 +229,46 @@ check_patch () {\n> \tgrep -e \"^Subject:\" \"$1\"\n> }\n> \n> +test_expect_success 'format.from=false' '\n> +\n> +\tgit -c format.from=false format-patch --stdout master..side |\n> +\tsed -e \"/^\\$/q\" >patch &&\n> +\tcheck_patch patch &&\n> +\t! grep \"^From: C O Mitter \\$\" patch\n\nI know you are only mimicking the style of the existing tests but\nthe piping loses a potential error exit code from format-patch. I'd\nsuggest leaving this as low-hanging fruit for later, not fixing them\nas part of this series.\n\nThis stops at the blank after the header, so there is no point doing\nmaster..side to format three patches. Just do \"-1 side\" instead?\n\nMore importantly, this only checks the From: in the header, which is\njust the half of what --from does. Shouldn't we be testing that the\nin-body From: is added as necessary?\n\nThanks.\n"},{"id":"293336","messageId":"20160808045228.ag4mb55dwcsrro6y@x","threadId":"42970","inReplyTo":"xmqqpopsi61a.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 1/2] format-patch: Add a config option format.from to set the default for --from","fromName":"Josh Triplett","fromEmail":"josh@joshtriplett.org","sentAt":"2016-08-08T04:52:28Z","receivedAt":"2016-08-08T04:52:44Z","isPatch":true,"sender":{"key":"josh@joshtriplett.org","avatar":"https://avatars.githubusercontent.com/u/162737?v=4"},"body":"On Mon, Aug 01, 2016 at 01:32:49PM -0700, Junio C Hamano wrote:\n> Josh Triplett writes:\n> \n> > +static char *from;\n> > static const char *signature = git_version_string;\n> > static const char *signature_file;\n> > static int config_cover_letter;\n> > @@ -807,6 +808,17 @@ static int git_format_config(const char *var, const char *value, void *cb)\n> > \t\tbase_auto = git_config_bool(var, value);\n> > \t\treturn 0;\n> > \t}\n> > +\tif (!strcmp(var, \"format.from\")) {\n> > +\t\tint b = git_config_maybe_bool(var, value);\n> > +\t\tfree(from);\n> > +\t\tif (b < 0)\n> > +\t\t\tfrom = xstrdup(value);\n> > +\t\telse if (b)\n> > +\t\t\tfrom = xstrdup(git_committer_info(IDENT_NO_DATE));\n> > +\t\telse\n> > +\t\t\tfrom = NULL;\n> > +\t\treturn 0;\n> > +\t}\n> \n> One potential issue I see here is that if we ever plan to switch the\n> default, we may want to issue a warning message to users who do not\n> have any format.from configured when they do run the program on a\n> commit that will get a different result before and after the switch\n> in a release of Git before that default switch happens. The message\n> would say something like \"you are formatting somebody else's commit.\n> the output will change in future versions of Git and show an explicit\n> in-body From: header; if you want to keep the current behaviour, set\n> format.from configuration variable to false\".\n\nThe previous discussion between you and Jeff King seemed to suggest that\na mention in the release notes might suffice, rather than a \"noisy\ndeprecation\" warning.\n\n> But you cannot tell by looking at from that is NULL between two\n> cases, it is NULL because we defaulted to it (in which case we do\n> want to warn), or the user set it explicitly to false (we do not\n> want to give the warning).\n\nIf we wanted to issue such a warning, I'd suggest a separate boolean\n\"from_set\", set when either the configuration or command line sets\n\"from\", and then the code that handles \"from\" could emit a warning to\nstderr if it would produce different results and !from_set.\n\n- Josh Triplett\n"}]}