Skip to content

Commit

Permalink
Call parameter "repeatable"
Browse files Browse the repository at this point in the history
  • Loading branch information
fmeum committed Jun 30, 2022
1 parent f581749 commit e04ea41
Show file tree
Hide file tree
Showing 7 changed files with 45 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import com.google.devtools.build.lib.analysis.config.ExecutionTransitionFactory;
import com.google.devtools.build.lib.packages.BuildSetting;
import com.google.devtools.build.lib.starlarkbuildapi.StarlarkConfigApi;
import net.starlark.java.eval.EvalException;
import net.starlark.java.eval.Printer;
import net.starlark.java.eval.Starlark;

Expand All @@ -44,8 +45,11 @@ public BuildSetting stringSetting(Boolean flag, Boolean allowMultiple) {
}

@Override
public BuildSetting stringListSetting(Boolean flag, Boolean repeated) {
return BuildSetting.create(flag, STRING_LIST, false, repeated);
public BuildSetting stringListSetting(Boolean flag, Boolean repeatable) throws EvalException {
if (repeatable && !flag) {
throw Starlark.errorf("'repeatable' can only be set for a setting with 'flag = True'");
}
return BuildSetting.create(flag, STRING_LIST, false, repeatable);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,18 @@ public class BuildSetting implements BuildSettingApi {
private final boolean isFlag;
private final Type<?> type;
private final boolean allowMultiple;
private final boolean repeated;
private final boolean repeatable;

private BuildSetting(boolean isFlag, Type<?> type, boolean allowMultiple, boolean repeated) {
private BuildSetting(boolean isFlag, Type<?> type, boolean allowMultiple, boolean repeatable) {
this.isFlag = isFlag;
this.type = type;
this.allowMultiple = allowMultiple;
this.repeated = repeated;
this.repeatable = repeatable;
}

public static BuildSetting create(boolean isFlag, Type<?> type, boolean allowMultiple, boolean repeated) {
return new BuildSetting(isFlag, type, allowMultiple, repeated);
public static BuildSetting create(boolean isFlag, Type<?> type, boolean allowMultiple,
boolean repeatable) {
return new BuildSetting(isFlag, type, allowMultiple, repeatable);
}

public static BuildSetting create(boolean isFlag, Type<?> type) {
Expand All @@ -60,7 +61,7 @@ public boolean allowsMultiple() {
return allowMultiple;
}

public boolean usesRepeatedStyle() { return repeated; }
public boolean isRepeatableFlag() { return repeatable; }

@Override
public void repr(Printer printer) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ public void parse(ExtendedEventHandler eventHandler) throws OptionsParsingExcept
String.format("Unrecognized option: %s=%s", loadedFlag, unparsedValue));
}
Type<?> type = buildSetting.getType();
if (buildSetting.usesRepeatedStyle()) {
if (buildSetting.isRepeatableFlag()) {
type = Preconditions.checkNotNull(type.getListElementType());
}
Converter<?> converter = BUILD_SETTING_CONVERTERS.get(type);
Expand All @@ -182,7 +182,7 @@ public void parse(ExtendedEventHandler eventHandler) throws OptionsParsingExcept
loadedFlag, unparsedValue, unparsedValue, type),
e);
}
if (buildSetting.allowsMultiple() || buildSetting.usesRepeatedStyle()) {
if (buildSetting.allowsMultiple() || buildSetting.isRepeatableFlag()) {
List<Object> newValue;
if (buildSettingWithTargetAndValue.containsKey(loadedFlag)) {
newValue =
Expand Down Expand Up @@ -375,7 +375,7 @@ public OptionsParser getNativeOptionsParserFortesting() {

public boolean checkIfParsedOptionAllowsMultiple(String option) {
BuildSetting setting = parsedBuildSettings.get(option);
return setting.allowsMultiple() || setting.usesRepeatedStyle();
return setting.allowsMultiple() || setting.isRepeatableFlag();
}

public Type<?> getParsedOptionType(String option) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import net.starlark.java.annot.ParamType;
import net.starlark.java.annot.StarlarkBuiltin;
import net.starlark.java.annot.StarlarkMethod;
import net.starlark.java.eval.EvalException;
import net.starlark.java.eval.NoneType;
import net.starlark.java.eval.StarlarkValue;

Expand Down Expand Up @@ -91,7 +92,7 @@ public interface StarlarkConfigApi extends StarlarkValue {
defaultValue = "False",
doc =
"Deprecated, use a <code>string_list</code> setting with"
+ " <code>repeated = True</code> instead. If set, this flag is allowed to be"
+ " <code>repeatable = True</code> instead. If set, this flag is allowed to be"
+ " set multiple times on the command line. The Value of the flag as accessed"
+ " in transitions and build setting implementation function will be a list of"
+ " strings. Insertion order and repeated values are both maintained. This list"
Expand All @@ -115,7 +116,7 @@ public interface StarlarkConfigApi extends StarlarkValue {
named = true,
positional = false),
@Param(
name = "repeated",
name = "repeatable",
defaultValue = "False",
doc =
"If set, instead of expecting a comma-separated value, this flag is allowed to be"
Expand All @@ -126,7 +127,7 @@ public interface StarlarkConfigApi extends StarlarkValue {
named = true,
positional = false)
})
BuildSettingApi stringListSetting(Boolean flag, Boolean repeated);
BuildSettingApi stringListSetting(Boolean flag, Boolean repeatable) throws EvalException;

/** The API for build setting descriptors. */
@StarlarkBuiltin(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1775,14 +1775,14 @@ public void buildsettings_allowMultipleWorks() throws Exception {
}

@Test
public void buildsettings_repeatedWorks() throws Exception {
public void buildsettings_repeatableWorks() throws Exception {
scratch.file(
"test/build_settings.bzl",
"def _impl(ctx):",
" return []",
"string_list_flag = rule(",
" implementation = _impl,",
" build_setting = config.string_list(flag = True, repeated = True),",
" build_setting = config.string_list(flag = True, repeatable = True),",
")");
scratch.file(
"test/BUILD",
Expand All @@ -1794,10 +1794,31 @@ public void buildsettings_repeatedWorks() throws Exception {
" },",
")",
"string_list_flag(name = 'cheese', build_setting_default = ['gouda'])");

useConfiguration(ImmutableMap.of("//test:cheese", ImmutableList.of("pepperjack", "brie")));
assertThat(getConfigMatchingProvider("//test:match").matches()).isTrue();
}

@Test
public void buildsettings_repeatableWithoutFlagErrors() throws Exception {
scratch.file(
"test/build_settings.bzl",
"def _impl(ctx):",
" return []",
"string_list_setting = rule(",
" implementation = _impl,",
" build_setting = config.string_list(repeatable = True),",
")");
scratch.file(
"test/BUILD",
"load('//test:build_settings.bzl', 'string_list_setting')",
"string_list_setting(name = 'cheese', build_setting_default = ['gouda'])");

reporter.removeHandler(failFastHandler);
getConfiguredTarget("//test:cheese");
assertContainsEvent("'repeatable' can only be set for a setting with 'flag = True'");
}

@Test
public void notBuildSettingOrFeatureFlag() throws Exception {
scratch.file(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,7 @@ public void testRepeatedStringListFlag() throws Exception {
" return []",
"repeated_flag = rule(",
" implementation = _build_setting_impl,",
" build_setting = config.string_list(flag=True, repeated=True)",
" build_setting = config.string_list(flag=True, repeatable=True)",
")");
scratch.file(
"test/BUILD",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3036,7 +3036,7 @@ public void testBuildSettingValue_isRepeatedSetting() throws Exception {
"",
"string_list_flag = rule(",
" implementation = _impl,",
" build_setting = config.string_list(flag = True, repeated = True),",
" build_setting = config.string_list(flag = True, repeatable = True),",
")");
scratch.file(
"test/BUILD",
Expand Down

0 comments on commit e04ea41

Please sign in to comment.