Skip to content

Commit

Permalink
Allow user provided platform constraints (#371)
Browse files Browse the repository at this point in the history
An implementation of the fix suggested in
#361 to allow
users to specify additional platform constraints for each toolchain.

My personal use case was building some targets with musl and the
toolchains here were interferring.
  • Loading branch information
jkurland-roku authored Sep 6, 2024
1 parent a1a5013 commit 1cd9e36
Show file tree
Hide file tree
Showing 10 changed files with 227 additions and 5 deletions.
63 changes: 62 additions & 1 deletion tests/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

load("@bazel_skylib//rules:build_test.bzl", "build_test")
load("@rules_cc//cc:defs.bzl", "cc_binary", "cc_library", "cc_test")
load(":transitions.bzl", "dwp_file")
load(":transitions.bzl", "dwp_file", "transition_library_to_platform")

cc_library(
name = "stdlib",
Expand Down Expand Up @@ -139,3 +139,64 @@ toolchain(
toolchain = "@@rules_foreign_cc~~tools~ninja_1.11.1_mac//:ninja_tool",
toolchain_type = "@rules_foreign_cc//toolchains:ninja_toolchain",
)

# Testing extra_target_compatible_with
constraint_setting(
name = "cxx_standard",
default_constraint_value = ":cxx17",
visibility = ["//visibility:public"],
)

constraint_value(
name = "cxx20",
constraint_setting = ":cxx_standard",
visibility = ["//visibility:public"],
)

constraint_value(
name = "cxx17",
constraint_setting = ":cxx_standard",
visibility = ["//visibility:public"],
)

platform(
name = "cxx20_platform",
constraint_values = [
":cxx20",
],
parents = ["@platforms//host"],
visibility = ["//visibility:public"],
)

cc_library(
name = "test_cxx_standard_lib",
srcs = ["test_cxx_standard.cc"],
)

cc_test(
name = "test_cxx_standard_is_17",
size = "small",
srcs = ["test_cxx_standard_main.cc"],
args = ["201703"],
deps = [":test_cxx_standard_lib"],
)

transition_library_to_platform(
name = "test_cxx_standard_lib_transitioned",
lib = ":test_cxx_standard_lib",
platform = ":cxx20_platform",
)

cc_test(
name = "test_cxx_standard_is_20",
size = "small",
srcs = ["test_cxx_standard_main.cc"],
args = ["202002"],

# Since some platforms require special toolchains (e.g. llvm 13.0.0) this
# target won't build on those platforms unless we create a new toolchain per
# platform with c++20. So instead just only run this test on platforms that
# can use the default toolchain
tags = ["manual"],
deps = [":test_cxx_standard_lib_transitioned"],
)
19 changes: 18 additions & 1 deletion tests/MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,28 @@ LLVM_VERSIONS = {
llvm.toolchain(
name = "llvm_toolchain",
llvm_versions = LLVM_VERSIONS,
cxx_standard = {"": "c++17"},
)
llvm.extra_target_compatible_with(
name = "llvm_toolchain",
constraints = ["@//:cxx17"],
)
use_repo(llvm, "llvm_toolchain", "llvm_toolchain_llvm")

register_toolchains("@llvm_toolchain//:all")

llvm.toolchain(
name = "llvm_toolchain_cxx20",
llvm_versions = LLVM_VERSIONS,
cxx_standard = {"": "c++20"},
)
llvm.extra_target_compatible_with(
name = "llvm_toolchain_cxx20",
constraints = ["//:cxx20"],
)
use_repo(llvm, "llvm_toolchain_cxx20")
register_toolchains("@llvm_toolchain_cxx20//:all")


# Example toolchain with user provided URLs.
# TODO(siddharthab): Add test.
llvm.toolchain(
Expand Down
30 changes: 30 additions & 0 deletions tests/WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,19 @@ LLVM_VERSIONS = {

llvm_toolchain(
name = "llvm_toolchain",
cxx_standard = {"": "c++17"},
extra_target_compatible_with = {
"": ["@//:cxx17"],
},
llvm_versions = LLVM_VERSIONS,
)

llvm_toolchain(
name = "llvm_toolchain_cxx20",
cxx_standard = {"": "c++20"},
extra_target_compatible_with = {
"": ["@//:cxx20"],
},
llvm_versions = LLVM_VERSIONS,
)

Expand Down Expand Up @@ -75,6 +88,10 @@ load("@llvm_toolchain//:toolchains.bzl", "llvm_register_toolchains")

llvm_register_toolchains()

load("@llvm_toolchain_cxx20//:toolchains.bzl", llvm_register_toolchains_cxx20 = "llvm_register_toolchains")

llvm_register_toolchains_cxx20()

## Toolchain example with absolute paths; tested in GitHub CI.
llvm_toolchain(
name = "llvm_toolchain_with_absolute_paths",
Expand Down Expand Up @@ -230,3 +247,16 @@ http_archive(
"https://ftp.pcre.org/pub/pcre/pcre-8.43.tar.gz",
],
)

http_archive(
name = "platforms",
sha256 = "218efe8ee736d26a3572663b374a253c012b716d8af0c07e842e82f238a0a7ee",
urls = [
"https://mirror.bazel.build/github.com/bazelbuild/platforms/releases/download/0.0.10/platforms-0.0.10.tar.gz",
"https://github.com/bazelbuild/platforms/releases/download/0.0.10/platforms-0.0.10.tar.gz",
],
)

load("@platforms//host:extension.bzl", "host_platform_repo")

host_platform_repo(name = "host_platform")
12 changes: 11 additions & 1 deletion tests/scripts/run_tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,18 @@ test_args=(
"--linkopt=-Wl,-t"
)

targets=(
"//:all"
)
# :test_cxx_standard_is_20 builds with a version of the default toolchain, if
# we're trying to build with a different toolchain then it's likely the default
# toolchain won't work so :test_cxx_standard_is_20 won't build.
if [[ -z ${toolchain_name} ]]; then
targets+=("//:test_cxx_standard_is_20")
fi

"${bazel}" ${TEST_MIGRATION:+"--strict"} --bazelrc=/dev/null test \
"${common_test_args[@]}" "${test_args[@]}" //:all
"${common_test_args[@]}" "${test_args[@]}" "${targets[@]}"

# Note that the following flags are currently known to cause issues in migration tests:
# --incompatible_disallow_struct_provider_syntax # https://github.com/bazelbuild/bazel/issues/7347
Expand Down
22 changes: 22 additions & 0 deletions tests/test_cxx_standard.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
#include <iostream>
#include <cstdlib>

int run_test(int argc, char** argv) {
if (argc != 2) {
std::cout << "Not enough arguments" << std::endl;
return 1;
}

long expected_version = std::atol(argv[1]);

if (expected_version == 0) {
std::cout << "Invalid version argument, must be an integer" << std::endl;
return 1;
}

if (expected_version != __cplusplus) {
std::cout << "Expected version to be " << argv[1] << " but got " << __cplusplus << std::endl;
return 1;
}
return 0;
}
5 changes: 5 additions & 0 deletions tests/test_cxx_standard_main.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
int run_test(int argc, char** argv);

int main(int argc, char** argv) {
return run_test(argc, argv);
}
25 changes: 25 additions & 0 deletions tests/transitions.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -96,3 +96,28 @@ dwp_file = rule(
),
},
)

def _transition_library_to_platform_transition_impl(_, attr):
return {"//command_line_option:platforms": str(attr.platform)}

_transition_library_to_platform_transition = transition(
implementation = _transition_library_to_platform_transition_impl,
inputs = [],
outputs = ["//command_line_option:platforms"],
)

def _transition_library_to_platform_impl(ctx):
return [
ctx.attr.lib[0][CcInfo],
]

transition_library_to_platform = rule(
implementation = _transition_library_to_platform_impl,
attrs = {
"lib": attr.label(mandatory = True, cfg = _transition_library_to_platform_transition),
"platform": attr.label(mandatory = True),
"_allowlist_function_transition": attr.label(
default = "@bazel_tools//tools/allowlists/function_transition_allowlist",
),
},
)
36 changes: 36 additions & 0 deletions toolchain/extensions/llvm.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,20 @@ def _root_dict(roots, cls, name, strip_target):

return res

def _constraint_dict(tags, name):
constraints = {}

# Gather all the additional constraints for each target
for tag in tags:
targets = list(tag.targets)
if not targets:
targets = [""]
for target in targets:
constraints_for_target = constraints.setdefault(target, [])
constraints_for_target.extend([str(c) for c in tag.constraints])

return constraints

def _llvm_impl_(module_ctx):
for mod in module_ctx.modules:
if not mod.is_root:
Expand All @@ -49,6 +63,14 @@ def _llvm_impl_(module_ctx):
}
attrs["toolchain_roots"] = _root_dict([root for root in mod.tags.toolchain_root if root.name == name], "toolchain_root", name, True)
attrs["sysroot"] = _root_dict([sysroot for sysroot in mod.tags.sysroot if sysroot.name == name], "sysroot", name, False)
attrs["extra_exec_compatible_with"] = _constraint_dict(
[tag for tag in mod.tags.extra_exec_compatible_with if tag.name == name],
name,
)
attrs["extra_target_compatible_with"] = _constraint_dict(
[tag for tag in mod.tags.extra_target_compatible_with if tag.name == name],
name,
)

llvm_toolchain(
**attrs
Expand Down Expand Up @@ -95,5 +117,19 @@ llvm = module_extension(
"path": attr.string(doc = "Absolute path to the sysroot."),
},
),
"extra_exec_compatible_with": tag_class(
attrs = {
"name": attr.string(doc = "Same name as the toolchain tag.", default = "llvm_toolchain"),
"targets": attr.string_list(doc = "Specific targets, if any; empty list means this applies to all."),
"constraints": attr.label_list(doc = "List of extra constraints to add to exec_compatible_with for the generated toolchains."),
},
),
"extra_target_compatible_with": tag_class(
attrs = {
"name": attr.string(doc = "Same name as the toolchain tag.", default = "llvm_toolchain"),
"targets": attr.string_list(doc = "Specific targets, if any; empty list means this applies to all."),
"constraints": attr.label_list(doc = "List of extra constraints to add to target_compatible_with for the generated toolchains."),
},
),
},
)
12 changes: 10 additions & 2 deletions toolchain/internal/configure.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ def _join(path1, path2):
def llvm_config_impl(rctx):
_check_os_arch_keys(rctx.attr.sysroot)
_check_os_arch_keys(rctx.attr.cxx_builtin_include_directories)
_check_os_arch_keys(rctx.attr.extra_exec_compatible_with)
_check_os_arch_keys(rctx.attr.extra_target_compatible_with)

