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 91305aa7ff90f5..e895d07db9f896 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 @@ -680,7 +680,6 @@ public Descriptor boolAttribute( @Override public Descriptor outputAttribute( - Object defaultValue, // Label | StarlarkFunction String doc, Boolean mandatory, StarlarkThread thread) @@ -688,16 +687,12 @@ public Descriptor outputAttribute( BazelStarlarkContext.from(thread).checkLoadingOrWorkspacePhase("attr.output"); return createNonconfigurableAttrDescriptor( - "output", - optionMap(DEFAULT_ARG, defaultValue, MANDATORY_ARG, mandatory), - BuildType.OUTPUT, - thread); + "output", optionMap(MANDATORY_ARG, mandatory), BuildType.OUTPUT, thread); } @Override public Descriptor outputListAttribute( Boolean allowEmpty, - Object defaultValue, // Sequence | StarlarkFunction String doc, Boolean mandatory, Boolean nonEmpty, @@ -708,8 +703,6 @@ public Descriptor outputListAttribute( return createAttrDescriptor( "output_list", optionMap( - DEFAULT_ARG, - defaultValue, MANDATORY_ARG, mandatory, NON_EMPTY_ARG, 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 a5403796227c3d..dfb51397e79f5b 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 @@ -482,20 +482,6 @@ public class StarlarkSemanticsOptions extends OptionsBase implements Serializabl + "2019-10-24-file-visibility.md") public boolean incompatibleNoImplicitFileExport; - @Option( - name = "incompatible_no_output_attr_default", - defaultValue = "true", - 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 `default` parameter of the `attr.output` and " - + "`attr.output_list` attribute definition functions.") - public boolean incompatibleNoOutputAttrDefault; - @Option( name = "incompatible_no_rule_outputs_param", defaultValue = "false", @@ -704,7 +690,6 @@ public StarlarkSemantics toSkylarkSemantics() { .incompatibleNewActionsApi(incompatibleNewActionsApi) .incompatibleNoAttrLicense(incompatibleNoAttrLicense) .incompatibleNoImplicitFileExport(incompatibleNoImplicitFileExport) - .incompatibleNoOutputAttrDefault(incompatibleNoOutputAttrDefault) .incompatibleNoRuleOutputsParam(incompatibleNoRuleOutputsParam) .incompatibleNoSupportToolsInActionInputs(incompatibleNoSupportToolsInActionInputs) .incompatibleNoTargetOutputGroup(incompatibleNoTargetOutputGroup) 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 24f39a6d596d60..dca25cb3f839e2 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 @@ -727,19 +727,6 @@ Descriptor boolAttribute( name = "output", doc = "Creates a schema for an output (label) attribute." + OUTPUT_ATTR_TEXT, parameters = { - @Param( - name = DEFAULT_ARG, - allowedTypes = { - @ParamType(type = Label.class), - @ParamType(type = StarlarkFunction.class) - }, - noneable = true, - defaultValue = "None", - named = true, - positional = false, - disableWithFlag = FlagIdentifier.INCOMPATIBLE_NO_OUTPUT_ATTR_DEFAULT, - valueWhenDisabled = "None", - doc = DEFAULT_DOC), @Param( name = DOC_ARG, type = String.class, @@ -756,8 +743,7 @@ Descriptor boolAttribute( doc = MANDATORY_DOC) }, useStarlarkThread = true) - Descriptor outputAttribute( - Object defaultValue, String doc, Boolean mandatory, StarlarkThread thread) + Descriptor outputAttribute(String doc, Boolean mandatory, StarlarkThread thread) throws EvalException; @SkylarkCallable( @@ -770,19 +756,6 @@ Descriptor outputAttribute( defaultValue = "True", doc = ALLOW_EMPTY_DOC, named = true), - @Param( - name = DEFAULT_ARG, - allowedTypes = { - @ParamType(type = Sequence.class, generic1 = Label.class), - @ParamType(type = StarlarkFunction.class) - }, - noneable = true, - defaultValue = "None", - named = true, - positional = false, - disableWithFlag = FlagIdentifier.INCOMPATIBLE_NO_OUTPUT_ATTR_DEFAULT, - valueWhenDisabled = "None", - doc = DEFAULT_DOC), @Param( name = DOC_ARG, type = String.class, @@ -808,7 +781,6 @@ Descriptor outputAttribute( useStarlarkThread = true) Descriptor outputListAttribute( Boolean allowEmpty, - Object defaultValue, String doc, Boolean mandatory, Boolean nonEmpty, 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 28c7dd71c3303c..41ef0d794f6589 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 @@ -74,8 +74,6 @@ private FlagIdentifier() {} // uninstantiable "incompatible_applicable_licenses"; public static final String INCOMPATIBLE_DISABLE_DEPSET_INPUTS = "incompatible_disable_depset_inputs"; - public static final String INCOMPATIBLE_NO_OUTPUT_ATTR_DEFAULT = - "incompatible_no_output_attr_default"; public static final String INCOMPATIBLE_NO_RULE_OUTPUTS_PARAM = "incompatible_no_rule_outputs_param"; public static final String INCOMPATIBLE_NO_TARGET_OUTPUT_GROUP = @@ -127,8 +125,6 @@ public boolean flagValue(String flag) { return incompatibleApplicableLicenses(); case FlagIdentifier.INCOMPATIBLE_DISABLE_DEPSET_INPUTS: return incompatibleDisableDepsetItems(); - case FlagIdentifier.INCOMPATIBLE_NO_OUTPUT_ATTR_DEFAULT: - return incompatibleNoOutputAttrDefault(); case FlagIdentifier.INCOMPATIBLE_NO_RULE_OUTPUTS_PARAM: return incompatibleNoRuleOutputsParam(); case FlagIdentifier.INCOMPATIBLE_NO_TARGET_OUTPUT_GROUP: @@ -240,8 +236,6 @@ boolean isFeatureEnabledBasedOnTogglingFlags(String enablingFlag, String disabli public abstract boolean incompatibleNoImplicitFileExport(); - public abstract boolean incompatibleNoOutputAttrDefault(); - public abstract boolean incompatibleNoRuleOutputsParam(); public abstract boolean incompatibleNoSupportToolsInActionInputs(); @@ -336,7 +330,6 @@ public static Builder builderWithDefaults() { .incompatibleNewActionsApi(true) .incompatibleNoAttrLicense(true) .incompatibleNoImplicitFileExport(false) - .incompatibleNoOutputAttrDefault(true) .incompatibleNoRuleOutputsParam(false) .incompatibleNoSupportToolsInActionInputs(true) .incompatibleNoTargetOutputGroup(true) @@ -417,8 +410,6 @@ public abstract static class Builder { public abstract Builder incompatibleNoImplicitFileExport(boolean value); - public abstract Builder incompatibleNoOutputAttrDefault(boolean value); - public abstract Builder incompatibleNoRuleOutputsParam(boolean value); public abstract Builder incompatibleNoSupportToolsInActionInputs(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 e999b136c18e7f..762c1fe79102d5 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 @@ -166,22 +166,20 @@ public Descriptor boolAttribute( } @Override - public Descriptor outputAttribute( - Object defaultO, String doc, Boolean mandatory, StarlarkThread thread) throws EvalException { - return new FakeDescriptor(AttributeType.OUTPUT, doc, mandatory, ImmutableList.of(), defaultO); + public Descriptor outputAttribute(String doc, Boolean mandatory, StarlarkThread thread) + throws EvalException { + return new FakeDescriptor(AttributeType.OUTPUT, doc, mandatory, ImmutableList.of(), ""); } @Override public Descriptor outputListAttribute( Boolean allowEmpty, - Object defaultList, String doc, Boolean mandatory, Boolean nonEmpty, StarlarkThread thread) throws EvalException { - return new FakeDescriptor( - AttributeType.OUTPUT_LIST, doc, mandatory, ImmutableList.of(), defaultList); + return new FakeDescriptor(AttributeType.OUTPUT_LIST, doc, mandatory, ImmutableList.of(), ""); } @Override 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 e80bf34f90dd50..6a183e80328e03 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 @@ -154,7 +154,6 @@ private static StarlarkSemanticsOptions buildRandomOptions(Random rand) throws E "--incompatible_new_actions_api=" + rand.nextBoolean(), "--incompatible_no_attr_license=" + rand.nextBoolean(), "--incompatible_no_implicit_file_export=" + 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(), @@ -209,7 +208,6 @@ private static StarlarkSemantics buildRandomSemantics(Random rand) { .incompatibleNewActionsApi(rand.nextBoolean()) .incompatibleNoAttrLicense(rand.nextBoolean()) .incompatibleNoImplicitFileExport(rand.nextBoolean()) - .incompatibleNoOutputAttrDefault(rand.nextBoolean()) .incompatibleNoRuleOutputsParam(rand.nextBoolean()) .incompatibleNoSupportToolsInActionInputs(rand.nextBoolean()) .incompatibleNoTargetOutputGroup(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 774f52e88d4fa9..367341f5bb1bd4 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 @@ -1239,8 +1239,6 @@ public void testRuleClassImplicitOutputFunctionPrints() throws Exception { @Test public void testNoOutputAttrDefault() throws Exception { - setSkylarkSemanticsOptions("--incompatible_no_output_attr_default=true"); - scratch.file( "test/extension.bzl", "load('//myinfo:myinfo.bzl', 'MyInfo')", @@ -1263,15 +1261,11 @@ public void testNoOutputAttrDefault() throws Exception { reporter.removeHandler(failFastHandler); getConfiguredTarget("//test:r"); - assertContainsEvent( - "parameter 'default' is deprecated and will be removed soon. It may be " - + "temporarily re-enabled by setting --incompatible_no_output_attr_default=false"); + assertContainsEvent("got unexpected keyword argument 'default'"); } @Test public void testNoOutputListAttrDefault() throws Exception { - setSkylarkSemanticsOptions("--incompatible_no_output_attr_default=true"); - scratch.file( "test/extension.bzl", "def custom_rule_impl(ctx):", @@ -1288,45 +1282,7 @@ public void testNoOutputListAttrDefault() throws Exception { reporter.removeHandler(failFastHandler); getConfiguredTarget("//test:r"); - assertContainsEvent( - "parameter 'default' is deprecated and will be removed soon. It may be " - + "temporarily re-enabled by setting --incompatible_no_output_attr_default=false"); - } - - @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", - "load('//myinfo:myinfo.bzl', 'MyInfo')", - "def custom_rule_impl(ctx):", - " out_file = ctx.actions.declare_file(ctx.attr._o1.name)", - " ctx.actions.write(output=out_file, content='hi')", - " return [MyInfo(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"); - StructImpl myInfo = getMyInfoFromTarget(target); - assertThat(myInfo.getValue("o1")) - .isEqualTo(Label.parseAbsoluteUnchecked("//test/skylark:foo.txt")); - assertThat(myInfo.getValue("o2")).isEqualTo(StarlarkList.empty()); + assertContainsEvent("got unexpected keyword argument 'default'"); } @Test diff --git a/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/EnablingAndDisablingFlag.java b/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/EnablingAndDisablingFlag.java index 13d6c91f92119b..27e3882bf6722b 100644 --- a/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/EnablingAndDisablingFlag.java +++ b/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/EnablingAndDisablingFlag.java @@ -31,8 +31,8 @@ public class EnablingAndDisablingFlag implements StarlarkValue { @Param(name = "one", type = String.class, named = true), @Param(name = "two", type = Integer.class, named = true), }, - enableOnlyWithFlag = FlagIdentifier.INCOMPATIBLE_NO_OUTPUT_ATTR_DEFAULT, - disableWithFlag = FlagIdentifier.INCOMPATIBLE_NO_OUTPUT_ATTR_DEFAULT) + enableOnlyWithFlag = FlagIdentifier.INCOMPATIBLE_APPLICABLE_LICENSES, + disableWithFlag = FlagIdentifier.INCOMPATIBLE_APPLICABLE_LICENSES) public String someMethod(String one, Integer two) { return "foo"; }