@matheusbsbs / matheus-bagliano / commits / df0f502195

perf-lib.sh: remove GIT_TEST_INSTALLED from perf-lib.sh

Follow-up my preceding change which fixed the immediate "./run <revisions>" regression in 0baf78e7bc ("perf-lib.sh: rely on test-lib.sh for --tee handling", 2019-03-15) and entirely get rid of GIT_TEST_INSTALLED from perf-lib.sh (and aggregate.perl). As noted in that change the dance we're doing with GIT_TEST_INSTALLED perf-lib.sh isn't necessary, but there I was doing the most minimal set of changes to quickly fix a regression. But it's much simpler to never deal with the "GIT_TEST_INSTALLED" we were setting in perf-lib.sh at all. Instead the run_dirs_helper() sets the previously inferred $PERF_RESULTS_PREFIX directly. Setting this at the callsite that's already best positioned to exhaustively know about all the different cases we need to handle where PERF_RESULTS_PREFIX isn't what we want already (the empty string) makes the most sense. In one-off cases like: ./run ./p0000-perf-lib-sanity.sh ./p0000-perf-lib-sanity.sh We'll just do the right thing because PERF_RESULTS_PREFIX will be empty, and test-lib.sh takes care of finding where our git is. Any refactoring of this code needs to change both the shell code and the Perl code in aggregate.perl, because when running e.g.: ./run ../../ -- <test> The "../../" path to a relative bindir needs to be munged to a filename containing the results, and critically aggregate.perl does not get passed the path to those aggregations, just "../..". Let's fix cases where aggregate.perl would print e.g. ".." in its report output for this, and "git" for "/home/avar/g/git", i.e. it would always pick the last element. Now'll always print the full path instead. This also makes the code sturdier, e.g. you can feed "../.." to "./run" and then an absolute path to the aggregate.perl script, as long as the absolute path and "../.." resolved to the same directory printing the aggregation will work. Also simplify the "[_*]" on the RHS of "tr -c", we're trimming everything to "_", so we don't need that. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>

Ævar Arnfjörð Bjarmason committed May 7, 2019 at 12:54 UTC df0f5021951bd0ae5e7db0d89fd7e5c141c334be

3 files changed +38 -32

t/perf/aggregate.perl

+6 -4

@@ -6,6 +6,7 @@ use warnings;

6 use JSON;

7 use Getopt::Long;

8 use Git;

9 +use Cwd qw(realpath);

