Skip to content

Commit

Permalink
Remove workarounds for rules_go support (#95)
Browse files Browse the repository at this point in the history
It looks like rules_go can work without these now.

This will simplify the setup of the LLVM distribution repo.
  • Loading branch information
Siddhartha Bagaria authored Sep 22, 2021
1 parent e4b62c9 commit 6e42888
Show file tree
Hide file tree
Showing 5 changed files with 8 additions and 37 deletions.
11 changes: 5 additions & 6 deletions tests/scripts/run_external_tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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=/
Expand All @@ -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)')
11 changes: 2 additions & 9 deletions toolchain/BUILD.llvm_repo
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down
3 changes: 0 additions & 3 deletions toolchain/cc_wrapper.sh.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
17 changes: 1 addition & 16 deletions toolchain/internal/repo.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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")
3 changes: 0 additions & 3 deletions toolchain/osx_cc_wrapper.sh.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit 6e42888

Please sign in to comment.