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 bazelbuild#7977 for further details.

Implementation for bazelbuild#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 bazelbuild#7977 for more info.
PiperOrigin-RevId: 252063297

src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java

@@ -448,6 +448,18 @@ public class StarlarkSemanticsOptions extends OptionsBase implements Serializabl
               + "`attr.output_list` attribute definition functions.")
   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",
@@ -662,6 +674,7 @@ public StarlarkSemantics toSkylarkSemantics() {
         .incompatibleNoAttrLicense(incompatibleNoAttrLicense)
         .incompatibleNoKwargsInBuildFiles(incompatibleNoKwargsInBuildFiles)
         .incompatibleNoOutputAttrDefault(incompatibleNoOutputAttrDefault)
+        .incompatibleNoRuleOutputsParam(incompatibleNoRuleOutputsParam)
         .incompatibleNoSupportToolsInActionInputs(incompatibleNoSupportToolsInActionInputs)
         .incompatibleNoTargetOutputGroup(incompatibleNoTargetOutputGroup)
         .incompatibleNoTransitiveLoads(incompatibleNoTransitiveLoads)

src/main/java/com/google/devtools/build/lib/skylarkbuildapi/SkylarkRuleFunctionsApi.java

@@ -149,9 +149,10 @@ public interface SkylarkRuleFunctionsApi<FileApiT extends FileApi> {
             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. "

src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java

@@ -53,6 +53,7 @@ public enum FlagIdentifier {
     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 boolean flagValue(FlagIdentifier flagIdentifier) {
 
   public abstract boolean incompatibleNoOutputAttrDefault();
 
+  public abstract boolean incompatibleNoRuleOutputsParam();
+
   public abstract boolean incompatibleNoSupportToolsInActionInputs();
 
   public abstract boolean incompatibleNoTargetOutputGroup();
@@ -265,6 +268,7 @@ public static Builder builderWithDefaults() {
           .incompatibleNoAttrLicense(true)
           .incompatibleNoKwargsInBuildFiles(true)
           .incompatibleNoOutputAttrDefault(true)
+          .incompatibleNoRuleOutputsParam(false)
           .incompatibleNoSupportToolsInActionInputs(true)
           .incompatibleNoTargetOutputGroup(false)
           .incompatibleNoTransitiveLoads(true)
@@ -346,6 +350,8 @@ public abstract Builder incompatibleDisallowRuleExecutionPlatformConstraintsAllo
 
     public abstract Builder incompatibleNoOutputAttrDefault(boolean value);
 
+    public abstract Builder incompatibleNoRuleOutputsParam(boolean value);
+
     public abstract Builder incompatibleNoSupportToolsInActionInputs(boolean value);
 
     public abstract Builder incompatibleNoTargetOutputGroup(boolean value);

src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java

@@ -156,6 +156,7 @@ private static StarlarkSemanticsOptions buildRandomOptions(Random rand) throws E
         "--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 @@ private static StarlarkSemantics buildRandomSemantics(Random rand) {
         .incompatibleNoAttrLicense(rand.nextBoolean())
         .incompatibleNoKwargsInBuildFiles(rand.nextBoolean())
         .incompatibleNoOutputAttrDefault(rand.nextBoolean())
+        .incompatibleNoRuleOutputsParam(rand.nextBoolean())
         .incompatibleNoSupportToolsInActionInputs(rand.nextBoolean())
         .incompatibleNoTargetOutputGroup(rand.nextBoolean())
         .incompatibleNoTransitiveLoads(rand.nextBoolean())

src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java

@@ -2909,6 +2909,30 @@ public void testDisallowStructProviderSyntax() throws Exception {
             + "--incompatible_disallow_struct_provider_syntax=false");
   }
 
+  @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");

src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java

@@ -1925,7 +1925,8 @@ public void testNoAccessToDependencyActionsWithoutSkylarkTest() throws Exception
   @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 @@ public void testAbstractActionInterface() throws Exception {
   @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 @@ public void testSpawnActionInterface() throws Exception {
   @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

tools/build_defs/pkg/pkg.bzl

@@ -225,6 +225,12 @@ def _pkg_deb_impl(ctx):
         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 @@ _real_pkg_tar = rule(
         "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 @@ _real_pkg_tar = rule(
             allow_files = True,
         ),
     },
-    outputs = {
-        "out": "%{name}.%{extension}",
-    },
 )
 
 def pkg_tar(**kwargs):
@@ -273,10 +277,14 @@ def pkg_tar(**kwargs):
                       "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 @@ pkg_deb = rule(
             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
+    )

tools/jdk/default_java_toolchain.bzl

@@ -110,7 +110,7 @@ def java_runtime_files(name, srcs):
             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 @@ def _bootclasspath(ctx):
         arguments = [args],
     )
 
-    bootclasspath = ctx.outputs.jar
+    bootclasspath = ctx.outputs.output_jar
 
     inputs = class_outputs + ctx.files.host_javabase
 
@@ -168,9 +168,13 @@ def _bootclasspath(ctx):
         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 @@ bootclasspath = rule(
         "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
+    )