Remove feature requirements from tools.

BEGIN_PUBLIC
Remove feature requirements from tools.

It's such a niche use case, adds a fair amount of complexity, and there are multiple alternatives available:
* Now that we're using rule based things, you should just be able to add a bazel flag and select on it in your tool definition.
* If you really want precisely that behaviour, you can make your feature add "--use-foo-tool" to the args, and make your tool a wrapper tool that reads the command-line argument, and if provided, invokes a different tool.
END_PUBLIC

PiperOrigin-RevId: 657383729
Change-Id: Idb4f3ad66dc92d48ef81a1e8875bf6d3ba215aa4
This commit is contained in:
Googler 2024-07-29 17:36:39 -07:00 committed by Copybara-Service
parent e1c7ebb858
commit f97190f039
11 changed files with 13 additions and 85 deletions

View File

@ -151,7 +151,6 @@ ToolInfo = provider(
"label": "(Label) The label defining this provider. Place in error messages to simplify debugging", "label": "(Label) The label defining this provider. Place in error messages to simplify debugging",
"exe": "(File) The file corresponding to the tool", "exe": "(File) The file corresponding to the tool",
"runfiles": "(depset[File]) The files required to run the tool", "runfiles": "(depset[File]) The files required to run the tool",
"requires_any_of": "(Sequence[FeatureConstraintInfo]) A set of constraints, one of which is required to enable the tool. Equivalent to with_features",
"execution_requirements": "(Sequence[str]) A set of execution requirements of the tool", "execution_requirements": "(Sequence[str]) A set of execution requirements of the tool",
}, },
) )

View File

@ -106,7 +106,6 @@ def collect_tools(ctx, targets, fail = fail):
label = target.label, label = target.label,
exe = info.files_to_run.executable, exe = info.files_to_run.executable,
runfiles = collect_data(ctx, [target]), runfiles = collect_data(ctx, [target]),
requires_any_of = tuple(),
execution_requirements = tuple(), execution_requirements = tuple(),
)) ))
else: else:

View File

@ -128,10 +128,7 @@ def convert_tool(tool):
return legacy_tool( return legacy_tool(
tool = tool.exe, tool = tool.exe,
execution_requirements = list(tool.execution_requirements), execution_requirements = list(tool.execution_requirements),
with_features = [ with_features = [],
convert_feature_constraint(fc)
for fc in tool.requires_any_of
],
) )
def _convert_action_type_config(atc): def _convert_action_type_config(atc):

View File

