Skip to content

Commit

Permalink
[8.0.0] Re-enable symbolic macro attribute inheritance and remove --e…
Browse files Browse the repository at this point in the history
…xperimental_enable_macro_inherit_attrs flag (#24445)

After
a2f1f58,
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

a39d181

RELNOTES: Re-enable symbolic macro attribute inheritance.
PiperOrigin-RevId: 698884636
Change-Id: I7f682949e96b6739e12d8f569a87ef2daf0ea41c
  • Loading branch information
tetromino authored Nov 21, 2024
1 parent 27487cf commit 16b6096
Show file tree
Hide file tree
Showing 5 changed files with 1 addition and 54 deletions.
4 changes: 0 additions & 4 deletions site/en/extending/macros.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
"""
Expand All @@ -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",
"""
Expand All @@ -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",
"""
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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",
"""
Expand Down Expand Up @@ -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",
"""
Expand Down Expand Up @@ -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",
"""
Expand Down Expand Up @@ -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",
"""
Expand Down Expand Up @@ -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",
"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 16b6096

Please sign in to comment.