On Tue, May 07, 2019 at 01:23:09AM +0200, Ævar Arnfjörð Bjarmason wrote:
> @@ -79,7 +95,16 @@ run_dirs_helper () {
> if test $# -gt 0 -a "$1" = --; then
> shift
> fi
> - if [ ! -d "$mydir" ]; then
> +
> + PERF_RESULTS_PREFIX=
> + if test "$mydir" = "."
> + then
> + unset GIT_TEST_INSTALLED
> + elif test -d "$mydir"
> + then
> + PERF_RESULTS_PREFIX=$(cd $mydir && printf "%s" "$(pwd)" | tr -c "[a-zA-Z0-9]" "_").
> + set_git_test_installed "$mydir"
> + else
> rev=$(git rev-parse --verify "$mydir" 2>/dev/null) ||
> die "'$mydir' is neither a directory nor a valid revision"
> if [ ! -d build/$rev ]; then
OK, so this is basically the same cleanup I came up with. The big
difference is that you pushed the shared code into a function, rather
than the funky double-conditional in the original. I'm OK with that.
> diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl
> index 494907a892..c8f4a78903 100755
> --- a/t/perf/aggregate.perl
> +++ b/t/perf/aggregate.perl
> @@ -6,6 +6,7 @@
> use JSON;
> use Getopt::Long;
> use Git;
> +use Cwd qw(realpath);
>
> sub get_times {
> my $name = shift;
> @@ -103,13 +104,14 @@ sub format_size {
> if (! -d $arg) {
> my $rev = Git::command_oneline(qw(rev-parse --verify), $arg);
> $dir = "build/".$rev;
> + } elsif ($arg eq '.') {
> + $dir = '.';
> } else {
> - $arg =~ s{/*$}{};
> - $dir = $arg;
> - $dirabbrevs{$dir} = $dir;
> + $dir = realpath($arg);
> + $dirnames{$dir} = $dir;
> }
> push @dirs, $dir;
> - $dirnames{$dir} = $arg;
> + $dirnames{$dir} ||= $arg;
I'm not sure I get what's going on here. Why do we need the realpath in
aggregate.perl? We'd want to generate the same filename that "run"
decided to store things in, which we'd generate from the command-line
arguments (either passed on to us by "run", or direct from the user if
they're printing a previous run).
> @@ -312,9 +314,6 @@ sub print_codespeed_results {
> $environment = $reponame;
> } elsif (exists $ENV{GIT_PERF_REPO_NAME} and $ENV{GIT_PERF_REPO_NAME} ne "") {
> $environment = $ENV{GIT_PERF_REPO_NAME};
> - } elsif (exists $ENV{GIT_TEST_INSTALLED} and $ENV{GIT_TEST_INSTALLED} ne "") {
> - $environment = $ENV{GIT_TEST_INSTALLED};
> - $environment =~ s|/bin-wrappers$||;
> } else {
> $environment = `uname -r`;
> chomp $environment;
Is this codespeed thing a totally separate bug? Should it go into its
own patch?
-Peff
