incompatible_allow_tags_propagation and aspects cannot coexist
Description of the bug:
I'm trying to update our codebase so that we can enable --incompatible_allow_tags_propagation after the Bazel 6 -> 7 upgrade and I'm finding some surprising behavior.
We have many C++ test targets that set tags no-remote-exec and no-sandbox. These are the right settings for test execution.
We also have aspects that run over the build and try to extract metadata (like for code linting purposes). What I'm finding is that the actions spawned by the aspects for these targets inherit those tags and end up running locally / without the sandbox, which then makes them misbehave.
I'm also finding that it is impossible to undo the propagated tags. The Starlark rule uses ctx.actions.run_shell, and I've tried to override execution_requirements with an empty dictionary (no luck), a dictionary that has an irrelevant key (no luck either), and tried to "reset" the no-* propagated tags to 0 or False values. However, Bazel just checks for the presence of tags, not their values, so it's impossible to say "set no-sandbox to 0" and have Bazel re-enable the sandbox.
This feels like spooky-action-at-a-distance and not what I'd expect. Is this feature working as intended? What's the way out here?
Which category does this issue belong to?
Core
What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.
No response
Which operating system are you running Bazel on?
Linux
What is the output of bazel info release?
7.6.1
If bazel info release returns development version or (@non-git), tell us how you built Bazel.
No response
What's the output of git remote get-url origin; git rev-parse HEAD ?
If this is a regression, please try to identify the Bazel commit where the bug was introduced with bazelisk --bisect.
No response
Have you found anything relevant by searching the web?
No response
Any other information, logs, or outputs that you want to share?
No response
I've been able to workaround my problem by doing this:
def translate_tags(tags=[], exec_properties={}):
"""Translates Bazel-internal tags to exec_properties.
This function extracts all tags from the tags list that have an internal meaning to Bazel
and explicitly applies them to exec_properties, *removing* them from tags. Then returns
the modified tags and the new set of exec_properties.
This function is meant to be called by macros like this:
def the_macro(name, tags = [], exec_properties = {}):
tags, exec_properties = translate_tags(tags, exec_properties)
# Use the results as usual to instantiate actions.
The above is necessary to _prevent_ tag propagation to aspects when the "new" tag
propagation behavior in Bazel 7 (--incompatible_allow_tags_propagation) is enabled: we
have macros that expect to receive tags for the actions they define (e.g. macros that
expand tests that need to run locally sometimes) and those tags make no sense when run
over aspects. See https://github.com/bazelbuild/bazel/issues/26513.
Args:
tags: Original list of tags passed to rule or macro.
exec_properties: Original list of exec_properties passed to rule or macro.
Returns:
A set of tags and exec_properties to replace the original ones passed to the rule or macro.
"""
new_tags = []
new_exec_properties = {}
for tag in tags:
# List of Bazel-internal prefixes taken from https://github.com/bazelbuild/bazel/issues/8830.
if (tag.startswith("block-") or
tag.startswith("disable-") or
tag.startswith("no-") or
tag.startswith("requires-") or
tag.startswith("supports-") or
tag == "local"):
new_exec_properties[tag] = "1"
elif tag.startswith("cpu:") or tag.startswith("memory:"):
prefix, value = tag.split(":", 1)
new_exec_properties[prefix] = value
else:
new_tags.append(tag)
return (new_tags, exec_properties | new_exec_properties)
... and then patching (some of) my macros to "properly" filter the tags out so that they aren't picked up by the aspect. But this seems quite fragile and somehow unexpected behavior.
I got a tip to use test.no-remote-exec instead for our tests, which is a better tag anyway for what I want to achieve... but still, this propagation of tags into aspects feels weird. After all, aspects are intended to be their own entities, right?
Yes, I think that flipping --incompatible_allow_tags_propagation was a mistake (sorry!). But it was also a very popular flag for other reasons.
tags just doesn't look like the correct way to influence actions anymore. We should probably update the documentation to recommend exec_properties instead.
We could also consider rolling back the incompatible change as a new incompatible change or limiting its impact to aspects and non-test actions of test rules.
Cc @tjgq
We are running into this as well. We have tests tagged with no-remote-exec because they are incompatible with remote execution, and aspects that register their own validation actions, and we trigger remote builds from different platforms (host!=exec), but that combination doesn't work.
In particular actions with no-remote-exec need exec_compatible_with = HOST_CONSTRAINTS to ensure they run locally with binaries of the correct os/arch. For test and build actions, exec_compatible_with on the target works. But aspects inherit the no-remote-exec and not the exec_compatible_with, so they run locally with binaries for the wrong platform.
In order to work with --incompatible_allow_tags_propagation we have to, in the aspect, define an exec group with exec_compatible_with = HOST_CONSTRAINTS, and depend on exec deps both with and without that exec group, and in the implementation check if no-remote-exec is in the tags to know which to use.
We have this in a lib to make it easier to handle from our aspects but its still quite a large pain that can be avoided by disabling --incompatible_allow_tags_propagation.
load("@local_config_platform//:constraints.bzl"", "HOST_CONSTRAINTS")
LOCAL_EXEC_GROUPS = {
"local_exec_group": exec_group(
exec_compatible_with = HOST_CONSTRAINTS,
),
}
def exec_attr_label(name, **kwargs):
return {
name + "_remote": attr.label(
cfg = "exec",
**kwargs
),
name + "_local": attr.label(
cfg = config.exec("local_exec_group"),
**kwargs
),
}
def exec_attr_value(name, value):
return {
name + "_remote": value,
name + "_local": value,
}
def _is_no_remote_exec(tags):
return any([tag in tags for tag in ["no-remote-exec", "no-remote", "local"]])
def get_exec_attr(tags, attrs, name):
return getattr(attrs, name + "_local" if _is_no_remote_exec(tags) else name + "_remote")
def get_exec_group(tags):
return "local_exec_group" if _is_no_remote_exec(tags) else None
def _impl(target, ctx):
...
ctx.actions.run(
...
executable = get_exec_attr(ctx.rule.attr.tags, ctx.executable, "_validator"),
exec_group = get_exec_group(ctx.rule.attr.tags),
)
return [OutputGroupInfo(_validation = ...)]
some_validation_aspect = aspect(
implementation = _impl,
attrs = dict(
**exec_attr_label(
name = "_validator",
executable = True,
default = Label("//validator_thing"),
)
),
exec_groups = LOCAL_EXEC_GROUPS,
)