Skip to content

Commit

Permalink
Parse and load starlark-defined options in full package-path form.
Browse files Browse the repository at this point in the history
This includes the the following features available to native options parsing:
* --(no)boolean_flag syntax
* --flag=value syntax
* --flag value syntax
* single-dash syntax
* for multiple values of same option, last option wins
* internal-only options are treated like they don't exist

There is potential here to combine some of the code in StarlarkOptionsParser.java with OptionsParserImpl.java since they support the same formats of flags. This is complicated by the fact that for native flags, you have a lot of information (type, internal vs external) before parsing whereas for starlark flags, you need to parse out the name first and load it to get this information. So I haven't done the combining in this CL.

This brings up an interesting point that now all commands that use starlark flags will need to do loading. For now we limit starlark parsing to commands that already do building (which is possibly all the commands we care about).

This behavior is still guarded behind --experimental_build_setting_api

Work towards #5574 and #5577

PiperOrigin-RevId: 223600443
  • Loading branch information
juliexxia authored and Copybara-Service committed Dec 1, 2018
1 parent b7de274 commit cb9b2af
Show file tree
Hide file tree
Showing 12 changed files with 1,012 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -576,6 +576,8 @@ public BuildConfiguration getHostConfiguration() throws EvalException {
return ruleContext.getHostConfiguration();
}

// TODO(juliexxia): special-case label-typed build settings so they return the providers of the
// target represented by the label instead of the actual label.
@Override
@Nullable
public Object getBuildSettingValue() throws EvalException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -312,20 +312,7 @@ private BlazeCommandResult execExclusively(
// Early exit. We need to guarantee that the ErrOut and Reporter setup below never error out,
// so any invariants they need must be checked before this point.
if (!earlyExitCode.equals(ExitCode.SUCCESS)) {
// Partial replay of the printed events before we exit.
PrintingEventHandler printingEventHandler =
new PrintingEventHandler(outErr, EventKind.ALL_EVENTS);
for (String note : optionHandler.getRcfileNotes()) {
printingEventHandler.handle(Event.info(note));
}
for (Event event : storedEventHandler.getEvents()) {
printingEventHandler.handle(event);
}
for (Postable post : storedEventHandler.getPosts()) {
env.getEventBus().post(post);
}
result = BlazeCommandResult.exitCode(earlyExitCode);
return result;
return replayEarlyExitEvents(outErr, optionHandler, storedEventHandler, env, earlyExitCode);
}

Reporter reporter = env.getReporter();
Expand Down Expand Up @@ -480,6 +467,13 @@ private BlazeCommandResult execExclusively(
}
}

// Parse starlark options.
earlyExitCode = optionHandler.parseStarlarkOptions(env, storedEventHandler);
if (!earlyExitCode.equals(ExitCode.SUCCESS)) {
return replayEarlyExitEvents(outErr, optionHandler, storedEventHandler, env, earlyExitCode);
}
options = optionHandler.getOptionsResult();

result = command.exec(env, options);
ExitCode moduleExitCode = env.precompleteCommand(result.getExitCode());
// If Blaze did not suffer an infrastructure failure, check for errors in modules.
Expand Down Expand Up @@ -512,6 +506,26 @@ private BlazeCommandResult execExclusively(
}
}

private static BlazeCommandResult replayEarlyExitEvents(
OutErr outErr,
BlazeOptionHandler optionHandler,
StoredEventHandler storedEventHandler,
CommandEnvironment env,
ExitCode earlyExitCode) {
PrintingEventHandler printingEventHandler =
new PrintingEventHandler(outErr, EventKind.ALL_EVENTS);
for (String note : optionHandler.getRcfileNotes()) {
printingEventHandler.handle(Event.info(note));
}
for (Event event : storedEventHandler.getEvents()) {
printingEventHandler.handle(event);
}
for (Postable post : storedEventHandler.getPosts()) {
env.getEventBus().post(post);
}
return BlazeCommandResult.exitCode(earlyExitCode);
}

/**
* For testing ONLY. Same as {@link #exec(InvocationPolicy, List, OutErr, LockingMode, String,
* long, Optional<List<Pair<String, String>>>)}, but automatically uses the current time.
Expand All @@ -529,7 +543,6 @@ public BlazeCommandResult exec(List<String> args, String clientDescription, OutE
Optional.empty() /* startupOptionBundles */);
}


private OutErr bufferOut(OutErr outErr, boolean fully) {
OutputStream wrappedOut;
if (fully) {
Expand Down Expand Up @@ -560,7 +573,6 @@ private OutErr ansiStripErr(OutErr outErr) {
return OutErr.create(outErr.getOutputStream(), wrappedErr);
}


private OutErr tee(OutErr outErr, List<OutErr> additionalOutErrs) {
if (additionalOutErrs.isEmpty()) {
return outErr;
Expand Down Expand Up @@ -600,7 +612,7 @@ private OptionsParser createOptionsParser(BlazeCommand command)
throw new IllegalStateException(e);
}
Command annotation = command.getClass().getAnnotation(Command.class);
OptionsParser parser = OptionsParser.newOptionsParser(optionsData);
OptionsParser parser = OptionsParser.newOptionsParser(optionsData, "--//");
parser.setAllowResidue(annotation.allowResidue());
return parser;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,30 @@ private void parseArgsAndConfigs(List<String> args, ExtendedEventHandler eventHa
expandConfigOptions(eventHandler, commandToRcArgs);
}

/**
* TODO(bazel-team): When we move BuildConfiguration.Options options to be defined in starlark,
* make sure they're not passed in here during {@link #getOptionsResult}.
*/
ExitCode parseStarlarkOptions(CommandEnvironment env, ExtendedEventHandler eventHandler) {
// For now, restrict starlark options to commands that already build to ensure that loading
// will work. We may want to open this up to other commands in the future. The "info"
// and "clean" commands have builds=true set in their annotation but don't actually do any
// building (b/120041419).
if (!commandAnnotation.builds()
|| commandAnnotation.name().equals("info")
|| commandAnnotation.name().equals("clean")) {
return ExitCode.SUCCESS;
}
try {
StarlarkOptionsParser.newStarlarkOptionsParser(env, optionsParser, runtime)
.parse(commandAnnotation, eventHandler);
} catch (OptionsParsingException e) {
eventHandler.handle(Event.error(e.getMessage()));
return ExitCode.COMMAND_LINE_ERROR;
}
return ExitCode.SUCCESS;
}

/**
* Parses the options, taking care not to generate any output to outErr, return, or throw an
* exception.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -584,6 +584,22 @@ public void setupPackageCache(OptionsProvider options,
options);
}

public void syncPackageLoading(
PackageCacheOptions packageCacheOptions,
SkylarkSemanticsOptions skylarkSemanticsOptions,
String defaultsPackageContents)
throws AbruptExitException {
getSkyframeExecutor()
.syncPackageLoading(
packageCacheOptions,
packageLocator,
skylarkSemanticsOptions,
defaultsPackageContents,
getCommandId(),
clientEnv,
timestampGranularityMonitor);
}

public void recordLastExecutionTime() {
workspace.recordLastExecutionTime(getCommandStartTime());
}
Expand Down
Loading

0 comments on commit cb9b2af

Please sign in to comment.