From 08beb210eddd35b703857e005d99c60b963e8e10 Mon Sep 17 00:00:00 2001 From: Googler Date: Thu, 14 Nov 2024 10:57:49 -0800 Subject: [PATCH] Put macro attribute inheritance behind an off-by-default --experimental_enable_macro_inherit_attrs flag We still have open questions about how macro attribute inheritance ought to interact with the tracking of whether rule attributes were or were not explicitly provided. In effect, this re-opens https://github.com/bazelbuild/bazel/issues/24066 Part of addressing https://github.com/bazelbuild/bazel/issues/24319 RELNOTES: symbolic macro attribute inheritance is now marked experimental; set --experimental_enable_macro_inherit_attrs flag to enable it. PiperOrigin-RevId: 696582223 Change-Id: I3d7cb434bf8fe2da9cd10019e6075990205e7153 --- .../semantics/BuildLanguageOptions.java | 13 ++++++++ .../StarlarkRuleFunctionsApi.java | 2 ++ .../build/lib/analysis/SymbolicMacroTest.java | 32 +++++++++++++++++++ 3 files changed, 47 insertions(+) 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 100d13b4a11e48..39ffa6524a6719 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,6 +244,16 @@ 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", @@ -857,6 +867,7 @@ 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,6 +983,8 @@ 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 2a7123bb9d73cf..4ec84c53436589 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 @@ -264,6 +264,8 @@ 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 43eaf1d4dd2193..68f3b0143c5c96 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 @@ -1582,8 +1582,33 @@ 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", """ @@ -1608,6 +1633,7 @@ 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", """ @@ -1632,6 +1658,7 @@ 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", """ @@ -1694,6 +1721,7 @@ 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; @@ -1771,6 +1799,7 @@ def _impl(name, visibility, **kwargs): @Test public void inheritAttrs_fromExportedStarlarkRule() throws Exception { + setBuildLanguageOptions("--experimental_enable_macro_inherit_attrs"); scratch.file( "pkg/my_rule.bzl", """ @@ -1813,6 +1842,7 @@ def _my_macro_impl(name, visibility, **kwargs): @Test public void inheritAttrs_fromUnexportedStarlarkRule() throws Exception { + setBuildLanguageOptions("--experimental_enable_macro_inherit_attrs"); scratch.file( "pkg/my_macro.bzl", """ @@ -1850,6 +1880,7 @@ def _my_macro_impl(name, visibility, **kwargs): @Test public void inheritAttrs_fromExportedMacro() throws Exception { + setBuildLanguageOptions("--experimental_enable_macro_inherit_attrs"); scratch.file( "pkg/other_macro.bzl", """ @@ -1890,6 +1921,7 @@ def _my_macro_impl(name, visibility, **kwargs): @Test public void inheritAttrs_fromUnexportedMacro() throws Exception { + setBuildLanguageOptions("--experimental_enable_macro_inherit_attrs"); scratch.file( "pkg/my_macro.bzl", """