Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[8.0.0] Re-enable symbolic macro attribute inheritance and remove --experimental_enable_macro_inherit_attrs flag #24445

Merged
merged 1 commit into from
Nov 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading