Skip to content

Commit

Permalink
osx_cc_wrapper: add workaround to get_realpath for some bash versions
Browse files Browse the repository at this point in the history
It looks like in some bash versions under some conditions
with `set -e` bash may propagate sub-shell errors to parent shell,
but it shouldn't as `set -E` is the setting about sub-shell error propagation.

This manifests itself as `get_realpath` returning empty string.

Let's add explicit `|| true` to `$(readlink ...)` sub-shell. The loop
uses empty string as stop signal and should keep working for both
affected and unaffected bash versions.

EXTRA INFO

Adding `set -x` to debug it reveals that `next` is being set to empty
string but after that `echo "$previous"` isn't called. Which in turn
calls `install_name_tool` with missing argument and fails the build.

Example of the log
https://hydra.nixos.org/build/240805256/nixlog/1
it uses `bazel-6.4.0` to run cpp tests from `bazelbuild/examples`
in `x86_64-darwin` host.

Example of local run with `set -x`
```
++ get_otool_path bazel-out/darwin-fastbuild/bin/_solib_darwin/libexamples_Scpp_Slibhello-lib.dylib
++ get_realpath bazel-out/darwin-fastbuild/bin/_solib_darwin/libexamples_Scpp_Slibhello-lib.dylib
++ local previous=bazel-out/darwin-fastbuild/bin/_solib_darwin/libexamples_Scpp_Slibhello-lib.dylib
++ sed 's|^.*/bazel-out/|bazel-out/|'
+++ readlink bazel-out/darwin-fastbuild/bin/_solib_darwin/libexamples_Scpp_Slibhello-lib.dylib
++ local next=/private/tmp/nix-build-bazel-6.4.0.drv-1/_bazel__nixbld1/ff905cebe41cc2b7000dbd8f60c2df58/execroot/io_bazel/bazel-out/darwin-fastbuild/bin/_solib_darwin/libexamples_Scpp_Slibhello-lib.dylib
++ '[' -n /private/tmp/nix-build-bazel-6.4.0.drv-1/_bazel__nixbld1/ff905cebe41cc2b7000dbd8f60c2df58/execroot/io_bazel/bazel-out/darwin-fastbuild/bin/_solib_darwin/libexamples_Scpp_Slibhello-lib.dylib ']'
++ previous=/private/tmp/nix-build-bazel-6.4.0.drv-1/_bazel__nixbld1/ff905cebe41cc2b7000dbd8f60c2df58/execroot/io_bazel/bazel-out/darwin-fastbuild/bin/_solib_darwin/libexamples_Scpp_Slibhello-lib.dylib
+++ readlink /private/tmp/nix-build-bazel-6.4.0.drv-1/_bazel__nixbld1/ff905cebe41cc2b7000dbd8f60c2df58/execroot/io_bazel/bazel-out/darwin-fastbuild/bin/_solib_darwin/libexamples_Scpp_Slibhello-lib.dylib
++ next=/private/tmp/nix-build-bazel-6.4.0.drv-1/_bazel__nixbld1/ff905cebe41cc2b7000dbd8f60c2df58/execroot/io_bazel/bazel-out/darwin-fastbuild/bin/examples/cpp/libhello-lib.dylib
++ '[' -n /private/tmp/nix-build-bazel-6.4.0.drv-1/_bazel__nixbld1/ff905cebe41cc2b7000dbd8f60c2df58/execroot/io_bazel/bazel-out/darwin-fastbuild/bin/examples/cpp/libhello-lib.dylib ']'
++ previous=/private/tmp/nix-build-bazel-6.4.0.drv-1/_bazel__nixbld1/ff905cebe41cc2b7000dbd8f60c2df58/execroot/io_bazel/bazel-out/darwin-fastbuild/bin/examples/cpp/libhello-lib.dylib
+++ readlink /private/tmp/nix-build-bazel-6.4.0.drv-1/_bazel__nixbld1/ff905cebe41cc2b7000dbd8f60c2df58/execroot/io_bazel/bazel-out/darwin-fastbuild/bin/examples/cpp/libhello-lib.dylib
++ next=
+ /nix/store/hfwdjfb4203x8rl4s6q565016hi3m0d3-cctools-llvm-11.1.0-973.0.1/bin/install_name_tool -change @loader_path/../../_solib_darwin//libexamples_Scpp_Slibhello-lib.dylib bazel-out/darwin-fastbuild/bin/examples/cpp/hello-success_test
```
note that `@loader_path...` should've been the 2nd argument to `-change`

Local host bash version is
```
% bash -version
GNU bash, version 3.2.57(1)-release (x86_64-apple-darwin21)
Copyright (C) 2007 Free Software Foundation, Inc.
```
Though notably aarch64 host with
```
% bash -version
GNU bash, version 3.2.57(1)-release (arm64-apple-darwin22)
Copyright (C) 2007 Free Software Foundation, Inc.
```
doesn't hit the issue, so it may be sensitive to some other things too.

Bash changelogs do have mentions of bugs around `set -e`
https://github.com/bminor/bash/blob/master/CHANGES
though I haven't identified the exact version and bug entry.
  • Loading branch information
divanorama committed Nov 15, 2023
1 parent 8f8a4b2 commit 2319ddc
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions tools/cpp/osx_cc_wrapper.sh.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,10 @@ function get_library_path() {
# and multi-level symlinks.
function get_realpath() {
local previous="$1"
local next=$(readlink "${previous}")
local next=$(readlink "${previous}" || true)
while [ -n "${next}" ]; do
previous="${next}"
next=$(readlink "${previous}")
next=$(readlink "${previous}" || true)
done
echo "${previous}"
}
Expand Down

0 comments on commit 2319ddc

Please sign in to comment.