Skip to content

Commit df0f502

Browse files
avargitster
avar
authored and
gitster
committed
perf-lib.sh: remove GIT_TEST_INSTALLED from perf-lib.sh
Follow-up my preceding change which fixed the immediate "./run <revisions>" regression in 0baf78e ("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>
1 parent 90e3815 commit df0f502

File tree

3 files changed

+38
-32
lines changed

3 files changed

+38
-32
lines changed

‎t/perf/aggregate.perl‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
use JSON;
77
use Getopt::Long;
88
use Git;
9+
use Cwd qw(realpath);
910

1011
sub get_times {
1112
my $name = shift;
@@ -103,13 +104,14 @@ sub format_size {
103104
if (! -d $arg) {
104105
my $rev = Git::command_oneline(qw(rev-parse --verify), $arg);
105106
$dir = "build/".$rev;
107+
} elsif ($arg eq '.') {
108+
$dir = '.';
106109
} else {
107-
$arg =~ s{/*$}{};
108-
$dir = $arg;
109-
$dirabbrevs{$dir} = $dir;
110+
$dir = realpath($arg);
111+
$dirnames{$dir} = $dir;
110112
}
111113
push @dirs, $dir;
112-
$dirnames{$dir} = $arg;
114+
$dirnames{$dir} ||= $arg;
113115
my $prefix = $dir;
114116
$prefix =~ tr/^a-zA-Z0-9/_/c;
115117
$prefixes{$dir} = $prefix . '.';

‎t/perf/perf-lib.sh‎

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -21,25 +21,12 @@
2121
# because it will change our working directory.
2222
TEST_DIRECTORY=$(pwd)/..
2323
TEST_OUTPUT_DIRECTORY=$(pwd)
24-
ABSOLUTE_GIT_TEST_INSTALLED=$(
25-
test -n "$GIT_TEST_INSTALLED" && cd "$GIT_TEST_INSTALLED" && pwd)
2624

2725
TEST_NO_CREATE_REPO=t
2826
TEST_NO_MALLOC_CHECK=t
2927

3028
. ../test-lib.sh
3129

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-
4330
# Variables from test-lib that are normally internal to the tests; we
4431
# need to export them for test_perf subshells
4532
export TEST_DIRECTORY TRASH_DIRECTORY GIT_BUILD_DIR GIT_TEST_CMP
@@ -183,7 +170,7 @@ test_wrapper_ () {
183170
base=$(basename "$0" .sh)
184171
echo "$test_count" >>"$perf_results_dir"/$base.subtests
185172
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"
187174
"$test_wrapper_func_" "$@"
188175
fi
189176

‎t/perf/run‎

Lines changed: 31 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,22 @@ build_git_rev () {
7070
) || die "failed to build revision '$mydir'"
7171
}
7272

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+
7389
run_dirs_helper () {
7490
mydir=${1%/}
7591
shift
@@ -79,28 +95,29 @@ run_dirs_helper () {
7995
if test $# -gt 0 -a "$1" = --; then
8096
shift
8197
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
83108
rev=$(git rev-parse --verify "$mydir" 2>/dev/null) ||
84109
die "'$mydir' is neither a directory nor a valid revision"
85110
if [ ! -d build/$rev ]; then
86111
unpack_git_rev $rev
87112
fi
88113
build_git_rev $rev "$mydir"
89114
mydir=build/$rev
115+
116+
PERF_RESULTS_PREFIX=build_$rev.
117+
set_git_test_installed "$mydir"
90118
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+
104121
run_one_dir "$@"
105122
}
106123

0 commit comments

Comments
 (0)