Skip to content

Commit

Permalink
Automated rollback of commit bbe47a1.
Browse files Browse the repository at this point in the history
*** 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 #7977 for more info.
PiperOrigin-RevId: 246792521
  • Loading branch information
aehlig authored and copybara-github committed May 6, 2019
1 parent 898d7b6 commit c2001a4
Show file tree
Hide file tree
Showing 8 changed files with 20 additions and 108 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -656,7 +644,6 @@ public StarlarkSemantics toSkylarkSemantics() {
.incompatibleNoAttrLicense(incompatibleNoAttrLicense)
.incompatibleNoKwargsInBuildFiles(incompatibleNoKwargsInBuildFiles)
.incompatibleNoOutputAttrDefault(incompatibleNoOutputAttrDefault)
.incompatibleNoRuleOutputsParam(incompatibleNoRuleOutputsParam)
.incompatibleNoSupportToolsInActionInputs(incompatibleNoSupportToolsInActionInputs)
.incompatibleNoTargetOutputGroup(incompatibleNoTargetOutputGroup)
.incompatibleNoTransitiveLoads(incompatibleNoTransitiveLoads)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,10 +149,9 @@ public interface SkylarkRuleFunctionsApi<FileApiT extends FileApi> {
callbackEnabled = true,
noneable = true,
defaultValue = "None",
valueWhenDisabled = "None",
disableWithFlag = FlagIdentifier.INCOMPATIBLE_NO_RULE_OUTPUTS_PARAM,
doc =
"A schema for defining predeclared outputs. Unlike "
"<b>Experimental:</b> This API is in the process of being redesigned."
+ "<p>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. "
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -246,7 +243,6 @@ public static Builder builderWithDefaults() {
.incompatibleNoAttrLicense(true)
.incompatibleNoKwargsInBuildFiles(false)
.incompatibleNoOutputAttrDefault(true)
.incompatibleNoRuleOutputsParam(false)
.incompatibleNoSupportToolsInActionInputs(false)
.incompatibleNoTargetOutputGroup(false)
.incompatibleNoTransitiveLoads(true)
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
48 changes: 10 additions & 38 deletions tools/build_defs/pkg/pkg.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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(),
Expand All @@ -258,6 +251,9 @@ _real_pkg_tar = rule(
allow_files = True,
),
},
outputs = {
"out": "%{name}.%{extension}",
},
)

def pkg_tar(**kwargs):
Expand All @@ -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),
Expand Down Expand Up @@ -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
)
21 changes: 5 additions & 16 deletions tools/jdk/default_java_toolchain.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand All @@ -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",
Expand All @@ -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
)

0 comments on commit c2001a4

Please sign in to comment.