@ -85,14 +85,6 @@ def _validate_requires_any_of(fn, self, known_features, fail):
if self.requires_any_of and not valid: if self.requires_any_of and not valid:
fail(_INVALID_CONSTRAINT_ERR.format(provider = self.label)) fail(_INVALID_CONSTRAINT_ERR.format(provider = self.label))
def _validate_requires_any_of_constraint(self, known_features, fail):
return _validate_requires_any_of(
lambda constraint: constraint.all_of.to_list(),
self,
known_features,
fail,
)
def _validate_requires_any_of_feature_set(self, known_features, fail): def _validate_requires_any_of_feature_set(self, known_features, fail):
return _validate_requires_any_of( return _validate_requires_any_of(
lambda feature_set: feature_set.features.to_list(), lambda feature_set: feature_set.features.to_list(),
@ -107,15 +99,15 @@ def _validate_implies(self, known_features, fail = fail):
fail(_UNKNOWN_FEATURE_ERR.format(self = self.label, ft = ft.label)) fail(_UNKNOWN_FEATURE_ERR.format(self = self.label, ft = ft.label))
def _validate_args(self, known_features, fail): def _validate_args(self, known_features, fail):
_validate_requires_any_of_constraint(self, known_features, fail = fail) return _validate_requires_any_of(
lambda constraint: constraint.all_of.to_list(),
def _validate_tool(self, known_features, fail): self,
_validate_requires_any_of_constraint(self, known_features, fail = fail) known_features,
fail,
)
def _validate_action_config(self, known_features, fail): def _validate_action_config(self, known_features, fail):
_validate_implies(self, known_features, fail = fail) _validate_implies(self, known_features, fail = fail)
for tool in self.tools:
_validate_tool(tool, known_features, fail = fail)
def _validate_feature(self, known_features, fail): def _validate_feature(self, known_features, fail):
_validate_requires_any_of_feature_set(self, known_features, fail = fail) _validate_requires_any_of_feature_set(self, known_features, fail = fail)

View File

@ -13,10 +13,9 @@
# limitations under the License. # limitations under the License.
"""Implementation of cc_tool""" """Implementation of cc_tool"""
load("//cc/toolchains/impl:collect.bzl", "collect_data", "collect_provider") load("//cc/toolchains/impl:collect.bzl", "collect_data")
load( load(
":cc_toolchain_info.bzl", ":cc_toolchain_info.bzl",
"FeatureConstraintInfo",
"ToolInfo", "ToolInfo",
) )
@ -34,10 +33,6 @@ def _cc_tool_impl(ctx):
label = ctx.label, label = ctx.label,
exe = exe, exe = exe,
runfiles = runfiles, runfiles = runfiles,
requires_any_of = tuple(collect_provider(
ctx.attr.requires_any_of,
FeatureConstraintInfo,
)),
execution_requirements = tuple(ctx.attr.execution_requirements), execution_requirements = tuple(ctx.attr.execution_requirements),
) )
@ -78,13 +73,6 @@ executable label.
"execution_requirements": attr.string_list( "execution_requirements": attr.string_list(
doc = "A list of strings that provide hints for execution environment compatibility (e.g. `requires-network`).", doc = "A list of strings that provide hints for execution environment compatibility (e.g. `requires-network`).",
), ),
"requires_any_of": attr.label_list(
providers = [FeatureConstraintInfo],
doc = """This will be enabled when any of the constraints are met.
If omitted, this tool will be enabled unconditionally.
""",
),
}, },
provides = [ToolInfo], provides = [ToolInfo],
doc = """Declares a tool that can be bound to action configs. doc = """Declares a tool that can be bound to action configs.

View File

@ -119,7 +119,7 @@ def _format_args_test(env, targets):
_expect_that_formatted( _expect_that_formatted(
env, env,
["{foo} {bar}"], ["{foo} {bar}"],
{"foo": targets.foo, "bar": targets.foo}, {"bar": targets.foo, "foo": targets.foo},
).err().contains('"{foo} {bar}" contained multiple variables') ).err().contains('"{foo} {bar}" contained multiple variables')
def _iterate_over_test(env, targets): def _iterate_over_test(env, targets):

View File

@ -179,7 +179,6 @@ _ToolFactory = generate_factory(
dict( dict(
exe = _subjects.file, exe = _subjects.file,
runfiles = runfiles_subject, runfiles = runfiles_subject,
requires_any_of = ProviderSequence(_FeatureConstraintFactory),
execution_requirements = _subjects.collection, execution_requirements = _subjects.collection,
), ),
) )

View File

