On Jan 30, 2015, at 15:05, brian m. carlson wrote:
> 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@xxxxxxxxxxxxxxxxxxx>
>> ---
>> 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:
> 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@xxxxxxxxxxxxxxxxxxx>
Signed-off-by: Kyle J. McKay <mackyle@xxxxxxxxx>
---
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;
--
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html