Skip to content

Commit

Permalink
Use llvm-libtool-darwin instead of llvm-ar
Browse files Browse the repository at this point in the history
This removes the need to provide `--features=-libtool` on macOS.
  • Loading branch information
Siddhartha Bagaria authored and siddharthab committed Mar 11, 2024
1 parent 2dc6ba0 commit 62fab6f
Show file tree
Hide file tree
Showing 11 changed files with 24 additions and 50 deletions.
18 changes: 1 addition & 17 deletions .github/workflows/release_notes_template.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
Minimum bazel version: **6.0.0** (6.1.0 if using blzmod)
Minimum bazel version: **7.0.0**

If you're using `bzlmod`, add the following to `MODULE.bazel`:

Expand Down Expand Up @@ -53,19 +53,3 @@ load("@llvm_toolchain//:toolchains.bzl", "llvm_register_toolchains")

llvm_register_toolchains()
```

And add the following section to your .bazelrc file:

```sh
# Not needed after https://github.com/bazelbuild/bazel/issues/7260 is closed
build --incompatible_enable_cc_toolchain_resolution

# For macOS only:

# Needed for Bazel versions before 7.
# Without this, one can use `--linkopt='-undefined dynamic_lookup'`.
# This feature is intentionally not supported on macOS.
build --features=-supports_dynamic_linker
# Not needed after https://github.com/bazel-contrib/toolchains_llvm/pull/229.
build --features=-libtool
```
8 changes: 1 addition & 7 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,8 @@ jobs:
fail-fast: false
matrix:
os: [macos-latest, ubuntu-latest]
bazel_version: [6.0.0, 6.1.0, latest]
bazel_version: [7.0.0, latest] # Minimum supported Bazel version is 7.0.0.
bzlmod: [true, false]
# bzlmod needs 6.1.0 (issue unrelated to this project)
exclude:
- bazel_version: 6.0.0
bzlmod: true
- bazel_version: 6.1.0
bzlmod: false
runs-on: ${{ matrix.os }}
steps:
- if: startsWith(matrix.os, 'ubuntu')
Expand Down
1 change: 1 addition & 0 deletions MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ module(
name = "toolchains_llvm",
version = "0.0.0",
compatibility_level = 0,
bazel_compatibility = [">=7.0.0"],
)

bazel_dep(name = "bazel_skylib", version = "1.4.2")
Expand Down
3 changes: 0 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,6 @@ For specifying unregistered toolchains on the command line, please use the
`--extra_toolchains` flag. For example,
`--extra_toolchains=@llvm_toolchain//:cc-toolchain-x86_64-linux`.

We no longer support the `--crosstool_top=@llvm_toolchain//:toolchain` flag,
and instead rely on the `--incompatible_enable_cc_toolchain_resolution` flag.

### Bring Your Own LLVM

The following mechanisms are available for using an LLVM toolchain:
Expand Down
1 change: 0 additions & 1 deletion tests/.bazelrc
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
build --incompatible_enable_cc_toolchain_resolution
build --features=layering_check
13 changes: 1 addition & 12 deletions tests/scripts/bazel.sh
Original file line number Diff line number Diff line change
Expand Up @@ -40,27 +40,16 @@ common_args=(
"--enable_bzlmod=${USE_BZLMOD:-false}"
)

# shellcheck disable=SC2034
common_test_args=(
"${common_args[@]}"
"--symlink_prefix=/"
"--incompatible_enable_cc_toolchain_resolution"
"--color=yes"
"--show_progress_rate_limit=30"
"--keep_going"
"--test_output=errors"
)

if [[ ${short_uname} == 'Darwin' ]]; then
common_test_args+=(
# Needed for Bazel versions before 7.
# Without this, one can use `--linkopt='-undefined dynamic_lookup'`.
# This feature is intentionally not supported on macOS.
--features=-supports_dynamic_linker
# Not needed after https://github.com/bazel-contrib/toolchains_llvm/pull/229.
--features=-libtool
)
fi

# Do not run autoconf to configure local CC toolchains.
export BAZEL_DO_NOT_DETECT_CPP_TOOLCHAIN=1

Expand Down
8 changes: 4 additions & 4 deletions tests/scripts/run_xcompile_tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,13 @@ check_with_image() {
return
fi
local image="$1"
docker run --rm --mount "type=bind,source=${binpath},target=/stdlib_test" "${image}" /stdlib_test
docker run --rm -it --platform=linux/amd64 \
--mount "type=bind,source=${binpath},target=/stdlib_test" "${image}" /stdlib_test
}