os = _os(rctx)
if os == "windows":
Expand Down Expand Up @@ -166,6 +168,8 @@ def llvm_config_impl(rctx):
unfiltered_compile_flags_dict = rctx.attr.unfiltered_compile_flags,
llvm_version = llvm_version,
extra_compiler_files = rctx.attr.extra_compiler_files,
extra_exec_compatible_with = rctx.attr.extra_exec_compatible_with,
extra_target_compatible_with = rctx.attr.extra_target_compatible_with,
)
exec_dl_ext = "dylib" if os == "darwin" else "so"
cc_toolchains_str, toolchain_labels_str = _cc_toolchains_str(
Expand Down Expand Up @@ -380,11 +384,11 @@ toolchain(
exec_compatible_with = [
"@platforms//cpu:{exec_arch}",
"@platforms//os:{exec_os_bzl}",
],
] + {extra_exec_compatible_with_specific} + {extra_exec_compatible_with_all_targets},
target_compatible_with = [
"@platforms//cpu:{target_arch}",
"@platforms//os:{target_os_bzl}",
],
] + {extra_target_compatible_with_specific} + {extra_target_compatible_with_all_targets},
target_settings = {target_settings},
toolchain = ":cc-clang-{suffix}",
toolchain_type = "@bazel_tools//tools/cpp:toolchain_type",
Expand Down Expand Up @@ -543,6 +547,10 @@ cc_toolchain(
]),
extra_compiler_files = ("\"%s\"," % str(toolchain_info.extra_compiler_files)) if toolchain_info.extra_compiler_files else "",
major_llvm_version = major_llvm_version,
extra_exec_compatible_with_specific = toolchain_info.extra_exec_compatible_with.get(target_pair, []),
extra_target_compatible_with_specific = toolchain_info.extra_target_compatible_with.get(target_pair, []),
extra_exec_compatible_with_all_targets = toolchain_info.extra_exec_compatible_with.get("", []),
extra_target_compatible_with_all_targets = toolchain_info.extra_target_compatible_with.get("", []),
)

def _convenience_targets_str(rctx, use_absolute_paths, llvm_dist_rel_path, llvm_dist_label_prefix, exec_dl_ext):
Expand Down
8 changes: 8 additions & 0 deletions toolchain/internal/repo.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,14 @@ llvm_config_attrs.update({
default = False,
doc = "Use absolute paths in the toolchain. Avoids sandbox overhead.",
),
"extra_exec_compatible_with": attr.string_list_dict(
mandatory = False,
doc = "Extra constraints to be added to exec_compatible_with for each target",
),
"extra_target_compatible_with": attr.string_list_dict(
mandatory = False,
doc = "Extra constraints to be added to target_compatible_with for each target",
),
"_cc_toolchain_config_bzl": attr.label(
default = "//toolchain:cc_toolchain_config.bzl",
),
Expand Down

0 comments on commit 1cd9e36

Please sign in to comment.