From 03f583d52e74b507cadec8128c3db1bec3922070 Mon Sep 17 00:00:00 2001 From: Googler Date: Thu, 21 Nov 2024 12:52:55 -0800 Subject: [PATCH] [8.0.0] Re-enable symbolic macro attribute inheritance and remove --experimental_enable_macro_inherit_attrs flag After https://github.com/bazelbuild/bazel/commit/a2f1f58cdff07d240c9f715d5091672bd9abd03f, macro inheritance API is in a good shape - and without macro inheritance, we wouldn't have any good way for symbolic macros in Bazel 8 to use `testonly`. Considering for how little time the --experimental_enable_macro_inherit_attrs existed, and that no references to it was marked experimental, it seems safe to remove it directly without graveyarding. Cherry-pick of https://github.com/bazelbuild/bazel/commit/a39d18122ae824697691813a103ccb65871ff033 RELNOTES: Re-enable symbolic macro attribute inheritance. PiperOrigin-RevId: 698884636 Change-Id: I7f682949e96b6739e12d8f569a87ef2daf0ea41c --- site/en/extending/macros.md | 4 --- .../semantics/BuildLanguageOptions.java | 13 -------- .../StarlarkRuleFunctionsApi.java | 2 -- .../build/lib/analysis/SymbolicMacroTest.java | 33 ------------------- .../ModuleInfoExtractorTest.java | 3 +- 5 files changed, 1 insertion(+), 54 deletions(-) diff --git a/site/en/extending/macros.md b/site/en/extending/macros.md index 840dbebc2dd0d1..a958a8935f96bf 100644 --- a/site/en/extending/macros.md +++ b/site/en/extending/macros.md @@ -58,10 +58,6 @@ as an unconfigurable `select` – `"foo"` will become #### Attribute inheritance {:#attribute-inheritance} -IMPORTANT: Attribute inheritance is an experimental feature enabled by the -`--experimental_enable_macro_inherit_attrs` flag. Some of the behaviors -described in this section may change before the feature is enabled by default. - Macros are often intended to wrap a rule (or another macro), and the macro's author often wants to forward the bulk of the wrapped symbol's attributes unchanged, using `**kwargs`, to the macro's main target (or main inner macro). diff --git a/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java b/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java index 3c7e0ec498a9fa..8805b243ef5c4d 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java +++ b/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java @@ -244,16 +244,6 @@ public final class BuildLanguageOptions extends OptionsBase { help = "If set to true, enables the `macro()` construct for defining symbolic macros.") public boolean experimentalEnableFirstClassMacros; - @Option( - name = "experimental_enable_macro_inherit_attrs", - defaultValue = "false", - documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS, - effectTags = OptionEffectTag.BUILD_FILE_SEMANTICS, - help = - "If set to true, enables attribute inheritance in symbolic macros and the inherit_attrs" - + " parameter in the macro() built-in") - public boolean experimentalEnableMacroInheritAttrs; - @Option( name = "experimental_enable_scl_dialect", defaultValue = "true", @@ -858,7 +848,6 @@ public StarlarkSemantics toStarlarkSemantics() { EXPERIMENTAL_SINGLE_PACKAGE_TOOLCHAIN_BINDING, experimentalSinglePackageToolchainBinding) .setBool(EXPERIMENTAL_ENABLE_FIRST_CLASS_MACROS, experimentalEnableFirstClassMacros) - .setBool(EXPERIMENTAL_ENABLE_MACRO_INHERIT_ATTRS, experimentalEnableMacroInheritAttrs) .setBool(EXPERIMENTAL_ENABLE_SCL_DIALECT, experimentalEnableSclDialect) .setBool(ENABLE_BZLMOD, enableBzlmod) .setBool(ENABLE_WORKSPACE, enableWorkspace) @@ -972,8 +961,6 @@ public StarlarkSemantics toStarlarkSemantics() { "-experimental_single_package_toolchain_binding"; public static final String EXPERIMENTAL_ENABLE_FIRST_CLASS_MACROS = "+experimental_enable_first_class_macros"; - public static final String EXPERIMENTAL_ENABLE_MACRO_INHERIT_ATTRS = - "-experimental_enable_macro_inherit_attrs"; public static final String EXPERIMENTAL_ENABLE_SCL_DIALECT = "+experimental_enable_scl_dialect"; public static final String ENABLE_BZLMOD = "+enable_bzlmod"; public static final String ENABLE_WORKSPACE = "-enable_workspace"; diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkRuleFunctionsApi.java b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkRuleFunctionsApi.java index b28a0e99e7cbe7..82f7dae9ae5c57 100644 --- a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkRuleFunctionsApi.java +++ b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkRuleFunctionsApi.java @@ -301,8 +301,6 @@ site of the rule. Such attributes can be assigned a default value (as in positional = false, named = true, defaultValue = "None", - enableOnlyWithFlag = BuildLanguageOptions.EXPERIMENTAL_ENABLE_MACRO_INHERIT_ATTRS, - valueWhenDisabled = "None", doc = """ A rule symbol, macro symbol, or the name of a built-in common attribute list (see below) from which diff --git a/src/test/java/com/google/devtools/build/lib/analysis/SymbolicMacroTest.java b/src/test/java/com/google/devtools/build/lib/analysis/SymbolicMacroTest.java index 94c4ca77ea7bfd..aa5ee751412b1b 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/SymbolicMacroTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/SymbolicMacroTest.java @@ -1597,33 +1597,8 @@ def _impl(name, visibility, **kwargs): .doesNotContainKey("disabled_attr"); } - @Test - public void inheritAttrs_disabledByDefault() throws Exception { - scratch.file( - "pkg/foo.bzl", - """ - def _my_macro_impl(name, visibility, **kwargs): - pass - - my_macro = macro( - implementation = _my_macro_impl, - inherit_attrs = native.cc_library, - ) - """); - scratch.file( - "pkg/BUILD", - """ - load(":foo.bzl", "my_macro") - """); - reporter.removeHandler(failFastHandler); - assertThat(getPackage("pkg")).isNull(); - assertContainsEvent( - "parameter 'inherit_attrs' is experimental and thus unavailable with the current flags"); - } - @Test public void inheritAttrs_fromInvalidSource_fails() throws Exception { - setBuildLanguageOptions("--experimental_enable_macro_inherit_attrs"); scratch.file( "pkg/foo.bzl", """ @@ -1648,7 +1623,6 @@ def _my_macro_impl(name, visibility, **kwargs): @Test public void inheritAttrs_withoutKwargsInImplementation_fails() throws Exception { - setBuildLanguageOptions("--experimental_enable_macro_inherit_attrs"); scratch.file( "pkg/foo.bzl", """ @@ -1673,7 +1647,6 @@ def _my_macro_impl(name, visibility, tags): @Test public void inheritAttrs_fromCommon_withOverrides() throws Exception { - setBuildLanguageOptions("--experimental_enable_macro_inherit_attrs"); scratch.file( "pkg/my_macro.bzl", """ @@ -1738,7 +1711,6 @@ public void inheritAttrs_fromAnyNativeRule() throws Exception { // * a new AttributeValueSource or a new attribute type is introduced, and symbolic macros // cannot inherit an attribute with a default with this source or of such a type (to fix, add // a check for it in MacroClass#forceDefaultToNone). - setBuildLanguageOptions("--experimental_enable_macro_inherit_attrs"); for (RuleClass ruleClass : getBuiltinRuleClasses(false)) { if (ruleClass.getAttributes().isEmpty()) { continue; @@ -1819,7 +1791,6 @@ public void inheritAttrs_fromGenrule_producesTargetThatPassesAnalysis() throws E // inheritAttrs_fromAnyNativeRule() above is loading-phase only; by contrast, this test verifies // that we can wrap a native rule (in this case, genrule) in a macro with inherit_attrs, and // that the macro-wrapped rule target passes analysis. - setBuildLanguageOptions("--experimental_enable_macro_inherit_attrs"); scratch.file( "pkg/my_genrule.bzl", """ @@ -1857,7 +1828,6 @@ def _my_genrule_impl(name, visibility, tags, **kwargs): @Test public void inheritAttrs_fromExportedStarlarkRule() throws Exception { - setBuildLanguageOptions("--experimental_enable_macro_inherit_attrs"); scratch.file( "pkg/my_macro.bzl", """ @@ -1895,7 +1865,6 @@ def _my_macro_impl(name, visibility, **kwargs): @Test public void inheritAttrs_fromUnexportedStarlarkRule_fails() throws Exception { - setBuildLanguageOptions("--experimental_enable_macro_inherit_attrs"); scratch.file( "pkg/my_macro.bzl", """ @@ -1934,7 +1903,6 @@ def _my_macro_impl(name, visibility, **kwargs): @Test public void inheritAttrs_fromExportedMacro() throws Exception { - setBuildLanguageOptions("--experimental_enable_macro_inherit_attrs"); scratch.file( "pkg/my_macro.bzl", """ @@ -1976,7 +1944,6 @@ def _my_macro_impl(name, visibility, tags, **kwargs): @Test public void inheritAttrs_fromUnexportedMacro_fails() throws Exception { - setBuildLanguageOptions("--experimental_enable_macro_inherit_attrs"); scratch.file( "pkg/my_macro.bzl", """ diff --git a/src/test/java/com/google/devtools/build/lib/starlarkdocextract/ModuleInfoExtractorTest.java b/src/test/java/com/google/devtools/build/lib/starlarkdocextract/ModuleInfoExtractorTest.java index a083f26cbe8289..ad98bcd9b8b2ab 100644 --- a/src/test/java/com/google/devtools/build/lib/starlarkdocextract/ModuleInfoExtractorTest.java +++ b/src/test/java/com/google/devtools/build/lib/starlarkdocextract/ModuleInfoExtractorTest.java @@ -975,8 +975,7 @@ def _my_impl(name): @Test public void macroInheritedAttributes() throws Exception { Module module = - execWithOptions( - ImmutableList.of("--experimental_enable_macro_inherit_attrs"), + exec( """ def _my_rule_impl(ctx): pass