Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixups to use of Labels to support bzlmod #872

Merged
merged 5 commits into from
Apr 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion foreign_cc/built_tools/make_build.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ make_tool = rule(
output_to_genfiles = True,
implementation = _make_tool_impl,
toolchains = [
str(Label("//foreign_cc/private/framework:shell_toolchain")),
"@rules_foreign_cc//foreign_cc/private/framework:shell_toolchain",
"@bazel_tools//tools/cpp:toolchain_type",
],
)
Expand Down
2 changes: 1 addition & 1 deletion foreign_cc/built_tools/ninja_build.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ ninja_tool = rule(
output_to_genfiles = True,
implementation = _ninja_tool_impl,
toolchains = [
str(Label("//foreign_cc/private/framework:shell_toolchain")),
"@rules_foreign_cc//foreign_cc/private/framework:shell_toolchain",
"@bazel_tools//tools/cpp:toolchain_type",
],
)
31 changes: 7 additions & 24 deletions foreign_cc/private/framework/toolchain.bzl
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
"""A module containing the implementation of the foreign_cc framework's toolchains"""

load("//foreign_cc/private/framework/toolchains:commands.bzl", "PLATFORM_COMMANDS")
load("//foreign_cc/private/framework/toolchains:mappings.bzl", "TOOLCHAIN_MAPPINGS")

_BUILD_FILE_CONTENT = """\
Expand All @@ -17,7 +16,7 @@ foreign_cc_framework_toolchain(
toolchain(
name = "toolchain",
toolchain_type = "@rules_foreign_cc//foreign_cc/private/framework:shell_toolchain",
toolchain = "//:commands",
toolchain = ":commands",
exec_compatible_with = {exec_compat},
target_compatible_with = {target_compat},
)
Expand All @@ -26,11 +25,7 @@ toolchain(
_DEFS_BZL_CONTENT = """\
load(
"{commands_src}",
{symbols}
)

commands = struct(
{commands}
"commands"
)

def _foreign_cc_framework_toolchain_impl(ctx):
Expand All @@ -51,16 +46,8 @@ def _framework_toolchain_repository_impl(repository_ctx):
repository_ctx (repository_ctx): The rule's context object
"""

# Ensure we always have an absolute label. This may not be the case
# when building within the `@rules_foreign_cc` workspace.
absolute_label = str(repository_ctx.attr.commands_src)
if not absolute_label.startswith("@"):
absolute_label = "@rules_foreign_cc" + absolute_label

repository_ctx.file("defs.bzl", _DEFS_BZL_CONTENT.format(
commands_src = absolute_label,
symbols = "\n ".join(["\"{}\",".format(symbol) for symbol in PLATFORM_COMMANDS.keys()]),
commands = "\n ".join(["{cmd} = {cmd},".format(cmd = symbol) for symbol in PLATFORM_COMMANDS.keys()]),
commands_src = repository_ctx.attr.commands_src,
))

repository_ctx.file("BUILD.bazel", _BUILD_FILE_CONTENT.format(
Expand All @@ -72,9 +59,8 @@ framework_toolchain_repository = repository_rule(
doc = "A repository rule which defines a `@rules_foreign_cc//foreign_cc/private/framework:shell_toolchain` toolchain.",
implementation = _framework_toolchain_repository_impl,
attrs = {
"commands_src": attr.label(
doc = "The label of a `.bzl` source which defines toolchain commands",
allow_files = [".bzl"],
"commands_src": attr.string(
doc = "The string of a `.bzl` source which defines toolchain commands",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why remove the .bzl restriction?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because it becomes a string here - bzlmod remaps the names of the repos and so we can't evaluate the Label at this point as it gets used in the context of another repo so unfortunately we need to pass it through as a string here. It's a bit brittle but as its all internal it works but isn't great.
I guess the real solution is to move the .bzl into the generated repo itself so we don't need to resolve a label to access it?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oooh, I didn't notice that the type changed. I'm more nervous about making larger changes than needing to restrict the values here. This seems fine.

),
"exec_compatible_with": attr.string_list(
doc = "A list of constraint_values that must be present in the execution platform for this target.",
Expand All @@ -95,17 +81,14 @@ def register_framework_toolchains(register_toolchains = True):
toolchains = []

for item in TOOLCHAIN_MAPPINGS:
# Generate a toolchain name without the `.bzl` suffix
toolchain_name = "rules_foreign_cc_framework_toolchain_" + item.file.name[:-len(".bzl")]

framework_toolchain_repository(
name = toolchain_name,
name = item.repo_name,
commands_src = item.file,
exec_compatible_with = item.exec_compatible_with,
target_compatible_with = item.target_compatible_with,
)

toolchains.append("@{}//:toolchain".format(toolchain_name))
toolchains.append("@{}//:toolchain".format(item.repo_name))

if register_toolchains:
native.register_toolchains(*toolchains)
2 changes: 1 addition & 1 deletion foreign_cc/private/framework/toolchains/access.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def create_context(ctx):
- prelude (dict): A cache for rendered functions
"""
return struct(
shell = ctx.toolchains[str(Label("//foreign_cc/private/framework:shell_toolchain"))].commands,
shell = ctx.toolchains[Label("//foreign_cc/private/framework:shell_toolchain")].commands,
prelude = {},
)

