Skip to content

Commit

Permalink
Fixes in preparation for symbolic macros (#137)
Browse files Browse the repository at this point in the history
* Recognize symbolic macros as supported kinds
* Fix handling of main repo labels in `select`s
* Fix handling of `None` attribute values
* Ensure that all rewritten labels are prefixed with the macro name
including subdirs
* Enable dynamic_deps test on macOS with Bazel 8 and later
* Apply buildifier
  • Loading branch information
fmeum authored Dec 6, 2024
1 parent 5257b0e commit 65b3c02
Show file tree
Hide file tree
Showing 7 changed files with 44 additions and 33 deletions.
1 change: 1 addition & 0 deletions examples/MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ local_path_override(
path = "..",
)

bazel_dep(name = "bazel_features", version = "1.21.0")
bazel_dep(name = "bazel_skylib", version = "1.7.1")
bazel_dep(name = "contrib_rules_jvm", version = "0.27.0")
bazel_dep(name = "platforms", version = "0.0.10")
Expand Down
1 change: 1 addition & 0 deletions examples/builder_reuse/builder.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ second_builder = _first_builder.clone()

# Demonstrate that the builder does not capture mutable values by reference.
_first_builder_copt_default[0] = "-DC_VALUE=\"unexpected\""

# Demonstrate that clone() does not share mutable state with the original builder.
second_builder.set(
"cxxopt",
Expand Down
13 changes: 6 additions & 7 deletions examples/dynamic_deps/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
load("@bazel_features//:features.bzl", "bazel_features")
load("//dynamic_deps/rules:runfiles_dir.bzl", "runfiles_dir")
load(":cc_define_binary.bzl", "cc_define_binary")

Expand Down Expand Up @@ -29,18 +30,16 @@ sh_test(
"//conditions:default": ["$(rlocationpath :bin_runfiles)/bin"],
}),
data = [":bin_runfiles"],
deps = ["@bazel_tools//tools/bash/runfiles"],
target_compatible_with = select({
# TODO: Get this test to pass on Windows.
"@platforms//os:windows": ["@platforms//:incompatible"],
# TODO: Reenable on macOS after:
# 1. Bazel is released with the fix in https://github.com/bazelbuild/bazel/pull/23089.
# 2. Enabling --incompatible_macos_set_install_name.
# 3. Adding a dep on apple_support, as the default Unix toolchain only supports this flag
# after https://github.com/bazelbuild/bazel/pull/23090.
"@platforms//os:macos": ["@platforms//:incompatible"],
# Requires https://github.com/bazelbuild/bazel/pull/23089 and the flip
# of --incompatible_macos_set_install_name on macOS. Use a feature
# introduced in Bazel 8 as a proxy for the fix.
"@platforms//os:macos": [] if bazel_features.rules.rule_extension_apis_available else ["@platforms//:incompatible"],
"//conditions:default": [],
}),
deps = ["@bazel_tools//tools/bash/runfiles"],
)

