diff options
author | Ulf Adams <ulfjack@google.com> | 2016-11-15 17:30:45 +0000 |
---|---|---|
committer | Kristina Chodorow <kchodorow@google.com> | 2016-11-16 15:56:25 +0000 |
commit | e134f794337054562d3119b2bf151fc83fcd9a66 (patch) | |
tree | 5059a70572bd4260bc284b78b296776ea0939dfc /src/main/java/com | |
parent | 461a772d45c2b7a7624b46d7aee44a460527b792 (diff) |
Code cleanup for interleaving target pattern eval and config creation.
--
MOS_MIGRATED_REVID=139209942
Diffstat (limited to 'src/main/java/com')
4 files changed, 27 insertions, 37 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java index 522f753586..8eafd9d6fc 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java @@ -150,10 +150,7 @@ public final class BuildTool { executionTool.init(); } - // Loading phase. - loadingResult = runLoadingPhase(request, validator); - - // Create the build configurations. + // Error out early if multi_cpus is set, but we're not in build or test command. if (!request.getMultiCpus().isEmpty()) { getReporter().handle(Event.warn( "The --experimental_multi_cpu option is _very_ experimental and only intended for " @@ -165,9 +162,25 @@ public final class BuildTool { + "'test' right now!"); } } - configurations = env.getSkyframeExecutor().createConfigurations( - env.getReporter(), runtime.getConfigurationFactory(), buildOptions, - request.getMultiCpus(), request.getViewOptions().keepGoing); + + // Exit if there are any pending exceptions from modules. + env.throwPendingException(); + + // Target pattern evaluation. + loadingResult = evaluateTargetPatterns(request, validator); + + // Exit if there are any pending exceptions from modules. + env.throwPendingException(); + + // Configuration creation. + configurations = + env.getSkyframeExecutor() + .createConfigurations( + env.getReporter(), + runtime.getConfigurationFactory(), + buildOptions, + request.getMultiCpus(), + request.getViewOptions().keepGoing); env.throwPendingException(); if (configurations.getTargetConfigurations().size() == 1) { @@ -389,14 +402,10 @@ public final class BuildTool { return !(request.getViewOptions().keepGoing && request.getExecutionOptions().testKeepGoing); } - @VisibleForTesting - protected final LoadingResult runLoadingPhase(final BuildRequest request, - final TargetValidator validator) - throws LoadingFailedException, TargetParsingException, InterruptedException, - AbruptExitException { + private final LoadingResult evaluateTargetPatterns( + final BuildRequest request, final TargetValidator validator) + throws LoadingFailedException, TargetParsingException, InterruptedException { Profiler.instance().markPhase(ProfilePhase.LOAD); - env.throwPendingException(); - initializeOutputFilter(request); final boolean keepGoing = request.getViewOptions().keepGoing; @@ -420,11 +429,9 @@ public final class BuildTool { request.getTargets(), env.getRelativeWorkingDirectory(), request.getLoadingOptions(), - runtime.createBuildOptions(request).getAllLabels(), keepGoing, request.shouldRunTests(), callback); - env.throwPendingException(); return result; } diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/LegacyLoadingPhaseRunner.java b/src/main/java/com/google/devtools/build/lib/pkgcache/LegacyLoadingPhaseRunner.java index 448faf661e..018e03208d 100644 --- a/src/main/java/com/google/devtools/build/lib/pkgcache/LegacyLoadingPhaseRunner.java +++ b/src/main/java/com/google/devtools/build/lib/pkgcache/LegacyLoadingPhaseRunner.java @@ -15,7 +15,6 @@ package com.google.devtools.build.lib.pkgcache; import com.google.common.base.Stopwatch; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.ListMultimap; import com.google.common.collect.Sets; import com.google.common.eventbus.EventBus; import com.google.devtools.build.lib.cmdline.Label; @@ -101,7 +100,6 @@ public final class LegacyLoadingPhaseRunner extends LoadingPhaseRunner { List<String> targetPatterns, PathFragment relativeWorkingDirectory, LoadingOptions options, - ListMultimap<String, Label> labelsToLoadUnconditionally, boolean keepGoing, boolean determineTests, @Nullable LoadingCallback callback) diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunner.java b/src/main/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunner.java index 7c7fbcad1d..1ef720f110 100644 --- a/src/main/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunner.java +++ b/src/main/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunner.java @@ -15,9 +15,7 @@ package com.google.devtools.build.lib.pkgcache; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; -import com.google.common.collect.ListMultimap; import com.google.common.eventbus.EventBus; -import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.PackageIdentifier; import com.google.devtools.build.lib.cmdline.TargetParsingException; import com.google.devtools.build.lib.events.Event; @@ -38,9 +36,6 @@ import javax.annotation.Nullable; * <ul> * <li>target pattern evaluation * <li>test suite expansion - * <li>loading the labels needed to construct the build configuration - * <li>loading the labels needed for the analysis with the build configuration - * <li>loading the transitive closure of the targets and the configuration labels * </ul> * * <p>In order to ensure correctness of incremental loading and of full cache hits, this class is @@ -52,18 +47,13 @@ import javax.annotation.Nullable; * maximize caching, it is vital that these change as rarely as possible. */ public abstract class LoadingPhaseRunner { - /** - * Performs target pattern evaluation, test suite expansion (if requested), and loads the - * transitive closure of the resulting targets as well as of the targets needed to use the given - * build configuration provider. - */ + /** Performs target pattern evaluation and test suite expansion (if requested). */ public abstract LoadingResult execute( EventHandler eventHandler, EventBus eventBus, List<String> targetPatterns, PathFragment relativeWorkingDirectory, LoadingOptions options, - ListMultimap<String, Label> labelsToLoadUnconditionally, boolean keepGoing, boolean determineTests, @Nullable LoadingCallback callback) diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java index b4cb3059fd..a8fa1ae0b0 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java @@ -29,7 +29,6 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedSet; import com.google.common.collect.Iterables; -import com.google.common.collect.ListMultimap; import com.google.common.collect.Maps; import com.google.common.collect.Range; import com.google.common.eventbus.EventBus; @@ -1040,8 +1039,8 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { SkyKey skyKey = ConfigurationCollectionValue.key( buildOptions, ImmutableSortedSet.copyOf(multiCpu)); - EvaluationResult<ConfigurationCollectionValue> result = buildDriver.evaluate( - Arrays.asList(skyKey), keepGoing, DEFAULT_THREAD_COUNT, eventHandler); + EvaluationResult<ConfigurationCollectionValue> result = + buildDriver.evaluate(Arrays.asList(skyKey), keepGoing, DEFAULT_THREAD_COUNT, eventHandler); if (result.hasError()) { Throwable e = result.getError(skyKey).getException(); // Wrap loading failed exceptions @@ -1052,10 +1051,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { throw new IllegalStateException( "Unknown error during ConfigurationCollectionValue evaluation", e); } - Preconditions.checkState(result.values().size() == 1, - "Result of evaluate() must contain exactly one value %s", result); - ConfigurationCollectionValue configurationValue = - Iterables.getOnlyElement(result.values()); + ConfigurationCollectionValue configurationValue = result.get(skyKey); return configurationValue.getConfigurationCollection(); } @@ -1845,7 +1841,6 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { List<String> targetPatterns, PathFragment relativeWorkingDirectory, LoadingOptions options, - ListMultimap<String, Label> labelsToLoadUnconditionally, boolean keepGoing, boolean determineTests, @Nullable LoadingCallback callback) |