Rollforward "Disable outputs param of rule function" with fix
Original change broke //scripts/packages/debian:bazel-debian.deb
This rollforward manually tested to not break that target.
*** Original change description ***
Disable outputs param of rule function
This constitutes an incompatible change guarded by flag --incompatible_no_rule_outputs_param. See #7977 for further details.
Implementation for #7977.
RELNOTES: The `outputs` parameter of the `rule()` function is deprecated and attached to flag `--incompatible_no_rule_outputs_param`. Migrate rules to use `OutputGroupInfo` or `attr.output` instead. See https://github.com/bazelbuild/bazel/issues/7977 for more info.
PiperOrigin-RevId: 252063297
diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java
index 4d52868..587e959 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java
@@ -449,6 +449,18 @@
public boolean incompatibleNoOutputAttrDefault;
@Option(
+ name = "incompatible_no_rule_outputs_param",
+ defaultValue = "false",
+ documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
+ effectTags = {OptionEffectTag.BUILD_FILE_SEMANTICS},
+ metadataTags = {
+ OptionMetadataTag.INCOMPATIBLE_CHANGE,
+ OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
+ },
+ help = "If set to true, disables the `outputs` parameter of the `rule()` Starlark function.")
+ public boolean incompatibleNoRuleOutputsParam;
+
+ @Option(
name = "incompatible_no_support_tools_in_action_inputs",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
@@ -662,6 +674,7 @@
.incompatibleNoAttrLicense(incompatibleNoAttrLicense)
.incompatibleNoKwargsInBuildFiles(incompatibleNoKwargsInBuildFiles)
.incompatibleNoOutputAttrDefault(incompatibleNoOutputAttrDefault)
+ .incompatibleNoRuleOutputsParam(incompatibleNoRuleOutputsParam)
.incompatibleNoSupportToolsInActionInputs(incompatibleNoSupportToolsInActionInputs)
.incompatibleNoTargetOutputGroup(incompatibleNoTargetOutputGroup)
.incompatibleNoTransitiveLoads(incompatibleNoTransitiveLoads)
diff --git a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/SkylarkRuleFunctionsApi.java b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/SkylarkRuleFunctionsApi.java
index 20d280b..a00b633 100644
--- a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/SkylarkRuleFunctionsApi.java
+++ b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/SkylarkRuleFunctionsApi.java
@@ -149,9 +149,10 @@
callbackEnabled = true,
noneable = true,
defaultValue = "None",
+ valueWhenDisabled = "None",
+ disableWithFlag = FlagIdentifier.INCOMPATIBLE_NO_RULE_OUTPUTS_PARAM,
doc =
- "<b>Experimental:</b> This API is in the process of being redesigned."
- + "<p>A schema for defining predeclared outputs. Unlike "
+ "A schema for defining predeclared outputs. Unlike "
+ "<a href='attr.html#output'><code>output</code></a> and "
+ "<a href='attr.html#output_list'><code>output_list</code></a> attributes, "
+ "the user does not specify the labels for these files. "
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java
index 9f3e960..987de0a 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java
@@ -53,6 +53,7 @@
INCOMPATIBLE_DISABLE_OBJC_PROVIDER_RESOURCES(
StarlarkSemantics::incompatibleDisableObjcProviderResources),
INCOMPATIBLE_NO_OUTPUT_ATTR_DEFAULT(StarlarkSemantics::incompatibleNoOutputAttrDefault),
+ INCOMPATIBLE_NO_RULE_OUTPUTS_PARAM(StarlarkSemantics::incompatibleNoRuleOutputsParam),
INCOMPATIBLE_NO_TARGET_OUTPUT_GROUP(StarlarkSemantics::incompatibleNoTargetOutputGroup),
INCOMPATIBLE_NO_ATTR_LICENSE(StarlarkSemantics::incompatibleNoAttrLicense),
INCOMPATIBLE_OBJC_FRAMEWORK_CLEANUP(StarlarkSemantics::incompatibleObjcFrameworkCleanup),
@@ -180,6 +181,8 @@
public abstract boolean incompatibleNoOutputAttrDefault();
+ public abstract boolean incompatibleNoRuleOutputsParam();
+
public abstract boolean incompatibleNoSupportToolsInActionInputs();
public abstract boolean incompatibleNoTargetOutputGroup();
@@ -265,6 +268,7 @@
.incompatibleNoAttrLicense(true)
.incompatibleNoKwargsInBuildFiles(true)
.incompatibleNoOutputAttrDefault(true)
+ .incompatibleNoRuleOutputsParam(false)
.incompatibleNoSupportToolsInActionInputs(true)
.incompatibleNoTargetOutputGroup(false)
.incompatibleNoTransitiveLoads(true)
@@ -346,6 +350,8 @@
public abstract Builder incompatibleNoOutputAttrDefault(boolean value);
+ public abstract Builder incompatibleNoRuleOutputsParam(boolean value);
+
public abstract Builder incompatibleNoSupportToolsInActionInputs(boolean value);
public abstract Builder incompatibleNoTargetOutputGroup(boolean value);
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 01ebea9..50ff341 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
@@ -156,6 +156,7 @@
"--incompatible_no_attr_license=" + rand.nextBoolean(),
"--incompatible_no_kwargs_in_build_files=" + rand.nextBoolean(),
"--incompatible_no_output_attr_default=" + rand.nextBoolean(),
+ "--incompatible_no_rule_outputs_param=" + rand.nextBoolean(),
"--incompatible_no_support_tools_in_action_inputs=" + rand.nextBoolean(),
"--incompatible_no_target_output_group=" + rand.nextBoolean(),
"--incompatible_no_transitive_loads=" + rand.nextBoolean(),
@@ -210,6 +211,7 @@
.incompatibleNoAttrLicense(rand.nextBoolean())
.incompatibleNoKwargsInBuildFiles(rand.nextBoolean())
.incompatibleNoOutputAttrDefault(rand.nextBoolean())
+ .incompatibleNoRuleOutputsParam(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 f07fc4f..5564531 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
@@ -2910,6 +2910,30 @@
}
@Test
+ public void testNoRuleOutputsParam() throws Exception {
+ setSkylarkSemanticsOptions("--incompatible_no_rule_outputs_param=true");
+ scratch.file(
+ "test/skylark/test_rule.bzl",
+ "def _impl(ctx):",
+ " output = ctx.outputs.out",
+ " ctx.actions.write(output = output, content = 'hello')",
+ "",
+ "my_rule = rule(",
+ " implementation = _impl,",
+ " outputs = {\"out\": \"%{name}.txt\"})");
+ scratch.file(
+ "test/skylark/BUILD",
+ "load('//test/skylark:test_rule.bzl', 'my_rule')",
+ "my_rule(name = 'target')");
+
+ reporter.removeHandler(failFastHandler);
+ getConfiguredTarget("//test/skylark:target");
+ assertContainsEvent(
+ "parameter 'outputs' is deprecated and will be removed soon. It may be temporarily "
+ + "re-enabled by setting --incompatible_no_rule_outputs_param=false");
+ }
+
+ @Test
public void testExecutableNotInRunfiles() throws Exception {
setSkylarkSemanticsOptions("--incompatible_disallow_struct_provider_syntax=false");
scratch.file(
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
index 1b54e1c..054d5a8 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
@@ -1925,7 +1925,8 @@
@Test
public void testAbstractActionInterface() throws Exception {
setSkylarkSemanticsOptions(
- "--incompatible_disallow_struct_provider_syntax=false");
+ "--incompatible_disallow_struct_provider_syntax=false",
+ "--incompatible_no_rule_outputs_param=false");
scratch.file("test/rules.bzl",
"def _undertest_impl(ctx):",
" out1 = ctx.outputs.out1",
@@ -1969,7 +1970,8 @@
@Test
public void testCreatedActions() throws Exception {
setSkylarkSemanticsOptions(
- "--incompatible_disallow_struct_provider_syntax=false");
+ "--incompatible_disallow_struct_provider_syntax=false",
+ "--incompatible_no_rule_outputs_param=false");
// createRuleContext() gives us the context for a rule upon entry into its analysis function.
// But we need to inspect the result of calling created_actions() after the rule context has
// been modified by creating actions. So we'll call created_actions() from within the analysis
@@ -2052,7 +2054,8 @@
@Test
public void testRunShellUsesHelperScriptForLongCommand() throws Exception {
setSkylarkSemanticsOptions(
- "--incompatible_disallow_struct_provider_syntax=false");
+ "--incompatible_disallow_struct_provider_syntax=false",
+ "--incompatible_no_rule_outputs_param=false");
// createRuleContext() gives us the context for a rule upon entry into its analysis function.
// But we need to inspect the result of calling created_actions() after the rule context has
// been modified by creating actions. So we'll call created_actions() from within the analysis
diff --git a/tools/build_defs/pkg/pkg.bzl b/tools/build_defs/pkg/pkg.bzl
index 9074865..c2deaee 100644
--- a/tools/build_defs/pkg/pkg.bzl
+++ b/tools/build_defs/pkg/pkg.bzl
@@ -225,6 +225,12 @@
inputs = [ctx.outputs.deb],
outputs = [ctx.outputs.out],
)
+ output_groups = {"out": [ctx.outputs.out]}
+ if hasattr(ctx.outputs, "deb"):
+ output_groups["deb"] = [ctx.outputs.deb]
+ if hasattr(ctx.outputs, "changes"):
+ output_groups["changes"] = [ctx.outputs.changes]
+ return OutputGroupInfo(**output_groups)
# A rule for creating a tar file, see README.md
_real_pkg_tar = rule(
@@ -239,6 +245,7 @@
"modes": attr.string_dict(),
"mtime": attr.int(default = -1),
"portable_mtime": attr.bool(default = True),
+ "out": attr.output(),
"owner": attr.string(default = "0.0"),
"ownername": attr.string(default = "."),
"owners": attr.string_dict(),
@@ -257,9 +264,6 @@
allow_files = True,
),
},
- outputs = {
- "out": "%{name}.%{extension}",
- },
)
def pkg_tar(**kwargs):
@@ -273,10 +277,14 @@
"This attribute was renamed to `srcs`. " +
"Consider renaming it in your BUILD file.")
kwargs["srcs"] = kwargs.pop("files")
- _real_pkg_tar(**kwargs)
+ extension = kwargs.get("extension") or "tar"
+ _real_pkg_tar(
+ out = kwargs["name"] + "." + extension,
+ **kwargs
+ )
# A rule for creating a deb file, see README.md
-pkg_deb = rule(
+_pkg_deb = rule(
implementation = _pkg_deb_impl,
attrs = {
"data": attr.label(mandatory = True, allow_single_file = tar_filetype),
@@ -316,10 +324,24 @@
executable = True,
allow_files = True,
),
- },
- outputs = {
- "out": "%{name}.deb",
- "deb": "%{package}_%{version}_%{architecture}.deb",
- "changes": "%{package}_%{version}_%{architecture}.changes",
+ # Outputs.
+ "out": attr.output(mandatory = True),
+ "deb": attr.output(mandatory = True),
+ "changes": attr.output(mandatory = True),
},
)
+
+def pkg_deb(name, package, **kwargs):
+ """Creates a deb file. See README.md."""
+ version = kwargs.get("version", "")
+ architecture = kwargs.get("architecture", "all")
+ out_deb = "%s_%s_%s.deb" % (package, version, architecture)
+ out_changes = "%s_%s_%s.changes" % (package, version, architecture)
+ _pkg_deb(
+ name = name,
+ package = package,
+ out = name + ".deb",
+ deb = out_deb,
+ changes = out_changes,
+ **kwargs
+ )
diff --git a/tools/jdk/default_java_toolchain.bzl b/tools/jdk/default_java_toolchain.bzl
index da8f118..308b42a 100644
--- a/tools/jdk/default_java_toolchain.bzl
+++ b/tools/jdk/default_java_toolchain.bzl
@@ -110,7 +110,7 @@
outs = [src],
)
-def _bootclasspath(ctx):
+def _bootclasspath_impl(ctx):
host_javabase = ctx.attr.host_javabase[java_common.JavaRuntimeInfo]
# explicitly list output files instead of using TreeArtifact to work around
@@ -143,7 +143,7 @@
arguments = [args],
)
- bootclasspath = ctx.outputs.jar
+ bootclasspath = ctx.outputs.output_jar
inputs = class_outputs + ctx.files.host_javabase
@@ -168,9 +168,13 @@
outputs = [bootclasspath],
arguments = [args],
)
+ return [
+ DefaultInfo(files = depset([bootclasspath])),
+ OutputGroupInfo(jar = [bootclasspath]),
+ ]
-bootclasspath = rule(
- implementation = _bootclasspath,
+_bootclasspath = rule(
+ implementation = _bootclasspath_impl,
attrs = {
"host_javabase": attr.label(
cfg = "host",
@@ -183,6 +187,13 @@
"target_javabase": attr.label(
providers = [java_common.JavaRuntimeInfo],
),
+ "output_jar": attr.output(mandatory = True),
},
- outputs = {"jar": "%{name}.jar"},
)
+
+def bootclasspath(name, **kwargs):
+ _bootclasspath(
+ name = name,
+ output_jar = name + ".jar",
+ **kwargs
+ )
