Skip to content

Commit

Permalink
FlagSetFunction returns a list of options in command line form, inste…
Browse files Browse the repository at this point in the history
…ad of a BuildOptions, and use the OptionsParser to parse these options.

This is the groundwork for emitting an event that contains the updated options.

PiperOrigin-RevId: 707924039
Change-Id: I8c99cbaa9d267c58ae685f22595fe7134958b051
  • Loading branch information
susinmotion authored and copybara-github committed Dec 19, 2024
1 parent 2d01556 commit 4c649ba
Show file tree
Hide file tree
Showing 15 changed files with 193 additions and 123 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ private BuildResult fetchTargets(OptionsParsingResult options, List<String> targ
.setStartTimeMillis(env.getCommandStartTime())
.build();

BuildResult result = new BuildTool(env).processRequest(request, null);
BuildResult result = new BuildTool(env).processRequest(request, null, options);
if (!result.getSuccess()) {
throw new TargetFetcherException(
"Fetching some target dependencies failed with errors: "
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,11 +180,14 @@ public static AnalysisResult execute(

/** A simple container for storing processed evaluation results of the PROJECT.scl file. */
record ProjectEvaluationResult(
BuildOptions buildOptions, Optional<PathFragmentPrefixTrie> activeDirectoriesMatcher) {
ImmutableSet<String> buildOptions,
Optional<PathFragmentPrefixTrie> activeDirectoriesMatcher,
Optional<Label> projectFile) {

public ProjectEvaluationResult {
checkArgument(buildOptions != null, "buildOptions cannot be null.");
checkArgument(activeDirectoriesMatcher != null, "activeDirectoriesMatcher cannot be null.");
checkArgument(projectFile != null, "projectFile cannot be null.");
}
}

Expand Down Expand Up @@ -230,7 +233,7 @@ static ProjectEvaluationResult evaluateProjectFile(

if (featureFlags.isEmpty()) {
// All feature flags disabled.
return new ProjectEvaluationResult(buildOptions, Optional.empty());
return new ProjectEvaluationResult(ImmutableSet.of(), Optional.empty(), Optional.empty());
}

Label projectFile = null;
Expand Down Expand Up @@ -273,17 +276,20 @@ static ProjectEvaluationResult evaluateProjectFile(

if (featureFlags.contains(SCL_CONFIG) && projectFile != null) {
// Do not apply canonical configurations if the project file doesn't exist.
buildOptions =
ImmutableSet<String> options =
BuildTool.applySclConfigs(
buildOptions,
userOptions,
projectFile,
request.getBuildOptions().enforceProjectConfigs,
env.getSkyframeExecutor(),
env.getReporter());
return new ProjectEvaluationResult(
options, Optional.ofNullable(projectMatcher), Optional.ofNullable(projectFile));
}

return new ProjectEvaluationResult(buildOptions, Optional.ofNullable(projectMatcher));
return new ProjectEvaluationResult(
ImmutableSet.of(), Optional.ofNullable(projectMatcher), Optional.ofNullable(projectFile));
}

static void postAbortedEventsForSkippedTargets(
Expand Down Expand Up @@ -427,7 +433,7 @@ private static void postTopLevelStatusEvents(
TestAnalyzedEvent.create(
configuredTarget,
configurationMap.get(configuredTarget.getConfigurationKey()),
/*isSkipped=*/ analysisResult.getTargetsToSkip().contains(configuredTarget)));
/* isSkipped= */ analysisResult.getTargetsToSkip().contains(configuredTarget)));
}
}

Expand Down Expand Up @@ -486,7 +492,8 @@ private static ImmutableSet<Label> getExplicitTargetPatterns(
List<String> requestedTargetPatterns,
boolean keepGoing,
int loadingPhaseThreads)
throws ViewCreationFailedException, RepositoryMappingResolutionException,
throws ViewCreationFailedException,
RepositoryMappingResolutionException,
InterruptedException {
ImmutableSet.Builder<Label> explicitTargetPatterns = ImmutableSet.builder();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,10 @@
import com.google.devtools.build.skyframe.IntVersion;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
import com.google.devtools.common.options.OptionPriority.PriorityCategory;
import com.google.devtools.common.options.OptionsParser;
import com.google.devtools.common.options.OptionsParsingException;
import com.google.devtools.common.options.OptionsParsingResult;
import com.google.devtools.common.options.RegexPatternOption;
import java.io.BufferedOutputStream;
import java.io.IOException;
Expand Down Expand Up @@ -196,7 +200,11 @@ public BuildTool(CommandEnvironment env, AnalysisPostProcessor postProcessor) {
* @param result the build result that is the mutable result of this build
* @param validator target validator
*/
public void buildTargets(BuildRequest request, BuildResult result, TargetValidator validator)
public void buildTargets(
BuildRequest request,
BuildResult result,
TargetValidator validator,
OptionsParser optionsParser)
throws BuildFailedException,
InterruptedException,
ViewCreationFailedException,
Expand All @@ -207,7 +215,8 @@ public void buildTargets(BuildRequest request, BuildResult result, TargetValidat
TestExecException,
ExitException,
PostExecutionDumpException,
RepositoryMappingResolutionException {
RepositoryMappingResolutionException,
OptionsParsingException {
try (SilentCloseable c = Profiler.instance().profile("validateOptions")) {
validateOptions(request);
}
Expand Down Expand Up @@ -239,25 +248,24 @@ public void buildTargets(BuildRequest request, BuildResult result, TargetValidat
evaluateProjectFile(
request, buildOptions, request.getUserOptions(), targetPatternPhaseValue, env);

if (!projectEvaluationResult.buildOptions().isEmpty()) {
optionsParser.parse(
PriorityCategory.COMMAND_LINE,
projectEvaluationResult.projectFile().get().toString(),
projectEvaluationResult.buildOptions().asList());
}
buildOptions = runtime.createBuildOptions(optionsParser);
var analysisCachingDeps =
RemoteAnalysisCachingDependenciesProviderImpl.forAnalysis(
env, projectEvaluationResult.activeDirectoriesMatcher());

if (env.withMergedAnalysisAndExecutionSourceOfTruth()) {
// a.k.a. Skymeld.
buildTargetsWithMergedAnalysisExecution(
request,
result,
targetPatternPhaseValue,
projectEvaluationResult.buildOptions(),
analysisCachingDeps);
request, result, targetPatternPhaseValue, buildOptions, analysisCachingDeps);
} else {
buildTargetsWithoutMergedAnalysisExecution(
request,
result,
targetPatternPhaseValue,
projectEvaluationResult.buildOptions(),
analysisCachingDeps);
request, result, targetPatternPhaseValue, buildOptions, analysisCachingDeps);
}

logAnalysisCachingStatsAndMaybeUploadFrontier(
Expand Down Expand Up @@ -687,8 +695,9 @@ private void reportExceptionError(Exception e) {
}
}

public BuildResult processRequest(BuildRequest request, TargetValidator validator) {
return processRequest(request, validator, /* postBuildCallback= */ null);
public BuildResult processRequest(
BuildRequest request, TargetValidator validator, OptionsParsingResult options) {
return processRequest(request, validator, /* postBuildCallback= */ null, options);
}

/**
Expand All @@ -709,18 +718,27 @@ public BuildResult processRequest(BuildRequest request, TargetValidator validato
* the request object are populated
* @param validator an optional target validator
* @param postBuildCallback an optional callback called after the build has been completed
* successfully
* successfully.
* @param options the options parsing result containing the options parsed so far, excluding those
* from flagsets. This will be cast to an {@link OptionsParser} in order to add any options
* from flagsets.
* @return the result as a {@link BuildResult} object
*/
public BuildResult processRequest(
BuildRequest request, TargetValidator validator, PostBuildCallback postBuildCallback) {
BuildRequest request,
TargetValidator validator,
PostBuildCallback postBuildCallback,
OptionsParsingResult options) {
BuildResult result = new BuildResult(request.getStartTime());
maybeSetStopOnFirstFailure(request, result);
Throwable crash = null;
DetailedExitCode detailedExitCode = null;
try {
try (SilentCloseable c = Profiler.instance().profile("buildTargets")) {
buildTargets(request, result, validator);
// This OptionsParsingResult is essentially a wrapper around the OptionsParser in
// https://github.com/bazelbuild/bazel/blob/master/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java#L341. Casting it back to
// an OptionsParser is safe, and necessary in order to add any options from flagsets.
buildTargets(request, result, validator, (OptionsParser) options);
}
detailedExitCode = DetailedExitCode.success();
if (postBuildCallback != null) {
Expand Down Expand Up @@ -986,8 +1004,8 @@ public static PathFragmentPrefixTrie getWorkingSetMatcherForSkyfocus(
return PathFragmentPrefixTrie.of(((ProjectValue) result.get(key)).getDefaultActiveDirectory());
}

/** Creates a BuildOptions class for the given options taken from an {@link OptionsProvider}. */
public static BuildOptions applySclConfigs(
/** Returns the set of options from the selected {@code projectFile} in command line format. */
public static ImmutableSet<String> applySclConfigs(
BuildOptions buildOptionsBeforeFlagSets,
ImmutableMap<String, String> userOptions,
Label projectFile,
Expand All @@ -1005,8 +1023,8 @@ public static BuildOptions applySclConfigs(
eventHandler,
skyframeExecutor);

// BuildOptions after Flagsets
return flagSetValue.getTopLevelBuildOptions();
// Options from the selected project config.
return flagSetValue.getOptionsFromFlagset();
}

private Reporter getReporter() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,9 @@ public BlazeCommandResult exec(CommandEnvironment env, OptionsParsingResult opti
}
try {
return BlazeCommandResult.detailedExitCode(
new BuildTool(env, aqueryBuildTool).processRequest(request, null).getDetailedExitCode());
new BuildTool(env, aqueryBuildTool)
.processRequest(request, null, options)
.getDetailedExitCode());
} catch (StackOverflowError e) {
String message = "Aquery output was too large to handle: " + query;
env.getReporter().handle(Event.error(message));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ public BlazeCommandResult exec(CommandEnvironment env, OptionsParsingResult opti
.build();
}
DetailedExitCode detailedExitCode =
new BuildTool(env).processRequest(request, null).getDetailedExitCode();
new BuildTool(env).processRequest(request, null, options).getDetailedExitCode();
return BlazeCommandResult.detailedExitCode(detailedExitCode);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ public BlazeCommandResult exec(CommandEnvironment env, OptionsParsingResult opti
.build();
DetailedExitCode detailedExitCode =
new BuildTool(env, new CqueryProcessor(expr, mainRepoTargetParser))
.processRequest(request, null)
.processRequest(request, null, options)
.getDetailedExitCode();
return BlazeCommandResult.detailedExitCode(detailedExitCode);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ private BuildResult gatherActionsForTargets(CommandEnvironment env, List<String>
.setTargets(targets)
.setStartTimeMillis(env.getCommandStartTime())
.build();
BuildResult result = new BuildTool(env).processRequest(request, null);
BuildResult result = new BuildTool(env).processRequest(request, null, options);
if (hasFatalBuildFailure(result)) {
return result;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,8 @@ private static BuiltTargets runBuild(
request,
(Collection<Target> tgts, boolean keepGoing) ->
validateTargets(
env.getReporter(), request.getTargets(), tgts, runUnder, keepGoing));
env.getReporter(), request.getTargets(), tgts, runUnder, keepGoing),
options);
if (!buildResult.getSuccess()) {
env.getReporter().handle(Event.error("Build failed. Not running target"));
throw new RunCommandException(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ private BlazeCommandResult doTest(
}
BuildRequest request = builder.build();

BuildResult buildResult = new BuildTool(env).processRequest(request, null);
BuildResult buildResult = new BuildTool(env).processRequest(request, null, options);

Collection<ConfiguredTarget> testTargets = buildResult.getTestTargets();
// TODO(bazel-team): don't handle isEmpty here or fix up a bunch of tests
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,8 @@ public BlazeCommandResult exec(CommandEnvironment env, OptionsParsingResult opti
/* validator= */ null,
successfulTargets ->
doMobileInstall(
env, options, runTargetArgs, successfulTargets, deployerRequestRef));
env, options, runTargetArgs, successfulTargets, deployerRequestRef),
options);
if (!result.getSuccess()) {
env.getReporter().handle(Event.error("Build failed. Not running mobile-install on target."));
return BlazeCommandResult.detailedExitCode(result.getDetailedExitCode());
Expand Down
Loading

0 comments on commit 4c649ba

Please sign in to comment.