Revert "Replace which with command -v"

This reverts commit 42a8d12.
distributed-system-analysis · May 24, 2022 · e79ad4d · e79ad4d
1 parent cd70a64
commit e79ad4d
Show file tree

Hide file tree

Showing 10 changed files with 39 additions and 13 deletions.
diff --git a/agent/base b/agent/base
@@ -293,10 +293,8 @@ function generate_inventory {
 
 function resolve_benchmark_bin {
     # Encapsulation of method for resolving the actual benchmark
-    # binary ignoring `bash` aliases and functions.
-    unalias "${1}" > /dev/null 2>&1
-    unset -f "${1}" > /dev/null 2>&1
-    command -v "${1}"
+    # binary.
+    which --skip-alias --skip-functions "${1}"
 }
 
 function vercmp {

diff --git a/agent/bench-scripts/pbench-fio b/agent/bench-scripts/pbench-fio
@@ -470,9 +470,9 @@ function local_pre_check() {
 
 	debug_log "local_pre_check(devs=\"${devs}\", ver=\"${ver}\", match=\"${match}\")"
 
-	resolve_benchmark_bin "${benchmark}" > /dev/null 2>&1
+	resolve_benchmark_bin_err="$(resolve_benchmark_bin "${benchmark}" 2>&1)"
 	if [[ ${?} -ne 0 ]]; then
-		error_log "[${script_name}] ${benchmark} executable not found"
+		error_log "[${script_name}] ${benchmark} executable not found, '${resolve_benchmark_bin_err}'"
 		exit 1
 	fi
 

diff --git a/agent/bench-scripts/pbench-uperf b/agent/bench-scripts/pbench-uperf
@@ -197,9 +197,9 @@ function local_pre_check {
 	local _ver="${1}"
 	local _match="${2}"
 
-	resolve_benchmark_bin "${benchmark}" > /dev/null 2>&1
+	resolve_benchmark_bin_err="$(resolve_benchmark_bin "${benchmark}" 2>&1)"
 	if [[ ${?} -ne 0 ]]; then
-		error_log "${script_name}: ${benchmark} executable not found"
+		error_log "${script_name}: ${benchmark} executable not found, '${resolve_benchmark_bin_err}'"
 		exit 1
 	fi
 

diff --git a/agent/bench-scripts/test-bin/which b/agent/bench-scripts/test-bin/which
@@ -0,0 +1,20 @@
+#!/bin/bash
+
+# Mock the operation of `which` so that the value of `PATH` is never emitted
+# during the test runs, thus avoiding it landing in a gold file.
+
+if [[ $# -eq 0 ]] ;then
+    echo "$0 requires at least one argument" >&2
+    exit 255
+fi
+args=( "${@}" )
+last=${args[-1]}
+
+stdout=$(/usr/bin/which ${last} 2> /dev/null)
+res=${?}
+if [[ ${res} -eq 0 ]]; then
+    echo ${stdout}
+else
+    echo "which: no ${last} in (\$PATH)" >&2
+fi
+exit ${res}
diff --git a/agent/bench-scripts/tests/pbench-fio/test-22.txt b/agent/bench-scripts/tests/pbench-fio/test-22.txt
@@ -1,5 +1,5 @@
 +++ Running test-22 pbench-fio
-[error][1900-01-01T00:00:00.000000] [pbench-fio] fio executable not found
+[error][1900-01-01T00:00:00.000000] [pbench-fio] fio executable not found, 'which: no fio in ($PATH)'
 --- Finished test-22 pbench-fio (status=1)
 +++ pbench tree state
 /var/tmp/pbench-test-bench/pbench-agent
@@ -15,5 +15,5 @@
 [debug][1900-01-01T00:00:00.000000] fio_pre_check(targets="/tmp/fio", clients="", ver="3.21", match="gte")
 [debug][1900-01-01T00:00:00.000000] Running pre-check locally
 [debug][1900-01-01T00:00:00.000000] local_pre_check(devs="", ver="3.21", match="gte")
-[error][1900-01-01T00:00:00.000000] [pbench-fio] fio executable not found
+[error][1900-01-01T00:00:00.000000] [pbench-fio] fio executable not found, 'which: no fio in ($PATH)'
 --- pbench.log file contents
diff --git a/agent/bench-scripts/tests/pbench-uperf/test-02.txt b/agent/bench-scripts/tests/pbench-uperf/test-02.txt
@@ -1,5 +1,5 @@
 +++ Running test-02 pbench-uperf
-[error][1900-01-01T00:00:00.000000] pbench-uperf: uperf executable not found
+[error][1900-01-01T00:00:00.000000] pbench-uperf: uperf executable not found, 'which: no uperf in ($PATH)'
 --- Finished test-02 pbench-uperf (status=1)
 +++ pbench tree state
 /var/tmp/pbench-test-bench/pbench-agent
@@ -12,5 +12,5 @@
 /var/tmp/pbench-test-bench/pbench-agent/tools-v1-default/testhost.example.com/sar
 --- pbench tree state
 +++ pbench.log file contents
-[error][1900-01-01T00:00:00.000000] pbench-uperf: uperf executable not found
+[error][1900-01-01T00:00:00.000000] pbench-uperf: uperf executable not found, 'which: no uperf in ($PATH)'
 --- pbench.log file contents
diff --git a/agent/bench-scripts/unittests b/agent/bench-scripts/unittests
@@ -79,6 +79,9 @@ PATH=$(remove_path /opt/pbench-agent/util-scripts ${PATH})
 # Allows us to intercept scp, ssh, rsync, etc.
 export PATH=${_testopt}/unittest-scripts:${_testopt}/util-scripts:${_testopt}/bench-scripts:${PATH}
 
+# Allow us to intercept "which".
+unset -f which
+
 # Fixed timestamp output
 export _PBENCH_UNIT_TESTS=1
 

diff --git a/agent/rpm/pbench-agent.spec.j2 b/agent/rpm/pbench-agent.spec.j2
@@ -48,7 +48,7 @@ Requires:  perl-Time-HiRes
 
 Requires:  bc, bzip2, hostname, iproute, iputils, net-tools
 Requires:  openssh-clients, openssh-server, procps-ng, psmisc, redis
-Requires:  rpmdevtools, rsync, screen, sos, tar, xz
+Requires:  rpmdevtools, rsync, screen, sos, tar, which, xz
 
 Obsoletes: pbench <= 0.34
 Conflicts: pbench <= 0.34

diff --git a/jenkins/ci.fedora.Dockerfile b/jenkins/ci.fedora.Dockerfile
@@ -71,6 +71,10 @@ RUN \
         `# Base command used by CI unit test jobs` \
         `#` \
         tox \
+        `#` \
+        `# Required for pbench-fio and pbench-uperf` \
+        `#` \
+        which \
     && \
     `#` \
     `# Save space in the container image.` \

diff --git a/jenkins/development.Dockerfile.j2 b/jenkins/development.Dockerfile.j2
@@ -101,6 +101,7 @@ RUN \
         screen \
         sos \
         tar \
+        which \
         xz \
         \
         `#` \
-Original file line number
+Diff line change
@@ Expand Up / @@ -71,6 +71,10 @@ RUN \ @@
             `# Base command used by CI unit test jobs` \
             `#` \
             tox \
+            `#` \
+            `# Required for pbench-fio and pbench-uperf` \
+            `#` \
+            which \
         && \
         `#` \
         `# Save space in the container image.` \
@@ Expand Down @@