From c2001a4569483596d9dc74ba9cabcbe4b6f1887f Mon Sep 17 00:00:00 2001 From: aehlig Date: Mon, 6 May 2019 02:32:32 -0700 Subject: [PATCH] Automated rollback of commit bbe47a1564a832e1a175206f2dfbc92af94c120b. *** Reason for rollback *** Breaks Bazel's target //scripts/packages/debian:bazel-debian.deb which is needed for bazel releases. *** 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: 246792521 --- .../packages/StarlarkSemanticsOptions.java | 13 ----- .../SkylarkRuleFunctionsApi.java | 5 +- .../build/lib/syntax/StarlarkSemantics.java | 6 --- .../SkylarkSemanticsConsistencyTest.java | 2 - .../lib/skylark/SkylarkIntegrationTest.java | 24 ---------- .../lib/skylark/SkylarkRuleContextTest.java | 9 ++-- tools/build_defs/pkg/pkg.bzl | 48 ++++--------------- tools/jdk/default_java_toolchain.bzl | 21 ++------ 8 files changed, 20 insertions(+), 108 deletions(-) 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 b288956bf5f083..c039f87b17d6c6 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 @@ -443,18 +443,6 @@ 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 = "false", @@ -656,7 +644,6 @@ public StarlarkSemantics toSkylarkSemantics() { .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 a35c698784b139..304734911eec8a 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,10 +149,9 @@ public interface SkylarkRuleFunctionsApi { callbackEnabled = true, noneable = true, defaultValue = "None", - valueWhenDisabled = "None", - disableWithFlag = FlagIdentifier.INCOMPATIBLE_NO_RULE_OUTPUTS_PARAM, doc = - "A schema for defining predeclared outputs. Unlike " + "Experimental: This API is in the process of being redesigned." + + "

A schema for defining predeclared outputs. Unlike " + "output and " + "output_list 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 779ee1914068c8..bd34ac97b28f9b 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 @@ -50,7 +50,6 @@ 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), @@ -176,8 +175,6 @@ public boolean flagValue(FlagIdentifier flagIdentifier) { public abstract boolean incompatibleNoOutputAttrDefault(); - public abstract boolean incompatibleNoRuleOutputsParam(); - public abstract boolean incompatibleNoSupportToolsInActionInputs(); public abstract boolean incompatibleNoTargetOutputGroup(); @@ -246,7 +243,6 @@ public static Builder builderWithDefaults() { .incompatibleNoAttrLicense(true) .incompatibleNoKwargsInBuildFiles(false) .incompatibleNoOutputAttrDefault(true) - .incompatibleNoRuleOutputsParam(false) .incompatibleNoSupportToolsInActionInputs(false) .incompatibleNoTargetOutputGroup(false) .incompatibleNoTransitiveLoads(true) @@ -325,8 +321,6 @@ 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); 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 5ce9cf76854459..7a550db7d6a0de 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 @@ -155,7 +155,6 @@ 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(), @@ -208,7 +207,6 @@ 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()) 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 1d29f26817e486..7d1b2c884f6d77 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 @@ -2892,30 +2892,6 @@ 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"); 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 83382ceed8fd6b..9cac21757197db 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 @@ -1881,8 +1881,7 @@ public void testNoAccessToDependencyActionsWithoutSkylarkTest() throws Exception @Test public void testAbstractActionInterface() throws Exception { setSkylarkSemanticsOptions( - "--incompatible_disallow_struct_provider_syntax=false", - "--incompatible_no_rule_outputs_param=false"); + "--incompatible_disallow_struct_provider_syntax=false"); scratch.file("test/rules.bzl", "def _undertest_impl(ctx):", " out1 = ctx.outputs.out1", @@ -1926,8 +1925,7 @@ public void testAbstractActionInterface() throws Exception { @Test public void testCreatedActions() throws Exception { setSkylarkSemanticsOptions( - "--incompatible_disallow_struct_provider_syntax=false", - "--incompatible_no_rule_outputs_param=false"); + "--incompatible_disallow_struct_provider_syntax=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 @@ -2010,8 +2008,7 @@ public void testSpawnActionInterface() throws Exception { @Test public void testRunShellUsesHelperScriptForLongCommand() throws Exception { setSkylarkSemanticsOptions( - "--incompatible_disallow_struct_provider_syntax=false", - "--incompatible_no_rule_outputs_param=false"); + "--incompatible_disallow_struct_provider_syntax=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 5fcc2ce493537d..2a6396dc1d0046 100644 --- a/tools/build_defs/pkg/pkg.bzl +++ b/tools/build_defs/pkg/pkg.bzl @@ -219,12 +219,6 @@ def _pkg_deb_impl(ctx): inputs = [ctx.outputs.deb], outputs = [ctx.outputs.out], ) - output_groups = {"out": [ctx.outputs.out]} - if hasattr(ctx.outputs, "out_deb"): - output_groups["deb"] = ctx.outputs.deb - if hasattr(ctx.outputs, "out_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,7 +233,6 @@ _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(), @@ -258,6 +251,9 @@ _real_pkg_tar = rule( allow_files = True, ), }, + outputs = { + "out": "%{name}.%{extension}", + }, ) def pkg_tar(**kwargs): @@ -271,17 +267,10 @@ def pkg_tar(**kwargs): "This attribute was renamed to `srcs`. " + "Consider renaming it in your BUILD file.") kwargs["srcs"] = kwargs.pop("files") - if "extension" in kwargs and kwargs["extension"]: - extension = kwargs["extension"] - else: - extension = "tar" - _real_pkg_tar( - out = kwargs["name"] + "." + extension, - **kwargs - ) + _real_pkg_tar(**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), @@ -319,27 +308,10 @@ _pkg_deb = rule( executable = True, allow_files = True, ), - # Outputs. - "out": attr.output(mandatory = True), - "deb": attr.output(), - "changes": attr.output(), + }, + outputs = { + "out": "%{name}.deb", + "deb": "%{package}_%{version}_%{architecture}.deb", + "changes": "%{package}_%{version}_%{architecture}.changes", }, ) - -def pkg_deb(name, package, **kwargs): - """Creates a deb file. See README.md.""" - version = kwargs.get("version", None) - architecture = kwargs.get("architecture", "all") - out_deb = None - out_changes = None - if version and architecture: - 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 182419952bff11..3fdcfc1c7b540c 100644 --- a/tools/jdk/default_java_toolchain.bzl +++ b/tools/jdk/default_java_toolchain.bzl @@ -108,7 +108,7 @@ def java_runtime_files(name, srcs): outs = [src], ) -def _bootclasspath_impl(ctx): +def _bootclasspath(ctx): host_javabase = ctx.attr.host_javabase[java_common.JavaRuntimeInfo] # explicitly list output files instead of using TreeArtifact to work around @@ -141,7 +141,7 @@ def _bootclasspath_impl(ctx): arguments = [args], ) - bootclasspath = ctx.outputs.output_jar + bootclasspath = ctx.outputs.jar inputs = class_outputs + ctx.files.host_javabase @@ -166,13 +166,9 @@ def _bootclasspath_impl(ctx): outputs = [bootclasspath], arguments = [args], ) - return [ - DefaultInfo(files = depset([bootclasspath])), - OutputGroupInfo(jar = [bootclasspath]), - ] -_bootclasspath = rule( - implementation = _bootclasspath_impl, +bootclasspath = rule( + implementation = _bootclasspath, attrs = { "host_javabase": attr.label( cfg = "host", @@ -185,13 +181,6 @@ _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 - )