Improve sh_test execution compatibility on MacOS

[mark-thm]

While working on #22213 I ran into errors running any sh_test, all invocations terminated without running any of the tests, and ended with these two lines:

usage: dirname string [...]
usage: dirname string [...]

I'm working on an arm-based Mac with MacOS Sonoma 14.4.1 and with bash reporting a version of GNU bash, version 3.2.57(1)-release.

This PR adds quotes to dirname arguments to resolve the sh_test execution failures.

[mark-thm]

The changes to this file were not necessary but are also not harmful, so I've left them in the patchset.

[hvadehra]

@mark-thm I can't see how these changes would make any difference, did making these changes actually fix the failures? I have the same setup (MacOS Sonoma 14.4.1, GNU bash, version 3.2.57(1)-release) and the tests seem to be working fine.

By any chance, are you perhaps running into something similar to #21391 (comment)?

[mark-thm]

Unfortunately, no, that doesn't seem related. Here's with --announce_rc what I'm seeing on master:

$ bazel test --announce_rc --noincompatible_check_sharding_support --test_filter=test_download_and_extract src/test/shell/bazel:bazel_workspaces_test
INFO: Options provided by the client:
  Inherited 'common' options: --isatty=1 --terminal_columns=142
INFO: Reading rc options for 'test' from /Users/melliot/code/bazel/.bazelrc:
  Inherited 'common' options: --enable_platform_specific_config --check_direct_dependencies=error --experimental_downloader_config=bazel_downloader.cfg --incompatible_disallow_empty_glob
INFO: Reading rc options for 'test' from /Users/melliot/code/bazel/.bazelrc:
  Inherited 'build' options: --java_runtime_version=21 --java_language_version=21 --tool_java_language_version=21 --tool_java_runtime_version=21
INFO: Found applicable config definition build:macos in file /Users/melliot/code/bazel/.bazelrc: --macos_minimum_os=10.11 --cxxopt=-std=c++17 --host_cxxopt=-std=c++17
FAIL: //src/test/shell/bazel:bazel_workspaces_test (shard 3 of 3) (see /private/var/tmp/_bazel_melliot/bca87da07f637628ae875fb2424867d8/execroot/_main/bazel-out/darwin_arm64-fastbuild/testlogs/src/test/shell/bazel/bazel_workspaces_test/shard_3_of_3/test.log)
FAIL: //src/test/shell/bazel:bazel_workspaces_test (shard 2 of 3) (see /private/var/tmp/_bazel_melliot/bca87da07f637628ae875fb2424867d8/execroot/_main/bazel-out/darwin_arm64-fastbuild/testlogs/src/test/shell/bazel/bazel_workspaces_test/shard_2_of_3/test.log)
FAIL: //src/test/shell/bazel:bazel_workspaces_test (shard 1 of 3) (see /private/var/tmp/_bazel_melliot/bca87da07f637628ae875fb2424867d8/execroot/_main/bazel-out/darwin_arm64-fastbuild/testlogs/src/test/shell/bazel/bazel_workspaces_test/shard_1_of_3/test.log)

FAILED: //src/test/shell/bazel:bazel_workspaces_test (Summary)
      /private/var/tmp/_bazel_melliot/bca87da07f637628ae875fb2424867d8/execroot/_main/bazel-out/darwin_arm64-fastbuild/testlogs/src/test/shell/bazel/bazel_workspaces_test/shard_3_of_3/test.log
      /private/var/tmp/_bazel_melliot/bca87da07f637628ae875fb2424867d8/execroot/_main/bazel-out/darwin_arm64-fastbuild/testlogs/src/test/shell/bazel/bazel_workspaces_test/shard_2_of_3/test.log
      /private/var/tmp/_bazel_melliot/bca87da07f637628ae875fb2424867d8/execroot/_main/bazel-out/darwin_arm64-fastbuild/testlogs/src/test/shell/bazel/bazel_workspaces_test/shard_1_of_3/test.log
INFO: Found 1 test target...
Target //src/test/shell/bazel:bazel_workspaces_test up-to-date:
  bazel-bin/src/test/shell/bazel/bazel_workspaces_test
INFO: Elapsed time: 1.615s, Critical Path: 1.31s
INFO: 4 processes: 6 darwin-sandbox.
INFO: Build completed, 1 test FAILED, 4 total actions
//src/test/shell/bazel:bazel_workspaces_test                             FAILED in 3 out of 3 in 1.2s
  Stats over 3 runs: max = 1.2s, min = 1.2s, avg = 1.2s, dev = 0.0s
  /private/var/tmp/_bazel_melliot/bca87da07f637628ae875fb2424867d8/execroot/_main/bazel-out/darwin_arm64-fastbuild/testlogs/src/test/shell/bazel/bazel_workspaces_test/shard_3_of_3/test.log
  /private/var/tmp/_bazel_melliot/bca87da07f637628ae875fb2424867d8/execroot/_main/bazel-out/darwin_arm64-fastbuild/testlogs/src/test/shell/bazel/bazel_workspaces_test/shard_2_of_3/test.log
  /private/var/tmp/_bazel_melliot/bca87da07f637628ae875fb2424867d8/execroot/_main/bazel-out/darwin_arm64-fastbuild/testlogs/src/test/shell/bazel/bazel_workspaces_test/shard_1_of_3/test.log

