Skip to content

Commit 1cd9e36

Browse files
Allow user provided platform constraints (#371)
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.
1 parent a1a5013 commit 1cd9e36

File tree

10 files changed

+227
-5
lines changed

10 files changed

+227
-5
lines changed

tests/BUILD.bazel

Lines changed: 62 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414

1515
load("@bazel_skylib//rules:build_test.bzl", "build_test")
1616
load("@rules_cc//cc:defs.bzl", "cc_binary", "cc_library", "cc_test")
17-
load(":transitions.bzl", "dwp_file")
17+
load(":transitions.bzl", "dwp_file", "transition_library_to_platform")
1818

1919
cc_library(
2020
name = "stdlib",
@@ -139,3 +139,64 @@ toolchain(
139139
toolchain = "@@rules_foreign_cc~~tools~ninja_1.11.1_mac//:ninja_tool",
140140
toolchain_type = "@rules_foreign_cc//toolchains:ninja_toolchain",
141141
)
142+
143+
# Testing extra_target_compatible_with
144+
constraint_setting(
145+
name = "cxx_standard",
146+
default_constraint_value = ":cxx17",
147+
visibility = ["//visibility:public"],
148+
)
149+
150+
constraint_value(
151+
name = "cxx20",
152+
constraint_setting = ":cxx_standard",
153+
visibility = ["//visibility:public"],
154+
)
155+
156+
constraint_value(
157+
name = "cxx17",
158+
constraint_setting = ":cxx_standard",
159+
visibility = ["//visibility:public"],
160+
)
161+
162+
platform(
163+
name = "cxx20_platform",
164+
constraint_values = [
165+
":cxx20",
166+
],
167+
parents = ["@platforms//host"],
168+
visibility = ["//visibility:public"],
169+
)
170+
171+
cc_library(
172+
name = "test_cxx_standard_lib",
173+
srcs = ["test_cxx_standard.cc"],
174+
)
175+
176+
cc_test(
177+
name = "test_cxx_standard_is_17",
178+
size = "small",
179+
srcs = ["test_cxx_standard_main.cc"],
180+
args = ["201703"],
181+
deps = [":test_cxx_standard_lib"],
182+
)
183+
184+
transition_library_to_platform(
185+
name = "test_cxx_standard_lib_transitioned",
186+
lib = ":test_cxx_standard_lib",
187+
platform = ":cxx20_platform",
188+
)
189+
190+
cc_test(
191+
name = "test_cxx_standard_is_20",
192+
size = "small",
193+
srcs = ["test_cxx_standard_main.cc"],
194+
args = ["202002"],
195+
196+
# Since some platforms require special toolchains (e.g. llvm 13.0.0) this
197+
# target won't build on those platforms unless we create a new toolchain per
198+
# platform with c++20. So instead just only run this test on platforms that
199+
# can use the default toolchain
200+
tags = ["manual"],
201+
deps = [":test_cxx_standard_lib_transitioned"],
202+
)

tests/MODULE.bazel

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,11 +65,28 @@ LLVM_VERSIONS = {
6565
llvm.toolchain(
6666
name = "llvm_toolchain",
6767
llvm_versions = LLVM_VERSIONS,
68+
cxx_standard = {"": "c++17"},
69+
)
70+
llvm.extra_target_compatible_with(
71+
name = "llvm_toolchain",
72+
constraints = ["@//:cxx17"],
6873
)
6974
use_repo(llvm, "llvm_toolchain", "llvm_toolchain_llvm")
70-
7175
register_toolchains("@llvm_toolchain//:all")
7276

77+
llvm.toolchain(
78+
name = "llvm_toolchain_cxx20",
79+
llvm_versions = LLVM_VERSIONS,
80+
cxx_standard = {"": "c++20"},
81+
)
82+
llvm.extra_target_compatible_with(
83+
name = "llvm_toolchain_cxx20",
84+
constraints = ["//:cxx20"],
85+
)
86+
use_repo(llvm, "llvm_toolchain_cxx20")
87+
register_toolchains("@llvm_toolchain_cxx20//:all")
88+
89+
7390
# Example toolchain with user provided URLs.
7491
# TODO(siddharthab): Add test.
7592
llvm.toolchain(

tests/WORKSPACE

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,19 @@ LLVM_VERSIONS = {
3737

3838
llvm_toolchain(
3939
name = "llvm_toolchain",
40+
cxx_standard = {"": "c++17"},
41+
extra_target_compatible_with = {
42+
"": ["@//:cxx17"],
43+
},
44+
llvm_versions = LLVM_VERSIONS,
45+
)
46+
47+
llvm_toolchain(
48+
name = "llvm_toolchain_cxx20",
49+
cxx_standard = {"": "c++20"},
50+
extra_target_compatible_with = {
51+
"": ["@//:cxx20"],
52+
},
4053
llvm_versions = LLVM_VERSIONS,
4154
)
4255

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

7689
llvm_register_toolchains()
7790

91+
load("@llvm_toolchain_cxx20//:toolchains.bzl", llvm_register_toolchains_cxx20 = "llvm_register_toolchains")
92+
93+
llvm_register_toolchains_cxx20()
94+
7895
## Toolchain example with absolute paths; tested in GitHub CI.
7996
llvm_toolchain(
8097
name = "llvm_toolchain_with_absolute_paths",
@@ -230,3 +247,16 @@ http_archive(
230247
"https://ftp.pcre.org/pub/pcre/pcre-8.43.tar.gz",
231248
],
232249
)
250+
251+
http_archive(
252+
name = "platforms",
253+
sha256 = "218efe8ee736d26a3572663b374a253c012b716d8af0c07e842e82f238a0a7ee",
254+
urls = [
255+
"https://mirror.bazel.build/github.com/bazelbuild/platforms/releases/download/0.0.10/platforms-0.0.10.tar.gz",
256+
"https://github.com/bazelbuild/platforms/releases/download/0.0.10/platforms-0.0.10.tar.gz",
257+
],
258+
)
259+
260+
load("@platforms//host:extension.bzl", "host_platform_repo")
261+
262+
host_platform_repo(name = "host_platform")

tests/scripts/run_tests.sh

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,18 @@ test_args=(
4646
"--linkopt=-Wl,-t"
4747
)
4848

49+
targets=(
50+
"//:all"
51+
)
52+
# :test_cxx_standard_is_20 builds with a version of the default toolchain, if
53+
# we're trying to build with a different toolchain then it's likely the default
54+
# toolchain won't work so :test_cxx_standard_is_20 won't build.
55+
if [[ -z ${toolchain_name} ]]; then
56+
targets+=("//:test_cxx_standard_is_20")
57+
fi
58+
4959
"${bazel}" ${TEST_MIGRATION:+"--strict"} --bazelrc=/dev/null test \
50-
"${common_test_args[@]}" "${test_args[@]}" //:all
60+
"${common_test_args[@]}" "${test_args[@]}" "${targets[@]}"
5161

5262
# Note that the following flags are currently known to cause issues in migration tests:
5363
# --incompatible_disallow_struct_provider_syntax # https://github.com/bazelbuild/bazel/issues/7347

tests/test_cxx_standard.cc

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
#include <iostream>
2+
#include <cstdlib>
3+
4+
int run_test(int argc, char** argv) {
5+
if (argc != 2) {
6+
std::cout << "Not enough arguments" << std::endl;
7+
return 1;
8+
}
9+
10+
long expected_version = std::atol(argv[1]);
11+
12+
if (expected_version == 0) {
13+
std::cout << "Invalid version argument, must be an integer" << std::endl;
14+
return 1;
15+
}
16+
17+
if (expected_version != __cplusplus) {
18+
std::cout << "Expected version to be " << argv[1] << " but got " << __cplusplus << std::endl;
19+
return 1;
20+
}
21+
return 0;
22+
}

tests/test_cxx_standard_main.cc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
int run_test(int argc, char** argv);
2+
3+
int main(int argc, char** argv) {
4+
return run_test(argc, argv);
5+
}

tests/transitions.bzl

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,3 +96,28 @@ dwp_file = rule(
9696
),
9797
},
9898
)
99+
100+
def _transition_library_to_platform_transition_impl(_, attr):
101+
return {"//command_line_option:platforms": str(attr.platform)}
102+
103+
_transition_library_to_platform_transition = transition(
104+
implementation = _transition_library_to_platform_transition_impl,
105+
inputs = [],
106+
outputs = ["//command_line_option:platforms"],
107+
)
108+
109+
def _transition_library_to_platform_impl(ctx):
110+
return [
111+
ctx.attr.lib[0][CcInfo],
112+
]
113+
114+
transition_library_to_platform = rule(
115+
implementation = _transition_library_to_platform_impl,
116+
attrs = {
117+
"lib": attr.label(mandatory = True, cfg = _transition_library_to_platform_transition),
118+
"platform": attr.label(mandatory = True),
119+
"_allowlist_function_transition": attr.label(
120+
default = "@bazel_tools//tools/allowlists/function_transition_allowlist",
121+
),
122+
},
123+
)

toolchain/extensions/llvm.bzl

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,20 @@ def _root_dict(roots, cls, name, strip_target):
3434

3535
return res
3636

37+
def _constraint_dict(tags, name):
38+
constraints = {}
39+
40+
# Gather all the additional constraints for each target
41+
for tag in tags:
42+
targets = list(tag.targets)
43+
if not targets:
44+
targets = [""]
45+
for target in targets:
46+
constraints_for_target = constraints.setdefault(target, [])
47+
constraints_for_target.extend([str(c) for c in tag.constraints])
48+
49+
return constraints
50+
3751
def _llvm_impl_(module_ctx):
3852
for mod in module_ctx.modules:
3953
if not mod.is_root:
@@ -49,6 +63,14 @@ def _llvm_impl_(module_ctx):
4963
}
5064
attrs["toolchain_roots"] = _root_dict([root for root in mod.tags.toolchain_root if root.name == name], "toolchain_root", name, True)
5165
attrs["sysroot"] = _root_dict([sysroot for sysroot in mod.tags.sysroot if sysroot.name == name], "sysroot", name, False)
66+
attrs["extra_exec_compatible_with"] = _constraint_dict(
67+
[tag for tag in mod.tags.extra_exec_compatible_with if tag.name == name],
68+
name,
69+
)
70+
attrs["extra_target_compatible_with"] = _constraint_dict(
71+
[tag for tag in mod.tags.extra_target_compatible_with if tag.name == name],
72+
name,
73+
)
5274

5375
llvm_toolchain(
5476
**attrs
@@ -95,5 +117,19 @@ llvm = module_extension(
95117
"path": attr.string(doc = "Absolute path to the sysroot."),
96118
},
97119
),
120+
"extra_exec_compatible_with": tag_class(
121+
attrs = {
122+
"name": attr.string(doc = "Same name as the toolchain tag.", default = "llvm_toolchain"),
123+
"targets": attr.string_list(doc = "Specific targets, if any; empty list means this applies to all."),
124+
"constraints": attr.label_list(doc = "List of extra constraints to add to exec_compatible_with for the generated toolchains."),
125+
},
126+
),
127+
"extra_target_compatible_with": tag_class(
128+
attrs = {
129+
"name": attr.string(doc = "Same name as the toolchain tag.", default = "llvm_toolchain"),
130+
"targets": attr.string_list(doc = "Specific targets, if any; empty list means this applies to all."),
131+
"constraints": attr.label_list(doc = "List of extra constraints to add to target_compatible_with for the generated toolchains."),
132+
},
133+
),
98134
},
99135
)

