[PATCH v3 4/6] perf-lib.sh: remove GIT_TEST_INSTALLED from perf-lib.sh

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

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>
---
 t/perf/aggregate.perl | 10 ++++++----
 t/perf/perf-lib.sh    | 15 +--------------
 t/perf/run            | 45 +++++++++++++++++++++++++++++--------------
 3 files changed, 38 insertions(+), 32 deletions(-)

diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl
index f6518339dc..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;
 	my $prefix = $dir;
 	$prefix =~ tr/^a-zA-Z0-9/_/c;
 	$prefixes{$dir} = $prefix . '.';
diff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh
index b15ee1d262..9cdccba222 100644
--- a/t/perf/perf-lib.sh
+++ b/t/perf/perf-lib.sh
@@ -21,25 +21,12 @@
 # because it will change our working directory.
 TEST_DIRECTORY=$(pwd)/..
 TEST_OUTPUT_DIRECTORY=$(pwd)
-ABSOLUTE_GIT_TEST_INSTALLED=$(
-	test -n "$GIT_TEST_INSTALLED" && cd "$GIT_TEST_INSTALLED" && pwd)
 
 TEST_NO_CREATE_REPO=t
 TEST_NO_MALLOC_CHECK=t
 
 . ../test-lib.sh
 
-if test -z "$GIT_TEST_INSTALLED"; then
-	perf_results_prefix=
-else
-	if test -n "$GIT_PERF_DIR_MYDIR_REL"
-	then
-		GIT_TEST_INSTALLED=$GIT_PERF_DIR_MYDIR_REL
-	fi
-	perf_results_prefix=$(printf "%s" "${GIT_TEST_INSTALLED%/bin-wrappers}" | tr -c "[a-zA-Z0-9]" "[_*]")"."
-	GIT_TEST_INSTALLED=$ABSOLUTE_GIT_TEST_INSTALLED
-fi
-
 # Variables from test-lib that are normally internal to the tests; we
 # need to export them for test_perf subshells
 export TEST_DIRECTORY TRASH_DIRECTORY GIT_BUILD_DIR GIT_TEST_CMP
@@ -183,7 +170,7 @@ test_wrapper_ () {
 		base=$(basename "$0" .sh)
 		echo "$test_count" >>"$perf_results_dir"/$base.subtests
 		echo "$1" >"$perf_results_dir"/$base.$test_count.descr
-		base="$perf_results_dir"/"$perf_results_prefix$(basename "$0" .sh)"."$test_count"
+		base="$perf_results_dir"/"$PERF_RESULTS_PREFIX$(basename "$0" .sh)"."$test_count"
 		"$test_wrapper_func_" "$@"
 	fi
 
diff --git a/t/perf/run b/t/perf/run
index 0a7c8744ab..85b7bd31d5 100755
--- a/t/perf/run
+++ b/t/perf/run
@@ -70,6 +70,22 @@ build_git_rev () {
 	) || die "failed to build revision '$mydir'"
 }
 
+set_git_test_installed () {
+	mydir=$1
+
+	mydir_abs=$(cd $mydir && pwd)
+	mydir_abs_wrappers="$mydir_abs_wrappers/bin-wrappers"
+	if test -d "$mydir_abs_wrappers"
+	then
+		GIT_TEST_INSTALLED=$mydir_abs_wrappers
+	else
+		# Older versions of git lacked bin-wrappers;
+		# fallback to the files in the root.
+		GIT_TEST_INSTALLED=$mydir_abs
+	fi
+	export GIT_TEST_INSTALLED
+}
+
 run_dirs_helper () {
 	mydir=${1%/}
 	shift
@@ -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
@@ -87,20 +112,12 @@ run_dirs_helper () {
 		fi
 		build_git_rev $rev "$mydir"
 		mydir=build/$rev
+
+		PERF_RESULTS_PREFIX=build_$rev.
+		set_git_test_installed "$mydir"
 	fi
-	if test "$mydir" = .; then
-		unset GIT_TEST_INSTALLED
-	else
-		GIT_PERF_DIR_MYDIR_REL=$mydir
-		GIT_PERF_DIR_MYDIR_ABS=$(cd $mydir && pwd)
-		export GIT_PERF_DIR_MYDIR_REL GIT_PERF_DIR_MYDIR_ABS
-
-		GIT_TEST_INSTALLED="$GIT_PERF_DIR_MYDIR_ABS/bin-wrappers"
-		# Older versions of git lacked bin-wrappers; fallback to the
-		# files in the root.
-		test -d "$GIT_TEST_INSTALLED" || GIT_TEST_INSTALLED=$GIT_PERF_DIR_MYDIR_ABS
-		export GIT_TEST_INSTALLED
-	fi
+	export PERF_RESULTS_PREFIX
+
 	run_one_dir "$@"
 }
 
-- 
2.21.0.593.g511ec345e18