Symlink '/usr/bin/ld' to llvm toolchain directory on Mac

[dozzman]

Mac systems still use the local ld executable since lld.ld is still
experimental. Some applications, e.g. rustc when compiling rust, expect
to find ld within the same directory as the compiler and so when
compiling rust we run into an issue where the linker is not found.

The first immediate solution is to use the rust rule's
extra_rustc_flags to specify the linker explicitly (via
-Clinker=/usr/bin/ld. This works for some crates but fails for crates
using a build script, since the extra flags are not propogated to rustc
invocations within the exec configuration.

Another solution attempt was to link ld into the toolchain
configuration repository instead of directly in the llvm repository,
such that ld becomes a sibling file to cc_wrapper.sh. Unfortunately
rustc was still unable to find the linker with this configuration.

The final fully working solution is to link ld into the bin folder of
the llvm repository. This should be fine as LLVM's own linker exists
under ld.lld and symlinking the local linker will not clobber any
files.

[siddharthab]

Thank you for the PR.

We used to do this when GoCompile had the same problem. I removed these symlinks in 6e42888 because Go rules did not need it anymore. Will you be able to add some tests for Rust in the run_external_tests.sh script so we can track compatibility?

[rrbutani]

I'm still a little confused about why/how rustc seems to be searching the bin directory of the LLVM repository instead of the bin dir where cc_wrapper.sh lives but that doesn't need to block this PR.

I added a test to run_external_tests.sh that runs git2-rs's tests.

[rrbutani]

@siddharthab Adding rules_rust causes our migration test to fail for --incompatible_no_rule_outputs_param=true.

I can think of three ways to get around this:

1. add --incompatible_no_rule_outputs_param=false to test_args in run_tests.sh
   • this seems like a bad precedent: we're taking on the full set of migration problems of our test dependencies
   • in this specific case I don't think it's a big deal (we're pretty unlikely to accidentally grow any outputs in our rules since we're a toolchain, I think) but still
2. move rules_rust and its tests to another workspace
   • I don't think we can move all the tests to another workspace (or at least I don't think this would help test migrations) since some of the migrations affect more than just things in the load phase, I think (on the other hand we currently only really expose a repo rule anyways)
   • the downside here is duplication (though there wouldn't be that much of it)
3. gate the rules_rust bits in WORKSPACE on $TEST_MIGRATION by wrapping those bits in a repo rule
   • this gets around the duplication problem but is maybe a little too much magic? not sure

I'm partial to the third option but whatever you think is best works.

---

As a sidenote: we don't actually seem to run run_external_tests.sh in migration.yml yet run_external_tests.sh has checks for $TEST_MIGRATION; is this intentional?

[siddharthab]

I would not worry too much about the migration failure. Just adding an exception in run_tests.sh (option 1) seems fine. If possible, file an issue with rules_rust and mention the issue in comments before our exception.

For run_external_tests.sh, we can remove the block that adds migration check flags. I don't think I put too much thought into it when I added it there.

[dozzman]

The force push was a mistake -- thought I accidentally pushed some commits, didn't realise there was some other work done on the branch. Have reverted that.

[dozzman]

I would not worry too much about the migration failure. Just adding an exception in run_tests.sh (option 1) seems fine. If possible, file an issue with rules_rust and mention the issue in comments before our exception.

For run_external_tests.sh, we can remove the block that adds migration check flags. I don't think I put too much thought into it when I added it there.

Added the appropriate flag to run_tests.sh and run_external_tests.sh. The other scripts looked like they didn't need amending.

[dozzman]

Was sure I saw a message requesting to remove the $TEST_MIGRATION blocks but since I can no longer find it I will leave it.

[rrbutani]

Was sure I saw a message requesting to remove the $TEST_MIGRATION blocks but since I can no longer find it I will leave it.

This was the message I think. I've removed that bit from run_external_tests.sh.

[rrbutani]

I did a little more digging; rustc calls cc_wrapper.sh as the "linker" which then calls clang which then tries to shell out to ld. rustc itself does do some searching for the linker it calls but that's not what's failing here.

clang can't find ld when ld is next to cc_wrapper.sh but can when ld is next to clang itself (as in, in the LLVM repo's bin folder) which makes sense.

I'm still a little confused why we only seem to run into this when cc_wrapper.sh is called from rustc; nothing about the cc_wrapper.sh call rustc generates looks different. Maybe it's something odd about $PWD in that case.

Regardless, I think this PR is good to go .

[siddharthab]

I did a little more digging; rustc calls cc_wrapper.sh as the "linker" which then calls clang which then tries to shell out to ld. rustc itself does do some searching for the linker it calls but that's not what's failing here.

That's because in that function, PATH is set to /bin, but the linker on macOS is in /usr/bin.

[siddharthab]

I will merge this as-is and then maybe commit some more changes to clean up later.

[rrbutani]

I did a little more digging; rustc calls cc_wrapper.sh as the "linker" which then calls clang which then tries to shell out to ld. rustc itself does do some searching for the linker it calls but that's not what's failing here.

