Skip to content

Commit

Permalink
Provide a way for aspects to participate in swift_feature_allowlist
Browse files Browse the repository at this point in the history
… checking

This lets aspects enable/disable features without having to add every package where that aspect might be applied to targets to the `packages` of the allowlist.

PiperOrigin-RevId: 596900977
(cherry picked from commit 0206011)
Signed-off-by: Brentley Jones <github@brentleyjones.com>
  • Loading branch information
allevato authored and brentleyjones committed Oct 14, 2024
1 parent aceb372 commit e4de12b
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 10 deletions.
5 changes: 3 additions & 2 deletions doc/rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ the future, this rule is not recommended for other widespread use.
## swift_feature_allowlist

<pre>
swift_feature_allowlist(<a href="#swift_feature_allowlist-name">name</a>, <a href="#swift_feature_allowlist-managed_features">managed_features</a>, <a href="#swift_feature_allowlist-packages">packages</a>)
swift_feature_allowlist(<a href="#swift_feature_allowlist-name">name</a>, <a href="#swift_feature_allowlist-aspect_ids">aspect_ids</a>, <a href="#swift_feature_allowlist-managed_features">managed_features</a>, <a href="#swift_feature_allowlist-packages">packages</a>)
</pre>

Limits the ability to request or disable certain features to a set of packages
Expand All @@ -233,7 +233,8 @@ package.
| Name | Description | Type | Mandatory | Default |
| :------------- | :------------- | :------------- | :------------- | :------------- |
| <a id="swift_feature_allowlist-name"></a>name | A unique name for this target. | <a href="https://bazel.build/concepts/labels#target-names">Name</a> | required | |
| <a id="swift_feature_allowlist-managed_features"></a>managed_features | A list of feature strings that are permitted to be specified by the targets in the packages matched by the `packages` attribute. This list may include both feature names and/or negations (a name with a leading `-`); a regular feature name means that the targets in the matching packages may explicitly request that the feature be enabled, and a negated feature means that the target may explicitly request that the feature be disabled.<br><br>For example, `managed_features = ["foo", "-bar"]` means that targets in the allowlist's packages may request that feature `"foo"` be enabled and that feature `"bar"` be disabled. | List of strings | optional | `[]` |
| <a id="swift_feature_allowlist-aspect_ids"></a>aspect_ids | A list of strings representing the identifiers of aspects that are allowed to enable/disable the features in `managed_features`, even when the aspect is applied to packages not covered by the `packages` attribute.<br><br>Aspect identifiers are each expected to be of the form `<.bzl file label>%<aspect top-level name>` (i.e., the form one would use if invoking it from the command line, as described at https://bazel.build/extending/aspects#invoking_the_aspect_using_the_command_line). | List of strings | optional | `[]` |
| <a id="swift_feature_allowlist-managed_features"></a>managed_features | A list of feature strings that are permitted to be specified by the targets in the packages matched by the `packages` attribute *or* an aspect whose name matches the `aspect_ids` attribute (in any package). This list may include both feature names and/or negations (a name with a leading `-`); a regular feature name means that the matching targets/aspects may explicitly request that the feature be enabled, and a negated feature means that the target may explicitly request that the feature be disabled.<br><br>For example, `managed_features = ["foo", "-bar"]` means that targets in the allowlist's packages/aspects may request that feature `"foo"` be enabled and that feature `"bar"` be disabled. | List of strings | optional | `[]` |
| <a id="swift_feature_allowlist-packages"></a>packages | A list of strings representing packages (possibly recursive) whose targets are allowed to enable/disable the features in `managed_features`. Each package pattern is written in the syntax used by the `package_group` function:<br><br>* `//foo/bar`: Targets in the package `//foo/bar` but not in subpackages.<br><br>* `//foo/bar/...`: Targets in the package `//foo/bar` and any of its subpackages.<br><br>* A leading `-` excludes packages that would otherwise have been included by the patterns in the list.<br><br>Exclusions always take priority over inclusions; order in the list is irrelevant. | List of strings | required | |


Expand Down
25 changes: 25 additions & 0 deletions swift/internal/features.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,25 @@ def configure_features(
"parse_headers",
])

# HACK: This is the only way today to check whether the caller is inside an
# aspect. We have to do this because accessing `ctx.aspect_ids` halts the
# build if called from outside an aspect, but we can't use `hasattr` to
# check if it's safe because the attribute is always present on both rule
# and aspect contexts.
# TODO: b/319132714 - Replace this with a real API.
is_aspect = repr(ctx).startswith("<aspect context ")
if is_aspect and ctx.aspect_ids:
# It doesn't appear to be documented anywhere, but according to the
# Bazel team, the last element in this list is the currently running
# aspect.
aspect_id = ctx.aspect_ids[len(ctx.aspect_ids) - 1]
else:
aspect_id = None

