{"thread":{"id":"60519","subject":"[PATCH] send-email: avoid duplicate specification warnings","startedAt":"2023-11-14T16:39:22Z","lastAt":"2023-11-16T22:32:07Z","messageCount":16,"participants":["Todd Zullinger","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"484863","messageId":"20231114163826.207267-1-tmz@pobox.com","threadId":"60519","inReplyTo":null,"subject":"[PATCH] send-email: avoid duplicate specification warnings","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2023-11-14T16:38:19Z","receivedAt":"2023-11-14T16:39:22Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"With perl-Getopt-Long >= 2.55, a warning is issued for options which are\nspecified more than once.  In addition to causing users to see warnings,\nthis results in test failures which compare the output.  An example,\nfrom t9001-send-email.37:\n\n  | +++ diff -u expect actual\n  | --- expect      2023-11-14 10:38:23.854346488 +0000\n  | +++ actual      2023-11-14 10:38:23.848346466 +0000\n  | @@ -1,2 +1,7 @@\n  | +Duplicate specification \"no-chain-reply-to\" for option \"no-chain-reply-to\"\n  | +Duplicate specification \"to-cover|to-cover!\" for option \"to-cover\"\n  | +Duplicate specification \"cc-cover|cc-cover!\" for option \"cc-cover\"\n  | +Duplicate specification \"no-thread\" for option \"no-thread\"\n  | +Duplicate specification \"no-to-cover\" for option \"no-to-cover\"\n  |  fatal: longline.patch:35 is longer than 998 characters\n  |  warning: no patches were sent\n  | error: last command exited with $?=1\n  | not ok 37 - reject long lines\n\nRemove the duplicate option specs.\n\nSigned-off-by: Todd Zullinger <tmz@pobox.com>\n---\n\nI've run this through the full test suite.  I also compared the output of\n--help to ensure it only differs in the removal of the \"Duplicate\nspecification\" warnings.  I _think_ that's a good sign that no other changes\nwill result.  But I would be grateful to anyone who can confirm or reject that\ntheory.\n\n git-send-email.perl | 16 +++-------------\n 1 file changed, 3 insertions(+), 13 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex cacdbd6bb2..13d9c47fe5 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -491,7 +491,6 @@ sub config_regexp {\n \t\t    \"bcc=s\" => \\@getopt_bcc,\n \t\t    \"no-bcc\" => \\$no_bcc,\n \t\t    \"chain-reply-to!\" => \\$chain_reply_to,\n-\t\t    \"no-chain-reply-to\" => sub {$chain_reply_to = 0},\n \t\t    \"sendmail-cmd=s\" => \\$sendmail_cmd,\n \t\t    \"smtp-server=s\" => \\$smtp_server,\n \t\t    \"smtp-server-option=s\" => \\@smtp_server_options,\n@@ -506,36 +505,27 @@ sub config_regexp {\n \t\t    \"smtp-auth=s\" => \\$smtp_auth,\n \t\t    \"no-smtp-auth\" => sub {$smtp_auth = 'none'},\n \t\t    \"annotate!\" => \\$annotate,\n-\t\t    \"no-annotate\" => sub {$annotate = 0},\n \t\t    \"compose\" => \\$compose,\n \t\t    \"quiet\" => \\$quiet,\n \t\t    \"cc-cmd=s\" => \\$cc_cmd,\n \t\t    \"header-cmd=s\" => \\$header_cmd,\n \t\t    \"no-header-cmd\" => \\$no_header_cmd,\n \t\t    \"suppress-from!\" => \\$suppress_from,\n-\t\t    \"no-suppress-from\" => sub {$suppress_from = 0},\n \t\t    \"suppress-cc=s\" => \\@suppress_cc,\n-\t\t    \"signed-off-cc|signed-off-by-cc!\" => \\$signed_off_by_cc,\n-\t\t    \"no-signed-off-cc|no-signed-off-by-cc\" => sub {$signed_off_by_cc = 0},\n-\t\t    \"cc-cover|cc-cover!\" => \\$cover_cc,\n-\t\t    \"no-cc-cover\" => sub {$cover_cc = 0},\n-\t\t    \"to-cover|to-cover!\" => \\$cover_to,\n-\t\t    \"no-to-cover\" => sub {$cover_to = 0},\n+\t\t    \"signed-off-by-cc!\" => \\$signed_off_by_cc,\n+\t\t    \"cc-cover!\" => \\$cover_cc,\n+\t\t    \"to-cover!\" => \\$cover_to,\n \t\t    \"confirm=s\" => \\$confirm,\n \t\t    \"dry-run\" => \\$dry_run,\n \t\t    \"envelope-sender=s\" => \\$envelope_sender,\n \t\t    \"thread!\" => \\$thread,\n-\t\t    \"no-thread\" => sub {$thread = 0},\n \t\t    \"validate!\" => \\$validate,\n-\t\t    \"no-validate\" => sub {$validate = 0},\n \t\t    \"transfer-encoding=s\" => \\$target_xfer_encoding,\n \t\t    \"format-patch!\" => \\$format_patch,\n-\t\t    \"no-format-patch\" => sub {$format_patch = 0},\n \t\t    \"8bit-encoding=s\" => \\$auto_8bit_encoding,\n \t\t    \"compose-encoding=s\" => \\$compose_encoding,\n \t\t    \"force\" => \\$force,\n \t\t    \"xmailer!\" => \\$use_xmailer,\n-\t\t    \"no-xmailer\" => sub {$use_xmailer = 0},\n \t\t    \"batch-size=i\" => \\$batch_size,\n \t\t    \"relogin-delay=i\" => \\$relogin_delay,\n \t\t    \"git-completion-helper\" => \\$git_completion_helper,\n"},{"id":"484868","messageId":"xmqqjzqkxmuk.fsf@gitster.g","threadId":"60519","inReplyTo":"20231114163826.207267-1-tmz@pobox.com","subject":"Re: [PATCH] send-email: avoid duplicate specification warnings","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-11-14T17:32:03Z","receivedAt":"2023-11-14T17:32:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Todd Zullinger <tmz@pobox.com> writes:\n\n> With perl-Getopt-Long >= 2.55, a warning is issued for options which are\n> specified more than once.  In addition to causing users to see warnings,\n> this results in test failures which compare the output.  An example,\n> from t9001-send-email.37:\n>\n>   | +++ diff -u expect actual\n>   | --- expect      2023-11-14 10:38:23.854346488 +0000\n>   | +++ actual      2023-11-14 10:38:23.848346466 +0000\n>   | @@ -1,2 +1,7 @@\n>   | +Duplicate specification \"no-chain-reply-to\" for option \"no-chain-reply-to\"\n>   | +Duplicate specification \"to-cover|to-cover!\" for option \"to-cover\"\n>   | +Duplicate specification \"cc-cover|cc-cover!\" for option \"cc-cover\"\n>   | +Duplicate specification \"no-thread\" for option \"no-thread\"\n>   | +Duplicate specification \"no-to-cover\" for option \"no-to-cover\"\n>   |  fatal: longline.patch:35 is longer than 998 characters\n>   |  warning: no patches were sent\n>   | error: last command exited with $?=1\n>   | not ok 37 - reject long lines\n>\n> Remove the duplicate option specs.\n\nAs long as these manual implementation of \"no-\" are doing true\nopposite of the positive one, it should be sufficient to remove\nthem, so I'd prefer to see you explicitly say that you did audit\nthem all to make sure.\n\nFor example,\n\n>  \t\t    \"annotate!\" => \\$annotate,\n> -\t\t    \"no-annotate\" => sub {$annotate = 0},\n\nthis is an example of good pair.  With the former, \"--no-annotate\"\nand \"--annotate\" result in $annotate set to false and true, and the\nlatter attempts to set $annotate to false upon \"--no-annotate\", so\nthe net result of removing the latter should be a no-op.\n\n>  \t\t    \"suppress-from!\" => \\$suppress_from,\n> -\t\t    \"no-suppress-from\" => sub {$suppress_from = 0},\n\nDitto.\n\nAs it is very late at night here, I didn't do a though job to scan\nand validate all of them (some did not have their positive\ncounterparts in the context), though.  Thanks for woking on this.\n"},{"id":"484883","messageId":"20231114200009.GD2092538@coredump.intra.peff.net","threadId":"60519","inReplyTo":"20231114163826.207267-1-tmz@pobox.com","subject":"Re: [PATCH] send-email: avoid duplicate specification warnings","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-11-14T20:00:09Z","receivedAt":"2023-11-14T20:00:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Nov 14, 2023 at 11:38:19AM -0500, Todd Zullinger wrote:\n\n> With perl-Getopt-Long >= 2.55, a warning is issued for options which are\n> specified more than once.  In addition to causing users to see warnings,\n> this results in test failures which compare the output.  An example,\n> from t9001-send-email.37:\n\nThis made me wonder if the warnings are new, or if the duplicated\nauto-negated options are new. I.e., were the manual \"--no-foo\" option\nspecs doing something useful in the older versions (in which case we'd\nneed to do something more complicated)?\n\nBut I think the answer is no.  We've explicitly marked these with \"!\" to\nindicate that they're negatable. And certainly running with Getopt::Long\n2.52 (from perl 5.36, which is the current in Debian unstable) seems to\nsupport them.\n\nIt does make me wonder why some boolean options are not marked as\nnegatable (even if just to countermand an earlier option), but that is\noutside the scope of your patch.\n\n> I've run this through the full test suite.  I also compared the output of\n> --help to ensure it only differs in the removal of the \"Duplicate\n> specification\" warnings.  I _think_ that's a good sign that no other changes\n> will result.  But I would be grateful to anyone who can confirm or reject that\n> theory.\n\nI guess you meant \"-h\", not \"--help\", since the latter will just show\nthe manpage. But isn't \"-h\" just dumping a static usage message we\nwrote, and not auto-generated by the code?\n\nThe changes look good to me (even after double-checking Junio's question\nthat they are all appropriately matched with their \"positive\" sides).\nThis one is curious:\n\n> -\t\t    \"cc-cover|cc-cover!\" => \\$cover_cc,\n\nIt was an alternate name for itself? I think somebody just misunderstood\nhow the API was supposed to work. The \"!\" would applies to all names, if\nI understand correctly, so this really is doing nothing beyond just\n\"cc-cover!\", which is what your patch switches it to.\n\n-Peff\n"},{"id":"484886","messageId":"ZVPfvjoXyGVlKqvr@pobox.com","threadId":"60519","inReplyTo":"20231114200009.GD2092538@coredump.intra.peff.net","subject":"Re: [PATCH] send-email: avoid duplicate specification warnings","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2023-11-14T20:59:42Z","receivedAt":"2023-11-14T20:59:52Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"Jeff King wrote:\n> On Tue, Nov 14, 2023 at 11:38:19AM -0500, Todd Zullinger wrote:\n>> I've run this through the full test suite.  I also compared the output of\n>> --help to ensure it only differs in the removal of the \"Duplicate\n>> specification\" warnings.  I _think_ that's a good sign that no other changes\n>> will result.  But I would be grateful to anyone who can confirm or reject that\n>> theory.\n> \n> I guess you meant \"-h\", not \"--help\", since the latter will just show\n> the manpage. But isn't \"-h\" just dumping a static usage message we\n> wrote, and not auto-generated by the code?\n\nYes to both.  This is why I shouldn't submit patches within\na few hours of waking up.\n\n> The changes look good to me (even after double-checking Junio's question\n> that they are all appropriately matched with their \"positive\" sides).\n\nIndeed.  I need to go through them each to test that the\nresults match before and after.  With the fallback to\npassing options to format-patch, testing outside of a git\nrepo makes this rather convenient.  If I've dropped an\noption it will result in the \"Cannot run git format-patch\nfrom outside a repository\" error.  That's a good start to\nensure the changes don't cause any regressions.\n\nI did notice that I mistakenly dropped --[no-]signed-off-cc.\nI need to keep:\n\n    \"signed-off-cc|signed-off-by-cc!\" => \\$signed_off_by_cc,\n\nas is.\n\n> This one is curious:\n> \n>> -\t\t    \"cc-cover|cc-cover!\" => \\$cover_cc,\n> \n> It was an alternate name for itself? I think somebody just misunderstood\n> how the API was supposed to work. The \"!\" would applies to all names, if\n> I understand correctly, so this really is doing nothing beyond just\n> \"cc-cover!\", which is what your patch switches it to.\n\nI wondered about those as well.  Perhaps this is needed in\nsome older version of Getopt::Long?  I'll try to look\nthrough the history of the module to see if that's the case.\n\nSince this isn't anything new with 2.43, it doesn't need to\nbe fixed with much urgency.\n\nThanks both,\n\n-- \nTodd\n"},{"id":"484903","messageId":"xmqqy1ezx2mq.fsf@gitster.g","threadId":"60519","inReplyTo":"ZVPfvjoXyGVlKqvr@pobox.com","subject":"Re: [PATCH] send-email: avoid duplicate specification warnings","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-11-15T00:48:45Z","receivedAt":"2023-11-15T00:48:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Todd Zullinger <tmz@pobox.com> writes:\n\n> Since this isn't anything new with 2.43, it doesn't need to\n> be fixed with much urgency.\n\nTrue.  Unless the new version of Getopt::Long is quickly spreading\nthrough our user base, that is.\n\n> Thanks both,\n\nThanks for spotting the issue and acting on it quickly.\n"},{"id":"484941","messageId":"20231115173952.339303-1-tmz@pobox.com","threadId":"60519","inReplyTo":"xmqqy1ezx2mq.fsf@gitster.g","subject":"[RFC PATCH v2 0/2] send-email: avoid duplicate specification warnings","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2023-11-15T17:39:42Z","receivedAt":"2023-11-15T17:40:15Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"Changes since v1:\n\n    * Teach `--git-completion-helper` to output the '--no-' options.\n      They are not included in the options hash and would otherwise be\n      lost.\n\n    * Restore the `--signed-off-cc` alias which was mistakenly removed.\n\nTodd Zullinger (2):\n  send-email: avoid duplicate specification warnings\n  send-email: remove stray characters from usage\n\n git-send-email.perl | 23 ++++++++---------------\n 1 file changed, 8 insertions(+), 15 deletions(-)\n\n-- \n2.43.0.rc2\n\n"},{"id":"484942","messageId":"20231115173952.339303-2-tmz@pobox.com","threadId":"60519","inReplyTo":"20231115173952.339303-1-tmz@pobox.com","subject":"[RFC PATCH v2 1/2] send-email: avoid duplicate specification warnings","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2023-11-15T17:39:43Z","receivedAt":"2023-11-15T17:40:20Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"With perl-Getopt-Long >= 2.55 a warning is issued for options which are\nspecified more than once.  In addition to causing users to see warnings,\nthis results in test failures which compare the output.  An example,\nfrom t9001-send-email.37:\n\n  | +++ diff -u expect actual\n  | --- expect      2023-11-14 10:38:23.854346488 +0000\n  | +++ actual      2023-11-14 10:38:23.848346466 +0000\n  | @@ -1,2 +1,7 @@\n  | +Duplicate specification \"no-chain-reply-to\" for option \"no-chain-reply-to\"\n  | +Duplicate specification \"to-cover|to-cover!\" for option \"to-cover\"\n  | +Duplicate specification \"cc-cover|cc-cover!\" for option \"cc-cover\"\n  | +Duplicate specification \"no-thread\" for option \"no-thread\"\n  | +Duplicate specification \"no-to-cover\" for option \"no-to-cover\"\n  |  fatal: longline.patch:35 is longer than 998 characters\n  |  warning: no patches were sent\n  | error: last command exited with $?=1\n  | not ok 37 - reject long lines\n\nRemove the duplicate option specs.\n\nTeach `--git-completion-helper` to output the '--no-' options.  They are\nnot included in the options hash and would otherwise be lost.\n\nSigned-off-by: Todd Zullinger <tmz@pobox.com>\n---\nI compared the output from\n\n    git send-email --git-completion-helper | tr ' ' ' '\\n' | sort\n\nbefore and after the change to ensure no options were lost (or added).\n\nI also confirmed that each of the options changed did not result in any\nerror.  Unrecognized options result in an error from `git format-patch`,\ne.g.:\n\n    $ git send-email --foo\n    fatal: unrecognized argument: --foo\n    format-patch -o /tmp/PaqtbH3jCw --foo: command returned error: 128\n\nA little history:\n\n  Support for the '--no-' prefix was added in Getopt::Long >= 2.33, in\n  commit 8ca8b48 (Negatable options (with \"!\") now also support the\n  \"no-\" prefix., 2003-04-04).  Getopt::Long 2.34 was included in\n  perl-5.8.1 (2003-09-25), per Module::CoreList[1].\n  \n  We list perl-5.8 as the minimum version in INSTALL.  This would leave\n  users with perl-5.8.0 (2002-07-18) with non-working arguments for\n  options where we're removing the explicit 'no-' variant.\n  \n  The explicit 'no-' opts were added in f471494303 (git-send-email.perl:\n  support no- prefix with older GetOptions, 2015-01-30), specifically to\n  support perl-5.8.0 which includes the older Getopt::Long.\n    \nIt may be time to bump the Perl requirement to 5.8.1 (2003-09-25) or\neven 5.10.0 (2007-12-18).  We last bumped the requirement from 5.6 to\n5.8 in d48b284183 (perl: bump the required Perl version to 5.8 from\n5.6.[21], 2010-09-24).\n\nAnother option to avoid the warning from Getopt::Long >= 2.55 would be\nto remove the '!' negation, but that would drop support for the 'no'\nprefix variants (e.g.: `--nocc-cover`).  While these are not documented\n(and I don't think they ever were[2]), they have worked for a long, long\ntime.  Odds are good that some scripts rely on them and we don't want\nanyone yelling at Junio.\n\nI lean toward dropping support for the 21-year-old 5.8.0.\n\nIf there is a way to have our cake without any consequence, I'm happy to\nhear it.  If not, I'll add a commit which bumps the requirement in\ngeneral or notes that some git-send-email requires perl >= 5.8.1 and\nadjusts the 'use' line there to `use 5.008001;`.\n\n[1] http://perlpunks.de/corelist/mversion?module=Getopt%3A%3ALong\n\n[2] The 'no-' opts were added in f471494303 (git-send-email.perl:\n    support no- prefix with older GetOptions, 2015-01-30).  The commit\n    message says \"the help only mentions the 'no-' prefix and not the\n    'no' prefix, add explicit support for the 'no-' prefix to support\n    older GetOptions versions.\"\n\n git-send-email.perl | 19 ++++++-------------\n 1 file changed, 6 insertions(+), 13 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex cacdbd6bb2..94046e0fb7 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -119,13 +119,16 @@ sub completion_helper {\n \n \tforeach my $key (keys %$original_opts) {\n \t\tunless (exists $not_for_completion{$key}) {\n-\t\t\t$key =~ s/!$//;\n+\t\t\tmy $negate = ($key =~ s/!$//);\n \n \t\t\tif ($key =~ /[:=][si]$/) {\n \t\t\t\t$key =~ s/[:=][si]$//;\n \t\t\t\tpush (@send_email_opts, \"--$_=\") foreach (split (/\\|/, $key));\n \t\t\t} else {\n \t\t\t\tpush (@send_email_opts, \"--$_\") foreach (split (/\\|/, $key));\n+\t\t\t\tif ($negate) {\n+\t\t\t\t\tpush (@send_email_opts, \"--no-$_\") foreach (split (/\\|/, $key));\n+\t\t\t\t}\n \t\t\t}\n \t\t}\n \t}\n@@ -491,7 +494,6 @@ sub config_regexp {\n \t\t    \"bcc=s\" => \\@getopt_bcc,\n \t\t    \"no-bcc\" => \\$no_bcc,\n \t\t    \"chain-reply-to!\" => \\$chain_reply_to,\n-\t\t    \"no-chain-reply-to\" => sub {$chain_reply_to = 0},\n \t\t    \"sendmail-cmd=s\" => \\$sendmail_cmd,\n \t\t    \"smtp-server=s\" => \\$smtp_server,\n \t\t    \"smtp-server-option=s\" => \\@smtp_server_options,\n@@ -506,36 +508,27 @@ sub config_regexp {\n \t\t    \"smtp-auth=s\" => \\$smtp_auth,\n \t\t    \"no-smtp-auth\" => sub {$smtp_auth = 'none'},\n \t\t    \"annotate!\" => \\$annotate,\n-\t\t    \"no-annotate\" => sub {$annotate = 0},\n \t\t    \"compose\" => \\$compose,\n \t\t    \"quiet\" => \\$quiet,\n \t\t    \"cc-cmd=s\" => \\$cc_cmd,\n \t\t    \"header-cmd=s\" => \\$header_cmd,\n \t\t    \"no-header-cmd\" => \\$no_header_cmd,\n \t\t    \"suppress-from!\" => \\$suppress_from,\n-\t\t    \"no-suppress-from\" => sub {$suppress_from = 0},\n \t\t    \"suppress-cc=s\" => \\@suppress_cc,\n \t\t    \"signed-off-cc|signed-off-by-cc!\" => \\$signed_off_by_cc,\n-\t\t    \"no-signed-off-cc|no-signed-off-by-cc\" => sub {$signed_off_by_cc = 0},\n-\t\t    \"cc-cover|cc-cover!\" => \\$cover_cc,\n-\t\t    \"no-cc-cover\" => sub {$cover_cc = 0},\n-\t\t    \"to-cover|to-cover!\" => \\$cover_to,\n-\t\t    \"no-to-cover\" => sub {$cover_to = 0},\n+\t\t    \"cc-cover!\" => \\$cover_cc,\n+\t\t    \"to-cover!\" => \\$cover_to,\n \t\t    \"confirm=s\" => \\$confirm,\n \t\t    \"dry-run\" => \\$dry_run,\n \t\t    \"envelope-sender=s\" => \\$envelope_sender,\n \t\t    \"thread!\" => \\$thread,\n-\t\t    \"no-thread\" => sub {$thread = 0},\n \t\t    \"validate!\" => \\$validate,\n-\t\t    \"no-validate\" => sub {$validate = 0},\n \t\t    \"transfer-encoding=s\" => \\$target_xfer_encoding,\n \t\t    \"format-patch!\" => \\$format_patch,\n-\t\t    \"no-format-patch\" => sub {$format_patch = 0},\n \t\t    \"8bit-encoding=s\" => \\$auto_8bit_encoding,\n \t\t    \"compose-encoding=s\" => \\$compose_encoding,\n \t\t    \"force\" => \\$force,\n \t\t    \"xmailer!\" => \\$use_xmailer,\n-\t\t    \"no-xmailer\" => sub {$use_xmailer = 0},\n \t\t    \"batch-size=i\" => \\$batch_size,\n \t\t    \"relogin-delay=i\" => \\$relogin_delay,\n \t\t    \"git-completion-helper\" => \\$git_completion_helper,\n-- \n2.43.0.rc2\n\n"},{"id":"484943","messageId":"20231115173952.339303-3-tmz@pobox.com","threadId":"60519","inReplyTo":"20231115173952.339303-1-tmz@pobox.com","subject":"[RFC PATCH v2 2/2] send-email: remove stray characters from usage","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2023-11-15T17:39:44Z","receivedAt":"2023-11-15T17:40:25Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"A few stray single quotes crept into the usage string in a2ce608244\n(send-email docs: add format-patch options, 2021-10-25).  Remove them.\n\nSigned-off-by: Todd Zullinger <tmz@pobox.com>\n---\nThis is not scrictly tied to the previous commit.  It just stood out\nwhile I was reviewing the usage output.\n\n git-send-email.perl | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 94046e0fb7..cd2f0ae14e 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -28,8 +28,8 @@\n \n sub usage {\n \tprint <<EOT;\n-git send-email' [<options>] <file|directory>\n-git send-email' [<options>] <format-patch options>\n+git send-email [<options>] <file|directory>\n+git send-email [<options>] <format-patch options>\n git send-email --dump-aliases\n \n   Composing:\n-- \n2.43.0.rc2\n\n"},{"id":"484961","messageId":"xmqq4jhmthtg.fsf@gitster.g","threadId":"60519","inReplyTo":"20231115173952.339303-2-tmz@pobox.com","subject":"Re: [RFC PATCH v2 1/2] send-email: avoid duplicate specification warnings","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-11-16T04:58:51Z","receivedAt":"2023-11-16T04:58:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Todd Zullinger <tmz@pobox.com> writes:\n\n> With perl-Getopt-Long >= 2.55 a warning is issued for options which are\n> specified more than once.  In addition to causing users to see warnings,\n> this results in test failures which compare the output.  An example,\n> from t9001-send-email.37:\n>\n>   | +++ diff -u expect actual\n>   | --- expect      2023-11-14 10:38:23.854346488 +0000\n>   | +++ actual      2023-11-14 10:38:23.848346466 +0000\n>   | @@ -1,2 +1,7 @@\n>   | +Duplicate specification \"no-chain-reply-to\" for option \"no-chain-reply-to\"\n>   | +Duplicate specification \"to-cover|to-cover!\" for option \"to-cover\"\n>   | +Duplicate specification \"cc-cover|cc-cover!\" for option \"cc-cover\"\n>   | +Duplicate specification \"no-thread\" for option \"no-thread\"\n>   | +Duplicate specification \"no-to-cover\" for option \"no-to-cover\"\n>   |  fatal: longline.patch:35 is longer than 998 characters\n>   |  warning: no patches were sent\n>   | error: last command exited with $?=1\n>   | not ok 37 - reject long lines\n>\n> Remove the duplicate option specs.\n>\n> Teach `--git-completion-helper` to output the '--no-' options.  They are\n> not included in the options hash and would otherwise be lost.\n\nNice to see a careful handling of potential fallouts.\n\n> A little history:\n>\n>   Support for the '--no-' prefix was added in Getopt::Long >= 2.33, in\n>   commit 8ca8b48 (Negatable options (with \"!\") now also support the\n>   \"no-\" prefix., 2003-04-04).  Getopt::Long 2.34 was included in\n>   perl-5.8.1 (2003-09-25), per Module::CoreList[1].\n>   \n>   We list perl-5.8 as the minimum version in INSTALL.  This would leave\n>   users with perl-5.8.0 (2002-07-18) with non-working arguments for\n>   options where we're removing the explicit 'no-' variant.\n>   \n>   The explicit 'no-' opts were added in f471494303 (git-send-email.perl:\n>   support no- prefix with older GetOptions, 2015-01-30), specifically to\n>   support perl-5.8.0 which includes the older Getopt::Long.\n\nThese are all very much relevant and deserve to be in the log\nmessage, not hidden under the three-dash line, I would think.\nThanks for digging the history.  The first paragraph was a bit hard\nto read as it wasn't clear \"support\" on which side is being\ndiscussed, though.  If it were written perhaps like so:\n\n   Getopt::Long >= 2.33 started supporting the '--no-' prefix\n   natively by appending '!' to the option specification string,\n   which was shipped with perl-5.8.1 and not present in perl-5.8.0\n\nit would have been clear that it was talking about the support\ngiven by Getopt module, not on our side.\n\n> It may be time to bump the Perl requirement to 5.8.1 (2003-09-25) or\n> even 5.10.0 (2007-12-18).  We last bumped the requirement from 5.6 to\n> 5.8 in d48b284183 (perl: bump the required Perl version to 5.8 from\n> 5.6.[21], 2010-09-24).\n\nIsn't the position this patch takes a lot stronger than \"It may be\ntime\"?  If we applied this patch, it drops the support for folks\nwith Perl 5.8.0 (which I do not think is a bad thing, by the way).\n\nThis sounds like something that is worth describing in the log\nmessage (and Release Notes).\n\n> If there is a way to have our cake without any consequence, I'm happy to\n> hear it.  If not, I'll add a commit which bumps the requirement in\n> general or notes that some git-send-email requires perl >= 5.8.1 and\n> adjusts the 'use' line there to `use 5.008001;`.\n\nSounds like a plan.\n\n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index cacdbd6bb2..94046e0fb7 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -119,13 +119,16 @@ sub completion_helper {\n>  \n>  \tforeach my $key (keys %$original_opts) {\n>  \t\tunless (exists $not_for_completion{$key}) {\n> -\t\t\t$key =~ s/!$//;\n> +\t\t\tmy $negate = ($key =~ s/!$//);\n\nA very minor nit, but I'd call this $negatable if I were doing this\npatch.\n\nJust to make sure I did not misunderstand what you said below the\nthree-dash line, if we were to take the other option that allows us\nto live with 5.8.0, we would make this hunk ...\n\n>  \t\t    \"chain-reply-to!\" => \\$chain_reply_to,\n> -\t\t    \"no-chain-reply-to\" => sub {$chain_reply_to = 0},\n\n... look more like this?\n\n> -\t\t    \"chain-reply-to!\" => \\$chain_reply_to,\n> +\t\t    \"chain-reply-to\" => \\$chain_reply_to,\n>  \t\t    \"no-chain-reply-to\" => sub {$chain_reply_to = 0},\n> +\t\t    \"nochain-reply-to\" => sub {$chain_reply_to = 0},\n\nThat is, by removing the \"!\" suffix, we reject the native support of\n\"--no-*\" offered by Getopt::Long, and implement the negated variants\nourselves?\n\nThanks.\n"},{"id":"484962","messageId":"xmqqzfzes37f.fsf@gitster.g","threadId":"60519","inReplyTo":"20231115173952.339303-3-tmz@pobox.com","subject":"Re: [RFC PATCH v2 2/2] send-email: remove stray characters from usage","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-11-16T04:59:48Z","receivedAt":"2023-11-16T04:59:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Todd Zullinger <tmz@pobox.com> writes:\n\n> A few stray single quotes crept into the usage string in a2ce608244\n> (send-email docs: add format-patch options, 2021-10-25).  Remove them.\n>\n> Signed-off-by: Todd Zullinger <tmz@pobox.com>\n> ---\n> This is not scrictly tied to the previous commit.  It just stood out\n> while I was reviewing the usage output.\n\nThanks.  Let's split this out as a docfix patch and handle it\nseparately.\n\n\n>\n>  git-send-email.perl | 4 ++--\n>  1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index 94046e0fb7..cd2f0ae14e 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -28,8 +28,8 @@\n>  \n>  sub usage {\n>  \tprint <<EOT;\n> -git send-email' [<options>] <file|directory>\n> -git send-email' [<options>] <format-patch options>\n> +git send-email [<options>] <file|directory>\n> +git send-email [<options>] <format-patch options>\n>  git send-email --dump-aliases\n>  \n>    Composing:\n"},{"id":"484980","messageId":"20231116193014.470420-1-tmz@pobox.com","threadId":"60519","inReplyTo":"xmqq4jhmthtg.fsf@gitster.g","subject":"[PATCH v3 0/2] send-email: avoid duplicate specification warnings","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2023-11-16T19:30:09Z","receivedAt":"2023-11-16T19:30:28Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"Junio C Hamano wrote:\n> Todd Zullinger <tmz@pobox.com> writes:\n[...]\n>> A little history:\n>>\n>>   Support for the '--no-' prefix was added in Getopt::Long >= 2.33, in\n>>   commit 8ca8b48 (Negatable options (with \"!\") now also support the\n>>   \"no-\" prefix., 2003-04-04).  Getopt::Long 2.34 was included in\n>>   perl-5.8.1 (2003-09-25), per Module::CoreList[1].\n>>   \n>>   We list perl-5.8 as the minimum version in INSTALL.  This would leave\n>>   users with perl-5.8.0 (2002-07-18) with non-working arguments for\n>>   options where we're removing the explicit 'no-' variant.\n>>   \n>>   The explicit 'no-' opts were added in f471494303 (git-send-email.perl:\n>>   support no- prefix with older GetOptions, 2015-01-30), specifically to\n>>   support perl-5.8.0 which includes the older Getopt::Long.\n> \n> These are all very much relevant and deserve to be in the log\n> message, not hidden under the three-dash line, I would think.\n> Thanks for digging the history.  The first paragraph was a bit hard\n> to read as it wasn't clear \"support\" on which side is being\n> discussed, though.  If it were written perhaps like so:\n> \n>    Getopt::Long >= 2.33 started supporting the '--no-' prefix\n>    natively by appending '!' to the option specification string,\n>    which was shipped with perl-5.8.1 and not present in perl-5.8.0\n> \n> it would have been clear that it was talking about the support\n> given by Getopt module, not on our side.\n\nThat is much better.  I've adjusted the commit message similarly and\nhopefully kept your improved wording largely intact.\n\n>> It may be time to bump the Perl requirement to 5.8.1 (2003-09-25) or\n>> even 5.10.0 (2007-12-18).  We last bumped the requirement from 5.6 to\n>> 5.8 in d48b284183 (perl: bump the required Perl version to 5.8 from\n>> 5.6.[21], 2010-09-24).\n>\n> Isn't the position this patch takes a lot stronger than \"It may be\n> time\"?  If we applied this patch, it drops the support for folks\n> with Perl 5.8.0 (which I do not think is a bad thing, by the way).\n\nIndeed it is.  I should have mentioned that more explicitly.  I added\nthe RFC tag to this round because I was unsure whether we'd want to go\nthe route of bumping the Perl requirement.  But I managed to not\nactually say as much.\n\n> This sounds like something that is worth describing in the log\n> message (and Release Notes).\n\nI think the new commit messages describe the changes better.  I didn't\ninclude anything in RelNotes as I was presuming we'd leave this for\n2.44 rather than risk causing any problems this late in the 2.43 cycle.\nIf you think the risk is low and/or the benefit is high, I can add it to\nthe 2.43.0 RelNotes.\n\n>> diff --git a/git-send-email.perl b/git-send-email.perl\n>> index cacdbd6bb2..94046e0fb7 100755\n>> --- a/git-send-email.perl\n>> +++ b/git-send-email.perl\n>> @@ -119,13 +119,16 @@ sub completion_helper {\n>>  \n>>      foreach my $key (keys %$original_opts) {\n>>              unless (exists $not_for_completion{$key}) {\n>> -                    $key =~ s/!$//;\n>> +                    my $negate = ($key =~ s/!$//);\n> \n> A very minor nit, but I'd call this $negatable if I were doing this\n> patch.\n\nSounds good.\n\n> Just to make sure I did not misunderstand what you said below the\n> three-dash line, if we were to take the other option that allows us\n> to live with 5.8.0, we would make this hunk ...\n> \n>>                  \"chain-reply-to!\" => \\$chain_reply_to,\n>> -                \"no-chain-reply-to\" => sub {$chain_reply_to = 0},\n> \n> ... look more like this?\n> \n>> -                \"chain-reply-to!\" => \\$chain_reply_to,\n>> +                \"chain-reply-to\" => \\$chain_reply_to,\n>>                  \"no-chain-reply-to\" => sub {$chain_reply_to = 0},\n>> +                \"nochain-reply-to\" => sub {$chain_reply_to = 0},\n> \n> That is, by removing the \"!\" suffix, we reject the native support of\n> \"--no-*\" offered by Getopt::Long, and implement the negated variants\n> ourselves?\n\nExactly.  We could bundle the two no* options together, but that's a\ntrivial style issue, i.e.:\n\n> -                \"chain-reply-to!\" => \\$chain_reply_to,\n> -                \"no-chain-reply-to\" => sub {$chain_reply_to = 0},\n> +                \"chain-reply-to\" => \\$chain_reply_to,\n> +                \"no-chain-reply-to|nochain-reply-to\" => sub {$chain_reply_to = 0},\n\nThanks for a very helpful review, as always.\n\nTodd Zullinger (2):\n  perl: bump the required Perl version to 5.8.1 from 5.8.0\n  send-email: avoid duplicate specification warnings\n\n Documentation/CodingGuidelines          |  2 +-\n INSTALL                                 |  2 +-\n contrib/diff-highlight/DiffHighlight.pm |  2 +-\n contrib/mw-to-git/Git/Mediawiki.pm      |  2 +-\n git-archimport.perl                     |  2 +-\n git-cvsexportcommit.perl                |  2 +-\n git-cvsimport.perl                      |  2 +-\n git-cvsserver.perl                      |  2 +-\n git-send-email.perl                     | 23 ++++++++---------------\n git-svn.perl                            |  2 +-\n gitweb/INSTALL                          |  2 +-\n gitweb/gitweb.perl                      |  2 +-\n perl/Git.pm                             |  2 +-\n perl/Git/I18N.pm                        |  2 +-\n perl/Git/LoadCPAN.pm                    |  2 +-\n perl/Git/LoadCPAN/Error.pm              |  2 +-\n perl/Git/LoadCPAN/Mail/Address.pm       |  2 +-\n perl/Git/Packet.pm                      |  2 +-\n t/t0202/test.pl                         |  2 +-\n t/t5562/invoke-with-content-length.pl   |  2 +-\n t/t9700/test.pl                         |  2 +-\n t/test-terminal.perl                    |  2 +-\n 22 files changed, 29 insertions(+), 36 deletions(-)\n\nRange-diff against v2:\n-:  ---------- > 1:  b276216a53 perl: bump the required Perl version to 5.8.1 from 5.8.0\n1:  59e2c79085 ! 2:  e076a2ede5 send-email: avoid duplicate specification warnings\n    @@ Metadata\n      ## Commit message ##\n         send-email: avoid duplicate specification warnings\n     \n    -    With perl-Getopt-Long >= 2.55 a warning is issued for options which are\n    -    specified more than once.  In addition to causing users to see warnings,\n    -    this results in test failures which compare the output.  An example,\n    -    from t9001-send-email.37:\n    +    A warning is issued for options which are specified more than once\n    +    beginning with perl-Getopt-Long >= 2.55.  In addition to causing users\n    +    to see warnings, this results in test failures which compare the output.\n    +    An example, from t9001-send-email.37:\n     \n           | +++ diff -u expect actual\n           | --- expect      2023-11-14 10:38:23.854346488 +0000\n    @@ Commit message\n           | error: last command exited with $?=1\n           | not ok 37 - reject long lines\n     \n    -    Remove the duplicate option specs.\n    +    Remove the duplicate option specs.  These are primarily the explicit\n    +    '--no-' prefix opts which were added in f471494303 (git-send-email.perl:\n    +    support no- prefix with older GetOptions, 2015-01-30).  This was done\n    +    specifically to support perl-5.8.0 which includes Getopt::Long 2.32[1].\n    +\n    +    Getopt::Long 2.33 added support for the '--no-' prefix natively by\n    +    appending '!' to the option specification string, which was included in\n    +    perl-5.8.1 and is not present in perl-5.8.0.  The previous commit bumped\n    +    the minimum supported Perl version to 5.8.1 so we no longer need to\n    +    provide the '--no-' variants for negatable options manually.\n     \n         Teach `--git-completion-helper` to output the '--no-' options.  They are\n         not included in the options hash and would otherwise be lost.\n     \n         Signed-off-by: Todd Zullinger <tmz@pobox.com>\n     \n      ## git-send-email.perl ##\n     @@ git-send-email.perl: sub completion_helper {\n      \n      \tforeach my $key (keys %$original_opts) {\n      \t\tunless (exists $not_for_completion{$key}) {\n     -\t\t\t$key =~ s/!$//;\n    -+\t\t\tmy $negate = ($key =~ s/!$//);\n    ++\t\t\tmy $negatable = ($key =~ s/!$//);\n      \n      \t\t\tif ($key =~ /[:=][si]$/) {\n      \t\t\t\t$key =~ s/[:=][si]$//;\n      \t\t\t\tpush (@send_email_opts, \"--$_=\") foreach (split (/\\|/, $key));\n      \t\t\t} else {\n      \t\t\t\tpush (@send_email_opts, \"--$_\") foreach (split (/\\|/, $key));\n    -+\t\t\t\tif ($negate) {\n    ++\t\t\t\tif ($negatable) {\n     +\t\t\t\t\tpush (@send_email_opts, \"--no-$_\") foreach (split (/\\|/, $key));\n     +\t\t\t\t}\n      \t\t\t}\n2:  c1f37d4395 < -:  ---------- send-email: remove stray characters from usage\n-- \n2.43.0.rc2\n\n"},{"id":"484981","messageId":"20231116193014.470420-2-tmz@pobox.com","threadId":"60519","inReplyTo":"20231116193014.470420-1-tmz@pobox.com","subject":"[PATCH v3 1/2] perl: bump the required Perl version to 5.8.1 from 5.8.0","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2023-11-16T19:30:10Z","receivedAt":"2023-11-16T19:30:32Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"The following commit will make use of a Getopt::Long feature which is\nonly present in Perl >= 5.8.1.  Document that as the minimum version we\nsupport.\n\nMany of our Perl scripts will continue to run with 5.8.0 but this change\nallows us to adjust them as needed without breaking any promises to our\nusers.\n\nThe Perl requirement was last changed in d48b284183 (perl: bump the\nrequired Perl version to 5.8 from 5.6.[21], 2010-09-24).  At that time,\n5.8.0 was 8 years old.  It is now over 21 years old.\n\nSigned-off-by: Todd Zullinger <tmz@pobox.com>\n---\nI debated changing all the 'use 5.008;' lines here, as most don't\nactually require a newer Perl, but the previous bump did the same.\n\nI can see the merit in either direction.\n\nChanging it allows future contributors to be confident in relying on\n5.8.1 features.\n\nNot changing it allows anyone stuck on 5.8.0 to continue using the perl\nscripts which don't actually require 5.8.1.\n\nTangentially, the Perl docs for 'use' function recommend against the\n5.008001 form[1]:\n\n    Specifying VERSION as a numeric argument of the form 5.024001 should\n    generally be avoided as older less readable syntax compared to\n    v5.24.1. Before perl 5.8.0 released in 2002 the more verbose numeric\n    form was the only supported syntax, which is why you might see it in\n    older code.\n\n        use v5.24.1;    # compile time version check\n        use 5.24.1;     # ditto\n        use 5.024_001;  # ditto; older syntax compatible with perl 5.6\n\nI'm not enough of a Perl coder to have a strong preference or desire to\npush for such a change, but I thought it was worth mentioning in case\nothers wonder why we're using the 5.008001 form.\n\n[1] https://perldoc.perl.org/functions/use#use-VERSION\n\n Documentation/CodingGuidelines          | 2 +-\n INSTALL                                 | 2 +-\n contrib/diff-highlight/DiffHighlight.pm | 2 +-\n contrib/mw-to-git/Git/Mediawiki.pm      | 2 +-\n git-archimport.perl                     | 2 +-\n git-cvsexportcommit.perl                | 2 +-\n git-cvsimport.perl                      | 2 +-\n git-cvsserver.perl                      | 2 +-\n git-send-email.perl                     | 4 ++--\n git-svn.perl                            | 2 +-\n gitweb/INSTALL                          | 2 +-\n gitweb/gitweb.perl                      | 2 +-\n perl/Git.pm                             | 2 +-\n perl/Git/I18N.pm                        | 2 +-\n perl/Git/LoadCPAN.pm                    | 2 +-\n perl/Git/LoadCPAN/Error.pm              | 2 +-\n perl/Git/LoadCPAN/Mail/Address.pm       | 2 +-\n perl/Git/Packet.pm                      | 2 +-\n t/t0202/test.pl                         | 2 +-\n t/t5562/invoke-with-content-length.pl   | 2 +-\n t/t9700/test.pl                         | 2 +-\n t/test-terminal.perl                    | 2 +-\n 22 files changed, 23 insertions(+), 23 deletions(-)\n\ndiff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\nindex 8d3a467c01..39b9b7260f 100644\n--- a/Documentation/CodingGuidelines\n+++ b/Documentation/CodingGuidelines\n@@ -490,7 +490,7 @@ For Perl programs:\n \n  - Most of the C guidelines above apply.\n \n- - We try to support Perl 5.8 and later (\"use Perl 5.008\").\n+ - We try to support Perl 5.8.1 and later (\"use Perl 5.008001\").\n \n  - use strict and use warnings are strongly preferred.\n \ndiff --git a/INSTALL b/INSTALL\nindex 4b42288882..06f29a8ae7 100644\n--- a/INSTALL\n+++ b/INSTALL\n@@ -119,7 +119,7 @@ Issues of note:\n \t- A POSIX-compliant shell is required to run some scripts needed\n \t  for everyday use (e.g. \"bisect\", \"request-pull\").\n \n-\t- \"Perl\" version 5.8 or later is needed to use some of the\n+\t- \"Perl\" version 5.8.1 or later is needed to use some of the\n \t  features (e.g. sending patches using \"git send-email\",\n \t  interacting with svn repositories with \"git svn\").  If you can\n \t  live without these, use NO_PERL.  Note that recent releases of\ndiff --git a/contrib/diff-highlight/DiffHighlight.pm b/contrib/diff-highlight/DiffHighlight.pm\nindex 376f577737..636add6968 100644\n--- a/contrib/diff-highlight/DiffHighlight.pm\n+++ b/contrib/diff-highlight/DiffHighlight.pm\n@@ -1,6 +1,6 @@\n package DiffHighlight;\n \n-use 5.008;\n+use 5.008001;\n use warnings FATAL => 'all';\n use strict;\n \ndiff --git a/contrib/mw-to-git/Git/Mediawiki.pm b/contrib/mw-to-git/Git/Mediawiki.pm\nindex 917d9e2d32..ff7811225e 100644\n--- a/contrib/mw-to-git/Git/Mediawiki.pm\n+++ b/contrib/mw-to-git/Git/Mediawiki.pm\n@@ -1,6 +1,6 @@\n package Git::Mediawiki;\n \n-use 5.008;\n+use 5.008001;\n use strict;\n use POSIX;\n use Git;\ndiff --git a/git-archimport.perl b/git-archimport.perl\nindex b7c173c345..f5a317b899 100755\n--- a/git-archimport.perl\n+++ b/git-archimport.perl\n@@ -54,7 +54,7 @@ =head1 Devel Notes\n \n =cut\n \n-use 5.008;\n+use 5.008001;\n use strict;\n use warnings;\n use Getopt::Std;\ndiff --git a/git-cvsexportcommit.perl b/git-cvsexportcommit.perl\nindex 289d4bc684..1e03ba94d1 100755\n--- a/git-cvsexportcommit.perl\n+++ b/git-cvsexportcommit.perl\n@@ -1,6 +1,6 @@\n #!/usr/bin/perl\n \n-use 5.008;\n+use 5.008001;\n use strict;\n use warnings;\n use Getopt::Std;\ndiff --git a/git-cvsimport.perl b/git-cvsimport.perl\nindex 7bf3c12d67..07ea3443f7 100755\n--- a/git-cvsimport.perl\n+++ b/git-cvsimport.perl\n@@ -13,7 +13,7 @@\n # The head revision is on branch \"origin\" by default.\n # You can change that with the '-o' option.\n \n-use 5.008;\n+use 5.008001;\n use strict;\n use warnings;\n use Getopt::Long;\ndiff --git a/git-cvsserver.perl b/git-cvsserver.perl\nindex 7b757360e2..124f598bdc 100755\n--- a/git-cvsserver.perl\n+++ b/git-cvsserver.perl\n@@ -15,7 +15,7 @@\n ####\n ####\n \n-use 5.008;\n+use 5.008001;\n use strict;\n use warnings;\n use bytes;\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex cacdbd6bb2..d75a4a33dd 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -16,7 +16,7 @@\n #    and second line is the subject of the message.\n #\n \n-use 5.008;\n+use 5.008001;\n use strict;\n use warnings $ENV{GIT_PERL_FATAL_WARNINGS} ? qw(FATAL all) : ();\n use Getopt::Long;\n@@ -228,7 +228,7 @@ sub system_or_msg {\n \tmy @sprintf_args = ($cmd_name ? $cmd_name : $args->[0], $exit_code);\n \tif (defined $msg) {\n \t\t# Quiet the 'redundant' warning category, except we\n-\t\t# need to support down to Perl 5.8, so we can't do a\n+\t\t# need to support down to Perl 5.8.1, so we can't do a\n \t\t# \"no warnings 'redundant'\", since that category was\n \t\t# introduced in perl 5.22, and asking for it will die\n \t\t# on older perls.\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 4e8878f035..b0d0a50984 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -1,7 +1,7 @@\n #!/usr/bin/perl\n # Copyright (C) 2006, Eric Wong <normalperson@yhbt.net>\n # License: GPL v2 or later\n-use 5.008;\n+use 5.008001;\n use warnings $ENV{GIT_PERL_FATAL_WARNINGS} ? qw(FATAL all) : ();\n use strict;\n use vars qw/\t$AUTHOR $VERSION\ndiff --git a/gitweb/INSTALL b/gitweb/INSTALL\nindex a58e6b3c44..dadc6efa81 100644\n--- a/gitweb/INSTALL\n+++ b/gitweb/INSTALL\n@@ -29,7 +29,7 @@ Requirements\n ------------\n \n  - Core git tools\n- - Perl 5.8\n+ - Perl 5.8.1\n  - Perl modules: CGI, Encode, Fcntl, File::Find, File::Basename.\n  - web server\n \ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex e66eb3d9ba..55e7c6567e 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -7,7 +7,7 @@\n #\n # This program is licensed under the GPLv2\n \n-use 5.008;\n+use 5.008001;\n use strict;\n use warnings;\n # handle ACL in file access tests\ndiff --git a/perl/Git.pm b/perl/Git.pm\nindex 117765dc73..03bf570bf4 100644\n--- a/perl/Git.pm\n+++ b/perl/Git.pm\n@@ -7,7 +7,7 @@ =head1 NAME\n \n package Git;\n \n-use 5.008;\n+use 5.008001;\n use strict;\n use warnings $ENV{GIT_PERL_FATAL_WARNINGS} ? qw(FATAL all) : ();\n \ndiff --git a/perl/Git/I18N.pm b/perl/Git/I18N.pm\nindex 895e759c57..5454c3a6d2 100644\n--- a/perl/Git/I18N.pm\n+++ b/perl/Git/I18N.pm\n@@ -1,5 +1,5 @@\n package Git::I18N;\n-use 5.008;\n+use 5.008001;\n use strict;\n use warnings $ENV{GIT_PERL_FATAL_WARNINGS} ? qw(FATAL all) : ();\n BEGIN {\ndiff --git a/perl/Git/LoadCPAN.pm b/perl/Git/LoadCPAN.pm\nindex 0c360bc799..8c7fa805f9 100644\n--- a/perl/Git/LoadCPAN.pm\n+++ b/perl/Git/LoadCPAN.pm\n@@ -1,5 +1,5 @@\n package Git::LoadCPAN;\n-use 5.008;\n+use 5.008001;\n use strict;\n use warnings $ENV{GIT_PERL_FATAL_WARNINGS} ? qw(FATAL all) : ();\n \ndiff --git a/perl/Git/LoadCPAN/Error.pm b/perl/Git/LoadCPAN/Error.pm\nindex 5d84c20288..5cecb0fcd6 100644\n--- a/perl/Git/LoadCPAN/Error.pm\n+++ b/perl/Git/LoadCPAN/Error.pm\n@@ -1,5 +1,5 @@\n package Git::LoadCPAN::Error;\n-use 5.008;\n+use 5.008001;\n use strict;\n use warnings $ENV{GIT_PERL_FATAL_WARNINGS} ? qw(FATAL all) : ();\n use Git::LoadCPAN (\ndiff --git a/perl/Git/LoadCPAN/Mail/Address.pm b/perl/Git/LoadCPAN/Mail/Address.pm\nindex 340e88a7a5..9f808090a6 100644\n--- a/perl/Git/LoadCPAN/Mail/Address.pm\n+++ b/perl/Git/LoadCPAN/Mail/Address.pm\n@@ -1,5 +1,5 @@\n package Git::LoadCPAN::Mail::Address;\n-use 5.008;\n+use 5.008001;\n use strict;\n use warnings $ENV{GIT_PERL_FATAL_WARNINGS} ? qw(FATAL all) : ();\n use Git::LoadCPAN (\ndiff --git a/perl/Git/Packet.pm b/perl/Git/Packet.pm\nindex d144f5168f..d896e69523 100644\n--- a/perl/Git/Packet.pm\n+++ b/perl/Git/Packet.pm\n@@ -1,5 +1,5 @@\n package Git::Packet;\n-use 5.008;\n+use 5.008001;\n use strict;\n use warnings $ENV{GIT_PERL_FATAL_WARNINGS} ? qw(FATAL all) : ();\n BEGIN {\ndiff --git a/t/t0202/test.pl b/t/t0202/test.pl\nindex 2cbf7b9590..47d96a2a13 100755\n--- a/t/t0202/test.pl\n+++ b/t/t0202/test.pl\n@@ -1,5 +1,5 @@\n #!/usr/bin/perl\n-use 5.008;\n+use 5.008001;\n use lib (split(/:/, $ENV{GITPERLLIB}));\n use strict;\n use warnings;\ndiff --git a/t/t5562/invoke-with-content-length.pl b/t/t5562/invoke-with-content-length.pl\nindex 718dd9b49d..9babb9a375 100644\n--- a/t/t5562/invoke-with-content-length.pl\n+++ b/t/t5562/invoke-with-content-length.pl\n@@ -1,4 +1,4 @@\n-use 5.008;\n+use 5.008001;\n use strict;\n use warnings;\n \ndiff --git a/t/t9700/test.pl b/t/t9700/test.pl\nindex 6d753708d2..d8e85482ab 100755\n--- a/t/t9700/test.pl\n+++ b/t/t9700/test.pl\n@@ -1,7 +1,7 @@\n #!/usr/bin/perl\n use lib (split(/:/, $ENV{GITPERLLIB}));\n \n-use 5.008;\n+use 5.008001;\n use warnings;\n use strict;\n \ndiff --git a/t/test-terminal.perl b/t/test-terminal.perl\nindex 1bcf01a9a4..3810e9bb43 100755\n--- a/t/test-terminal.perl\n+++ b/t/test-terminal.perl\n@@ -1,5 +1,5 @@\n #!/usr/bin/perl\n-use 5.008;\n+use 5.008001;\n use strict;\n use warnings;\n use IO::Pty;\n-- \n2.43.0.rc2\n\n"},{"id":"484982","messageId":"20231116193014.470420-3-tmz@pobox.com","threadId":"60519","inReplyTo":"20231116193014.470420-1-tmz@pobox.com","subject":"[PATCH v3 2/2] send-email: avoid duplicate specification warnings","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2023-11-16T19:30:11Z","receivedAt":"2023-11-16T19:30:36Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"A warning is issued for options which are specified more than once\nbeginning with perl-Getopt-Long >= 2.55.  In addition to causing users\nto see warnings, this results in test failures which compare the output.\nAn example, from t9001-send-email.37:\n\n  | +++ diff -u expect actual\n  | --- expect      2023-11-14 10:38:23.854346488 +0000\n  | +++ actual      2023-11-14 10:38:23.848346466 +0000\n  | @@ -1,2 +1,7 @@\n  | +Duplicate specification \"no-chain-reply-to\" for option \"no-chain-reply-to\"\n  | +Duplicate specification \"to-cover|to-cover!\" for option \"to-cover\"\n  | +Duplicate specification \"cc-cover|cc-cover!\" for option \"cc-cover\"\n  | +Duplicate specification \"no-thread\" for option \"no-thread\"\n  | +Duplicate specification \"no-to-cover\" for option \"no-to-cover\"\n  |  fatal: longline.patch:35 is longer than 998 characters\n  |  warning: no patches were sent\n  | error: last command exited with $?=1\n  | not ok 37 - reject long lines\n\nRemove the duplicate option specs.  These are primarily the explicit\n'--no-' prefix opts which were added in f471494303 (git-send-email.perl:\nsupport no- prefix with older GetOptions, 2015-01-30).  This was done\nspecifically to support perl-5.8.0 which includes Getopt::Long 2.32[1].\n\nGetopt::Long 2.33 added support for the '--no-' prefix natively by\nappending '!' to the option specification string, which was included in\nperl-5.8.1 and is not present in perl-5.8.0.  The previous commit bumped\nthe minimum supported Perl version to 5.8.1 so we no longer need to\nprovide the '--no-' variants for negatable options manually.\n\nTeach `--git-completion-helper` to output the '--no-' options.  They are\nnot included in the options hash and would otherwise be lost.\n\nSigned-off-by: Todd Zullinger <tmz@pobox.com>\n---\n git-send-email.perl | 19 ++++++-------------\n 1 file changed, 6 insertions(+), 13 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex d75a4a33dd..f214bd4521 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -119,13 +119,16 @@ sub completion_helper {\n \n \tforeach my $key (keys %$original_opts) {\n \t\tunless (exists $not_for_completion{$key}) {\n-\t\t\t$key =~ s/!$//;\n+\t\t\tmy $negatable = ($key =~ s/!$//);\n \n \t\t\tif ($key =~ /[:=][si]$/) {\n \t\t\t\t$key =~ s/[:=][si]$//;\n \t\t\t\tpush (@send_email_opts, \"--$_=\") foreach (split (/\\|/, $key));\n \t\t\t} else {\n \t\t\t\tpush (@send_email_opts, \"--$_\") foreach (split (/\\|/, $key));\n+\t\t\t\tif ($negatable) {\n+\t\t\t\t\tpush (@send_email_opts, \"--no-$_\") foreach (split (/\\|/, $key));\n+\t\t\t\t}\n \t\t\t}\n \t\t}\n \t}\n@@ -491,7 +494,6 @@ sub config_regexp {\n \t\t    \"bcc=s\" => \\@getopt_bcc,\n \t\t    \"no-bcc\" => \\$no_bcc,\n \t\t    \"chain-reply-to!\" => \\$chain_reply_to,\n-\t\t    \"no-chain-reply-to\" => sub {$chain_reply_to = 0},\n \t\t    \"sendmail-cmd=s\" => \\$sendmail_cmd,\n \t\t    \"smtp-server=s\" => \\$smtp_server,\n \t\t    \"smtp-server-option=s\" => \\@smtp_server_options,\n@@ -506,36 +508,27 @@ sub config_regexp {\n \t\t    \"smtp-auth=s\" => \\$smtp_auth,\n \t\t    \"no-smtp-auth\" => sub {$smtp_auth = 'none'},\n \t\t    \"annotate!\" => \\$annotate,\n-\t\t    \"no-annotate\" => sub {$annotate = 0},\n \t\t    \"compose\" => \\$compose,\n \t\t    \"quiet\" => \\$quiet,\n \t\t    \"cc-cmd=s\" => \\$cc_cmd,\n \t\t    \"header-cmd=s\" => \\$header_cmd,\n \t\t    \"no-header-cmd\" => \\$no_header_cmd,\n \t\t    \"suppress-from!\" => \\$suppress_from,\n-\t\t    \"no-suppress-from\" => sub {$suppress_from = 0},\n \t\t    \"suppress-cc=s\" => \\@suppress_cc,\n \t\t    \"signed-off-cc|signed-off-by-cc!\" => \\$signed_off_by_cc,\n-\t\t    \"no-signed-off-cc|no-signed-off-by-cc\" => sub {$signed_off_by_cc = 0},\n-\t\t    \"cc-cover|cc-cover!\" => \\$cover_cc,\n-\t\t    \"no-cc-cover\" => sub {$cover_cc = 0},\n-\t\t    \"to-cover|to-cover!\" => \\$cover_to,\n-\t\t    \"no-to-cover\" => sub {$cover_to = 0},\n+\t\t    \"cc-cover!\" => \\$cover_cc,\n+\t\t    \"to-cover!\" => \\$cover_to,\n \t\t    \"confirm=s\" => \\$confirm,\n \t\t    \"dry-run\" => \\$dry_run,\n \t\t    \"envelope-sender=s\" => \\$envelope_sender,\n \t\t    \"thread!\" => \\$thread,\n-\t\t    \"no-thread\" => sub {$thread = 0},\n \t\t    \"validate!\" => \\$validate,\n-\t\t    \"no-validate\" => sub {$validate = 0},\n \t\t    \"transfer-encoding=s\" => \\$target_xfer_encoding,\n \t\t    \"format-patch!\" => \\$format_patch,\n-\t\t    \"no-format-patch\" => sub {$format_patch = 0},\n \t\t    \"8bit-encoding=s\" => \\$auto_8bit_encoding,\n \t\t    \"compose-encoding=s\" => \\$compose_encoding,\n \t\t    \"force\" => \\$force,\n \t\t    \"xmailer!\" => \\$use_xmailer,\n-\t\t    \"no-xmailer\" => sub {$use_xmailer = 0},\n \t\t    \"batch-size=i\" => \\$batch_size,\n \t\t    \"relogin-delay=i\" => \\$relogin_delay,\n \t\t    \"git-completion-helper\" => \\$git_completion_helper,\n-- \n2.43.0.rc2\n\n"},{"id":"484983","messageId":"20231116193623.471261-1-tmz@pobox.com","threadId":"60519","inReplyTo":"xmqqzfzes37f.fsf@gitster.g","subject":"[PATCH] send-email: remove stray characters from usage","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2023-11-16T19:36:21Z","receivedAt":"2023-11-16T19:36:32Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"A few stray single quotes crept into the usage string in a2ce608244\n(send-email docs: add format-patch options, 2021-10-25).  Remove them.\n\nSigned-off-by: Todd Zullinger <tmz@pobox.com>\n---\n[I trimmed the Cc: list]\n\nJunio C Hamano wrote:\n> Thanks.  Let's split this out as a docfix patch and handle it\n> separately.\n\nDone. :)\n\n git-send-email.perl | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex cacdbd6bb2..d24e981d61 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -28,8 +28,8 @@\n \n sub usage {\n \tprint <<EOT;\n-git send-email' [<options>] <file|directory>\n-git send-email' [<options>] <format-patch options>\n+git send-email [<options>] <file|directory>\n+git send-email [<options>] <format-patch options>\n git send-email --dump-aliases\n \n   Composing:\n-- \n2.43.0.rc2\n\n"},{"id":"484985","messageId":"20231116201618.GB1146561@coredump.intra.peff.net","threadId":"60519","inReplyTo":"20231116193014.470420-2-tmz@pobox.com","subject":"Re: [PATCH v3 1/2] perl: bump the required Perl version to 5.8.1 from 5.8.0","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-11-16T20:16:18Z","receivedAt":"2023-11-16T20:16:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Nov 16, 2023 at 02:30:10PM -0500, Todd Zullinger wrote:\n\n> The following commit will make use of a Getopt::Long feature which is\n> only present in Perl >= 5.8.1.  Document that as the minimum version we\n> support.\n> \n> Many of our Perl scripts will continue to run with 5.8.0 but this change\n> allows us to adjust them as needed without breaking any promises to our\n> users.\n> \n> The Perl requirement was last changed in d48b284183 (perl: bump the\n> required Perl version to 5.8 from 5.6.[21], 2010-09-24).  At that time,\n> 5.8.0 was 8 years old.  It is now over 21 years old.\n\nThanks, IMHO this is long overdue. You mentioned 5.10 elsewhere in the\nthread, and it came up recently in a discussion (it would allow the use\nof \"//\" for defined-or). So we could perhaps go a bit farther. But I am\nalso fine with 5.8.1 for now, if that is all it takes for this fix (and\nI expect the chance that it causes a problem for anybody to be close to\nzero).\n\n> Signed-off-by: Todd Zullinger <tmz@pobox.com>\n> ---\n> I debated changing all the 'use 5.008;' lines here, as most don't\n> actually require a newer Perl, but the previous bump did the same.\n> \n> I can see the merit in either direction.\n> \n> Changing it allows future contributors to be confident in relying on\n> 5.8.1 features.\n> \n> Not changing it allows anyone stuck on 5.8.0 to continue using the perl\n> scripts which don't actually require 5.8.1.\n\nYeah, I can see both sides of the argument. I think I'd err on the side\nof bumping (as you did here). That lets somebody who will be affected\nknow immediately, rather than only finding out when we randomly depend\non a feature later.\n\nAll of this discussion could likewise go in the commit message. :)\n\n> Tangentially, the Perl docs for 'use' function recommend against the\n> 5.008001 form[1]:\n> \n>     Specifying VERSION as a numeric argument of the form 5.024001 should\n>     generally be avoided as older less readable syntax compared to\n>     v5.24.1. Before perl 5.8.0 released in 2002 the more verbose numeric\n>     form was the only supported syntax, which is why you might see it in\n>     older code.\n> \n>         use v5.24.1;    # compile time version check\n>         use 5.24.1;     # ditto\n>         use 5.024_001;  # ditto; older syntax compatible with perl 5.6\n> \n> I'm not enough of a Perl coder to have a strong preference or desire to\n> push for such a change, but I thought it was worth mentioning in case\n> others wonder why we're using the 5.008001 form.\n\nI doubt it matters too much either way. I suspect at the time we moved\nto v5.8 the nicer syntax was still pretty new (having only been\nintroduced by v5.6, which we were moving off of) and that older versions\nof perl might not give as nice a message when they see it. But given\nthat 5.6 is now 23 years old, we can probably assume nobody will use it\n(or at least they will be accustomed to whatever ugly message it\nproduces).\n\nBut IMHO that should be done as a separate patch anyway.\n\n-Peff\n"},{"id":"484988","messageId":"xmqq1qcpqqhq.fsf@gitster.g","threadId":"60519","inReplyTo":"20231116193014.470420-1-tmz@pobox.com","subject":"Re: [PATCH v3 0/2] send-email: avoid duplicate specification warnings","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-11-16T22:32:01Z","receivedAt":"2023-11-16T22:32:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Todd Zullinger <tmz@pobox.com> writes:\n\n>> This sounds like something that is worth describing in the log\n>> message (and Release Notes).\n>\n> I think the new commit messages describe the changes better.  I didn't\n> include anything in RelNotes as I was presuming we'd leave this for\n> 2.44 rather than risk causing any problems this late in the 2.43 cycle.\n> If you think the risk is low and/or the benefit is high, I can add it to\n> the 2.43.0 RelNotes.\n\nPlease don't worry about the release notes, which I'll do only when\nthe topic hits the 'master' branch.  It was meant primarily as a\nnote to myself.  And I agree that this would be material for 2.44\nand later.\n\nBoth patches, plus the stray single quote fix patch, looked good.\n\nThanks.\n"}]}