I run zsh as my shell, and for good measure tried launching the build from bash, but the results are the same.

Here's test.log from shard_1_of_3, they're all fundamentally the same, though:

exec ${PAGER:-/usr/bin/less} "$0" || exit 1
Executing tests from //src/test/shell/bazel:bazel_workspaces_test
-----------------------------------------------------------------------------
INFO[bazel_workspaces_test 2024-05-07 15:17:11 (+0000)] bazel binary is at /private/var/tmp/_bazel_melliot/bca87da07f637628ae875fb2424867d8/sandbox/darwin-sandbox/1274/execroot/_main/bazel-out/darwin_arm64-fastbuild/bin/src/test/shell/bazel/bazel_workspaces_test.runfiles/_main/src/test/shell/bin
Add flags to prefer ipv6 network
INFO[bazel_workspaces_test 2024-05-07 15:17:11 (+0000)] setting up client in /private/var/tmp/_bazel_melliot/bca87da07f637628ae875fb2424867d8/sandbox/darwin-sandbox/1274/execroot/_main/_tmp/f667a654ce77c489c17e4c13ca626716/workspace
usage: dirname string [...]
usage: dirname string [...]

[mark-thm]

Adding some debugging:

diff --git a/src/test/shell/testenv.sh.tmpl b/src/test/shell/testenv.sh.tmpl
index 5d0fb91170..00dd6924f9 100755
--- a/src/test/shell/testenv.sh.tmpl
+++ b/src/test/shell/testenv.sh.tmpl
@@ -260,7 +260,12 @@ function setup_localjdk_javabase() {
   if [[ $PLATFORM =~ msys ]]; then
     jdk_dir="$(cygpath -m $(cd $(rlocation local_jdk/bin/java.exe)/../..; pwd))"
   else
-    jdk_dir="$(dirname "$(dirname "$(rlocation local_jdk/bin/java)")")"
+    # jdk_dir="$(dirname "$(dirname "$(rlocation local_jdk/bin/java)")")"
+    echo "rlocation=$(rlocation local_jdk/bin/java)"
+    echo "dirname+rlocation=$(dirname $(rlocation local_jdk/bin/java))"
+    echo "dirname+rlocation quoted=$(dirname "$(rlocation local_jdk/bin/java)")"
+    jdk_dir="$(dirname $(dirname $(rlocation local_jdk/bin/java)))"
+    echo "jdk_dir=$jdk_dir"
   fi
   bazel_javabase="${jdk_dir}"
 }

Results in the following output from the still failing tests:

exec ${PAGER:-/usr/bin/less} "$0" || exit 1
Executing tests from //src/test/shell/bazel:bazel_workspaces_test
-----------------------------------------------------------------------------
INFO[bazel_workspaces_test 2024-05-07 15:30:19 (+0000)] bazel binary is at /private/var/tmp/_bazel_melliot/bca87da07f637628ae875fb2424867d8/sandbox/darwin-sandbox/2558/execroot/_main/bazel-out/darwin_arm64-fastbuild/bin/src/test/shell/bazel/bazel_workspaces_test.runfiles/_main/src/test/shell/bin
Add flags to prefer ipv6 network
INFO[bazel_workspaces_test 2024-05-07 15:30:19 (+0000)] setting up client in /private/var/tmp/_bazel_melliot/bca87da07f637628ae875fb2424867d8/sandbox/darwin-sandbox/2558/execroot/_main/_tmp/f667a654ce77c489c17e4c13ca626716/workspace
rlocation=
usage: dirname string [...]
dirname+rlocation=
dirname+rlocation quoted=.
usage: dirname string [...]
usage: dirname string [...]

[mark-thm]

So ultimately this comes down to $(rlocation local_jdk/bin/java) returning empty for my setup.

But as to why this change helps the situation, here's some additional debug:

bash-3.2$ dirname ""
.
bash-3.2$ echo $?
0
bash-3.2$ dirname $(echo "")
usage: dirname string [...]
bash-3.2$ echo $?
1
bash-3.2$ dirname "$(echo "")"
.
bash-3.2$ echo $?
0

My default local JDK is:

$ java -version
openjdk version "22.0.1" 2024-04-16
OpenJDK Runtime Environment Zulu22.30+13-CA (build 22.0.1+8)
OpenJDK 64-Bit Server VM Zulu22.30+13-CA (build 22.0.1+8, mixed mode, sharing)

If I set my JDK to 21, the tests pass.

It'd probably be less cryptic to test the output of $(rlocation local_jdk/bin/java) is non-empty than to have things crash out -- in that respect it is related to the issue you linked, but only because I had the wrong JDK running.

[hvadehra]

@mark-thm Thanks a lot for the debugging, this was very helpful.

I was under the impression we'd improved the mismatched local jdk error in #21392 but that never got merged due to a poor interaction with CI.