That's because in that function, PATH is set to /bin, but the linker on macOS is in /usr/bin.

Sorry, that snippet is a little misleading; $PATH isn't being set to /bin but /bin is being added to a list of search directories. Regardless, rustc doesn't error there; cc_wrapper.sh is specified by rules_rust as the linker so rustc just calls cc_wrapper.sh (without doing any searching) which then errors when clang can't find ld.

The actual rustc invocation looks something like this on my machine:

bazel-out/darwin-opt-exec-2B5CBBC6/bin/external/rules_rust/util/process_wrapper/process_wrapper --subst 'pwd=${pwd}' -- external/rust_darwin_x86_64/bin/rustc external/crates__log__0_4_14/build.rs '--crate-name=log_build_script_script_' '--crate-type=bin' '--error-format=human' '--codegen=metadata=' '--out-dir=bazel-out/darwin-opt-exec-2B5CBBC6/bin/external/crates__log__0_4_14' '--codegen=extra-filename=' '--codegen=opt-level=3' '--codegen=debuginfo=0' '--remap-path-prefix=${pwd}=.' '--emit=dep-info,link' '--color=always' '--target=x86_64-apple-darwin' -L external/rust_darwin_x86_64/lib/rustlib/x86_64-apple-darwin/lib '--cap-lints=allow' '--codegen=linker=external/llvm_toolchain/bin/cc_wrapper.sh' --codegen 'link-args=-mlinker-version=450 --target=x86_64-apple-macosx -lm -no-canonical-prefixes -headerpad_max_install_names -undefined dynamic_lookup -lc++ -lc++abi --sysroot=/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk'

(note the '--codegen=linker=external/llvm_toolchain/bin/cc_wrapper.sh')

Which then leads to a cc_wrapper.sh invocation that looks like this:

