Skip to content

Commit

Permalink
Delete the flag --incompatible_no_output_attr_default.
Browse files Browse the repository at this point in the history
#7950

RELNOTES: The flag `incompatible_no_output_attr_default` is removed.
PiperOrigin-RevId: 302635771
  • Loading branch information
Googler authored and copybara-github committed Mar 24, 2020
1 parent d0ad67e commit 0a60a1b
Show file tree
Hide file tree
Showing 8 changed files with 10 additions and 117 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -680,24 +680,19 @@ public Descriptor boolAttribute(

@Override
public Descriptor outputAttribute(
Object defaultValue, // Label | StarlarkFunction
String doc,
Boolean mandatory,
StarlarkThread thread)
throws EvalException {
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,
Expand All @@ -708,8 +703,6 @@ public Descriptor outputListAttribute(
return createAttrDescriptor(
"output_list",
optionMap(
DEFAULT_ARG,
defaultValue,
MANDATORY_ARG,
mandatory,
NON_EMPTY_ARG,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -704,7 +690,6 @@ public StarlarkSemantics toSkylarkSemantics() {
.incompatibleNewActionsApi(incompatibleNewActionsApi)
.incompatibleNoAttrLicense(incompatibleNoAttrLicense)
.incompatibleNoImplicitFileExport(incompatibleNoImplicitFileExport)
.incompatibleNoOutputAttrDefault(incompatibleNoOutputAttrDefault)
.incompatibleNoRuleOutputsParam(incompatibleNoRuleOutputsParam)
.incompatibleNoSupportToolsInActionInputs(incompatibleNoSupportToolsInActionInputs)
.incompatibleNoTargetOutputGroup(incompatibleNoTargetOutputGroup)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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(
Expand All @@ -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,
Expand All @@ -808,7 +781,6 @@ Descriptor outputAttribute(
useStarlarkThread = true)
Descriptor outputListAttribute(
Boolean allowEmpty,
Object defaultValue,
String doc,
Boolean mandatory,
Boolean nonEmpty,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -336,7 +330,6 @@ public static Builder builderWithDefaults() {
.incompatibleNewActionsApi(true)
.incompatibleNoAttrLicense(true)
.incompatibleNoImplicitFileExport(false)
.incompatibleNoOutputAttrDefault(true)
.incompatibleNoRuleOutputsParam(false)
.incompatibleNoSupportToolsInActionInputs(true)
.incompatibleNoTargetOutputGroup(true)
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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')",
Expand All @@ -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):",
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
}
Expand Down

0 comments on commit 0a60a1b

Please sign in to comment.