bazel-contrib / toolchains_llvm

LLVM toolchain for bazel
Apache License 2.0
281 stars 207 forks source link

Option to use `--linkopt=-fuse-ld=ld64.lld` flag does not respect `absolute_paths = True` #324

Open jez opened 2 months ago

jez commented 2 months ago

https://github.com/bazel-contrib/toolchains_llvm/blob/f0a43a598b8f0e69c6402fe964e336220fc3c4c9/toolchain/osx_cc_wrapper.sh.tpl#L76-L77

When absolute_paths = True, the toolchain_path_prefix variable is an absolute path, so attempting to prefix it with execroot_abs_path means that the execroot absolute path is duplicated, leading clang to fail to invoke the linker.

❯ bazel build //parser/parser:generate_diagnostics
WARNING: Build option --linkopt has changed, discarding analysis cache (this can be expensive, see https://bazel.build/advanced/performance/iteration-speed).
INFO: Analyzed target //parser/parser:generate_diagnostics (68 packages loaded, 368 targets configured).
ERROR: /Users/jez/sandbox/bazel_example/parser/parser/BUILD:1:10: Linking parser/parser/generate_diagnostics failed: (Exit 1): cc_wrapper.sh failed: error executing CppLink command (from target //parser/parser:generate_diagnostics) external/llvm_toolchain_15_0_7/bin/cc_wrapper.sh @bazel-out/darwin_arm64-fastbuild/bin/parser/parser/generate_diagnostics-2.params

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
clang: error: invalid linker name in argument '-fuse-ld=/private/var/tmp/_bazel/2e52735c4d41272832995da68cda80bb/sandbox/darwin-sandbox/8/execroot/_main//private/var/tmp/_bazel/2e52735c4d41272832995da68cda80bb/external/llvm_toolchain_15_0_7_llvm/bin/ld64.lld'
Target //parser/parser:generate_diagnostics failed to build
Use --verbose_failures to see the command lines of failed build steps.
INFO: Elapsed time: 1.063s, Critical Path: 0.69s
INFO: 6 processes: 5 internal, 1 darwin-sandbox.
ERROR: Build did NOT complete successfully

In the error message, this:

/private/var/tmp/_bazel/2e52735c4d41272832995da68cda80bb/sandbox/darwin-sandbox/8/execroot/_main//private/var/tmp/_bazel/2e52735c4d41272832995da68cda80bb/external/llvm_toolchain_15_0_7_llvm/bin/ld64.lld

needs to be this:

/private/var/tmp/_bazel/2e52735c4d41272832995da68cda80bb/external/llvm_toolchain_15_0_7_llvm/bin/ld64.lld

or else the file does not exist.

This appeared in #286, which was a fix for #212.

jez commented 2 months ago

I didn't see an easy way to access absolute_paths from inside the cc_wrapper.sh file, so I just applied a patch like this to make progress:

    if [[ -f "%{toolchain_path_prefix}bin/ld64.lld" ]]; then
      echo "-fuse-ld=%{toolchain_path_prefix}bin/ld64.lld"
    else
      echo "-fuse-ld=${execroot_abs_path}%{toolchain_path_prefix}bin/ld64.lld"
    fi

which is bad for its own reasons (if that file happens to exist, it might be the wrong one?) but I'm not sure if that matters. At the very least, it's not explicit to the user what's going on and deserves a comment that this condition needs to be here in support of absolute_paths.

Feel free to use this code or another solution at your leisure.