if swift_toolchain.feature_allowlists:
_check_allowlists(
allowlists = swift_toolchain.feature_allowlists,
aspect_id = aspect_id,
label = ctx.label,
requested_features = requested_features,
unsupported_features = unsupported_features,
Expand Down Expand Up @@ -194,6 +210,7 @@ def is_feature_enabled(feature_configuration, feature_name):
def _check_allowlists(
*,
allowlists,
aspect_id,
label,
requested_features,
unsupported_features):
Expand All @@ -206,6 +223,9 @@ def _check_allowlists(
Args:
allowlists: A list of `SwiftFeatureAllowlistInfo` providers that will be
checked.
aspect_id: The identifier of the currently running aspect that has been
applied to the target that is creating the feature configuration, or
`None` if it is not being called from an aspect.
label: The label of the target being checked against the allowlist.
requested_features: The list of features to be enabled. This is
typically obtained using the `ctx.features` field in a rule
Expand All @@ -225,6 +245,11 @@ def _check_allowlists(
if feature_string not in allowlist.managed_features:
continue

# If the current aspect is permitted by the allowlist, we can allow
# the usage without looking at the package specs.
if aspect_id in allowlist.aspect_ids:
continue

if not label_matches_package_specs(
label = label,
package_specs = allowlist.package_specs,
Expand Down
9 changes: 7 additions & 2 deletions swift/providers.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ arbitrary libraries/binaries to depend on a plugin.

SwiftFeatureAllowlistInfo = provider(
doc = """\
Describes a set of features and the packages that are allowed to request or
disable them.
Describes a set of features and the packages and aspects that are allowed to
request or disable them.
This provider is an internal implementation detail of the rules; users should
not rely on it or assume that its structure is stable.
Expand All @@ -55,6 +55,11 @@ not rely on it or assume that its structure is stable.
"allowlist_label": """\
A string containing the label of the `swift_feature_allowlist` target that
created this provider.
""",
"aspect_ids": """\
A list of strings representing identifiers of aspects that are allowed to
request or disable a feature managed by the allowlist, even when the target the
aspect is being applied to does not match `package_specs`.
""",
"managed_features": """\
A list of strings representing feature names or their negations that packages in
Expand Down
27 changes: 21 additions & 6 deletions swift/swift_feature_allowlist.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ load(":providers.bzl", "SwiftFeatureAllowlistInfo")
def _swift_feature_allowlist_impl(ctx):
return [SwiftFeatureAllowlistInfo(
allowlist_label = str(ctx.label),
aspect_ids = ctx.attr.aspect_ids,
managed_features = ctx.attr.managed_features,
package_specs = parse_package_specs(
package_specs = ctx.attr.packages,
Expand All @@ -29,19 +30,33 @@ def _swift_feature_allowlist_impl(ctx):

swift_feature_allowlist = rule(
attrs = {
"aspect_ids": attr.string_list(
allow_empty = True,
doc = """\
A list of strings representing the identifiers of aspects that are allowed to
enable/disable the features in `managed_features`, even when the aspect is
applied to packages not covered by the `packages` attribute.
Aspect identifiers are each expected to be of the form
`<.bzl file label>%<aspect top-level name>` (i.e., the form one would use if
invoking it from the command line, as described at
https://bazel.build/extending/aspects#invoking_the_aspect_using_the_command_line).
""",
),
"managed_features": attr.string_list(
allow_empty = True,
doc = """\
A list of feature strings that are permitted to be specified by the targets in
the packages matched by the `packages` attribute. This list may include both
the packages matched by the `packages` attribute *or* an aspect whose name
matches the `aspect_ids` attribute (in any package). This list may include both
feature names and/or negations (a name with a leading `-`); a regular feature
name means that the targets in the matching packages may explicitly request that
the feature be enabled, and a negated feature means that the target may
explicitly request that the feature be disabled.
name means that the matching targets/aspects may explicitly request that the
feature be enabled, and a negated feature means that the target may explicitly
request that the feature be disabled.
For example, `managed_features = ["foo", "-bar"]` means that targets in the
allowlist's packages may request that feature `"foo"` be enabled and that
feature `"bar"` be disabled.
allowlist's packages/aspects may request that feature `"foo"` be enabled and
that feature `"bar"` be disabled.
""",
mandatory = False,
),
Expand Down

0 comments on commit e4de12b

Please sign in to comment.