Symlink '/usr/bin/ld' to llvm toolchain directory on Mac (bazel-contr…

…ib#105)

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

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 propagated 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.

* tests: add a `rules_rust` test

I picked `git2-rs` since it links against a C library and has tests but isn't _massive_.

There are a couple of hacks/patches here to workaround some bugs in `rules_rust`'s `crate-universe`
but on the whole it's not too messy.

* Add --incompatible_no_rule_outputs_param=false to run_external_tests.sh

The rules_rust repository is incompatible with this flag enabled. Have
disabled it for now.

* Add --incompatible_no_rule_outputs_param=false to run_tests.sh

Flag already added to 'run_external_tests.sh' but also needed here.

* Move 'git2-rs-cargo-toml.patch' to 'tests/rust'

* tests: move some shared test args for bazel into `bazel.sh`

Note that this also drops the check for `$TEST_MIGRATION` in run_external_tests.sh; we don't run that script in migration.yml

* tests: add a note about using `git2-rs` instead of the `@rules_rust` tests

Co-authored-by: Rahul Butani <[email protected]>

WORKSPACE

@@ -138,3 +138,104 @@ http_archive(
         "https://ftp.pcre.org/pub/pcre/pcre-8.43.tar.gz",
     ],
 )
+
+http_archive(
+    name = "rules_rust",
+    sha256 = "531bdd470728b61ce41cf7604dc4f9a115983e455d46ac1d0c1632f613ab9fc3",
+    strip_prefix = "rules_rust-d8238877c0e552639d3e057aadd6bfcf37592408",
+    urls = [
+        # `main` branch as of 2021-08-23
+        "https://github.com/bazelbuild/rules_rust/archive/d8238877c0e552639d3e057aadd6bfcf37592408.tar.gz",
+    ],
+)
+
+load("@rules_rust//rust:repositories.bzl", "rust_repositories")
+
+rust_repositories(
+    version = "1.55.0",
+    edition = "2018",
+)
+
+# We're using `git2` as our Rust test because it links against C code
+# (`libgit2-sys`) using `cc`, has tests, and is non-trivial but not _massive_.
+#
+# Ordinarily we'd just run the `rules_rust` tests but those break when run
+# from another workspace (some of the skylib unittests expect paths within the
+# main workspace and fail when paths like `external/rules_rust/...` are
+# produced) and we want to test usage of the cc_toolchain via the `cc` crate
+# anyways (as of this writing nothing in `@rules_rust//tests` seems to test
+# this).
+GIT2_RS_VER = "0.13.22"
+GIT2_RS_SHA = "9c1cbbfc9a1996c6af82c2b4caf828d2c653af4fcdbb0e5674cc966eee5a4197"
+
+http_archive(
+    name = "git2",
+    sha256 = GIT2_RS_SHA,
+    canonical_id = GIT2_RS_VER,
+    url = "https://crates.io/api/v1/crates/git2/{ver}/download".format(ver = GIT2_RS_VER),
+    type = "tar.gz",
+    strip_prefix = "git2-{ver}".format(ver = GIT2_RS_VER),
+    build_file_content = """
+package(default_visibility = ["//visibility:public"])
+
+load("@rules_rust//rust:defs.bzl", "rust_library", "rust_test")
+load("@crates//:defs.bzl", "crates_from", "dev_crates_from", "crate")
+
+DEV_CRATES = dev_crates_from("@git2//:Cargo.toml")
+DEV_CRATES.remove(crate("paste")) # This is a proc_macro crate!
+
+rust_library(
+    name = "git2",
+    srcs = glob(["src/**/*.rs"]),
+    deps = crates_from("@git2//:Cargo.toml"),
+)
+
+rust_test(
+    name = "git2-tests",
+    crate = ":git2",
+    deps = DEV_CRATES,
+    proc_macro_deps = [crate("paste")],
+)
+
+[
+    rust_test(
+        name = t[len("tests/"):][:-len(".rs")],
+        srcs = [t],
+        deps = [":git2"] + DEV_CRATES,
+        proc_macro_deps = [crate("paste")],
+    )
+    for t in glob(["tests/*.rs"])
+]
+""",
+    # We need to remove some `[target]` entries in `git2`'s `Cargo.toml` to
+    # make `crate-universe` happy.
+    #
+    # See: https://github.com/bazelbuild/rules_rust/issues/783
+    patches = ["//tests/rust:git2-rs-cargo-toml.patch"],
+    patch_args = ["-p1"]
+)
+
+# Snippets for `crate_universe`:
+RULES_RUST_CRATE_UNIVERSE_URL_TEMPLATE = "https://github.com/bazelbuild/rules_rust/releases/download/crate_universe-13/crate_universe_resolver-{host_triple}{extension}"
+RULES_RUST_CRATE_UNIVERSE_SHA256_CHECKSUMS = {
+    "aarch64-apple-darwin": "c6017cd8a4fee0f1796a8db184e9d64445dd340b7f48a65130d7ee61b97051b4",
+    "aarch64-unknown-linux-gnu": "d0a310b03b8147e234e44f6a93e8478c260a7c330e5b35515336e7dd67150f35",
+    "x86_64-apple-darwin": "762f1c77b3cf1de8e84d7471442af1314157efd90720c7e1f2fff68556830ee2",
+    "x86_64-pc-windows-gnu": "c44bd97373d690587e74448b13267077d133f04e89bedfc9d521ae8ba55dddb9",
+    "x86_64-unknown-linux-gnu": "aebf51af6a3dd33fdac463b35b0c3f4c47ab93e052099199673289e2025e5824",
+}
+
+load("@rules_rust//crate_universe:defs.bzl", "crate_universe")
+
+crate_universe(
+    name = "crates",
+    cargo_toml_files = [
+        "@git2//:Cargo.toml",
+    ],
+    resolver_download_url_template = RULES_RUST_CRATE_UNIVERSE_URL_TEMPLATE,
+    resolver_sha256s = RULES_RUST_CRATE_UNIVERSE_SHA256_CHECKSUMS,
+)
+
+load("@crates//:defs.bzl", "pinned_rust_install")
+
+pinned_rust_install()