11 sub get_times {

12 my $name = shift;

@@ -103,13 +104,14 @@ while (scalar @ARGV) {

104 if (! -d $arg) {

105 my $rev = Git::command_oneline(qw(rev-parse --verify), $arg);

106 $dir = "build/".$rev;

107 + } elsif ($arg eq '.') {

108 + $dir = '.';

109 } else {

107 - $arg =~ s{/*$}{};

108 - $dir = $arg;

109 - $dirabbrevs{$dir} = $dir;

110 + $dir = realpath($arg);

111 + $dirnames{$dir} = $dir;

112 }

113 push @dirs, $dir;

112 - $dirnames{$dir} = $arg;

114 + $dirnames{$dir} ||= $arg;

115 my $prefix = $dir;

116 $prefix =~ tr/^a-zA-Z0-9/_/c;

117 $prefixes{$dir} = $prefix . '.';

t/perf/perf-lib.sh

+1 -14

@@ -21,25 +21,12 @@

21 # because it will change our working directory.

22 TEST_DIRECTORY=$(pwd)/..

23 TEST_OUTPUT_DIRECTORY=$(pwd)

24 -ABSOLUTE_GIT_TEST_INSTALLED=$(

25 - test -n "$GIT_TEST_INSTALLED" && cd "$GIT_TEST_INSTALLED" && pwd)

25 TEST_NO_CREATE_REPO=t

26 TEST_NO_MALLOC_CHECK=t

28 . ../test-lib.sh

32 -if test -z "$GIT_TEST_INSTALLED"; then

33 - perf_results_prefix=

34 -else

35 - if test -n "$GIT_PERF_DIR_MYDIR_REL"

36 - then

37 - GIT_TEST_INSTALLED=$GIT_PERF_DIR_MYDIR_REL

38 - fi

39 - perf_results_prefix=$(printf "%s" "${GIT_TEST_INSTALLED%/bin-wrappers}" | tr -c "[a-zA-Z0-9]" "[_*]")"."

40 - GIT_TEST_INSTALLED=$ABSOLUTE_GIT_TEST_INSTALLED

41 -fi

42 -

30 # Variables from test-lib that are normally internal to the tests; we

31 # need to export them for test_perf subshells

32 export TEST_DIRECTORY TRASH_DIRECTORY GIT_BUILD_DIR GIT_TEST_CMP

@@ -183,7 +170,7 @@ test_wrapper_ () {

170 base=$(basename "$0" .sh)

171 echo "$test_count" >>"$perf_results_dir"/$base.subtests

172 echo "$1" >"$perf_results_dir"/$base.$test_count.descr

186 - base="$perf_results_dir"/"$perf_results_prefix$(basename "$0" .sh)"."$test_count"

173 + base="$perf_results_dir"/"$PERF_RESULTS_PREFIX$(basename "$0" .sh)"."$test_count"

174 "$test_wrapper_func_" "$@"

175 fi

176

t/perf/run

+31 -14

@@ -70,6 +70,22 @@ build_git_rev () {

70 ) || die "failed to build revision '$mydir'"

71 }

73 +set_git_test_installed () {

74 + mydir=$1

75 +

76 + mydir_abs=$(cd $mydir && pwd)

77 + mydir_abs_wrappers="$mydir_abs_wrappers/bin-wrappers"

78 + if test -d "$mydir_abs_wrappers"

79 + then

80 + GIT_TEST_INSTALLED=$mydir_abs_wrappers

81 + else

82 + # Older versions of git lacked bin-wrappers;

83 + # fallback to the files in the root.

84 + GIT_TEST_INSTALLED=$mydir_abs

85 + fi

86 + export GIT_TEST_INSTALLED

87 +}

88 +

89 run_dirs_helper () {

90 mydir=${1%/}

91 shift

@@ -79,7 +95,16 @@ run_dirs_helper () {

95 if test $# -gt 0 -a "$1" = --; then

96 shift

97 fi

82 - if [ ! -d "$mydir" ]; then

98 +

99 + PERF_RESULTS_PREFIX=

100 + if test "$mydir" = "."

101 + then

102 + unset GIT_TEST_INSTALLED

103 + elif test -d "$mydir"

104 + then

105 + PERF_RESULTS_PREFIX=$(cd $mydir && printf "%s" "$(pwd)" | tr -c "[a-zA-Z0-9]" "_").

106 + set_git_test_installed "$mydir"

107 + else

108 rev=$(git rev-parse --verify "$mydir" 2>/dev/null) ||

109 die "'$mydir' is neither a directory nor a valid revision"

110 if [ ! -d build/$rev ]; then

@@ -87,20 +112,12 @@ run_dirs_helper () {

112 fi

113 build_git_rev $rev "$mydir"

114 mydir=build/$rev

115 +

116 + PERF_RESULTS_PREFIX=build_$rev.

117 + set_git_test_installed "$mydir"

118 fi

91 - if test "$mydir" = .; then

92 - unset GIT_TEST_INSTALLED

93 - else

94 - GIT_PERF_DIR_MYDIR_REL=$mydir

95 - GIT_PERF_DIR_MYDIR_ABS=$(cd $mydir && pwd)

96 - export GIT_PERF_DIR_MYDIR_REL GIT_PERF_DIR_MYDIR_ABS

97 -

98 - GIT_TEST_INSTALLED="$GIT_PERF_DIR_MYDIR_ABS/bin-wrappers"

99 - # Older versions of git lacked bin-wrappers; fallback to the

100 - # files in the root.

101 - test -d "$GIT_TEST_INSTALLED" || GIT_TEST_INSTALLED=$GIT_PERF_DIR_MYDIR_ABS

102 - export GIT_TEST_INSTALLED

103 - fi

119 + export PERF_RESULTS_PREFIX

120 +

121 run_one_dir "$@"

122 }

123