Expand Down
35 changes: 35 additions & 0 deletions foreign_cc/private/framework/toolchains/freebsd_commands.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -261,3 +261,38 @@ if [[ -L "{file}" ]]; then
rm "{file}" && cp -a "${{target}}" "{file}"
fi
""".format(file = file)

commands = struct(
assert_script_errors = assert_script_errors,
cat = cat,
children_to_path = children_to_path,
cleanup_function = cleanup_function,
copy_dir_contents_to_dir = copy_dir_contents_to_dir,
define_absolute_paths = define_absolute_paths,
define_function = define_function,
define_sandbox_paths = define_sandbox_paths,
disable_tracing = disable_tracing,
echo = echo,
enable_tracing = enable_tracing,
env = env,
export_var = export_var,
if_else = if_else,
increment_pkg_config_path = increment_pkg_config_path,
local_var = local_var,
mkdirs = mkdirs,
path = path,
pwd = pwd,
redirect_out_err = redirect_out_err,
replace_absolute_paths = replace_absolute_paths,
replace_in_files = replace_in_files,
replace_sandbox_paths = replace_sandbox_paths,
replace_symlink = replace_symlink,
rm_rf = rm_rf,
script_extension = script_extension,
script_prelude = script_prelude,
shebang = shebang,
symlink_contents_to_dir = symlink_contents_to_dir,
symlink_to_dir = symlink_to_dir,
touch = touch,
use_var = use_var,
)
35 changes: 35 additions & 0 deletions foreign_cc/private/framework/toolchains/linux_commands.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -243,3 +243,38 @@ if [[ -L "{file}" ]]; then
rm "{file}" && cp -a "${{target}}" "{file}"
fi
""".format(file = file)

commands = struct(
assert_script_errors = assert_script_errors,
cat = cat,
children_to_path = children_to_path,
cleanup_function = cleanup_function,
copy_dir_contents_to_dir = copy_dir_contents_to_dir,
define_absolute_paths = define_absolute_paths,
define_function = define_function,
define_sandbox_paths = define_sandbox_paths,
disable_tracing = disable_tracing,
echo = echo,
enable_tracing = enable_tracing,
env = env,
export_var = export_var,
if_else = if_else,
increment_pkg_config_path = increment_pkg_config_path,
local_var = local_var,
mkdirs = mkdirs,
path = path,
pwd = pwd,
redirect_out_err = redirect_out_err,
replace_absolute_paths = replace_absolute_paths,
replace_in_files = replace_in_files,
replace_sandbox_paths = replace_sandbox_paths,
replace_symlink = replace_symlink,
rm_rf = rm_rf,
script_extension = script_extension,
script_prelude = script_prelude,
shebang = shebang,
symlink_contents_to_dir = symlink_contents_to_dir,
symlink_to_dir = symlink_to_dir,
touch = touch,
use_var = use_var,
)
35 changes: 35 additions & 0 deletions foreign_cc/private/framework/toolchains/macos_commands.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -256,3 +256,38 @@ if [[ -L "{file}" ]]; then
rm "{file}" && cp -a "${{target}}" "{file}"
fi
""".format(file = file)

commands = struct(
assert_script_errors = assert_script_errors,
cat = cat,
children_to_path = children_to_path,
cleanup_function = cleanup_function,
copy_dir_contents_to_dir = copy_dir_contents_to_dir,
define_absolute_paths = define_absolute_paths,
define_function = define_function,
define_sandbox_paths = define_sandbox_paths,
disable_tracing = disable_tracing,
echo = echo,
enable_tracing = enable_tracing,
env = env,
export_var = export_var,
if_else = if_else,
increment_pkg_config_path = increment_pkg_config_path,
local_var = local_var,
mkdirs = mkdirs,
path = path,
pwd = pwd,
redirect_out_err = redirect_out_err,
replace_absolute_paths = replace_absolute_paths,
replace_in_files = replace_in_files,
replace_sandbox_paths = replace_sandbox_paths,
replace_symlink = replace_symlink,
rm_rf = rm_rf,
script_extension = script_extension,
script_prelude = script_prelude,
shebang = shebang,
symlink_contents_to_dir = symlink_contents_to_dir,
symlink_to_dir = symlink_to_dir,
touch = touch,
use_var = use_var,
)
18 changes: 12 additions & 6 deletions foreign_cc/private/framework/toolchains/mappings.bzl
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
"""A module defining default toolchain info for the foreign_cc framework"""

def _toolchain_mapping(file, exec_compatible_with = [], target_compatible_with = []):
def _toolchain_mapping(file, repo_name, exec_compatible_with = [], target_compatible_with = []):
"""Mapping of toolchain definition files to platform constraints