tests/rust/git2-rs-cargo-toml.patch

@@ -0,0 +1,15 @@
+diff --git a/Cargo.toml b/Cargo.toml
+index 5837a47..d6ba77f 100644
+--- a/Cargo.toml
++++ b/Cargo.toml
+@@ -60,10 +60,3 @@ unstable = []
+ vendored-libgit2 = ["libgit2-sys/vendored"]
+ vendored-openssl = ["openssl-sys/vendored", "libgit2-sys/vendored-openssl"]
+ zlib-ng-compat = ["libgit2-sys/zlib-ng-compat"]
+-[target."cfg(all(unix, not(target_os = \"macos\")))".dependencies.openssl-probe]
+-version = "0.1"
+-optional = true
+-
+-[target."cfg(all(unix, not(target_os = \"macos\")))".dependencies.openssl-sys]
+-version = "0.9.0"
+-optional = true

tests/scripts/bazel.sh

@@ -17,5 +17,14 @@ readonly url="https://github.com/bazelbuild/bazelisk/releases/download/${bazelis
 bazel="${TMPDIR:-/tmp}/bazelisk"
 readonly bazel
 
+readonly common_test_args=(
+  --incompatible_enable_cc_toolchain_resolution
+  --symlink_prefix=/
+  --color=yes
+  --show_progress_rate_limit=30
+  --keep_going
+  --test_output=errors
+)
+
 curl -L -sSf -o "${bazel}" "${url}"
 chmod a+x "${bazel}"

tests/scripts/run_external_tests.sh

@@ -22,29 +22,14 @@ source "$(dirname "${BASH_SOURCE[0]}")/bazel.sh"
 "${bazel}" fetch @io_bazel_rules_go//tests/core/cgo:all
 "$("${bazel}" info output_base)/external/io_bazel_rules_go/tests/core/cgo/generate_imported_dylib.sh"
 
-test_args=(
-  --incompatible_enable_cc_toolchain_resolution
-  --symlink_prefix=/
-  --color=yes
-  --show_progress_rate_limit=30
-  --keep_going
-  --test_output=errors
-)
-if [[ "${TEST_MIGRATION:-}" ]]; then
-  # We can not use bazelisk to test migration because bazelisk does not allow
-  # us to selectively ignore a migration flag.
-  test_args+=("--all_incompatible_changes")
-  # This flag is not quite ready -- https://github.com/bazelbuild/bazel/issues/7347
-  test_args+=("--incompatible_disallow_struct_provider_syntax=false")
-fi
-
 # We exclude the following targets:
 # - cc_libs_test from rules_go because it assumes that stdlibc++ has been dynamically linked, but we
 #   link it statically on linux.
 # - opts_test from rules_go because its include path assumes that the main repo is rules_go (see
 #   https://github.com/bazelbuild/rules_go/issues/2955).
-"${bazel}" --bazelrc=/dev/null test "${test_args[@]}" \
+"${bazel}" --bazelrc=/dev/null test "${common_test_args[@]}" \
   //tests/foreign:pcre \
+  @git2//:all \
   @openssl//:libssl \
   $("${bazel}" query 'attr(timeout, short, tests(@com_google_absl//absl/...))') \
   $("${bazel}" query 'tests(@io_bazel_rules_go//tests/core/cgo:all) except set(@io_bazel_rules_go//tests/core/cgo:cc_libs_test @io_bazel_rules_go//tests/core/cgo:opts_test)')

tests/scripts/run_tests.sh

@@ -34,20 +34,17 @@ source "$(dirname "${BASH_SOURCE[0]}")/bazel.sh"
 set -x
 test_args=(
   --extra_toolchains="${toolchain_name}"
-  --incompatible_enable_cc_toolchain_resolution
   --copt=-v
   --linkopt=-Wl,-t
-  --symlink_prefix=/
-  --color=yes
-  --show_progress_rate_limit=30
-  --keep_going
-  --test_output=errors
 )
 if [[ "${TEST_MIGRATION:-}" ]]; then
   # We can not use bazelisk to test migration because bazelisk does not allow
   # us to selectively ignore a migration flag.
   test_args+=("--all_incompatible_changes")
   # This flag is not quite ready -- https://github.com/bazelbuild/bazel/issues/7347
   test_args+=("--incompatible_disallow_struct_provider_syntax=false")
+  # the rules_rust repo included in the WORKSPACE is currently incompatible with
+  # '--incompatible_no_rule_outputs_param=true', setting this to false for now.
+  test_args+=("--incompatible_no_rule_outputs_param=false")
 fi
-"${bazel}"  --bazelrc=/dev/null test "${test_args[@]}" //tests:all
+"${bazel}"  --bazelrc=/dev/null test "${common_test_args[@]}" "${test_args[@]}" //tests:all

toolchain/internal/repo.bzl

@@ -34,3 +34,8 @@ def llvm_repo_impl(rctx):
     )
 
     _download_llvm_preconfigured(rctx)
+
+    # darwin may use local 'ld' so symlink it to bin directory to help
+    # other programs locate it when called directly (e.g. rustc)
+    if os == "darwin":
+        rctx.symlink("/usr/bin/ld", "bin/ld")