Flip symbolic macro flags
This enables `--experimental_enable_first_class_macros` and `--incompatible_simplify_unconditional_selects_in_rule_attrs` to true. The former releases Symbolic Macros, and with it, Finalizers and Macro-Aware Visibility. The latter is a quality-of-life improvement for macro users that is technically a breaking change: Any rule instantiated with an attribute value of `select({"//conditions:default": foo})` instead sees just `foo`.
Main milestone of #19922.
RELNOTES: Symbolic Macros -- and with them, Finalizers and the new Macro-Aware Visibility model -- are now generally available (`--experimental_enable_first_class_macros` now defaults to true). Trivial `select()` values are automatically unwrapped (`--incompatible_simplify_unconditional_selects_in_rule_attrs` now defaults to true).
PiperOrigin-RevId: 680779247
Change-Id: Icdbf4b47bc893ba61417eddc22c79d85b6f5d920
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/AspectAwareAttributeMapperTest.java b/src/test/java/com/google/devtools/build/lib/analysis/AspectAwareAttributeMapperTest.java
index aa47230..424c276 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/AspectAwareAttributeMapperTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/AspectAwareAttributeMapperTest.java
@@ -46,10 +46,22 @@
scratchConfiguredTargetAndData(
"foo",
"myrule",
- "cc_binary(",
- " name = 'myrule',",
- " srcs = [':a.cc'],",
- " linkstatic = select({'//conditions:default': 1}))");
+ """
+ # Needed to avoid select() being eliminated as trivial.
+ config_setting(
+ name = "config",
+ values = {"define": "pi=3"},
+ )
+
+ cc_binary(
+ name = "myrule",
+ srcs = [":a.cc"],
+ linkstatic = select({
+ ":config": 1,
+ "//conditions:default": 1,
+ }),
+ )
+ """);
RuleConfiguredTarget ct = (RuleConfiguredTarget) ctad.getConfiguredTarget();
rule = (Rule) ctad.getTargetForTesting();
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/ConfigurableAttributesTest.java b/src/test/java/com/google/devtools/build/lib/analysis/ConfigurableAttributesTest.java
index d791045..82ad6e45 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/ConfigurableAttributesTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/ConfigurableAttributesTest.java
@@ -2146,9 +2146,8 @@
}
@Test
- public void incompatibleSimplifyUnconditionalSelectsInRuleAttrs_checksForNonconfigurableAttrs()
+ public void assigningSelectToNonconfigurableAttr_fails_evenIfSelectIsSimplifiableUnconditional()
throws Exception {
- setBuildLanguageOptions("--incompatible_simplify_unconditional_selects_in_rule_attrs=true");
writeConfigRules();
scratch.file(
"foo/BUILD",
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/MacroVisibilityTest.java b/src/test/java/com/google/devtools/build/lib/analysis/MacroVisibilityTest.java
index 240f10c..7541032 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/MacroVisibilityTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/MacroVisibilityTest.java
@@ -20,7 +20,6 @@
import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
import com.google.testing.junit.testparameterinjector.TestParameterInjector;
import com.google.testing.junit.testparameterinjector.TestParameters;
-import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -34,11 +33,6 @@
@RunWith(TestParameterInjector.class)
public final class MacroVisibilityTest extends BuildViewTestCase {
- @Before
- public void setUp() throws Exception {
- setBuildLanguageOptions("--experimental_enable_first_class_macros");
- }
-
@Override
protected String getDefaultVisibility() {
// We're testing visibility. Avoid having to litter our test cases with `visibility=` attribute
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 b4c7123..18dc318 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
@@ -35,7 +35,6 @@
import java.util.List;
import javax.annotation.Nullable;
import org.junit.Assert;
-import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -43,13 +42,6 @@
@RunWith(TestParameterInjector.class)
public final class SymbolicMacroTest extends BuildViewTestCase {
- @Before
- public void setUp() throws Exception {
- setBuildLanguageOptions(
- "--experimental_enable_first_class_macros",
- "--incompatible_simplify_unconditional_selects_in_rule_attrs");
- }
-
/**
* Returns a package by the given name (no leading "//"), or null upon {@link
* NoSuchPackageException}.
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/VisibilityProviderTest.java b/src/test/java/com/google/devtools/build/lib/analysis/VisibilityProviderTest.java
index 0dfbe9a..59f889e 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/VisibilityProviderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/VisibilityProviderTest.java
@@ -32,7 +32,6 @@
@Before
public void setUp() throws Exception {
setBuildLanguageOptions(
- "--experimental_enable_first_class_macros",
// Let's test the case where input files have proper visibilities by default.
"--incompatible_no_implicit_file_export");
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/select/ConfiguredAttributeMapperTest.java b/src/test/java/com/google/devtools/build/lib/analysis/select/ConfiguredAttributeMapperTest.java
index df59dbf..e681ee6 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/select/ConfiguredAttributeMapperTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/select/ConfiguredAttributeMapperTest.java
@@ -306,7 +306,20 @@
@Test
public void testNoneValueOnMandatoryAttribute() throws Exception {
- scratch.file("a/BUILD", "alias(name='a', actual=select({'//conditions:default': None}))");
+ scratch.file(
+ "a/BUILD",
+ """
+ # Needed to avoid select() being eliminated as trivial.
+ config_setting(
+ name = "config",
+ values = {"define": "pi=3"},
+ )
+
+ alias(
+ name = "a",
+ actual = select({":config": None, "//conditions:default": None}),
+ )
+ """);
reporter.removeHandler(failFastHandler);
getConfiguredTarget("//a:a");
assertContainsEvent("Mandatory attribute 'actual' resolved to 'None'");
diff --git a/src/test/java/com/google/devtools/build/lib/packages/NativeExistingRulesTest.java b/src/test/java/com/google/devtools/build/lib/packages/NativeExistingRulesTest.java
index 7d52622..73e5d58 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/NativeExistingRulesTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/NativeExistingRulesTest.java
@@ -123,9 +123,18 @@
"""
load("//test/starlark:rulestr.bzl", "my_rule", "save_dep")
+ # Needed to avoid select() being eliminated as trivial.
+ config_setting(
+ name = "config",
+ values = {"define": "pi=3"},
+ )
+
my_rule(
name = "x",
- dep = select({"//conditions:default": None}),
+ dep = select({
+ ":config": None,
+ "//conditions:default": None,
+ }),
)
save_dep("x")
@@ -135,7 +144,8 @@
assertThat(getConfiguredTarget("//test/getrule:x")).isNotNull();
// We have to compare by stringification because SelectorValue has reference equality semantics.
- assertThat(getSaved("dep").toString()).isEqualTo("select({\"//conditions:default\": None})");
+ assertThat(getSaved("dep").toString())
+ .isEqualTo("select({\":config\": None, \"//conditions:default\": None})");
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java b/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java
index 6612b1f..8dd5525 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java
@@ -1181,7 +1181,6 @@
* <p>The macro does not define any targets.
*/
private void defineEmptyMacroBzl() throws Exception {
- setBuildLanguageOptions("--experimental_enable_first_class_macros");
scratch.file(
"pkg/my_macro.bzl",
"""
@@ -1372,7 +1371,6 @@
@Test
public void testSymbolicMacro_implicitCreationOfInputFilesIsNotTriggeredByMacros()
throws Exception {
- setBuildLanguageOptions("--experimental_enable_first_class_macros");
scratch.file(
"pkg/my_macro.bzl",
"""
@@ -1401,7 +1399,6 @@
@Test
public void testSymbolicMacro_deferredEvaluationExpandsTransitively() throws Exception {
- setBuildLanguageOptions("--experimental_enable_first_class_macros");
scratch.file(
"pkg/my_macro.bzl",
"""
@@ -1456,7 +1453,6 @@
@Test
public void testSymbolicMacro_recursionProhibitedWithEagerEvaluation() throws Exception {
- setBuildLanguageOptions("--experimental_enable_first_class_macros");
defineRecursiveMacro(/* deferredEvaluation= */ false);
expectEvalError(
"""
@@ -1475,7 +1471,6 @@
@Test
public void testSymbolicMacro_recursionProhibitedWithDeferredEvaluation() throws Exception {
- setBuildLanguageOptions("--experimental_enable_first_class_macros");
defineRecursiveMacro(/* deferredEvaluation= */ true);
expectEvalError(
"""
@@ -1494,7 +1489,6 @@
@Test
public void testSymbolicMacro_indirectRecursionAlsoProhibited() throws Exception {
- setBuildLanguageOptions("--experimental_enable_first_class_macros");
// Define a pair of macros where A calls B calls A (and then would stop, if allowed to get that
// far). Wrap it in a different entry point to test that the non-cyclic part is included in the
// traceback.
@@ -1561,7 +1555,6 @@
}
private void enableMacrosAndUsePrivateVisibility() throws Exception {
- setBuildLanguageOptions("--experimental_enable_first_class_macros");
// BuildViewTestCase makes everything public by default.
setPackageOptions("--default_visibility=private");
}
diff --git a/src/test/java/com/google/devtools/build/lib/packages/RuleFinalizerTest.java b/src/test/java/com/google/devtools/build/lib/packages/RuleFinalizerTest.java
index 39b8aea..ec25016 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/RuleFinalizerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/RuleFinalizerTest.java
@@ -20,7 +20,6 @@
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import javax.annotation.Nullable;
-import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
@@ -29,11 +28,6 @@
@RunWith(JUnit4.class)
public final class RuleFinalizerTest extends BuildViewTestCase {
- @Before
- public void setUp() throws Exception {
- setBuildLanguageOptions("--experimental_enable_first_class_macros");
- }
-
/**
* Returns a package by the given name (no leading "//"), or null upon {@link
* NoSuchPackageException}.
diff --git a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkDefinedAspectsTest.java b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkDefinedAspectsTest.java
index d07a71c..093c816 100644
--- a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkDefinedAspectsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkDefinedAspectsTest.java
@@ -1678,8 +1678,21 @@
scratch.file(
"test/BUILD",
"""
- load('//test:aspect.bzl', 'my_rule')
- my_rule(name = 'xxx', my_attr = select({'//conditions:default': 'foo'}))
+ load("//test:aspect.bzl", "my_rule")
+
+ # Needed to avoid the select() being eliminated as trivial.
+ config_setting(
+ name = "config",
+ values = {"defines": "something"},
+ )
+
+ my_rule(
+ name = "xxx",
+ my_attr = select({
+ ":config": "foo",
+ "//conditions:default": "bar",
+ }),
+ )
""");
reporter.removeHandler(failFastHandler);
diff --git a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkOptionsParsingTest.java b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkOptionsParsingTest.java
index ab12ccd..774452d 100644
--- a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkOptionsParsingTest.java
+++ b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkOptionsParsingTest.java
@@ -660,9 +660,18 @@
scratch.file(
"test/pkg/BUILD",
"""
+ # Needed to avoid select() being eliminated as trivial.
+ config_setting(
+ name = "config",
+ values = {"define": "pi=3"},
+ )
+
alias(
name = "two",
- actual = select({"//conditions:default": "//test:three"}),
+ actual = select({
+ ":config": "//test:three",
+ "//conditions:default": "//test:three",
+ }),
)
""");
diff --git a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java
index 4b0a86a..eeee50f 100644
--- a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java
@@ -291,8 +291,6 @@
@Test
public void testSymbolicMacro_instantiationRegistersOnPackage() throws Exception {
- setBuildLanguageOptions("--experimental_enable_first_class_macros");
-
scratch.file(
"pkg/foo.bzl",
"""
@@ -316,8 +314,6 @@
@Test
public void testSymbolicMacro_instantiationRequiresExport() throws Exception {
- setBuildLanguageOptions("--experimental_enable_first_class_macros");
-
scratch.file(
"pkg/foo.bzl",
"""
@@ -341,8 +337,6 @@
@Test
public void testSymbolicMacro_cannotInstantiateInBzlThread() throws Exception {
- setBuildLanguageOptions("--experimental_enable_first_class_macros");
-
scratch.file(
"pkg/foo.bzl",
"""
@@ -371,8 +365,6 @@
@Test
public void testSymbolicMacro_requiresNameAttribute() throws Exception {
- setBuildLanguageOptions("--experimental_enable_first_class_macros");
-
scratch.file(
"pkg/foo.bzl",
"""
@@ -396,8 +388,6 @@
@Test
public void testSymbolicMacro_prohibitsPositionalArgs() throws Exception {
- setBuildLanguageOptions("--experimental_enable_first_class_macros");
-
scratch.file(
"pkg/foo.bzl",
"""
@@ -421,8 +411,6 @@
@Test
public void testSymbolicMacroCanAcceptAttributes() throws Exception {
- setBuildLanguageOptions("--experimental_enable_first_class_macros");
-
scratch.file(
"pkg/foo.bzl",
"""
@@ -452,8 +440,6 @@
@Test
public void testSymbolicMacro_rejectsUnknownAttribute() throws Exception {
- setBuildLanguageOptions("--experimental_enable_first_class_macros");
-
scratch.file(
"pkg/foo.bzl",
"""
@@ -485,8 +471,6 @@
@Test
public void testSymbolicMacro_rejectsReservedAttributeName() throws Exception {
- ev.setSemantics("--experimental_enable_first_class_macros");
-
ev.setFailFast(false);
evalAndExport(
ev,
@@ -506,8 +490,6 @@
@Test
public void testSymbolicMacro_requiresMandatoryAttribute() throws Exception {
- setBuildLanguageOptions("--experimental_enable_first_class_macros");
-
scratch.file(
"pkg/foo.bzl",
"""
@@ -536,8 +518,6 @@
@Test
public void testSymbolicMacro_cannotOverrideImplicitAttribute() throws Exception {
- setBuildLanguageOptions("--experimental_enable_first_class_macros");
-
scratch.file(
"pkg/foo.bzl",
"""
@@ -568,8 +548,6 @@
@Test
public void testSymbolicMacro_doesNotSupportComputedDefaults() throws Exception {
- ev.setSemantics("--experimental_enable_first_class_macros");
-
ev.checkEvalErrorContains(
"In macro attribute 'xyz': Macros do not support computed defaults or late-bound defaults",
"""
@@ -595,8 +573,6 @@
// getPackage() returns null and no events are emitted.)
ev.setFragmentNameToClass(ImmutableMap.of("cpp", CppConfiguration.class));
- ev.setSemantics("--experimental_enable_first_class_macros");
-
ev.checkEvalErrorContains(
"In macro attribute 'xyz': Macros do not support computed defaults or late-bound defaults",
"""
@@ -613,8 +589,6 @@
@Test
public void testSymbolicMacro_macroFunctionApi() throws Exception {
- ev.setSemantics("--experimental_enable_first_class_macros");
-
evalAndExport(
ev,
"""
@@ -4411,8 +4385,8 @@
getConfiguredTarget("//initializer_testing:my_target");
ev.assertContainsError(
- "existing_rules() can only be used while evaluating a BUILD file (or macro) or a WORKSPACE"
- + " file");
+ "existing_rules() can only be used while evaluating a BUILD file (or legacy macro), a rule"
+ + " finalizer, or a WORKSPACE file");
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleImplementationFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleImplementationFunctionsTest.java
index d9084b4..e765965 100644
--- a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleImplementationFunctionsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleImplementationFunctionsTest.java
@@ -2105,7 +2105,7 @@
""");
reporter.removeHandler(failFastHandler);
getConfiguredTarget("//test:my_glob");
- assertContainsEvent("glob() can only be used while evaluating a BUILD file (or macro)");
+ assertContainsEvent("glob() can only be used while evaluating a BUILD file (or legacy macro)");
}
@Test
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 d0d4415..cb89faf 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
@@ -905,8 +905,7 @@
@Test
public void macroDocstring() throws Exception {
Module module =
- execWithOptions(
- ImmutableList.of("--experimental_enable_first_class_macros"),
+ exec(
"""
def _my_impl(name, visibility):
pass
@@ -940,8 +939,7 @@
@Test
public void macroAttributes() throws Exception {
Module module =
- execWithOptions(
- ImmutableList.of("--experimental_enable_first_class_macros"),
+ exec(
"""
def _my_impl(name):
pass
@@ -977,8 +975,7 @@
@Test
public void unexportedMacro_notDocumented() throws Exception {
Module module =
- execWithOptions(
- ImmutableList.of("--experimental_enable_first_class_macros"),
+ exec(
"""
def _my_impl(name):
pass