Args:
file (Label): Toolchain definition file
file (str): Toolchain definition file
repo_name (str): name of repository to create for this toolchain
exec_compatible_with (list): A list of compatible execution platform constraints.
target_compatible_with (list): Compatible target platform constraints

Expand All @@ -13,34 +14,39 @@ def _toolchain_mapping(file, exec_compatible_with = [], target_compatible_with =
"""
return struct(
file = file,
repo_name = repo_name,
exec_compatible_with = exec_compatible_with,
target_compatible_with = target_compatible_with,
)

# This list is the single entrypoint for all foreign_cc framework toolchains.
TOOLCHAIN_MAPPINGS = [
_toolchain_mapping(
repo_name = "rules_foreign_cc_framework_toolchain_linux",
exec_compatible_with = [
"@platforms//os:linux",
],
file = Label("@rules_foreign_cc//foreign_cc/private/framework/toolchains:linux_commands.bzl"),
file = "@rules_foreign_cc//foreign_cc/private/framework/toolchains:linux_commands.bzl",
),
_toolchain_mapping(
repo_name = "rules_foreign_cc_framework_toolchain_freebsd",
exec_compatible_with = [
"@platforms//os:freebsd",
],
file = Label("@rules_foreign_cc//foreign_cc/private/framework/toolchains:freebsd_commands.bzl"),
file = "@rules_foreign_cc//foreign_cc/private/framework/toolchains:freebsd_commands.bzl",
),
_toolchain_mapping(
repo_name = "rules_foreign_cc_framework_toolchain_windows",
exec_compatible_with = [
"@platforms//os:windows",
],
file = Label("@rules_foreign_cc//foreign_cc/private/framework/toolchains:windows_commands.bzl"),
file = "@rules_foreign_cc//foreign_cc/private/framework/toolchains:windows_commands.bzl",
),
_toolchain_mapping(
repo_name = "rules_foreign_cc_framework_toolchain_macos",
exec_compatible_with = [
"@platforms//os:macos",
],
file = Label("@rules_foreign_cc//foreign_cc/private/framework/toolchains:macos_commands.bzl"),
file = "@rules_foreign_cc//foreign_cc/private/framework/toolchains:macos_commands.bzl",
),
]
35 changes: 35 additions & 0 deletions foreign_cc/private/framework/toolchains/windows_commands.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -257,3 +257,38 @@ if [[ -L "{file}" ]]; then
rm "{file}" && cp -a "${{target}}" "{file}"
fi
""".format(file = file)

commands = struct(
assert_script_errors = assert_script_errors,
cat = cat,
children_to_path = children_to_path,
cleanup_function = cleanup_function,
copy_dir_contents_to_dir = copy_dir_contents_to_dir,
define_absolute_paths = define_absolute_paths,
define_function = define_function,
define_sandbox_paths = define_sandbox_paths,
disable_tracing = disable_tracing,
echo = echo,
enable_tracing = enable_tracing,
env = env,
export_var = export_var,
if_else = if_else,
increment_pkg_config_path = increment_pkg_config_path,
local_var = local_var,
mkdirs = mkdirs,
path = path,
pwd = pwd,
redirect_out_err = redirect_out_err,
replace_absolute_paths = replace_absolute_paths,
replace_in_files = replace_in_files,
replace_sandbox_paths = replace_sandbox_paths,
replace_symlink = replace_symlink,
rm_rf = rm_rf,
script_extension = script_extension,
script_prelude = script_prelude,
shebang = shebang,
symlink_contents_to_dir = symlink_contents_to_dir,
symlink_to_dir = symlink_to_dir,
touch = touch,
use_var = use_var,
)
8 changes: 4 additions & 4 deletions foreign_cc/repositories.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,10 @@ def rules_foreign_cc_dependencies(
native.register_toolchains(*native_tools_toolchains)

native.register_toolchains(
str(Label("//toolchains:preinstalled_autoconf_toolchain")),
str(Label("//toolchains:preinstalled_automake_toolchain")),
str(Label("//toolchains:preinstalled_m4_toolchain")),
str(Label("//toolchains:preinstalled_pkgconfig_toolchain")),
"@rules_foreign_cc//toolchains:preinstalled_autoconf_toolchain",
"@rules_foreign_cc//toolchains:preinstalled_automake_toolchain",
"@rules_foreign_cc//toolchains:preinstalled_m4_toolchain",
"@rules_foreign_cc//toolchains:preinstalled_pkgconfig_toolchain",
)

if register_default_tools:
Expand Down
2 changes: 1 addition & 1 deletion toolchains/built_toolchains.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,7 @@ def _make_toolchain(version, register_toolchains):
http_archive,
name = "gnumake_src",
build_file_content = _ALL_CONTENT,
patches = [str(Label("//toolchains:make-reproducible-bootstrap.patch"))],
patches = [Label("//toolchains:make-reproducible-bootstrap.patch")],
sha256 = "e05fdde47c5f7ca45cb697e973894ff4f5d79e13b750ed57d7b66d8defc78e19",
strip_prefix = "make-4.3",
urls = [
Expand Down
16 changes: 8 additions & 8 deletions toolchains/native_tools/tool_access.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ def access_tool(toolchain_type_, ctx):
"""A helper macro for getting the path to a build tool's executable

Args:
toolchain_type_ (str): The name of the toolchain type
toolchain_type_ (Label): The name of the toolchain type
ctx (ctx): The rule's context object

Returns:
Expand All @@ -18,25 +18,25 @@ def access_tool(toolchain_type_, ctx):
fail("No toolchain found for " + toolchain_type_)

def get_autoconf_data(ctx):
return _access_and_expect_label_copied(str(Label("//toolchains:autoconf_toolchain")), ctx)
return _access_and_expect_label_copied(Label("//toolchains:autoconf_toolchain"), ctx)

def get_automake_data(ctx):
return _access_and_expect_label_copied(str(Label("//toolchains:automake_toolchain")), ctx)
return _access_and_expect_label_copied(Label("//toolchains:automake_toolchain"), ctx)

def get_cmake_data(ctx):
return _access_and_expect_label_copied(str(Label("//toolchains:cmake_toolchain")), ctx)
return _access_and_expect_label_copied(Label("//toolchains:cmake_toolchain"), ctx)

def get_m4_data(ctx):
return _access_and_expect_label_copied(str(Label("//toolchains:m4_toolchain")), ctx)
return _access_and_expect_label_copied(Label("//toolchains:m4_toolchain"), ctx)

def get_make_data(ctx):
return _access_and_expect_label_copied(str(Label("//toolchains:make_toolchain")), ctx)
return _access_and_expect_label_copied(Label("//toolchains:make_toolchain"), ctx)

def get_ninja_data(ctx):
return _access_and_expect_label_copied(str(Label("//toolchains:ninja_toolchain")), ctx)
return _access_and_expect_label_copied(Label("//toolchains:ninja_toolchain"), ctx)

def get_pkgconfig_data(ctx):
return _access_and_expect_label_copied(str(Label("//toolchains:pkgconfig_toolchain")), ctx)
return _access_and_expect_label_copied(Label("//toolchains:pkgconfig_toolchain"), ctx)

def _access_and_expect_label_copied(toolchain_type_, ctx):
tool_data = access_tool(toolchain_type_, ctx)
Expand Down
6 changes: 3 additions & 3 deletions toolchains/toolchains.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ prebuilt_toolchains = _prebuilt_toolchains
def preinstalled_toolchains():
"""Register toolchains for various build tools expected to be installed on the exec host"""
native.register_toolchains(
str(Label("//toolchains:preinstalled_cmake_toolchain")),
str(Label("//toolchains:preinstalled_make_toolchain")),
str(Label("//toolchains:preinstalled_ninja_toolchain")),
"@rules_foreign_cc//toolchains:preinstalled_cmake_toolchain",
"@rules_foreign_cc//toolchains:preinstalled_make_toolchain",
"@rules_foreign_cc//toolchains:preinstalled_ninja_toolchain",
)

def _current_toolchain_impl(ctx):
Expand Down