Skip to content

Commit

Permalink
Starlarkify all starlark build configurations-related work thus far (…
Browse files Browse the repository at this point in the history
…s/skylark/starlark)

#5577

PiperOrigin-RevId: 223357995
  • Loading branch information
juliexxia authored and Copybara-Service committed Nov 29, 2018
1 parent 4da4591 commit 692d148
Show file tree
Hide file tree
Showing 18 changed files with 145 additions and 153 deletions.

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -587,7 +587,7 @@ public Object getBuildSettingValue() throws EvalException {
ruleLabelCanonicalName));
}
ImmutableMap<String, Object> skylarkFlagSettings =
ruleContext.getConfiguration().getOptions().getSkylarkOptions();
ruleContext.getConfiguration().getOptions().getStarlarkOptions();

Type<?> buildSettingType =
ruleContext.getRule().getRuleClassObject().getBuildSetting().getType();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,12 @@
import static com.google.devtools.build.lib.syntax.Type.STRING_LIST;

import com.google.devtools.build.lib.packages.BuildSetting;
import com.google.devtools.build.lib.skylarkbuildapi.SkylarkConfigApi;
import com.google.devtools.build.lib.skylarkbuildapi.StarlarkConfigApi;
import com.google.devtools.build.lib.skylarkinterface.SkylarkPrinter;

/**
* Skylark namespace for creating build settings.
* TODO(juliexxia): Consider adding more types of build settings, specifically other label types.
*/
public class SkylarkConfig implements SkylarkConfigApi {
/** Starlark namespace for creating build settings. */
// TODO(juliexxia): Consider adding more types of build settings, specifically other label types.
public class StarlarkConfig implements StarlarkConfigApi {

@Override
public BuildSetting intSetting(Boolean flag) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public BuildOptions patch(BuildOptions originalOptions) {
BuildOptions.Builder builder = BuildOptions.builder();
for (FragmentOptions options : originalOptions.getNativeOptions()) {
if (!(options instanceof TestOptions)) {
builder.add(options);
builder.addFragmentOptions(options);
}
}
return builder.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
public class BuildRequest implements OptionsProvider {
private final UUID id;
private final LoadingCache<Class<? extends OptionsBase>, Optional<OptionsBase>> optionsCache;
private final Map<String, Object> skylarkOptions;
private final Map<String, Object> starlarkOptions;

/** A human-readable description of all the non-default option settings. */
private final String optionsDescription;
Expand Down Expand Up @@ -107,17 +107,16 @@ public Optional<OptionsBase> load(Class<? extends OptionsBase> key) throws Excep
return Optional.fromNullable(result);
}
});
this.skylarkOptions = options.getSkylarkOptions();
this.starlarkOptions = options.getStarlarkOptions();

for (Class<? extends OptionsBase> optionsClass : MANDATORY_OPTIONS) {
Preconditions.checkNotNull(getOptions(optionsClass));
}
}