toolchain/internal/configure.bzl

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,8 @@ def _join(path1, path2):
5959
def llvm_config_impl(rctx):
6060
_check_os_arch_keys(rctx.attr.sysroot)
6161
_check_os_arch_keys(rctx.attr.cxx_builtin_include_directories)
62+
_check_os_arch_keys(rctx.attr.extra_exec_compatible_with)
63+
_check_os_arch_keys(rctx.attr.extra_target_compatible_with)
6264

6365
os = _os(rctx)
6466
if os == "windows":
@@ -166,6 +168,8 @@ def llvm_config_impl(rctx):
166168
unfiltered_compile_flags_dict = rctx.attr.unfiltered_compile_flags,
167169
llvm_version = llvm_version,
168170
extra_compiler_files = rctx.attr.extra_compiler_files,
171+
extra_exec_compatible_with = rctx.attr.extra_exec_compatible_with,
172+
extra_target_compatible_with = rctx.attr.extra_target_compatible_with,
169173
)
170174
exec_dl_ext = "dylib" if os == "darwin" else "so"
171175
cc_toolchains_str, toolchain_labels_str = _cc_toolchains_str(
@@ -380,11 +384,11 @@ toolchain(
380384
exec_compatible_with = [
381385
"@platforms//cpu:{exec_arch}",
382386
"@platforms//os:{exec_os_bzl}",
383-
],
387+
] + {extra_exec_compatible_with_specific} + {extra_exec_compatible_with_all_targets},
384388
target_compatible_with = [
385389
"@platforms//cpu:{target_arch}",
386390
"@platforms//os:{target_os_bzl}",
387-
],
391+
] + {extra_target_compatible_with_specific} + {extra_target_compatible_with_all_targets},
388392
target_settings = {target_settings},
389393
toolchain = ":cc-clang-{suffix}",
390394
toolchain_type = "@bazel_tools//tools/cpp:toolchain_type",
@@ -543,6 +547,10 @@ cc_toolchain(
543547
]),
544548
extra_compiler_files = ("\"%s\"," % str(toolchain_info.extra_compiler_files)) if toolchain_info.extra_compiler_files else "",
545549
major_llvm_version = major_llvm_version,
550+
extra_exec_compatible_with_specific = toolchain_info.extra_exec_compatible_with.get(target_pair, []),
551+
extra_target_compatible_with_specific = toolchain_info.extra_target_compatible_with.get(target_pair, []),
552+
extra_exec_compatible_with_all_targets = toolchain_info.extra_exec_compatible_with.get("", []),
553+
extra_target_compatible_with_all_targets = toolchain_info.extra_target_compatible_with.get("", []),
546554
)
547555

548556
def _convenience_targets_str(rctx, use_absolute_paths, llvm_dist_rel_path, llvm_dist_label_prefix, exec_dl_ext):

toolchain/internal/repo.bzl

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,14 @@ llvm_config_attrs.update({
271271
default = False,
272272
doc = "Use absolute paths in the toolchain. Avoids sandbox overhead.",
273273
),
274+
"extra_exec_compatible_with": attr.string_list_dict(
275+
mandatory = False,
276+
doc = "Extra constraints to be added to exec_compatible_with for each target",
277+
),
278+
"extra_target_compatible_with": attr.string_list_dict(
279+
mandatory = False,
280+
doc = "Extra constraints to be added to target_compatible_with for each target",
281+
),
274282
"_cc_toolchain_config_bzl": attr.label(
275283
default = "//toolchain:cc_toolchain_config.bzl",
276284
),

0 commit comments

Comments
 (0)