From 2319ddc346ef04c67e490b8c74cdb850de2aee24 Mon Sep 17 00:00:00 2001 From: Dmitry Ivankov Date: Wed, 15 Nov 2023 14:02:42 +0100 Subject: [PATCH] osx_cc_wrapper: add workaround to get_realpath for some bash versions 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. --- tools/cpp/osx_cc_wrapper.sh.tpl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/cpp/osx_cc_wrapper.sh.tpl b/tools/cpp/osx_cc_wrapper.sh.tpl index 8264090c29604d..6e38c9df509228 100644 --- a/tools/cpp/osx_cc_wrapper.sh.tpl +++ b/tools/cpp/osx_cc_wrapper.sh.tpl @@ -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}" }