@ -10,7 +10,6 @@ util.helper_target(
src = "//tests/rule_based_toolchain/testdata:bin_wrapper.sh", src = "//tests/rule_based_toolchain/testdata:bin_wrapper.sh",
data = ["//tests/rule_based_toolchain/testdata:bin"], data = ["//tests/rule_based_toolchain/testdata:bin"],
execution_requirements = ["requires-network"], execution_requirements = ["requires-network"],
requires_any_of = ["//tests/rule_based_toolchain/features:direct_constraint"],
) )
util.helper_target( util.helper_target(
@ -26,7 +25,6 @@ cc_directory_tool(
directory = "//tests/rule_based_toolchain/testdata:directory", directory = "//tests/rule_based_toolchain/testdata:directory",
executable = "bin_wrapper.sh", executable = "bin_wrapper.sh",
execution_requirements = ["requires-network"], execution_requirements = ["requires-network"],
requires_any_of = ["//tests/rule_based_toolchain/features:direct_constraint"],
) )
analysis_test_suite( analysis_test_suite(

View File

@ -13,10 +13,6 @@
# limitations under the License. # limitations under the License.
"""Tests for the cc_args rule.""" """Tests for the cc_args rule."""
load(
"//cc:cc_toolchain_config_lib.bzl",
legacy_with_feature_set = "with_feature_set",
)
load("//cc/toolchains:cc_toolchain_info.bzl", "ToolInfo") load("//cc/toolchains:cc_toolchain_info.bzl", "ToolInfo")
load("//cc/toolchains/impl:collect.bzl", _collect_tools = "collect_tools") load("//cc/toolchains/impl:collect.bzl", _collect_tools = "collect_tools")
load("//cc/toolchains/impl:legacy_converter.bzl", "convert_tool") load("//cc/toolchains/impl:legacy_converter.bzl", "convert_tool")
@ -37,7 +33,7 @@ _BIN_WRAPPER_SYMLINK = "tests/rule_based_toolchain/testdata/bin_wrapper"
_BIN_WRAPPER = "tests/rule_based_toolchain/testdata/bin_wrapper.sh" _BIN_WRAPPER = "tests/rule_based_toolchain/testdata/bin_wrapper.sh"
_BIN = "tests/rule_based_toolchain/testdata/bin" _BIN = "tests/rule_based_toolchain/testdata/bin"
def _tool_test(env, targets, target): def _tool_test(env, target):
tool = env.expect.that_target(target).provider(ToolInfo) tool = env.expect.that_target(target).provider(ToolInfo)
tool.exe().short_path_equals(_BIN_WRAPPER) tool.exe().short_path_equals(_BIN_WRAPPER)
tool.execution_requirements().contains_exactly(["requires-network"]) tool.execution_requirements().contains_exactly(["requires-network"])
@ -45,17 +41,11 @@ def _tool_test(env, targets, target):
_BIN_WRAPPER, _BIN_WRAPPER,
_BIN, _BIN,
]) ])
tool.requires_any_of().contains_exactly([targets.direct_constraint.label])
legacy = convert_tool(tool.actual) legacy = convert_tool(tool.actual)
env.expect.that_file(legacy.tool).equals(tool.actual.exe) env.expect.that_file(legacy.tool).equals(tool.actual.exe)
env.expect.that_collection(legacy.execution_requirements).contains_exactly(["requires-network"]) env.expect.that_collection(legacy.execution_requirements).contains_exactly(["requires-network"])
env.expect.that_collection(legacy.with_features).contains_exactly([ env.expect.that_collection(legacy.with_features).contains_exactly([])
legacy_with_feature_set(
features = ["feature_name"],
not_features = ["simple2"],
),
])
def _wrapped_tool_includes_runfiles_test(env, targets): def _wrapped_tool_includes_runfiles_test(env, targets):
tool = env.expect.that_target(targets.wrapped_tool).provider(ToolInfo) tool = env.expect.that_target(targets.wrapped_tool).provider(ToolInfo)
@ -115,8 +105,8 @@ TARGETS = [
# @unsorted-dict-items # @unsorted-dict-items
TESTS = { TESTS = {
"tool_test": lambda env, targets: _tool_test(env, targets, targets.tool), "tool_test": lambda env, targets: _tool_test(env, targets.tool),
"directory_tool_test": lambda env, targets: _tool_test(env, targets, targets.directory_tool), "directory_tool_test": lambda env, targets: _tool_test(env, targets.directory_tool),
"wrapped_tool_includes_runfiles_test": _wrapped_tool_includes_runfiles_test, "wrapped_tool_includes_runfiles_test": _wrapped_tool_includes_runfiles_test,
"collect_tools_collects_tools_test": _collect_tools_collects_tools_test, "collect_tools_collects_tools_test": _collect_tools_collects_tools_test,
"collect_tools_collects_binaries_test": _collect_tools_collects_binaries_test, "collect_tools_collects_binaries_test": _collect_tools_collects_binaries_test,

View File

@ -3,7 +3,6 @@ load("//cc/toolchains:action_type_config.bzl", "cc_action_type_config")
load("//cc/toolchains:args.bzl", "cc_args") load("//cc/toolchains:args.bzl", "cc_args")
load("//cc/toolchains:feature.bzl", "cc_feature") load("//cc/toolchains:feature.bzl", "cc_feature")
load("//cc/toolchains:feature_set.bzl", "cc_feature_set") load("//cc/toolchains:feature_set.bzl", "cc_feature_set")
load("//cc/toolchains:tool.bzl", "cc_tool")
load("//cc/toolchains/impl:external_feature.bzl", "cc_external_feature") load("//cc/toolchains/impl:external_feature.bzl", "cc_external_feature")
load("//cc/toolchains/impl:toolchain_config.bzl", "cc_legacy_file_group", "cc_toolchain_config") load("//cc/toolchains/impl:toolchain_config.bzl", "cc_legacy_file_group", "cc_toolchain_config")
load("//tests/rule_based_toolchain:analysis_test_suite.bzl", "analysis_test_suite") load("//tests/rule_based_toolchain:analysis_test_suite.bzl", "analysis_test_suite")
@ -137,20 +136,6 @@ util.helper_target(
requires_any_of = [":all_simple_features"], requires_any_of = [":all_simple_features"],
) )
util.helper_target(
cc_tool,
name = "requires_all_simple_tool",
src = "//tests/rule_based_toolchain/testdata:bin_wrapper.sh",
requires_any_of = [":all_simple_features"],
)
util.helper_target(
cc_action_type_config,
name = "requires_all_simple_action_type_config",
action_types = ["//tests/rule_based_toolchain/actions:c_compile"],
tools = [":requires_all_simple_tool"],
)
util.helper_target( util.helper_target(
cc_feature, cc_feature,
name = "requires_any_simple_feature", name = "requires_any_simple_feature",

View File

@ -169,22 +169,6 @@ def _args_missing_requirements_invalid_test(env, targets):
"It is impossible to enable %s" % targets.requires_all_simple_args.label, "It is impossible to enable %s" % targets.requires_all_simple_args.label,
) )
def _tool_missing_requirements_invalid_test(env, targets):
_expect_that_toolchain(
env,
action_type_configs = [targets.requires_all_simple_action_type_config],
features = [targets.simple_feature, targets.simple_feature2],
expr = "has_both",
).ok()
_expect_that_toolchain(
env,
action_type_configs = [targets.requires_all_simple_action_type_config],
features = [targets.simple_feature],
expr = "has_only_one",
).err().contains(
"It is impossible to enable %s" % targets.requires_all_simple_tool.label,
)
def _toolchain_collects_files_test(env, targets): def _toolchain_collects_files_test(env, targets):
tc = env.expect.that_target( tc = env.expect.that_target(
targets.collects_files_toolchain_config, targets.collects_files_toolchain_config,
@ -256,8 +240,6 @@ TARGETS = [
":requires_any_simple_feature", ":requires_any_simple_feature",
":requires_all_simple_feature", ":requires_all_simple_feature",
":requires_all_simple_args", ":requires_all_simple_args",
":requires_all_simple_action_type_config",
":requires_all_simple_tool",
":simple_feature", ":simple_feature",
":simple_feature2", ":simple_feature2",
":same_feature_name", ":same_feature_name",
@ -272,6 +254,5 @@ TESTS = {
"feature_config_implies_missing_feature_invalid_test": _feature_config_implies_missing_feature_invalid_test, "feature_config_implies_missing_feature_invalid_test": _feature_config_implies_missing_feature_invalid_test,
"feature_missing_requirements_invalid_test": _feature_missing_requirements_invalid_test, "feature_missing_requirements_invalid_test": _feature_missing_requirements_invalid_test,
"args_missing_requirements_invalid_test": _args_missing_requirements_invalid_test, "args_missing_requirements_invalid_test": _args_missing_requirements_invalid_test,
"tool_missing_requirements_invalid_test": _tool_missing_requirements_invalid_test,
"toolchain_collects_files_test": _toolchain_collects_files_test, "toolchain_collects_files_test": _toolchain_collects_files_test,
} }