sh_test(
Expand Down
6 changes: 3 additions & 3 deletions examples/dynamic_deps/lib/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@ cc_library(
name = "lib",
srcs = ["lib.cpp"],
hdrs = ["lib.h"],
visibility = ["//dynamic_deps:__pkg__"],
tags = ["manual"],
visibility = ["//dynamic_deps:__pkg__"],
)

cc_shared_library(
name = "shared",
deps = [":lib"],
visibility = ["//dynamic_deps:__pkg__"],
tags = ["manual"],
visibility = ["//dynamic_deps:__pkg__"],
deps = [":lib"],
)
8 changes: 7 additions & 1 deletion with_cfg/private/select.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,14 @@ def consume_single_value(r, pos):
string, after_string = _consume_string(r, pos)

# On Bazel 6, stringification of Labels may not include the leading `@@`.
# Labels in the main repository are not prefixed with `@` at all since
# their `repr` implementation doesn't use the unambiguous form as of
# Bazel 8.
if not string.startswith("@@"):
string = "@" + string
if string.startswith("@"):
string = "@" + string
else:
string = "@@" + string

# Skip over `)`.
return Label(string), after_string + 1
Expand Down
10 changes: 6 additions & 4 deletions with_cfg/private/with_cfg.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -130,10 +130,12 @@ def with_cfg(
)
return make_builder(rule_info)

def get_rule_name(rule_or_macro):
s = str(rule_or_macro)
def get_rule_name(kind):
s = str(kind)
if s.startswith("<rule "):
return s.removeprefix("<rule ").removesuffix(">")
elif s.startswith("<macro "):
return s.removeprefix("<macro ").removesuffix(">")
elif s.startswith("<built-in rule "):
return s.removeprefix("<built-in rule ").removesuffix(">")
elif s.startswith("<function "):
Expand All @@ -150,8 +152,8 @@ def is_test(rule_name):
# shouldn't become configurable.
return rule_name.endswith("_test") or rule_name == "go_test_macro"

def _is_native(rule_or_macro):
return str(rule_or_macro).startswith("<built-in rule ")
def _is_native(kind):
return str(kind).startswith("<built-in rule ")

def get_implicit_targets(rule_name):
return IMPLICIT_TARGETS.get(rule_name, [])
38 changes: 20 additions & 18 deletions with_cfg/private/wrapper.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ def _wrapper(
tags = ["manual"],
visibility = ["//visibility:private"],
),
basename = basename,
name_prefix = name,
)
rule_info.kind(
name = original_name,
Expand Down Expand Up @@ -192,7 +192,7 @@ def _wrapper(
**(alias_attrs | common_attrs)
)

def _process_attrs_for_reset(*, attrs, attrs_to_reset, reset_target, basename):
def _process_attrs_for_reset(*, attrs, attrs_to_reset, reset_target, name_prefix):
if not attrs_to_reset:
return attrs

Expand All @@ -209,7 +209,7 @@ def _process_attrs_for_reset(*, attrs, attrs_to_reset, reset_target, basename):
dep = dep,
label_map = label_map,
reset_target = reset_target,
base_target_name = basename + "__" + attr,
name_prefix = name_prefix + "__" + attr,
mutable_num_calls = mutable_num_calls,
)
first_pass_attrs[attr] = map_attr(attr_func, attrs[attr])
Expand All @@ -223,47 +223,49 @@ def _process_attrs_for_reset(*, attrs, attrs_to_reset, reset_target, basename):

return second_pass_attrs

def _replace_dep_attr(*, dep, label_map, reset_target, base_target_name, mutable_num_calls):
def _replace_dep_attr(*, dep, label_map, reset_target, name_prefix, mutable_num_calls):
if dep == None:
return dep
if is_list(dep):
# attr.label_list
return [
_replace_single_dep(
label_string = label_string,
label_map = label_map,
reset_target = reset_target,
base_target_name = base_target_name,
name_prefix = name_prefix,
mutable_num_calls = mutable_num_calls,
)
for label_string in dep
]
elif is_dict(dep):
if is_dict(dep):
# attr.label_keyed_string_dict (only the keys represent deps)
return {
_replace_single_dep(
label_string = label_string,
label_map = label_map,
reset_target = reset_target,
base_target_name = base_target_name,
name_prefix = name_prefix,
mutable_num_calls = mutable_num_calls,
): v
for label_string, v in dep.items()
}
else:
# attr.label
return _replace_single_dep(
label_string = dep,
label_map = label_map,
reset_target = reset_target,
base_target_name = base_target_name,
mutable_num_calls = mutable_num_calls,
)

# attr.label
return _replace_single_dep(
label_string = dep,
label_map = label_map,
reset_target = reset_target,
name_prefix = name_prefix,
mutable_num_calls = mutable_num_calls,
)

def _replace_single_dep(
*,
label_string,
label_map,
reset_target,
base_target_name,
name_prefix,
mutable_num_calls):
use_label = is_label(label_string)
if not use_label and not is_string(label_string):
Expand All @@ -273,7 +275,7 @@ def _replace_single_dep(
target_label_string = label_map[label]
else:
# Multiple targets can be referenced in a single logical dep if that dep is a select.
target_name = base_target_name + "_" + str(mutable_num_calls[0])
target_name = name_prefix + "_" + str(mutable_num_calls[0])
reset_target(
name = target_name,
exports = label,
Expand Down

0 comments on commit 65b3c02

Please sign in to comment.