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

Provide a way for aspects to participate in swift_feature_allowlist checking #1377

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
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