Skip to content

Commit

Permalink
configure: specify labels to sysroots in sysroot_label_map
Browse files Browse the repository at this point in the history
This change has the same motivation and caveats as the `toolchain_roots`
change (previous commit).

For `sysroot` I considered just flipping the map since I think forcing a
`1:1` mapping would be more acceptable but:
  - this is a potentially more confusing breaking change
  - this would force us to drop support for absolute paths to a sysroot
  • Loading branch information
rrbutani committed Oct 22, 2023
1 parent e80b2ad commit aebe2d3
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 9 deletions.
5 changes: 4 additions & 1 deletion tests/MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,10 @@ llvm.toolchain(
name = "llvm_toolchain_with_sysroot",
llvm_versions = LLVM_VERSIONS,
sysroot = {
"linux-x86_64": "@org_chromium_sysroot_linux_x64//:sysroot",
"linux-x86_64": "chromium_x64_sysroot",
},
sysroot_label_map = {
"@org_chromium_sysroot_linux_x64//:sysroot": "chromium_x64_sysroot",
},
# We can share the downloaded LLVM distribution with the first configuration.
toolchain_roots = {
Expand Down
5 changes: 4 additions & 1 deletion tests/WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,10 @@ llvm_toolchain(
name = "llvm_toolchain_with_sysroot",
llvm_versions = LLVM_VERSIONS,
sysroot = {
"linux-x86_64": "@org_chromium_sysroot_linux_x64//:sysroot",
"linux-x86_64": "chromium_x64_sysroot",
},
sysroot_label_map = {
"@org_chromium_sysroot_linux_x64//:sysroot": "chromium_x64_sysroot",
},
# We can share the downloaded LLVM distribution with the first configuration.
toolchain_roots = {
Expand Down
2 changes: 1 addition & 1 deletion toolchain/internal/common.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ BZLMOD_ENABLED = "@@" in str(Label("//:unused"))

def pkg_name_from_label(label):
if label.workspace_name:
return "@" + label.workspace_name + "//" + label.package
return ("@@" if BZLMOD_ENABLED else "@") + label.workspace_name + "//" + label.package
else:
return label.package

Expand Down
3 changes: 3 additions & 0 deletions toolchain/internal/configure.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -145,9 +145,12 @@ def llvm_register_toolchains():
tools_path_prefix = llvm_dist_path_prefix + "bin/"
symlinked_tools_str = ""

# Flip the sysroot label map (note: no error on duplicate "keys" for now)
_sysroot_label_map = { key: label for label, key in rctx.attr.sysroot_label_map.items() }
sysroot_paths_dict, sysroot_labels_dict = _sysroot_paths_dict(
rctx,
rctx.attr.sysroot,
_sysroot_label_map,
use_absolute_paths_sysroot,
)

Expand Down
15 changes: 12 additions & 3 deletions toolchain/internal/repo.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,18 @@ _compiler_configuration_attrs = {
"({}), ".format(_target_pairs) +
"used to indicate the set of files that form the sysroot for the compiler. " +
"If the value begins with exactly one forward slash '/', then the value is " +
"assumed to be a system path. Else, the value will be assumed to be a label " +
"containing the files and the sysroot path will be taken as the path to the " +
"package of this label."),
"assumed to be a system path. Else, the value will be assumed to be a key into " +
"`sysroot_label_map` that then points to a label containing the files. The " +
"sysroot path will be taken as the path to the package of this label."),
),
# NOTE: see the comments on `toolchain_roots_label_map`; this exists for
# similar reasons.
"sysroot_label_map": attr.label_keyed_string_dict(
mandatory = False,
doc = ("Inverted map from sysroot Label to string identifier.\n" +
"This attribute is to be used with `sysroot` in order to specify sysroots " +
"that come from a Bazel package. Keys can be any identifier that do not start " +
"with a forward slash '/`.\n"),
),
"cxx_builtin_include_directories": attr.string_list_dict(
mandatory = False,
Expand Down
16 changes: 13 additions & 3 deletions toolchain/internal/sysroot.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ def default_sysroot_path(rctx, os):
return ""

# Return the sysroot path and the label to the files, if sysroot is not a system path.
def _sysroot_path(sysroot_dict, os, arch):
def _sysroot_path(sysroot_dict, label_map, os, arch):
sysroot = sysroot_dict.get(_os_arch_pair(os, arch))
if not sysroot:
return (None, None)
Expand All @@ -50,18 +50,26 @@ def _sysroot_path(sysroot_dict, os, arch):
if sysroot[0] == "/" and (len(sysroot) == 1 or sysroot[1] != "/"):
return (sysroot, None)

label = Label(sysroot)
# Otherwise, consult the label map:
if sysroot not in label_map:
fail(("Value in `sysroot` {key} was interpreted as a `sysroot_label_map` " +
"key but it is not present in the map: {map}").format(
key = sysroot, map = label_map,
))
label = label_map[sysroot]

sysroot_path = _pkg_path_from_label(label)
return (sysroot_path, label)

# Return dictionaries for paths (relative or absolute) and labels if the
# sysroot needs to be included in the build sandbox.
def sysroot_paths_dict(rctx, sysroot_dict, use_absolute_paths):
def sysroot_paths_dict(rctx, sysroot_dict, sysroot_label_map, use_absolute_paths):
paths_dict = dict()
labels_dict = dict()
for (target_os, target_arch) in _supported_targets:
path, label = _sysroot_path(
sysroot_dict,
sysroot_label_map,
target_os,
target_arch,
)
Expand All @@ -71,6 +79,8 @@ def sysroot_paths_dict(rctx, sysroot_dict, use_absolute_paths):
if label and use_absolute_paths:
# Get a label for a regular file in the sysroot package.
# Exact target does not matter.
#
# NOTE: this path has to exist
label = Label(_pkg_name_from_label(label) + ":BUILD.bazel")
path = _canonical_dir_path(str(rctx.path(label).dirname))
label = None
Expand Down

0 comments on commit aebe2d3

Please sign in to comment.