Skip to content

Commit 38368cb

Browse files
chriscoolgitster
authored andcommitted
perf/aggregate: use Getopt::Long for option parsing
When passing an option '--foo' that it does not recognize, the aggregate.perl script should die with an helpful error message like: Unknown option: foo ./aggregate.perl [options] [--] [<dir_or_rev>...] [--] \ [<test_script>...] > Options: --codespeed * Format output for Codespeed --reponame <str> * Send given reponame to codespeed --sort-by <str> * Sort output (only "regression" \ criteria is supported) rather than: fatal: Needed a single revision rev-parse --verify --foo: command returned error: 128 To implement that let's use Getopt::Long for option parsing instead of the current manual and sloppy parsing. This should save some code and make option parsing simpler, tighter and safer. This will avoid something like 'foo--sort-by=regression' to be handled as if '--sort-by=regression' had been used, for example. As Getopt::Long eats '--' at the end of options, this changes a bit the way '--' is handled as we can now have '--' both after the options and before the scripts. Signed-off-by: Christian Couder <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 1f1cddd commit 38368cb

File tree

1 file changed

+26
-36
lines changed

1 file changed

+26
-36
lines changed

t/perf/aggregate.perl

Lines changed: 26 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
use strict;
55
use warnings;
66
use JSON;
7+
use Getopt::Long;
78
use Git;
89

910
sub get_times {
@@ -36,46 +37,34 @@ sub format_times {
3637
return $out;
3738
}
3839

40+
sub usage {
41+
print <<EOT;
42+
./aggregate.perl [options] [--] [<dir_or_rev>...] [--] [<test_script>...] >
43+
44+
Options:
45+
--codespeed * Format output for Codespeed
46+
--reponame <str> * Send given reponame to codespeed
47+
--sort-by <str> * Sort output (only "regression" criteria is supported)
48+
--subsection <str> * Use results from given subsection
49+
50+
EOT
51+
exit(1);
52+
}
53+
3954
my (@dirs, %dirnames, %dirabbrevs, %prefixes, @tests,
4055
$codespeed, $sortby, $subsection, $reponame);
56+
57+
Getopt::Long::Configure qw/ require_order /;
58+
59+
my $rc = GetOptions("codespeed" => \$codespeed,
60+
"reponame=s" => \$reponame,
61+
"sort-by=s" => \$sortby,
62+
"subsection=s" => \$subsection);
63+
usage() unless $rc;
64+
4165
while (scalar @ARGV) {
4266
my $arg = $ARGV[0];
4367
my $dir;
44-
if ($arg eq "--codespeed") {
45-
$codespeed = 1;
46-
shift @ARGV;
47-
next;
48-
}
49-
if ($arg =~ /--sort-by(?:=(.*))?/) {
50-
shift @ARGV;
51-
if (defined $1) {
52-
$sortby = $1;
53-
} else {
54-
$sortby = shift @ARGV;
55-
if (! defined $sortby) {
56-
die "'--sort-by' requires an argument";
57-
}
58-
}
59-
next;
60-
}
61-
if ($arg eq "--subsection") {
62-
shift @ARGV;
63-
$subsection = $ARGV[0];
64-
shift @ARGV;
65-
if (! $subsection) {
66-
die "empty subsection";
67-
}
68-
next;
69-
}
70-
if ($arg eq "--reponame") {
71-
shift @ARGV;
72-
$reponame = $ARGV[0];
73-
shift @ARGV;
74-
if (! $reponame) {
75-
die "empty reponame";
76-
}
77-
next;
78-
}
7968
last if -f $arg or $arg eq "--";
8069
if (! -d $arg) {
8170
my $rev = Git::command_oneline(qw(rev-parse --verify), $arg);
@@ -225,7 +214,8 @@ sub print_sorted_results {
225214
my ($sortby) = @_;
226215

227216
if ($sortby ne "regression") {
228-
die "only 'regression' is supported as '--sort-by' argument";
217+
print "Only 'regression' is supported as '--sort-by' argument\n";
218+
usage();
229219
}
230220

231221
my @evolutions;

0 commit comments

Comments
 (0)