"external/llvm_toolchain/bin/cc_wrapper.sh" "-m64" "-arch" "x86_64" "bazel-out/darwin-opt-exec-2B5CBBC6/bin/external/crates__log__0_4_14/log_build_script_script_.log_build_script_script_.11355ed9-cgu.0.rcgu.o" "bazel-out/darwin-opt-exec-2B5CBBC6/bin/external/crates__log__0_4_14/log_build_script_script_.log_build_script_script_.11355ed9-cgu.1.rcgu.o" "bazel-out/darwin-opt-exec-2B5CBBC6/bin/external/crates__log__0_4_14/log_build_script_script_.log_build_script_script_.11355ed9-cgu.2.rcgu.o" "bazel-out/darwin-opt-exec-2B5CBBC6/bin/external/crates__log__0_4_14/log_build_script_script_.log_build_script_script_.11355ed9-cgu.3.rcgu.o" "bazel-out/darwin-opt-exec-2B5CBBC6/bin/external/crates__log__0_4_14/log_build_script_script_.log_build_script_script_.11355ed9-cgu.4.rcgu.o" "bazel-out/darwin-opt-exec-2B5CBBC6/bin/external/crates__log__0_4_14/log_build_script_script_.log_build_script_script_.11355ed9-cgu.5.rcgu.o" "bazel-out/darwin-opt-exec-2B5CBBC6/bin/external/crates__log__0_4_14/log_build_script_script_.log_build_script_script_.11355ed9-cgu.6.rcgu.o" "bazel-out/darwin-opt-exec-2B5CBBC6/bin/external/crates__log__0_4_14/log_build_script_script_.log_build_script_script_.11355ed9-cgu.7.rcgu.o" "bazel-out/darwin-opt-exec-2B5CBBC6/bin/external/crates__log__0_4_14/log_build_script_script_.log_build_script_script_.11355ed9-cgu.8.rcgu.o" "bazel-out/darwin-opt-exec-2B5CBBC6/bin/external/crates__log__0_4_14/log_build_script_script_.5esazdxjfc5wg0qb.rcgu.o" "-L" "external/rust_darwin_x86_64/lib/rustlib/x86_64-apple-darwin/lib" "-L" "/private/var/tmp/_bazel_rahulbutani/729ef14ca5fc8012394a84396ed546ff/external/rust_darwin_x86_64/lib/rustlib/x86_64-apple-darwin/lib" "/private/var/tmp/_bazel_rahulbutani/729ef14ca5fc8012394a84396ed546ff/external/rust_darwin_x86_64/lib/rustlib/x86_64-apple-darwin/lib/libstd-35b14ca5e62c7d66.rlib" "/private/var/tmp/_bazel_rahulbutani/729ef14ca5fc8012394a84396ed546ff/external/rust_darwin_x86_64/lib/rustlib/x86_64-apple-darwin/lib/libpanic_unwind-329b6ae10a542427.rlib" "/private/var/tmp/_bazel_rahulbutani/729ef14ca5fc8012394a84396ed546ff/external/rust_darwin_x86_64/lib/rustlib/x86_64-apple-darwin/lib/libobject-09e9356fd4907aa1.rlib" "/private/var/tmp/_bazel_rahulbutani/729ef14ca5fc8012394a84396ed546ff/external/rust_darwin_x86_64/lib/rustlib/x86_64-apple-darwin/lib/libaddr2line-f077c0a492da74c3.rlib" "/private/var/tmp/_bazel_rahulbutani/729ef14ca5fc8012394a84396ed546ff/external/rust_darwin_x86_64/lib/rustlib/x86_64-apple-darwin/lib/libgimli-01a2a2a1730ab072.rlib" "/private/var/tmp/_bazel_rahulbutani/729ef14ca5fc8012394a84396ed546ff/external/rust_darwin_x86_64/lib/rustlib/x86_64-apple-darwin/lib/libstd_detect-736ff6c48d99c951.rlib" "/private/var/tmp/_bazel_rahulbutani/729ef14ca5fc8012394a84396ed546ff/external/rust_darwin_x86_64/lib/rustlib/x86_64-apple-darwin/lib/librustc_demangle-f6b7985fe24842c3.rlib" "/private/var/tmp/_bazel_rahulbutani/729ef14ca5fc8012394a84396ed546ff/external/rust_darwin_x86_64/lib/rustlib/x86_64-apple-darwin/lib/libhashbrown-6ccb889196a16125.rlib" "/private/var/tmp/_bazel_rahulbutani/729ef14ca5fc8012394a84396ed546ff/external/rust_darwin_x86_64/lib/rustlib/x86_64-apple-darwin/lib/librustc_std_workspace_alloc-2bda93c117f030a0.rlib" "/private/var/tmp/_bazel_rahulbutani/729ef14ca5fc8012394a84396ed546ff/external/rust_darwin_x86_64/lib/rustlib/x86_64-apple-darwin/lib/libunwind-bcbd09b25c64fcac.rlib" "/private/var/tmp/_bazel_rahulbutani/729ef14ca5fc8012394a84396ed546ff/external/rust_darwin_x86_64/lib/rustlib/x86_64-apple-darwin/lib/libcfg_if-ef7e97011650176d.rlib" "/private/var/tmp/_bazel_rahulbutani/729ef14ca5fc8012394a84396ed546ff/external/rust_darwin_x86_64/lib/rustlib/x86_64-apple-darwin/lib/liblibc-104a032674bfeab8.rlib" "/private/var/tmp/_bazel_rahulbutani/729ef14ca5fc8012394a84396ed546ff/external/rust_darwin_x86_64/lib/rustlib/x86_64-apple-darwin/lib/liballoc-ba3aef5f5e0e8573.rlib" "/private/var/tmp/_bazel_rahulbutani/729ef14ca5fc8012394a84396ed546ff/external/rust_darwin_x86_64/lib/rustlib/x86_64-apple-darwin/lib/librustc_std_workspace_core-9ee1c33f7a829255.rlib" "/private/var/tmp/_bazel_rahulbutani/729ef14ca5fc8012394a84396ed546ff/external/rust_darwin_x86_64/lib/rustlib/x86_64-apple-darwin/lib/libcore-20d0dacb579b8ca8.rlib" "/private/var/tmp/_bazel_rahulbutani/729ef14ca5fc8012394a84396ed546ff/external/rust_darwin_x86_64/lib/rustlib/x86_64-apple-darwin/lib/libcompiler_builtins-2fee5b6fdc3293e9.rlib" "-lSystem" "-lresolv" "-lc" "-lm" "-liconv" "-L" "/private/var/tmp/_bazel_rahulbutani/729ef14ca5fc8012394a84396ed546ff/external/rust_darwin_x86_64/lib/rustlib/x86_64-apple-darwin/lib" "-o" "bazel-out/darwin-opt-exec-2B5CBBC6/bin/external/crates__log__0_4_14/log_build_script_script_" "-Wl,-dead_strip" "-nodefaultlibs" "-mlinker-version=450" "--target=x86_64-apple-macosx" "-lm" "-no-canonical-prefixes" "-headerpad_max_install_names" "-undefined" "dynamic_lookup" "-lc++" "-lc++abi" "--sysroot=/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk"

I can try to run the rustc cc_wrapper.sh invocation under dtruss and compare it to a "regular" cc_wrapper.sh invocation from a rules_cc target to figure out where the discrepancy is coming from later.

[rrbutani]

@siddharthab what other cleanup needs to happen?

edit: whoops, just saw the other PR

[siddharthab]

Sorry, that snippet is a little misleading; $PATH isn't being set to /bin but /bin is being added to a list of search directories.

You can verify my claim by adding a echo $PATH >> /tmp/foo in osx_cc_wrapper.sh, and then running the test.

[rrbutani]

@siddharthab you're absolutely right; good catch

I'm still fairly convinced it's not rustc that's doing this; I think it's actually this from rules_rust (it'd explain why we only see the problem for build scripts too edit: this happens for rust_binary targets too which makes sense since both ultimately seem to source such env vars from cc_common.get_environment_variables; the only thing unique to build script targets is that they aren't passed -Clinker= flags).