From 6e42888e8fc2d6560ee8ffb6ab77a0e64a1f1d6c Mon Sep 17 00:00:00 2001 From: Siddhartha Bagaria Date: Wed, 22 Sep 2021 01:14:55 -0700 Subject: [PATCH] Remove workarounds for rules_go support (#95) It looks like rules_go can work without these now. This will simplify the setup of the LLVM distribution repo. --- tests/scripts/run_external_tests.sh | 11 +++++------ toolchain/BUILD.llvm_repo | 11 ++--------- toolchain/cc_wrapper.sh.tpl | 3 --- toolchain/internal/repo.bzl | 17 +---------------- toolchain/osx_cc_wrapper.sh.tpl | 3 --- 5 files changed, 8 insertions(+), 37 deletions(-) diff --git a/tests/scripts/run_external_tests.sh b/tests/scripts/run_external_tests.sh index 35f75755..bff78469 100755 --- a/tests/scripts/run_external_tests.sh +++ b/tests/scripts/run_external_tests.sh @@ -22,11 +22,6 @@ 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" -# 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). test_args=( --incompatible_enable_cc_toolchain_resolution --symlink_prefix=/ @@ -42,8 +37,12 @@ if [[ "${TEST_MIGRATION:-}" ]]; then # 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: +# - 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[@]}" \ //tests/foreign:pcre \ @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)') + $("${bazel}" query 'tests(@io_bazel_rules_go//tests/core/cgo:all) except set(@io_bazel_rules_go//tests/core/cgo:opts_test)') diff --git a/toolchain/BUILD.llvm_repo b/toolchain/BUILD.llvm_repo index 2fac3278..7db02957 100644 --- a/toolchain/BUILD.llvm_repo +++ b/toolchain/BUILD.llvm_repo @@ -35,11 +35,7 @@ filegroup( name = "ld", srcs = [ "bin/ld.lld", - ] + glob([ - # Optional symlinks needed to support rules_go and some other applications. - "bin/ld", # -> /usr/bin/ld - "bin/ld.gold", # -> /usr/bin/ld.gold (or empty) - ]), + ], ) filegroup( @@ -78,10 +74,7 @@ filegroup( filegroup( name = "ar", - srcs = ["bin/llvm-ar"] + glob([ - # Optional symlinks needed to support rules_go and some other applications. - "bin/ar", # -> /usr/bin/ar - ]), + srcs = ["bin/llvm-ar"], ) filegroup( diff --git a/toolchain/cc_wrapper.sh.tpl b/toolchain/cc_wrapper.sh.tpl index e86e4bc1..451e768e 100644 --- a/toolchain/cc_wrapper.sh.tpl +++ b/toolchain/cc_wrapper.sh.tpl @@ -33,9 +33,6 @@ set -eu # Call the C++ compiler. if [[ -f %{toolchain_path_prefix}bin/clang ]]; then %{toolchain_path_prefix}bin/clang "$@" -elif [[ ":${PATH}:" == *":%{toolchain_path_prefix}bin:"* ]]; then - # GoCompile sets the PATH to the directory containing the linker, and changes CWD. - clang "$@" elif [[ "${BASH_SOURCE[0]}" == "/"* ]]; then # Some consumers of `CcToolchainConfigInfo` (e.g. `cmake` from rules_foreign_cc) # change CWD and call $CC (this script) with its absolute path. diff --git a/toolchain/internal/repo.bzl b/toolchain/internal/repo.bzl index dda05300..025d3756 100644 --- a/toolchain/internal/repo.bzl +++ b/toolchain/internal/repo.bzl @@ -40,21 +40,6 @@ def llvm_repo_impl(rctx): executable = False, ) + # TODO: Replace download_llvm with standard http_archive rules. if not _download_llvm(rctx, _os_arch_pair(os, arch)): _download_llvm_preconfigured(rctx) - - # TODO: Are these symlinks still needed? Remove from here and - # BUILD.llvm_repo if not. If these symlinks are removed, then we can - # leverage http_archive rule instead of _download_llvm. Or maybe we can - # still leverage it with a patch file. - - rctx.symlink("/usr/bin/ar", "bin/ar") # For GoLink. - - # For GoCompile on macOS; compiler path is set from linker path. - # It also helps clang driver sometimes for the linker to be colocated with the compiler. - rctx.symlink("/usr/bin/ld", "bin/ld") - if rctx.path("/usr/bin/ld.gold").exists: - rctx.symlink("/usr/bin/ld.gold", "bin/ld.gold") - else: - # Add dummy file for non-linux so we don't have to put conditional logic in BUILD. - rctx.file("bin/ld.gold") diff --git a/toolchain/osx_cc_wrapper.sh.tpl b/toolchain/osx_cc_wrapper.sh.tpl index 5de7bab8..84d7d7a3 100755 --- a/toolchain/osx_cc_wrapper.sh.tpl +++ b/toolchain/osx_cc_wrapper.sh.tpl @@ -52,9 +52,6 @@ done # Call the C++ compiler. if [[ -f %{toolchain_path_prefix}bin/clang ]]; then %{toolchain_path_prefix}bin/clang "$@" -elif [[ ":${PATH}:" == *":%{toolchain_path_prefix}bin:"* ]]; then - # GoCompile sets the PATH to the directory containing the linker, and changes CWD. - clang "$@" elif [[ "${BASH_SOURCE[0]}" == "/"* ]]; then # Some consumers of `CcToolchainConfigInfo` (e.g. `cmake` from rules_foreign_cc) # change CWD and call $CC (this script) with its absolute path.