git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] t9001: use older Getopt::Long boolean prefix '--no' rather than '--no-'

From
Kyle J. McKay <mackyle@gmail.com>
Date
Jan 31, 2015, 02:40 UTC
Message-ID
<a924a58108ea8ad8aece1ee66cbdc3f@74d39fa044aa309eaea14b9f57fe79c>
In-Reply-To
<20150130230516.GA7867@vauxhall.crustytoothpaste.net>
On Jan 30, 2015, at 15:05, brian m. carlson wrote:
Show 39 quoted lines
> On Fri, Jan 30, 2015 at 07:24:45AM +0100, Tom G. Christensen wrote:
>> The '--no-xmailer' option is a Getopt::Long boolean option. The
>> '--no-' prefix (as in --no-xmailer) for boolean options is not
>> supported in Getopt::Long version 2.32 which was released with Perl  
>> 5.8.0.
>> This version only supports '--no' as in '--noxmailer'.  More recent
>> versions of Getopt::Long, such as version 2.34, support either  
>> prefix. So
>> use the older form in the tests.
>>
>> See also:
>>
>> d2559f734bba7fe5257720356a92f3b7a5b0d37c
>> 907a0b1e04ea31cb368e9422df93d8ebb0187914
>> 84eeb687de7a6c7c42af3fb51b176e0f412a979e
>> 3fee1fe87144360a1913eab86af9ad136c810076
>>
>> Signed-off-by: Tom G. Christensen <tgc@statsbiblioteket.dk>
>> ---
>> t/t9001-send-email.sh | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
>> index af6a3e8..30df6ae 100755
>> --- a/t/t9001-send-email.sh
>> +++ b/t/t9001-send-email.sh
>> @@ -1580,20 +1580,20 @@ do_xmailer_test () {
>>
>> test_expect_success $PREREQ '--[no-]xmailer without any  
>> configuration' '
>> 	do_xmailer_test 1 "--xmailer" &&
>> -	do_xmailer_test 0 "--no-xmailer"
>> +	do_xmailer_test 0 "--noxmailer"
>
> I don't think this is an adequate fix.  The documented option is -- 
> no-xmailer.  If your version of Getopt::Long is not capable of that,  
> then the program doesn't work as documented, and the test is  
> correctly failing.  --noxmailer is not documented at all, so it's  
> not something we should be testing.

It is not alone. From the git-send-email help these are all boolean options:

Show 17 quoted lines
> git send-email
>
>   Composing:
>     --[no-]xmailer
>     --[no-]annotate
>
>   Automating:
>     --[no-]cc-cover
>     --[no-]to-cover
>     --[no-]signed-off-by-cc
>     --[no-]suppress-from
>     --[no-]chain-reply-to
>     --[no-]thread
>
>   Administering:
>     --[no-]validate
>     --[no-]format-patch

Anything done to fix --no-xmailer should be applied for all the other --no-... options as well.

> We should probably require a certain version of Getopt::Long or  
> explicitly handle this in the parsing code itself.  I think the  
> former is a better choice, since no security-supported OS still  
> ships with such a positively ancient version.
I don't really like that second option because all the .perl files have:
> use 5.008;

So either that needs to change or the code should properly deal with the version of Getopt::Long that comes with 5.8.0.

Since it's really not very difficult or invasive to add support for the no- variants, here's a patch to do so:

-- 8< --
Subject: [PATCH] git-send-email.perl: support no- prefix with older GetOptions

Only Perl version 5.8.0 or later is required, but that comes with an older Getopt::Long (2.32) that does not support the 'no-' prefix. Support for that was added in Getopt::Long version 2.33.

Since the help only mentions the 'no-' prefix and not the 'no' prefix, add explicit support for the 'no-' prefix when running with older GetOptions versions.

Reported-by: Tom G. Christensen <tgc@statsbiblioteket.dk>
Signed-off-by: Kyle J. McKay <mackyle@gmail.com>
---
 git-send-email.perl | 10 ++++++++++
 1 file changed, 10 insertions(+)
diff --git a/git-send-email.perl b/git-send-email.perl
index 3092ab35..a18a7959 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -299,6 +299,7 @@ my $rc = GetOptions("h" => \$help,
 		    "bcc=s" => \@bcclist,
 		    "no-bcc" => \$no_bcc,
 		    "chain-reply-to!" => \$chain_reply_to,
+		    "no-chain-reply-to" => sub {$chain_reply_to = 0},
 		    "smtp-server=s" => \$smtp_server,
 		    "smtp-server-option=s" => \@smtp_server_options,
 		    "smtp-server-port=s" => \$smtp_server_port,
@@ -311,25 +312,34 @@ my $rc = GetOptions("h" => \$help,
 		    "smtp-domain:s" => \$smtp_domain,
 		    "identity=s" => \$identity,
 		    "annotate!" => \$annotate,
+		    "no-annotate" => sub {$annotate = 0},
 		    "compose" => \$compose,
 		    "quiet" => \$quiet,
 		    "cc-cmd=s" => \$cc_cmd,
 		    "suppress-from!" => \$suppress_from,
+		    "no-suppress-from" => sub {$suppress_from = 0},
 		    "suppress-cc=s" => \@suppress_cc,
 		    "signed-off-cc|signed-off-by-cc!" => \$signed_off_by_cc,
+		    "no-signed-off-cc|no-signed-off-by-cc" => sub {$signed_off_by_cc = 0},
 		    "cc-cover|cc-cover!" => \$cover_cc,
+		    "no-cc-cover" => sub {$cover_cc = 0},
 		    "to-cover|to-cover!" => \$cover_to,
+		    "no-to-cover" => sub {$cover_to = 0},
 		    "confirm=s" => \$confirm,
 		    "dry-run" => \$dry_run,
 		    "envelope-sender=s" => \$envelope_sender,
 		    "thread!" => \$thread,
+		    "no-thread" => sub {$thread = 0},
 		    "validate!" => \$validate,
+		    "no-validate" => sub {$validate = 0},
 		    "transfer-encoding=s" => \$target_xfer_encoding,
 		    "format-patch!" => \$format_patch,
+		    "no-format-patch" => sub {$format_patch = 0},
 		    "8bit-encoding=s" => \$auto_8bit_encoding,
 		    "compose-encoding=s" => \$compose_encoding,
 		    "force" => \$force,
 		    "xmailer!" => \$use_xmailer,
+		    "no-xmailer" => sub {$use_xmailer = 0},
 	 );
 
 usage() if $help;
--
Previous: brian m. carlsonNext: Junio C Hamano
Message 18 of 31 in “[ANNOUNCE] Git v2.3.0-rc2”
  1. Junio C HamanoJan 27, 2015
  2. Broken makefile check for curl version on el4 [Re: [ANNOUNCE] Git v2.3.0-rc2]Tom G. Christensen, Jan 29, 2015
  3. Makefile: Handle broken curl version number in version checkTom G. Christensen, Jan 30, 2015
  4. Andreas SchwabJan 30, 2015
  5. Tom G. ChristensenJan 30, 2015
  6. Kyle J. McKayJan 30, 2015
  7. Junio C HamanoJan 30, 2015
  8. All gnupg tests broken on el4 [Re: [ANNOUNCE] Git v2.3.0-rc2]Tom G. Christensen, Jan 29, 2015
  9. Jeff KingJan 29, 2015
  10. Jeff KingJan 29, 2015
  11. Tom G. ChristensenJan 29, 2015
  12. Junio C HamanoJan 29, 2015
  13. Testsuite regression with perl 5.8.0 [Re: [ANNOUNCE] Git v2.3.0-rc2]Tom G. Christensen, Jan 29, 2015
  14. Jeff KingJan 29, 2015
  15. Tom G. ChristensenJan 30, 2015
  16. t9001: use older Getopt::Long boolean prefix '--no' rather than '--no-'Tom G. Christensen, Jan 30, 2015
  17. brian m. carlsonJan 30, 2015
  18. Kyle J. McKayJan 31, 2015
  19. Junio C HamanoFeb 2, 2015
  20. Kyle J. McKayFeb 2, 2015
  21. Junio C HamanoFeb 2, 2015
  22. Junio C HamanoFeb 12, 2015
  23. 0/2 Getopt::Long workaround in send-emailJunio C Hamano, Feb 13, 2015
  24. 1/2 git-send-email.perl: support no- prefix with older GetOptionsJunio C Hamano, Feb 13, 2015
  25. Brandon CaseyFeb 15, 2015
  26. 2/2 SQUASH??? t9001: turn --no$option workarounds to --no-$optionJunio C Hamano, Feb 13, 2015
  27. Kyle J. McKayFeb 13, 2015
  28. brian m. carlsonFeb 13, 2015
  29. Brandon CaseyFeb 15, 2015
  30. Tom G. ChristensenFeb 16, 2015
  31. Brandon CaseyFeb 16, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.