Disable the 'default' parameter of attr.output and attr.output_list
This is attached to a new Starlark semantics flag, --incompatible_no_output_attr_default
RELNOTES: The 'default' parameter of attr.output and attr.output_list is removed. This is controlled by --incompatible_no_output_attr_default
PiperOrigin-RevId: 215805602
diff --git a/site/docs/skylark/backward-compatibility.md b/site/docs/skylark/backward-compatibility.md
index 37cad78..536b4e0 100644
--- a/site/docs/skylark/backward-compatibility.md
+++ b/site/docs/skylark/backward-compatibility.md
@@ -42,6 +42,7 @@
* [New args API](#new-args-api)
* [Disable objc provider resources](#disable-objc-provider-resources)
* [Disable output group field on Target](#disable-output-group-field-on-target)
+* [Disable default parameter of output attributes](#disable-default-parameter-of-output-attributes)
* [Remove native git repository](#remove-native-git-repository)
* [Remove native http archive](#remove-native-http-archive)
* [New-style JavaInfo constructor](#new-style-java_info)
@@ -277,6 +278,48 @@
* Default: `false`
+### Disable default parameter of output attributes
+
+This flag disables the `default` parameter on `attr.output` and
+`attr.output_list`. Use Starlark macros to specify defaults for these attributes
+instead.
+
+For example, replace:
+
+```python
+my_rule = rule(
+ ...
+ attrs = { "out" : attr.output(default = "foo.txt") }
+ ...
+```
+
+with:
+
+```python
+# myrule.bzl
+my_rule = rule(
+ ...
+ attrs = { "out" : attr.output() }
+ ...
+
+# mymacro.bzl
+load(":myrule.bzl", _my_rule=”my_rule”)
+
+def my_rule(name):
+ _my_rule(
+ name = name,
+ output = "%s_out.txt" % name
+ )
+```
+
+The previous `default` parameter of these attribute types was severely
+bug-prone, as two targets of the same rule would be unable to exist in the same
+package under default behavior. (Two targets both generating `foo.txt` in the
+same package would conflict.)
+
+* Flag: `--incompatible_no_output_attr_default`
+* Default: `false`
+
### Remove native git repository
When set, the native `git_repository` and `new_git_repository` rules are
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkAttr.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkAttr.java
index 13cce13..51019a7 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkAttr.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkAttr.java
@@ -683,6 +683,13 @@
Object defaultO, String doc, Boolean mandatory, FuncallExpression ast, Environment env)
throws EvalException {
SkylarkUtils.checkLoadingOrWorkspacePhase(env, "attr.output", ast.getLocation());
+
+ if (env.getSemantics().incompatibleNoOutputAttrDefault() && defaultO != Runtime.NONE) {
+ throw new EvalException(ast.getLocation(),
+ "'default' is no longer a supported parameter for attr.output. Use Starlark macros "
+ + "to set the default of output or output_list parameters instead. You can use "
+ + "--incompatible_no_output_attr_default=false to temporarily disable this check.");
+ }
return createNonconfigurableAttrDescriptor(
"output",
EvalUtils.<String, Object>optionMap(env, DEFAULT_ARG, defaultO, MANDATORY_ARG, mandatory),
@@ -694,7 +701,7 @@
@Override
public Descriptor outputListAttribute(
Boolean allowEmpty,
- SkylarkList defaultList,
+ Object defaultList,
String doc,
Boolean mandatory,
Boolean nonEmpty,
@@ -702,6 +709,14 @@
Environment env)
throws EvalException {
SkylarkUtils.checkLoadingOrWorkspacePhase(env, "attr.output_list", ast.getLocation());
+
+ if (env.getSemantics().incompatibleNoOutputAttrDefault() && defaultList != Runtime.NONE) {
+ throw new EvalException(ast.getLocation(),
+ "'default' is no longer a supported parameter for attr.output_list. Use Starlark macros "
+ + "to set the default of output or output_list parameters instead. You can use "
+ + "--incompatible_no_output_attr_default=false to temporarily disable this check.");
+ }
+
return createAttrDescriptor(
"output_list",
EvalUtils.<String, Object>optionMap(
diff --git a/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java b/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java
index e253319..9c19e2b 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java
@@ -326,6 +326,19 @@
public boolean incompatibleNewActionsApi;
@Option(
+ name = "incompatible_no_output_attr_default",
+ defaultValue = "false",
+ documentationCategory = OptionDocumentationCategory.SKYLARK_SEMANTICS,
+ effectTags = {OptionEffectTag.BUILD_FILE_SEMANTICS},
+ metadataTags = {
+ OptionMetadataTag.INCOMPATIBLE_CHANGE,
+ OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
+ },
+ help = "If set to true, disables the `default` parameter of the `attr.output` and "
+ + "`attr.output_list` attribute definition functions.")
+ public boolean incompatibleNoOutputAttrDefault;
+
+ @Option(
name = "incompatible_no_support_tools_in_action_inputs",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.SKYLARK_SEMANTICS,
@@ -501,6 +514,7 @@
.incompatibleExpandDirectories(incompatibleExpandDirectories)
.incompatibleGenerateJavaCommonSourceJar(incompatibleGenerateJavaCommonSourceJar)
.incompatibleNewActionsApi(incompatibleNewActionsApi)
+ .incompatibleNoOutputAttrDefault(incompatibleNoOutputAttrDefault)
.incompatibleNoSupportToolsInActionInputs(incompatibleNoSupportToolsInActionInputs)
.incompatibleNoTargetOutputGroup(incompatibleNoTargetOutputGroup)
.incompatibleNoTransitiveLoads(incompatibleNoTransitiveLoads)
diff --git a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/SkylarkAttrApi.java b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/SkylarkAttrApi.java
index 96e1f4e..b688d70 100644
--- a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/SkylarkAttrApi.java
+++ b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/SkylarkAttrApi.java
@@ -756,7 +756,9 @@
defaultValue = "None",
named = true,
positional = false,
- doc = DEFAULT_DOC),
+ doc = "<b><code>default<code> is deprecated for <code>attr.output</code>. Use "
+ + "Starlark macros to set the default for <code>output<code> and "
+ + "<code>output_list</code> attributes. <p>" + DEFAULT_DOC),
@Param(
name = DOC_ARG,
type = String.class,
@@ -796,10 +798,13 @@
@ParamType(type = SkylarkList.class, generic1 = Label.class),
@ParamType(type = UserDefinedFunction.class)
},
- defaultValue = "[]",
+ noneable = true,
+ defaultValue = "None",
named = true,
positional = false,
- doc = DEFAULT_DOC),
+ doc = "<b><code>default<code> is deprecated for <code>attr.output_list</code>. Use "
+ + "Starlark macros to set the default for <code>output<code> and "
+ + "<code>output_list</code> attributes. <p>" + DEFAULT_DOC),
@Param(
name = DOC_ARG,
type = String.class,
@@ -826,7 +831,7 @@
useEnvironment = true)
public Descriptor outputListAttribute(
Boolean allowEmpty,
- SkylarkList<?> defaultList,
+ Object defaultList,
String doc,
Boolean mandatory,
Boolean nonEmpty,
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java
index 9805d61..bad29a9 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java
@@ -131,6 +131,8 @@
public abstract boolean incompatibleNewActionsApi();
+ public abstract boolean incompatibleNoOutputAttrDefault();
+
public abstract boolean incompatibleNoSupportToolsInActionInputs();
public abstract boolean incompatibleNoTargetOutputGroup();
@@ -188,6 +190,7 @@
.incompatibleExpandDirectories(false)
.incompatibleGenerateJavaCommonSourceJar(false)
.incompatibleNewActionsApi(false)
+ .incompatibleNoOutputAttrDefault(false)
.incompatibleNoSupportToolsInActionInputs(false)
.incompatibleNoTargetOutputGroup(false)
.incompatibleNoTransitiveLoads(false)
@@ -246,6 +249,8 @@
public abstract Builder incompatibleNewActionsApi(boolean value);
+ public abstract Builder incompatibleNoOutputAttrDefault(boolean value);
+
public abstract Builder incompatibleNoSupportToolsInActionInputs(boolean value);
public abstract Builder incompatibleNoTargetOutputGroup(boolean value);
diff --git a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeSkylarkAttrApi.java b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeSkylarkAttrApi.java
index f0ea8a6..d72333a 100644
--- a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeSkylarkAttrApi.java
+++ b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeSkylarkAttrApi.java
@@ -91,7 +91,7 @@
}
@Override
- public Descriptor outputListAttribute(Boolean allowEmpty, SkylarkList<?> defaultList, String doc,
+ public Descriptor outputListAttribute(Boolean allowEmpty, Object defaultList, String doc,
Boolean mandatory, Boolean nonEmpty, FuncallExpression ast, Environment env)
throws EvalException {
return new FakeDescriptor(Type.OUTPUT_LIST, doc, mandatory);
diff --git a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
index 933ef11..3ce2097 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
@@ -141,6 +141,7 @@
"--incompatible_expand_directories=" + rand.nextBoolean(),
"--incompatible_generate_javacommon_source_jar=" + rand.nextBoolean(),
"--incompatible_new_actions_api=" + rand.nextBoolean(),
+ "--incompatible_no_output_attr_default=" + rand.nextBoolean(),
"--incompatible_no_support_tools_in_action_inputs=" + rand.nextBoolean(),
"--incompatible_no_target_output_group=" + rand.nextBoolean(),
"--incompatible_no_transitive_loads=" + rand.nextBoolean(),
@@ -182,6 +183,7 @@
.incompatibleExpandDirectories(rand.nextBoolean())
.incompatibleGenerateJavaCommonSourceJar(rand.nextBoolean())
.incompatibleNewActionsApi(rand.nextBoolean())
+ .incompatibleNoOutputAttrDefault(rand.nextBoolean())
.incompatibleNoSupportToolsInActionInputs(rand.nextBoolean())
.incompatibleNoTargetOutputGroup(rand.nextBoolean())
.incompatibleNoTransitiveLoads(rand.nextBoolean())
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java
index c8734de..9d4a1e3 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java
@@ -1053,6 +1053,90 @@
}
@Test
+ public void testNoOutputAttrDefault() throws Exception {
+ setSkylarkSemanticsOptions("--incompatible_no_output_attr_default=true");
+
+ scratch.file(
+ "test/extension.bzl",
+ "def custom_rule_impl(ctx):",
+ " out_file = ctx.actions.declare_file(ctx.attr._o1.name)",
+ " ctx.actions.write(output=out_file, content='hi')",
+ " return struct(o1=ctx.attr._o1)",
+ "",
+ "def output_fn():",
+ " return Label('//test/skylark:foo.txt')",
+ "",
+ "custom_rule = rule(implementation = custom_rule_impl,",
+ " attrs = {'_o1': attr.output(default = output_fn)})");
+
+ scratch.file(
+ "test/BUILD",
+ "load('//test:extension.bzl', 'custom_rule')",
+ "",
+ "custom_rule(name = 'r')");
+
+ reporter.removeHandler(failFastHandler);
+ getConfiguredTarget("//test:r");
+ assertContainsEvent("'default' is no longer a supported parameter for attr.output");
+ }
+
+ @Test
+ public void testNoOutputListAttrDefault() throws Exception {
+ setSkylarkSemanticsOptions("--incompatible_no_output_attr_default=true");
+
+ scratch.file(
+ "test/extension.bzl",
+ "def custom_rule_impl(ctx):",
+ " return []",
+ "",
+ "custom_rule = rule(implementation = custom_rule_impl,",
+ " attrs = {'outs': attr.output_list(default = [])})");
+
+ scratch.file(
+ "test/BUILD",
+ "load('//test:extension.bzl', 'custom_rule')",
+ "",
+ "custom_rule(name = 'r')");
+
+ reporter.removeHandler(failFastHandler);
+ getConfiguredTarget("//test:r");
+ assertContainsEvent("'default' is no longer a supported parameter for attr.output_list");
+ }
+
+ @Test
+ public void testLegacyOutputAttrDefault() throws Exception {
+ // Note that use of the "default" parameter of attr.output and attr.output_label is deprecated
+ // and barely functional. This test simply serves as proof-of-concept verification that the
+ // legacy behavior remains intact.
+ setSkylarkSemanticsOptions("--incompatible_no_output_attr_default=false");
+
+ scratch.file(
+ "test/skylark/extension.bzl",
+ "def custom_rule_impl(ctx):",
+ " out_file = ctx.actions.declare_file(ctx.attr._o1.name)",
+ " ctx.actions.write(output=out_file, content='hi')",
+ " return struct(o1=ctx.attr._o1,",
+ " o2=ctx.attr.o2)",
+ "",
+ "def output_fn():",
+ " return Label('//test/skylark:foo.txt')",
+ "",
+ "custom_rule = rule(implementation = custom_rule_impl,",
+ " attrs = {'_o1': attr.output(default = output_fn),",
+ " 'o2': attr.output_list(default = [])})");
+
+ scratch.file(
+ "test/skylark/BUILD",
+ "load('//test/skylark:extension.bzl', 'custom_rule')",
+ "",
+ "custom_rule(name = 'cr')");
+
+ ConfiguredTarget target = getConfiguredTarget("//test/skylark:cr");
+ assertThat(target.get("o1")).isEqualTo(Label.parseAbsoluteUnchecked("//test/skylark:foo.txt"));
+ assertThat(target.get("o2")).isEqualTo(MutableList.empty());
+ }
+
+ @Test
public void testRuleClassNonMandatoryEmptyOutputs() throws Exception {
scratch.file(
"test/skylark/extension.bzl",