@Override
public Map<String, Object> getSkylarkOptions() {
return skylarkOptions;
public Map<String, Object> getStarlarkOptions() {
return starlarkOptions;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
package com.google.devtools.build.lib.packages;

import com.google.common.annotations.VisibleForTesting;
import com.google.devtools.build.lib.skylarkbuildapi.SkylarkConfigApi.BuildSettingApi;
import com.google.devtools.build.lib.skylarkbuildapi.StarlarkConfigApi.BuildSettingApi;
import com.google.devtools.build.lib.skylarkinterface.SkylarkPrinter;
import com.google.devtools.build.lib.syntax.Type;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import com.google.devtools.build.lib.analysis.BaseRuleClasses;
import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider.RuleSet;
import com.google.devtools.build.lib.analysis.skylark.SkylarkConfig;
import com.google.devtools.build.lib.analysis.skylark.StarlarkConfig;
import com.google.devtools.build.lib.rules.core.CoreRules;
import com.google.devtools.build.lib.rules.platform.PlatformRules;
import com.google.devtools.build.lib.skylarkbuildapi.config.ConfigBootstrap;
Expand All @@ -44,9 +44,7 @@ public void init(ConfiguredRuleClassProvider.Builder builder) {
builder.addRuleDefinition(new ConfigRuleClasses.ConfigFeatureFlagRule());
builder.addSkylarkBootstrap(
new ConfigBootstrap(
new ConfigSkylarkCommon(),
new SkylarkConfig(),
new ConfigGlobalLibrary()));
new ConfigSkylarkCommon(), new StarlarkConfig(), new ConfigGlobalLibrary()));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.events.Location;
import com.google.devtools.build.lib.skylarkbuildapi.SkylarkConfigApi.BuildSettingApi;
import com.google.devtools.build.lib.skylarkbuildapi.StarlarkConfigApi.BuildSettingApi;
import com.google.devtools.build.lib.skylarkinterface.Param;
import com.google.devtools.build.lib.skylarkinterface.ParamType;
import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
/**
* The "config" module of the Build API.
*
* <p>This exposes methods to describe what kind of build setting (if any) a skylark rule is using
* <p>This exposes methods to describe what kind of build setting (if any) a starlark rule is using
* the {@code build_setting} attr of the {@code rule(...)} function.
*/
@SkylarkModule(
Expand All @@ -46,9 +46,9 @@
+ " build_setting = config.int(flag = True),\n"
+ " ...\n"
+ " )</pre>")
// TODO(juliexxia): Create formal documentation for skylark build configuration efforts
// TODO(juliexxia): Create formal documentation for starlark build configuration efforts
// (b/112545834)
public interface SkylarkConfigApi extends SkylarkValue {
public interface StarlarkConfigApi extends SkylarkValue {

static final String FLAG_ARG = "flag";
static final String FLAG_ARG_DOC =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

import com.google.common.collect.ImmutableMap.Builder;
import com.google.devtools.build.lib.skylarkbuildapi.Bootstrap;
import com.google.devtools.build.lib.skylarkbuildapi.SkylarkConfigApi;
import com.google.devtools.build.lib.skylarkbuildapi.StarlarkConfigApi;
import com.google.devtools.build.lib.syntax.Runtime;

/**
Expand All @@ -25,22 +25,22 @@
public class ConfigBootstrap implements Bootstrap {

private final ConfigSkylarkCommonApi configSkylarkCommonApi;
private final SkylarkConfigApi skylarkConfigApi;
private final StarlarkConfigApi starlarkConfigApi;
private final ConfigGlobalLibraryApi configGlobalLibrary;

public ConfigBootstrap(
ConfigSkylarkCommonApi configSkylarkCommonApi,
SkylarkConfigApi skylarkConfigApi,
StarlarkConfigApi starlarkConfigApi,
ConfigGlobalLibraryApi configGlobalLibrary) {
this.configSkylarkCommonApi = configSkylarkCommonApi;
this.skylarkConfigApi = skylarkConfigApi;
this.starlarkConfigApi = starlarkConfigApi;
this.configGlobalLibrary = configGlobalLibrary;
}

@Override
public void addBindingsToBuilder(Builder<String, Object> builder) {
builder.put("config_common", configSkylarkCommonApi);
builder.put("config", skylarkConfigApi);
builder.put("config", starlarkConfigApi);
Runtime.setupSkylarkLibrary(builder, configGlobalLibrary);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

package com.google.devtools.build.skydoc.fakebuildapi;

import com.google.devtools.build.lib.skylarkbuildapi.SkylarkConfigApi.BuildSettingApi;
import com.google.devtools.build.lib.skylarkbuildapi.StarlarkConfigApi.BuildSettingApi;
import com.google.devtools.build.lib.skylarkinterface.SkylarkPrinter;

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,11 @@

package com.google.devtools.build.skydoc.fakebuildapi;

import com.google.devtools.build.lib.skylarkbuildapi.SkylarkConfigApi;
import com.google.devtools.build.lib.skylarkbuildapi.StarlarkConfigApi;
import com.google.devtools.build.lib.skylarkinterface.SkylarkPrinter;

/**
* Fake implementation of {@link SkylarkConfigApi}.
*/
public class FakeConfigApi implements SkylarkConfigApi {
/** Fake implementation of {@link StarlarkConfigApi}. */
public class FakeConfigApi implements StarlarkConfigApi {

@Override
public BuildSettingApi intSetting(Boolean flag) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ public static OptionsParser newOptionsParser(OpaqueOptionsData optionsData) {
private final List<String> residue = new ArrayList<String>();
private final List<String> postDoubleDashResidue = new ArrayList<>();
private boolean allowResidue = true;
private Map<String, Object> skylarkOptions = new HashMap<>();
private Map<String, Object> starlarkOptions = new HashMap<>();

OptionsParser(OptionsData optionsData) {
impl = new OptionsParserImpl(optionsData);
Expand All @@ -197,13 +197,13 @@ public void setAllowResidue(boolean allowResidue) {
}

@Override
public Map<String, Object> getSkylarkOptions() {
return skylarkOptions;
public Map<String, Object> getStarlarkOptions() {
return starlarkOptions;
}

@VisibleForTesting
public void setSkylarkOptionsForTesting(Map<String, Object> skylarkOptions) {
this.skylarkOptions = skylarkOptions;
public void setStarlarkOptionsForTesting(Map<String, Object> starlarkOptions) {
this.starlarkOptions = starlarkOptions;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,19 @@
* a specific class, but not e.g. the residue any other information pertaining to the command line.
*/
public interface OptionsProvider {
public static final OptionsProvider EMPTY = new OptionsProvider() {
@Override @Nullable
public <O extends OptionsBase> O getOptions(Class<O> optionsClass) {
return null;
}
public static final OptionsProvider EMPTY =
new OptionsProvider() {
@Override
@Nullable
public <O extends OptionsBase> O getOptions(Class<O> optionsClass) {
return null;
}

@Override
public Map<String, Object> getSkylarkOptions() {
return ImmutableMap.of();
}
};
@Override
public Map<String, Object> getStarlarkOptions() {
return ImmutableMap.of();
}
};

/**
* Returns the options instance for the given {@code optionsClass}, that is,
Expand All @@ -44,13 +46,13 @@ public Map<String, Object> getSkylarkOptions() {
@Nullable <O extends OptionsBase> O getOptions(Class<O> optionsClass);

/**
* Returns the skylark options in a name:value map.
* Returns the starlark options in a name:value map.
*
* <p>These follow the basics of the option syntax, --<name>=<value> but are parsed and stored
* differently than native options based on <name> starting with "//". This is a sufficient
* demarcation between skylark flags and native flags for now since all skylark flags are targets
* and are identified by their package path. But in the future when we implement short names for
* skylark options, this will need to change.
* demarcation between starlark flags and native flags for now since all starlark flags are
* targets and are identified by their package path. But in the future when we implement short
* names for starlark options, this will need to change.
*/
Map<String, Object> getSkylarkOptions();
Map<String, Object> getStarlarkOptions();
}
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ public void applyDiff_nativeOptions() throws Exception {
}

@Test
public void optionsDiff_sameSkylarkOptions() throws Exception {
public void optionsDiff_sameStarlarkOptions() throws Exception {
String flagName = "//foo/flag";
String flagValue = "value";
BuildOptions one = BuildOptions.of(ImmutableMap.of(flagName, flagValue));
Expand All @@ -197,7 +197,7 @@ public void optionsDiff_sameSkylarkOptions() throws Exception {
}

@Test
public void optionsDiff_differentSkylarkOptions() throws Exception {
public void optionsDiff_differentStarlarkOptions() throws Exception {
String flagName = "//bar/flag";
String flagValueOne = "valueOne";
String flagValueTwo = "valueTwo";
Expand All @@ -207,15 +207,15 @@ public void optionsDiff_differentSkylarkOptions() throws Exception {
OptionsDiff diff = BuildOptions.diff(one, two);

assertThat(diff.areSame()).isFalse();
assertThat(diff.getSkylarkFirstForTesting().keySet())
.isEqualTo(diff.getSkylarkSecondForTesting().keySet());
assertThat(diff.getSkylarkFirstForTesting().keySet()).containsExactly(flagName);
assertThat(diff.getSkylarkFirstForTesting().values()).containsExactly(flagValueOne);
assertThat(diff.getSkylarkSecondForTesting().values()).containsExactly(flagValueTwo);
assertThat(diff.getStarlarkFirstForTesting().keySet())
.isEqualTo(diff.getStarlarkSecondForTesting().keySet());
assertThat(diff.getStarlarkFirstForTesting().keySet()).containsExactly(flagName);
assertThat(diff.getStarlarkFirstForTesting().values()).containsExactly(flagValueOne);
assertThat(diff.getStarlarkSecondForTesting().values()).containsExactly(flagValueTwo);
}

@Test
public void optionsDiff_extraSkylarkOptions() throws Exception {
public void optionsDiff_extraStarlarkOptions() throws Exception {
String flagNameOne = "//extra/flag/one";
String flagNameTwo = "//extra/flag/two";
String flagValue = "foo";
Expand All @@ -225,13 +225,13 @@ public void optionsDiff_extraSkylarkOptions() throws Exception {
OptionsDiff diff = BuildOptions.diff(one, two);

assertThat(diff.areSame()).isFalse();
assertThat(diff.getExtraSkylarkOptionsFirstForTesting()).containsExactly(flagNameOne);
assertThat(diff.getExtraSkylarkOptionsSecondForTesting().entrySet())
assertThat(diff.getExtraStarlarkOptionsFirstForTesting()).containsExactly(flagNameOne);
assertThat(diff.getExtraStarlarkOptionsSecondForTesting().entrySet())
.containsExactly(Maps.immutableEntry(flagNameTwo, flagValue));
}

@Test
public void applyDiff_sameSkylarkOptions() throws Exception {
public void applyDiff_sameStarlarkOptions() throws Exception {
String flagName = "//foo/flag";
String flagValue = "value";
BuildOptions one = BuildOptions.of(ImmutableMap.of(flagName, flagValue));
Expand All @@ -248,7 +248,7 @@ public void applyDiff_sameSkylarkOptions() throws Exception {
}

@Test
public void applyDiff_differentSkylarkOptions() throws Exception {
public void applyDiff_differentStarlarkOptions() throws Exception {
String flagName = "//bar/flag";
String flagValueOne = "valueOne";
String flagValueTwo = "valueTwo";
Expand All @@ -262,7 +262,7 @@ public void applyDiff_differentSkylarkOptions() throws Exception {
}

@Test
public void applyDiff_extraSkylarkOptions() throws Exception {
public void applyDiff_extraStarlarkOptions() throws Exception {
String flagNameOne = "//extra/flag/one";
String flagNameTwo = "//extra/flag/two";
String flagValue = "foo";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -358,8 +358,8 @@ private BuildConfigurationCollection createConfigurations(
optionsParser.parse(allArgs);
optionsParser.parse(args);

// TODO(juliexxia): when the skylark options parsing work goes in, add type verification here.
optionsParser.setSkylarkOptionsForTesting(skylarkOptions);
// TODO(juliexxia): when the starlark options parsing work goes in, add type verification here.
optionsParser.setStarlarkOptionsForTesting(skylarkOptions);

InvocationPolicyEnforcer optionsPolicyEnforcer =
getAnalysisMock().getInvocationPolicyEnforcer();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public <O extends OptionsBase> O getOptions(Class<O> optionsClass) {
}

@Override
public Map<String, Object> getSkylarkOptions() {
public Map<String, Object> getStarlarkOptions() {
return ImmutableMap.of();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ public <O extends OptionsBase> O getOptions(Class<O> optionsClass) {
}

@Override
public Map<String, Object> getSkylarkOptions() {
public Map<String, Object> getStarlarkOptions() {
return ImmutableMap.of();
}
}
Expand Down

0 comments on commit 692d148

Please sign in to comment.