From 9befdcd90e859729d4f43bfc9272d9395ec007b4 Mon Sep 17 00:00:00 2001 From: Googler Date: Tue, 5 Mar 2024 14:54:47 -0800 Subject: [PATCH] Remove macros wrapping rules that take in features. Based on the comments in unknown commit, I created this CL BEGIN_PUBLIC Remove macros wrapping rules that take in features. END_PUBLIC PiperOrigin-RevId: 612979047 Change-Id: I5690717b164432c9cecebf87ef9dda41f9fa846f --- cc/toolchains/feature_constraint.bzl | 6 ++--- cc/toolchains/feature_set.bzl | 10 ++++---- cc/toolchains/features/BUILD | 4 ++-- cc/toolchains/features/legacy/BUILD | 2 +- cc/toolchains/impl/features_attr.bzl | 29 ----------------------- tests/rule_based_toolchain/features/BUILD | 2 +- 6 files changed, 12 insertions(+), 41 deletions(-) delete mode 100644 cc/toolchains/impl/features_attr.bzl diff --git a/cc/toolchains/feature_constraint.bzl b/cc/toolchains/feature_constraint.bzl index b02d420..c6ae44a 100644 --- a/cc/toolchains/feature_constraint.bzl +++ b/cc/toolchains/feature_constraint.bzl @@ -18,7 +18,6 @@ load( "collect_features", "collect_provider", ) -load("//cc/toolchains/impl:features_attr.bzl", "disallow_features_attr") load( ":cc_toolchain_info.bzl", "FeatureConstraintInfo", @@ -26,6 +25,8 @@ load( ) def _cc_feature_constraint_impl(ctx): + if ctx.attr.features: + fail("Features is a reserved attribute in bazel. Use the attributes `all_of` and `none_of` for feature constraints") all_of = collect_provider(ctx.attr.all_of, FeatureConstraintInfo) none_of = [collect_features(ctx.attr.none_of)] none_of.extend([fc.none_of for fc in all_of]) @@ -35,7 +36,7 @@ def _cc_feature_constraint_impl(ctx): none_of = depset(transitive = none_of), )] -_cc_feature_constraint = rule( +cc_feature_constraint = rule( implementation = _cc_feature_constraint_impl, attrs = { "all_of": attr.label_list( @@ -51,4 +52,3 @@ _cc_feature_constraint = rule( Can be used with require_any_of to specify that something is only enabled when a constraint is met.""", ) -cc_feature_constraint = disallow_features_attr(_cc_feature_constraint) diff --git a/cc/toolchains/feature_set.bzl b/cc/toolchains/feature_set.bzl index 369de9c..07af6d1 100644 --- a/cc/toolchains/feature_set.bzl +++ b/cc/toolchains/feature_set.bzl @@ -14,7 +14,6 @@ """Implementation of the cc_feature_set rule.""" load("//cc/toolchains/impl:collect.bzl", "collect_features") -load("//cc/toolchains/impl:features_attr.bzl", "require_features_attr") load( ":cc_toolchain_info.bzl", "FeatureConstraintInfo", @@ -22,7 +21,9 @@ load( ) def _cc_feature_set_impl(ctx): - features = collect_features(ctx.attr.features_) + if ctx.attr.features: + fail("Features is a reserved attribute in bazel. cc_feature_set takes `all_of` instead.") + features = collect_features(ctx.attr.all_of) return [ FeatureSetInfo(label = ctx.label, features = features), FeatureConstraintInfo( @@ -32,10 +33,10 @@ def _cc_feature_set_impl(ctx): ), ] -_cc_feature_set = rule( +cc_feature_set = rule( implementation = _cc_feature_set_impl, attrs = { - "features_": attr.label_list( + "all_of": attr.label_list( providers = [FeatureSetInfo], doc = "A set of features", ), @@ -54,4 +55,3 @@ Example: ) """, ) -cc_feature_set = require_features_attr(_cc_feature_set) diff --git a/cc/toolchains/features/BUILD b/cc/toolchains/features/BUILD index c9ad527..6c6088b 100644 --- a/cc/toolchains/features/BUILD +++ b/cc/toolchains/features/BUILD @@ -73,7 +73,7 @@ cc_external_feature( cc_feature_set( name = "all_non_legacy_builtin_features", - features = [ + all_of = [ ":opt", ":dbg", ":fastbuild", @@ -91,7 +91,7 @@ cc_feature_set( cc_feature_set( name = "all_builtin_features", - features = [ + all_of = [ ":all_non_legacy_builtin_features", "//cc/toolchains/features/legacy:all_legacy_builtin_features", ], diff --git a/cc/toolchains/features/legacy/BUILD b/cc/toolchains/features/legacy/BUILD index 60d9613..c7953a0 100644 --- a/cc/toolchains/features/legacy/BUILD +++ b/cc/toolchains/features/legacy/BUILD @@ -235,7 +235,7 @@ cc_external_feature( cc_feature_set( name = "all_legacy_builtin_features", - features = [ + all_of = [ ":legacy_compile_flags", ":default_compile_flags", ":dependency_file", diff --git a/cc/toolchains/impl/features_attr.bzl b/cc/toolchains/impl/features_attr.bzl deleted file mode 100644 index ab8d83a..0000000 --- a/cc/toolchains/impl/features_attr.bzl +++ /dev/null @@ -1,29 +0,0 @@ -# Copyright 2024 The Bazel Authors. All rights reserved. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. -"""Helpers for dealing with the fact that features is a reserved attribute.""" - -# buildifier: disable=unnamed-macro -def disallow_features_attr(rule): - def rule_wrapper(*, name, **kwargs): - if "features" in kwargs: - fail("Cannot use features in %s" % native.package_relative_label(name)) - rule(name = name, **kwargs) - - return rule_wrapper - -def require_features_attr(rule): - def rule_wrapper(*, name, features, **kwargs): - rule(name = name, features_ = features, **kwargs) - - return rule_wrapper diff --git a/tests/rule_based_toolchain/features/BUILD b/tests/rule_based_toolchain/features/BUILD index 2d0dce7..1d83660 100644 --- a/tests/rule_based_toolchain/features/BUILD +++ b/tests/rule_based_toolchain/features/BUILD @@ -35,7 +35,7 @@ util.helper_target( util.helper_target( cc_feature_set, name = "feature_set", - features = [ + all_of = [ ":simple", ":simple2", ],