echo ""
echo "Testing static linked user libraries and dynamic linked system libraries"
build_args=(
--incompatible_enable_cc_toolchain_resolution
--platforms=@toolchains_llvm//platforms:linux-x86_64
--extra_toolchains=@llvm_toolchain_with_sysroot//:cc-toolchain-x86_64-linux
--symlink_prefix=/
Expand All @@ -44,7 +44,7 @@ build_args=(
)
"${bazel}" --bazelrc=/dev/null build "${build_args[@]}" //:stdlib_test
file "${binpath}" | tee /dev/stderr | grep -q ELF
check_with_image "frolvlad/alpine-glibc" # Need glibc image for system libraries.
check_with_image "gcr.io/distroless/cc-debian11" # Need glibc image for system libraries.

echo ""
echo "Testing static linked user and system libraries"
Expand All @@ -53,4 +53,4 @@ build_args+=(
)
"${bazel}" --bazelrc=/dev/null build "${build_args[@]}" //:stdlib_test
file "${binpath}" | tee /dev/stderr | grep -q ELF
check_with_image "alpine"
check_with_image "gcr.io/distroless/static-debian11"
1 change: 0 additions & 1 deletion tests/transitions.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -95,5 +95,4 @@ dwp_file = rule(
default = "@bazel_tools//tools/allowlists/function_transition_allowlist",
),
},
incompatible_use_toolchain_transition = True,
)
2 changes: 1 addition & 1 deletion toolchain/cc_toolchain_config.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ def cc_toolchain_config(
# The tool names come from [here](https://github.com/bazelbuild/bazel/blob/c7e58e6ce0a78fdaff2d716b4864a5ace8917626/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java#L76-L90):
# NOTE: Ensure these are listed in toolchain_tools in toolchain/internal/common.bzl.
tool_paths = {
"ar": tools_path_prefix + "llvm-ar",
"ar": tools_path_prefix + ("llvm-ar" if (host_os != "darwin" or is_xcompile) else "llvm-libtool-darwin"),
"cpp": tools_path_prefix + "clang-cpp",
"dwp": tools_path_prefix + "llvm-dwp",
"gcc": wrapper_bin_prefix + "cc_wrapper.sh",
Expand Down
12 changes: 11 additions & 1 deletion toolchain/internal/common.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

SUPPORTED_TARGETS = [("linux", "x86_64"), ("linux", "aarch64"), ("darwin", "x86_64"), ("darwin", "aarch64")]

toolchain_tools = [
_toolchain_tools = [
"clang-cpp",
"ld.lld",
"llvm-ar",
Expand All @@ -27,6 +27,10 @@ toolchain_tools = [
"llvm-strip",
]

_toolchain_tools_darwin = [
"llvm-libtool-darwin",
]

def host_os_key(rctx):
(os, version, arch) = os_version_arch(rctx)
if version == "":
Expand Down Expand Up @@ -191,6 +195,12 @@ def attr_dict(attr):

return dict(tuples)

def toolchain_tools(os):
tools = list(_toolchain_tools)
if os == "darwin":
tools.extend(_toolchain_tools_darwin)
return tools

def _get_host_tool_info(rctx, tool_path, tool_key = None):
if tool_key == None:
tool_key = tool_path
Expand Down
7 changes: 4 additions & 3 deletions toolchain/internal/configure.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,12 @@ def llvm_register_toolchains():
# symlinked path from the wrapper.
wrapper_bin_prefix = "bin/"
tools_path_prefix = "bin/"
for tool_name in _toolchain_tools:
tools = _toolchain_tools(os)
for tool_name in tools:
rctx.symlink(llvm_dist_rel_path + "bin/" + tool_name, tools_path_prefix + tool_name)
symlinked_tools_str = "".join([
"\n" + (" " * 8) + "\"" + tools_path_prefix + name + "\","
for name in _toolchain_tools
"\n" + (" " * 8) + "\"" + tools_path_prefix + tool_name + "\","
for tool_name in tools
])
else:
llvm_dist_rel_path = llvm_dist_path_prefix
Expand Down

0 comments on commit 62fab6f

Please sign in to comment.