Remove LoadingResult
Instead, refactor the code to use TargetPatternPhaseValue exclusively. This
removes the need to convert from TargetPatternPhaseValue to LoadingResult, and
prepares for interleaving.
It also reduces the number of Skyframe calls which may speed up null builds a
bit, as a followup for https://github.com/bazelbuild/bazel/commit/1067310e18cb9ac203110726de0be53bdc403cea.
PiperOrigin-RevId: 205989338
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
index caca6a7..ec049cd 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
@@ -56,7 +56,6 @@
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.packages.TargetUtils;
-import com.google.devtools.build.lib.pkgcache.LoadingResult;
import com.google.devtools.build.lib.pkgcache.PackageManager.PackageManagerStatistics;
import com.google.devtools.build.lib.skyframe.AspectValue;
import com.google.devtools.build.lib.skyframe.AspectValue.AspectKey;
@@ -66,6 +65,7 @@
import com.google.devtools.build.lib.skyframe.SkyframeAnalysisResult;
import com.google.devtools.build.lib.skyframe.SkyframeBuildView;
import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
+import com.google.devtools.build.lib.skyframe.TargetPatternPhaseValue;
import com.google.devtools.build.lib.syntax.SkylarkImport;
import com.google.devtools.build.lib.syntax.SkylarkImports;
import com.google.devtools.build.lib.syntax.SkylarkImports.SkylarkImportSyntaxException;
@@ -174,9 +174,7 @@
/** Returns the collection of configured targets corresponding to any of the provided targets. */
@VisibleForTesting
static LinkedHashSet<ConfiguredTarget> filterTestsByTargets(
- Collection<ConfiguredTarget> targets, Collection<Target> allowedTargets) {
- Set<Label> allowedTargetLabels =
- allowedTargets.stream().map(Target::getLabel).collect(Collectors.toSet());
+ Collection<ConfiguredTarget> targets, Set<Label> allowedTargetLabels) {
return targets
.stream()
.filter(ct -> allowedTargetLabels.contains(ct.getLabel()))
@@ -185,7 +183,7 @@
@ThreadCompatible
public AnalysisResult update(
- LoadingResult loadingResult,
+ TargetPatternPhaseValue loadingResult,
BuildConfigurationCollection configurations,
List<String> aspects,
AnalysisOptions viewOptions,
@@ -201,7 +199,9 @@
skyframeBuildView.resetEvaluatedConfiguredTargetKeysSet();
skyframeBuildView.resetEvaluationActionCount();
- Collection<Target> targets = loadingResult.getTargets();
+ // TODO(ulfjack): Expensive. Maybe we don't actually need the targets, only the labels?
+ Collection<Target> targets =
+ loadingResult.getTargets(eventHandler, skyframeExecutor.getPackageManager());
eventBus.post(new AnalysisPhaseStartedEvent(targets));
skyframeBuildView.setConfigurations(eventHandler, configurations);
@@ -358,7 +358,7 @@
private AnalysisResult createResult(
ExtendedEventHandler eventHandler,
- LoadingResult loadingResult,
+ TargetPatternPhaseValue loadingResult,
BuildConfigurationCollection configurations,
TopLevelArtifactContext topLevelOptions,
AnalysisOptions viewOptions,
@@ -366,7 +366,7 @@
Set<ConfiguredTarget> targetsToSkip,
List<TargetAndConfiguration> topLevelTargetsWithConfigs)
throws InterruptedException {
- Collection<Target> testsToRun = loadingResult.getTestsToRun();
+ Set<Label> testsToRun = loadingResult.getTestsToRunLabels();
Set<ConfiguredTarget> configuredTargets =
Sets.newLinkedHashSet(skyframeAnalysisResult.getConfiguredTargets());
ImmutableSet<AspectValue> aspects = ImmutableSet.copyOf(skyframeAnalysisResult.getAspects());
@@ -469,10 +469,11 @@
@Nullable
public static String createErrorMessage(
- LoadingResult loadingResult, @Nullable SkyframeAnalysisResult skyframeAnalysisResult) {
- return loadingResult.hasTargetPatternError()
+ TargetPatternPhaseValue loadingResult,
+ @Nullable SkyframeAnalysisResult skyframeAnalysisResult) {
+ return loadingResult.hasError()
? "command succeeded, but there were errors parsing the target pattern"
- : loadingResult.hasLoadingError()
+ : loadingResult.hasPostExpansionError()
|| (skyframeAnalysisResult != null && skyframeAnalysisResult.hasLoadingError())
? "command succeeded, but there were loading phase errors"
: (skyframeAnalysisResult != null && skyframeAnalysisResult.hasAnalysisError())
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisPhaseRunner.java b/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisPhaseRunner.java
index 2f3e9b2..0cc40fe 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisPhaseRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisPhaseRunner.java
@@ -46,12 +46,12 @@
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.packages.TargetUtils;
import com.google.devtools.build.lib.pkgcache.LoadingFailedException;
-import com.google.devtools.build.lib.pkgcache.LoadingResult;
import com.google.devtools.build.lib.profiler.ProfilePhase;
import com.google.devtools.build.lib.profiler.Profiler;
import com.google.devtools.build.lib.profiler.SilentCloseable;
import com.google.devtools.build.lib.runtime.CommandEnvironment;
import com.google.devtools.build.lib.skyframe.BuildConfigurationValue;
+import com.google.devtools.build.lib.skyframe.TargetPatternPhaseValue;
import com.google.devtools.build.lib.util.AbruptExitException;
import com.google.devtools.build.lib.util.RegexFilter;
import com.google.devtools.common.options.OptionsParsingException;
@@ -81,7 +81,7 @@
InvalidConfigurationException {
// Target pattern evaluation.
- LoadingResult loadingResult;
+ TargetPatternPhaseValue loadingResult;
Profiler.instance().markPhase(ProfilePhase.LOAD);
try (SilentCloseable c = Profiler.instance().profile("evaluateTargetPatterns")) {
loadingResult = evaluateTargetPatterns(request, validator);
@@ -90,17 +90,22 @@
// Compute the heuristic instrumentation filter if needed.
if (request.needsInstrumentationFilter()) {
- String instrumentationFilter =
- InstrumentationFilterSupport.computeInstrumentationFilter(
- env.getReporter(), loadingResult.getTestsToRun());
- try {
- // We're modifying the buildOptions in place, which is not ideal, but we also don't want
- // to pay the price for making a copy. Maybe reconsider later if this turns out to be a
- // problem (and the performance loss may not be a big deal).
- buildOptions.get(BuildConfiguration.Options.class).instrumentationFilter =
- new RegexFilter.RegexFilterConverter().convert(instrumentationFilter);
- } catch (OptionsParsingException e) {
- throw new InvalidConfigurationException(e);
+ try (SilentCloseable c = Profiler.instance().profile("Compute instrumentation filter")) {
+ String instrumentationFilter =
+ InstrumentationFilterSupport.computeInstrumentationFilter(
+ env.getReporter(),
+ // TODO(ulfjack): Expensive. Make this part of the TargetPatternPhaseValue or write
+ // a new SkyFunction to compute it?
+ loadingResult.getTestsToRun(env.getReporter(), env.getPackageManager()));
+ try {
+ // We're modifying the buildOptions in place, which is not ideal, but we also don't want
+ // to pay the price for making a copy. Maybe reconsider later if this turns out to be a
+ // problem (and the performance loss may not be a big deal).
+ buildOptions.get(BuildConfiguration.Options.class).instrumentationFilter =
+ new RegexFilter.RegexFilterConverter().convert(instrumentationFilter);
+ } catch (OptionsParsingException e) {
+ throw new InvalidConfigurationException(e);
+ }
}
}
@@ -181,11 +186,11 @@
return analysisResult;
}
- private final LoadingResult evaluateTargetPatterns(
+ private final TargetPatternPhaseValue evaluateTargetPatterns(
final BuildRequest request, final TargetValidator validator)
throws LoadingFailedException, TargetParsingException, InterruptedException {
boolean keepGoing = request.getKeepGoing();
- LoadingResult result =
+ TargetPatternPhaseValue result =
env.getSkyframeExecutor()
.loadTargetPatterns(
env.getReporter(),
@@ -195,7 +200,8 @@
keepGoing,
request.shouldRunTests());
if (validator != null) {
- Collection<Target> targets = result.getTargets();
+ Collection<Target> targets =
+ result.getTargets(env.getReporter(), env.getSkyframeExecutor().getPackageManager());
validator.validateTargets(targets, keepGoing);
}
return result;
@@ -213,7 +219,7 @@
*/
private AnalysisResult runAnalysisPhase(
BuildRequest request,
- LoadingResult loadingResult,
+ TargetPatternPhaseValue loadingResult,
BuildConfigurationCollection configurations)
throws InterruptedException, ViewCreationFailedException {
Stopwatch timer = Stopwatch.createStarted();
diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/LoadingResult.java b/src/main/java/com/google/devtools/build/lib/pkgcache/LoadingResult.java
deleted file mode 100644
index 38377db..0000000
--- a/src/main/java/com/google/devtools/build/lib/pkgcache/LoadingResult.java
+++ /dev/null
@@ -1,77 +0,0 @@
-// Copyright 2015 The Bazel Authors. All rights reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-package com.google.devtools.build.lib.pkgcache;
-
-import com.google.common.base.MoreObjects;
-import com.google.common.collect.ImmutableSet;
-import com.google.devtools.build.lib.packages.Target;
-import java.util.Collection;
-
-/**
- * The result of the loading phase, i.e., whether there were errors, and which targets were
- * successfully loaded, plus some related metadata.
- */
-public final class LoadingResult {
- private final boolean hasTargetPatternError;
- private final boolean hasLoadingError;
- private final ImmutableSet<Target> targetsToAnalyze;
- private final ImmutableSet<Target> testsToRun;
- private final String workspaceName;
-
- public LoadingResult(boolean hasTargetPatternError, boolean hasLoadingError,
- Collection<Target> targetsToAnalyze, Collection<Target> testsToRun, String workspaceName) {
- this.hasTargetPatternError = hasTargetPatternError;
- this.hasLoadingError = hasLoadingError;
- this.targetsToAnalyze =
- targetsToAnalyze == null ? null : ImmutableSet.copyOf(targetsToAnalyze);
- this.testsToRun = testsToRun == null ? null : ImmutableSet.copyOf(testsToRun);
- this.workspaceName = workspaceName;
- }
-
- /** Whether there were errors during target pattern evaluation. */
- public boolean hasTargetPatternError() {
- return hasTargetPatternError;
- }
-
- /** Whether there were errors during the loading phase. */
- public boolean hasLoadingError() {
- return hasLoadingError;
- }
-
- /** Successfully loaded targets that should be built. */
- public Collection<Target> getTargets() {
- return targetsToAnalyze;
- }
-
- /** Successfully loaded targets that should be run as tests. Must be a subset of the targets. */
- public Collection<Target> getTestsToRun() {
- return testsToRun;
- }
-
- /** The name of the local workspace. */
- public String getWorkspaceName() {
- return workspaceName;
- }
-
- @Override
- public String toString() {
- return MoreObjects.toStringHelper(LoadingResult.class)
- .add("hasTargetPatternError", hasTargetPatternError)
- .add("hasLoadingError", hasLoadingError)
- .add("targetsToAnalyze", targetsToAnalyze)
- .add("testsToRun", testsToRun)
- .add("workspaceName", workspaceName)
- .toString();
- }
-}
diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/TargetParsingCompleteEvent.java b/src/main/java/com/google/devtools/build/lib/pkgcache/TargetParsingCompleteEvent.java
index 58d67f8..5266ae5 100644
--- a/src/main/java/com/google/devtools/build/lib/pkgcache/TargetParsingCompleteEvent.java
+++ b/src/main/java/com/google/devtools/build/lib/pkgcache/TargetParsingCompleteEvent.java
@@ -126,6 +126,14 @@
return Iterables.transform(targets, ThinTarget::getLabel);
}
+ public Iterable<Label> getFilteredLabels() {
+ return Iterables.transform(filteredTargets, ThinTarget::getLabel);
+ }
+
+ public Iterable<Label> getTestFilteredLabels() {
+ return Iterables.transform(testFilteredTargets, ThinTarget::getLabel);
+ }
+
/** @return the filtered targets (i.e., using -//foo:bar on the command-line) */
public ImmutableSet<ThinTarget> getFilteredTargets() {
return filteredTargets;
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 3fc7283..040306b 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
@@ -104,7 +104,6 @@
import com.google.devtools.build.lib.packages.RuleVisibility;
import com.google.devtools.build.lib.packages.SkylarkSemanticsOptions;
import com.google.devtools.build.lib.pkgcache.LoadingOptions;
-import com.google.devtools.build.lib.pkgcache.LoadingResult;
import com.google.devtools.build.lib.pkgcache.PackageCacheOptions;
import com.google.devtools.build.lib.pkgcache.PackageManager;
import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
@@ -2194,7 +2193,7 @@
*/
public abstract void deleteOldNodes(long versionWindowForDirtyGc);
- public LoadingResult loadTargetPatterns(
+ public TargetPatternPhaseValue loadTargetPatterns(
ExtendedEventHandler eventHandler,
List<String> targetPatterns,
PathFragment relativeWorkingDirectory,
@@ -2252,7 +2251,7 @@
eventHandler.post(new TargetParsingPhaseTimeEvent(timeMillis));
TargetPatternPhaseValue patternParsingValue = evalResult.get(key);
- return patternParsingValue.toLoadingResult(eventHandler, packageManager);
+ return patternParsingValue;
}
public Consumer<Artifact.SourceArtifact> getSourceDependencyListener(SkyKey key) {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseValue.java
index 7670dc0..224ea7e 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseValue.java
@@ -24,7 +24,6 @@
import com.google.devtools.build.lib.packages.NoSuchPackageException;
import com.google.devtools.build.lib.packages.NoSuchTargetException;
import com.google.devtools.build.lib.packages.Target;
-import com.google.devtools.build.lib.pkgcache.LoadingResult;
import com.google.devtools.build.lib.pkgcache.PackageManager;
import com.google.devtools.build.lib.pkgcache.TestFilter;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
@@ -82,6 +81,23 @@
.collect(ImmutableSet.toImmutableSet());
}
+ public ImmutableSet<Target> getTestsToRun(
+ ExtendedEventHandler eventHandler, PackageManager packageManager) {
+ return testsToRunLabels
+ .stream()
+ .map(
+ (label) -> {
+ try {
+ return packageManager
+ .getPackage(eventHandler, label.getPackageIdentifier())
+ .getTarget(label.getName());
+ } catch (NoSuchPackageException | NoSuchTargetException | InterruptedException e) {
+ throw new RuntimeException("Failed to get package from TargetPatternPhaseValue", e);
+ }
+ })
+ .collect(ImmutableSet.toImmutableSet());
+ }
+
public ImmutableSet<Label> getTargetLabels() {
return targetLabels;
}
@@ -103,48 +119,6 @@
return workspaceName;
}
- public LoadingResult toLoadingResult(
- ExtendedEventHandler eventHandler, PackageManager packageManager) {
- ImmutableSet<Target> targets =
- getTargetLabels()
- .stream()
- .map(
- (label) -> {
- try {
- return packageManager
- .getPackage(eventHandler, label.getPackageIdentifier())
- .getTarget(label.getName());
- } catch (NoSuchPackageException
- | NoSuchTargetException
- | InterruptedException e) {
- throw new RuntimeException(e);
- }
- })
- .collect(ImmutableSet.toImmutableSet());
- ImmutableSet<Target> testsToRun = null;
- if (testsToRunLabels != null) {
- testsToRun =
- getTestsToRunLabels()
- .stream()
- .map(
- (label) -> {
- try {
- return packageManager
- .getPackage(eventHandler, label.getPackageIdentifier())
- .getTarget(label.getName());
- } catch (NoSuchPackageException
- | NoSuchTargetException
- | InterruptedException e) {
- throw new RuntimeException(e);
- }
- })
- .collect(ImmutableSet.toImmutableSet());
- }
-
- return new LoadingResult(
- hasError(), hasPostExpansionError(), targets, testsToRun, getWorkspaceName());
- }
-
@Override
public boolean equals(Object obj) {
if (this == obj) {
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java b/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java
index 1d635c0..91d7438 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java
@@ -133,7 +133,8 @@
targets =
Lists.<ConfiguredTarget>newArrayList(
BuildView.filterTestsByTargets(
- targets, Sets.newHashSet(test1.getTarget(), suite.getTarget())));
+ targets,
+ Sets.newHashSet(test1.getTarget().getLabel(), suite.getTarget().getLabel())));
assertThat(targets).containsExactlyElementsIn(Sets.newHashSet(test1CT, suiteCT));
}
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
index b583a18..22d8fff 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
@@ -51,7 +51,6 @@
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.packages.util.MockToolsConfig;
import com.google.devtools.build.lib.pkgcache.LoadingOptions;
-import com.google.devtools.build.lib.pkgcache.LoadingResult;
import com.google.devtools.build.lib.pkgcache.PackageCacheOptions;
import com.google.devtools.build.lib.pkgcache.PackageManager;
import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
@@ -64,6 +63,7 @@
import com.google.devtools.build.lib.skyframe.PrecomputedValue;
import com.google.devtools.build.lib.skyframe.SequencedSkyframeExecutor;
import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
+import com.google.devtools.build.lib.skyframe.TargetPatternPhaseValue;
import com.google.devtools.build.lib.skyframe.util.SkyframeExecutorTestUtils;
import com.google.devtools.build.lib.testutil.FoundationTestCase;
import com.google.devtools.build.lib.testutil.TestConstants;
@@ -347,7 +347,7 @@
skyframeExecutor.invalidateFilesUnderPathForTesting(
reporter, ModifiedFileSet.EVERYTHING_MODIFIED, Root.fromPath(rootDirectory));
- LoadingResult loadingResult =
+ TargetPatternPhaseValue loadingResult =
skyframeExecutor.loadTargetPatterns(
reporter,
ImmutableList.copyOf(labels),
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java
index b3d9a06..a0fb40f 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java
@@ -74,12 +74,12 @@
import com.google.devtools.build.lib.packages.RawAttributeMapper;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.Target;
-import com.google.devtools.build.lib.pkgcache.LoadingResult;
import com.google.devtools.build.lib.skyframe.BuildConfigurationValue;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey;
import com.google.devtools.build.lib.skyframe.SkyframeBuildView;
import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
+import com.google.devtools.build.lib.skyframe.TargetPatternPhaseValue;
import com.google.devtools.build.lib.skyframe.ToolchainException;
import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.util.OrderedSetMultimap;
@@ -96,7 +96,6 @@
* tests access to Skyframe internals. The code largely predates the introduction of Skyframe, and
* mostly exists to avoid having to rewrite our tests to work with Skyframe natively.
*/
-@VisibleForTesting
public class BuildViewForTesting {
private final BuildView buildView;
private final SkyframeExecutor skyframeExecutor;
@@ -140,7 +139,7 @@
@ThreadCompatible
public AnalysisResult update(
- LoadingResult loadingResult,
+ TargetPatternPhaseValue loadingResult,
BuildConfigurationCollection configurations,
List<String> aspects,
AnalysisOptions viewOptions,
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
index a5d8368..739437d 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
@@ -123,7 +123,6 @@
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.packages.util.MockToolsConfig;
import com.google.devtools.build.lib.pkgcache.LoadingOptions;
-import com.google.devtools.build.lib.pkgcache.LoadingResult;
import com.google.devtools.build.lib.pkgcache.PackageCacheOptions;
import com.google.devtools.build.lib.pkgcache.PackageManager;
import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
@@ -139,6 +138,7 @@
import com.google.devtools.build.lib.skyframe.SequencedSkyframeExecutor;
import com.google.devtools.build.lib.skyframe.SkyValueDirtinessChecker;
import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
+import com.google.devtools.build.lib.skyframe.TargetPatternPhaseValue;
import com.google.devtools.build.lib.syntax.SkylarkSemantics;
import com.google.devtools.build.lib.testutil.BlazeTestUtils;
import com.google.devtools.build.lib.testutil.FoundationTestCase;
@@ -1730,7 +1730,7 @@
AnalysisOptions viewOptions = Options.getDefaults(AnalysisOptions.class);
- LoadingResult loadingResult =
+ TargetPatternPhaseValue loadingResult =
skyframeExecutor.loadTargetPatterns(
reporter,
targets,
diff --git a/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java b/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java
index 23d5ca1..cd9246e 100644
--- a/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java
@@ -50,6 +50,7 @@
import com.google.devtools.build.lib.skyframe.SequencedSkyframeExecutor;
import com.google.devtools.build.lib.skyframe.SkyValueDirtinessChecker;
import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
+import com.google.devtools.build.lib.skyframe.TargetPatternPhaseValue;
import com.google.devtools.build.lib.testutil.ManualClock;
import com.google.devtools.build.lib.testutil.MoreAsserts;
import com.google.devtools.build.lib.testutil.TestConstants;
@@ -66,12 +67,10 @@
import com.google.devtools.common.options.OptionsParsingException;
import java.io.IOException;
import java.util.ArrayList;
-import java.util.Collection;
import java.util.List;
import java.util.UUID;
import java.util.logging.Level;
import java.util.logging.Logger;
-import java.util.stream.Collectors;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -96,8 +95,12 @@
tester = new LoadingPhaseTester();
}
- protected List<Target> getTargets(String... targetNames) throws Exception {
- return tester.getTargets(targetNames);
+ private List<Label> getLabels(String... labels) throws Exception {
+ List<Label> result = new ArrayList<>();
+ for (String label : labels) {
+ result.add(Label.parseAbsoluteUnchecked(label));
+ }
+ return result;
}
private void assertCircularSymlinksDuringTargetParsing(String targetPattern) throws Exception {
@@ -110,9 +113,9 @@
}
}
- private LoadingResult assertNoErrors(LoadingResult loadingResult) {
- assertThat(loadingResult.hasTargetPatternError()).isFalse();
- assertThat(loadingResult.hasLoadingError()).isFalse();
+ private TargetPatternPhaseValue assertNoErrors(TargetPatternPhaseValue loadingResult) {
+ assertThat(loadingResult.hasError()).isFalse();
+ assertThat(loadingResult.hasPostExpansionError()).isFalse();
tester.assertNoEvents();
return loadingResult;
}
@@ -121,18 +124,19 @@
public void testSmoke() throws Exception {
tester.addFile("base/BUILD",
"filegroup(name = 'hello', srcs = ['foo.txt'])");
- LoadingResult loadingResult = assertNoErrors(tester.load("//base:hello"));
- assertThat(loadingResult.getTargets()).containsExactlyElementsIn(getTargets("//base:hello"));
- assertThat(loadingResult.getTestsToRun()).isNull();
+ TargetPatternPhaseValue loadingResult = assertNoErrors(tester.load("//base:hello"));
+ assertThat(loadingResult.getTargetLabels())
+ .containsExactlyElementsIn(getLabels("//base:hello"));
+ assertThat(loadingResult.getTestsToRunLabels()).isNull();
}
@Test
public void testNonExistentPackage() throws Exception {
- LoadingResult loadingResult = tester.loadKeepGoing("//base:missing");
- assertThat(loadingResult.hasTargetPatternError()).isTrue();
- assertThat(loadingResult.hasLoadingError()).isFalse();
- assertThat(loadingResult.getTargets()).isEmpty();
- assertThat(loadingResult.getTestsToRun()).isNull();
+ TargetPatternPhaseValue loadingResult = tester.loadKeepGoing("//base:missing");
+ assertThat(loadingResult.hasError()).isTrue();
+ assertThat(loadingResult.hasPostExpansionError()).isFalse();
+ assertThat(loadingResult.getTargetLabels()).isEmpty();
+ assertThat(loadingResult.getTestsToRunLabels()).isNull();
tester.assertContainsError("Skipping '//base:missing': no such package 'base'");
tester.assertContainsWarning("Target pattern parsing failed.");
PatternExpandingError err = tester.findPostOnce(PatternExpandingError.class);
@@ -165,11 +169,11 @@
@Test
public void testNonExistentTarget() throws Exception {
tester.addFile("base/BUILD");
- LoadingResult loadingResult = tester.loadKeepGoing("//base:missing");
- assertThat(loadingResult.hasTargetPatternError()).isTrue();
- assertThat(loadingResult.hasLoadingError()).isFalse();
- assertThat(loadingResult.getTargets()).isEmpty();
- assertThat(loadingResult.getTestsToRun()).isNull();
+ TargetPatternPhaseValue loadingResult = tester.loadKeepGoing("//base:missing");
+ assertThat(loadingResult.hasError()).isTrue();
+ assertThat(loadingResult.hasPostExpansionError()).isFalse();
+ assertThat(loadingResult.getTargetLabels()).isEmpty();
+ assertThat(loadingResult.getTestsToRunLabels()).isNull();
tester.assertContainsError("Skipping '//base:missing': no such target '//base:missing'");
tester.assertContainsWarning("Target pattern parsing failed.");
PatternExpandingError err = tester.findPostOnce(PatternExpandingError.class);
@@ -203,8 +207,8 @@
@Test
public void testMistypedTargetKeepGoing() throws Exception {
- LoadingResult result = tester.loadKeepGoing("foo//bar:missing");
- assertThat(result.hasTargetPatternError()).isTrue();
+ TargetPatternPhaseValue result = tester.loadKeepGoing("foo//bar:missing");
+ assertThat(result.hasError()).isTrue();
tester.assertContainsError(
"invalid target format 'foo//bar:missing': "
+ "invalid package name 'foo//bar': "
@@ -216,11 +220,11 @@
@Test
public void testBadTargetPatternWithTest() throws Exception {
tester.addFile("base/BUILD");
- LoadingResult loadingResult = tester.loadTestsKeepGoing("//base:missing");
- assertThat(loadingResult.hasTargetPatternError()).isTrue();
- assertThat(loadingResult.hasLoadingError()).isFalse();
- assertThat(loadingResult.getTargets()).isEmpty();
- assertThat(loadingResult.getTestsToRun()).isEmpty();
+ TargetPatternPhaseValue loadingResult = tester.loadTestsKeepGoing("//base:missing");
+ assertThat(loadingResult.hasError()).isTrue();
+ assertThat(loadingResult.hasPostExpansionError()).isFalse();
+ assertThat(loadingResult.getTargetLabels()).isEmpty();
+ assertThat(loadingResult.getTestsToRunLabels()).isEmpty();
tester.assertContainsError("Skipping '//base:missing': no such target '//base:missing'");
tester.assertContainsWarning("Target pattern parsing failed.");
}
@@ -229,12 +233,12 @@
public void testManualTarget() throws Exception {
AnalysisMock.get().ccSupport().setup(tester.mockToolsConfig);
tester.addFile("cc/BUILD", "cc_library(name = 'my_lib', srcs = ['lib.cc'], tags = ['manual'])");
- LoadingResult loadingResult = assertNoErrors(tester.load("//cc:all"));
- assertThat(loadingResult.getTargets()).containsExactlyElementsIn(getTargets());
+ TargetPatternPhaseValue loadingResult = assertNoErrors(tester.load("//cc:all"));
+ assertThat(loadingResult.getTargetLabels()).containsExactlyElementsIn(getLabels());
// Explicitly specified on the command line.
loadingResult = assertNoErrors(tester.load("//cc:my_lib"));
- assertThat(loadingResult.getTargets()).containsExactlyElementsIn(getTargets("//cc:my_lib"));
+ assertThat(loadingResult.getTargetLabels()).containsExactlyElementsIn(getLabels("//cc:my_lib"));
}
@Test
@@ -244,14 +248,12 @@
"cc_library(name = 'somelib', srcs = [ 'somelib.cc' ], hdrs = [ 'somelib.h' ])",
"config_setting(name = 'configa', values = { 'define': 'foo=a' })",
"config_setting(name = 'configb', values = { 'define': 'foo=b' })");
- LoadingResult loadingResult = assertNoErrors(tester.load("//config:all"));
- assertThat(loadingResult.getTargets())
- .containsExactlyElementsIn(getTargets("//config:somelib"));
+ TargetPatternPhaseValue result = assertNoErrors(tester.load("//config:all"));
+ assertThat(result.getTargetLabels()).containsExactlyElementsIn(getLabels("//config:somelib"));
// Explicitly specified on the command line.
- loadingResult = assertNoErrors(tester.load("//config:configa"));
- assertThat(loadingResult.getTargets())
- .containsExactlyElementsIn(getTargets("//config:configa"));
+ result = assertNoErrors(tester.load("//config:configa"));
+ assertThat(result.getTargetLabels()).containsExactlyElementsIn(getLabels("//config:configa"));
}
@Test
@@ -259,19 +261,8 @@
tester.addFile("my_test/BUILD",
"sh_test(name = 'my_test', srcs = ['test.cc'])");
assertNoErrors(tester.loadTests("-//my_test"));
- assertThinTargetsEqualToTargets(tester.getFilteredTargets(), getTargets());
- assertThinTargetsEqualToTargets(tester.getTestFilteredTargets(), getTargets());
- }
-
- private static void assertThinTargetsEqualToTargets(
- Collection<TargetParsingCompleteEvent.ThinTarget> thinTargets, Collection<Target> targets) {
- assertThat(
- thinTargets
- .stream()
- .map(TargetParsingCompleteEvent.ThinTarget::getLabel)
- .collect(Collectors.toList()))
- .containsExactlyElementsIn(
- targets.stream().map(Target::getLabel).collect(Collectors.toList()));
+ assertThat(tester.getFilteredTargets()).isEmpty();
+ assertThat(tester.getTestFilteredTargets()).isEmpty();
}
@Test
@@ -279,8 +270,8 @@
tester.addFile("my_library/BUILD",
"cc_library(name = 'my_library', srcs = ['test.cc'])");
assertNoErrors(tester.loadTests("-//my_library"));
- assertThinTargetsEqualToTargets(tester.getFilteredTargets(), getTargets());
- assertThinTargetsEqualToTargets(tester.getTestFilteredTargets(), getTargets());
+ assertThat(tester.getFilteredTargets()).isEmpty();
+ assertThat(tester.getTestFilteredTargets()).isEmpty();
}
private void writeBuildFilesForTestFiltering() throws Exception {
@@ -293,75 +284,75 @@
@Test
public void testTestFiltering() throws Exception {
writeBuildFilesForTestFiltering();
- LoadingResult loadingResult = assertNoErrors(tester.loadTests("//tests:all"));
- assertThat(loadingResult.getTargets())
- .containsExactlyElementsIn(getTargets("//tests:t1", "//tests:t2"));
- assertThat(loadingResult.getTestsToRun())
- .containsExactlyElementsIn(getTargets("//tests:t1", "//tests:t2"));
- assertThinTargetsEqualToTargets(tester.getFilteredTargets(), getTargets());
- assertThinTargetsEqualToTargets(tester.getTestFilteredTargets(), getTargets());
+ TargetPatternPhaseValue loadingResult = assertNoErrors(tester.loadTests("//tests:all"));
+ assertThat(loadingResult.getTargetLabels())
+ .containsExactlyElementsIn(getLabels("//tests:t1", "//tests:t2"));
+ assertThat(loadingResult.getTestsToRunLabels())
+ .containsExactlyElementsIn(getLabels("//tests:t1", "//tests:t2"));
+ assertThat(tester.getFilteredTargets()).isEmpty();
+ assertThat(tester.getTestFilteredTargets()).isEmpty();
}
@Test
public void testTestFilteringIncludingManual() throws Exception {
writeBuildFilesForTestFiltering();
tester.useLoadingOptions("--build_manual_tests");
- LoadingResult loadingResult = assertNoErrors(tester.loadTests("//tests:all"));
- assertThat(loadingResult.getTargets())
- .containsExactlyElementsIn(getTargets("//tests:t1", "//tests:t2", "//tests:t3"));
- assertThat(loadingResult.getTestsToRun())
- .containsExactlyElementsIn(getTargets("//tests:t1", "//tests:t2"));
- assertThinTargetsEqualToTargets(tester.getFilteredTargets(), getTargets());
- assertThinTargetsEqualToTargets(tester.getTestFilteredTargets(), getTargets());
+ TargetPatternPhaseValue loadingResult = assertNoErrors(tester.loadTests("//tests:all"));
+ assertThat(loadingResult.getTargetLabels())
+ .containsExactlyElementsIn(getLabels("//tests:t1", "//tests:t2", "//tests:t3"));
+ assertThat(loadingResult.getTestsToRunLabels())
+ .containsExactlyElementsIn(getLabels("//tests:t1", "//tests:t2"));
+ assertThat(tester.getFilteredTargets()).isEmpty();
+ assertThat(tester.getTestFilteredTargets()).isEmpty();
}
@Test
public void testTestFilteringBuildTestsOnly() throws Exception {
writeBuildFilesForTestFiltering();
tester.useLoadingOptions("--build_tests_only");
- LoadingResult loadingResult = assertNoErrors(tester.loadTests("//tests:all"));
- assertThat(loadingResult.getTargets())
- .containsExactlyElementsIn(getTargets("//tests:t1", "//tests:t2"));
- assertThat(loadingResult.getTestsToRun())
- .containsExactlyElementsIn(getTargets("//tests:t1", "//tests:t2"));
- assertThinTargetsEqualToTargets(tester.getFilteredTargets(), getTargets());
- assertThinTargetsEqualToTargets(tester.getTestFilteredTargets(), getTargets());
+ TargetPatternPhaseValue result = assertNoErrors(tester.loadTests("//tests:all"));
+ assertThat(result.getTargetLabels())
+ .containsExactlyElementsIn(getLabels("//tests:t1", "//tests:t2"));
+ assertThat(result.getTestsToRunLabels())
+ .containsExactlyElementsIn(getLabels("//tests:t1", "//tests:t2"));
+ assertThat(tester.getFilteredTargets()).isEmpty();
+ assertThat(tester.getTestFilteredTargets()).isEmpty();
}
@Test
public void testTestFilteringSize() throws Exception {
writeBuildFilesForTestFiltering();
tester.useLoadingOptions("--test_size_filters=small");
- LoadingResult loadingResult = assertNoErrors(tester.loadTests("//tests:all"));
- assertThat(loadingResult.getTargets())
- .containsExactlyElementsIn(getTargets("//tests:t1", "//tests:t2"));
- assertThat(loadingResult.getTestsToRun()).containsExactlyElementsIn(getTargets("//tests:t1"));
- assertThinTargetsEqualToTargets(tester.getFilteredTargets(), getTargets());
- assertThinTargetsEqualToTargets(tester.getTestFilteredTargets(), getTargets());
+ TargetPatternPhaseValue result = assertNoErrors(tester.loadTests("//tests:all"));
+ assertThat(result.getTargetLabels())
+ .containsExactlyElementsIn(getLabels("//tests:t1", "//tests:t2"));
+ assertThat(result.getTestsToRunLabels()).containsExactlyElementsIn(getLabels("//tests:t1"));
+ assertThat(tester.getFilteredTargets()).isEmpty();
+ assertThat(tester.getTestFilteredTargets()).isEmpty();
}
@Test
public void testTestFilteringSizeAndBuildTestsOnly() throws Exception {
writeBuildFilesForTestFiltering();
tester.useLoadingOptions("--test_size_filters=small", "--build_tests_only");
- LoadingResult loadingResult = assertNoErrors(tester.loadTests("//tests:all"));
- assertThat(loadingResult.getTargets()).containsExactlyElementsIn(getTargets("//tests:t1"));
- assertThat(loadingResult.getTestsToRun()).containsExactlyElementsIn(getTargets("//tests:t1"));
- assertThinTargetsEqualToTargets(tester.getFilteredTargets(), getTargets());
- assertThinTargetsEqualToTargets(tester.getTestFilteredTargets(), getTargets("//tests:t2"));
+ TargetPatternPhaseValue result = assertNoErrors(tester.loadTests("//tests:all"));
+ assertThat(result.getTargetLabels()).containsExactlyElementsIn(getLabels("//tests:t1"));
+ assertThat(result.getTestsToRunLabels()).containsExactlyElementsIn(getLabels("//tests:t1"));
+ assertThat(tester.getFilteredTargets()).isEmpty();
+ assertThat(tester.getTestFilteredTargets()).containsExactlyElementsIn(getLabels("//tests:t2"));
}
@Test
public void testTestFilteringLocalAndBuildTestsOnly() throws Exception {
writeBuildFilesForTestFiltering();
tester.useLoadingOptions("--test_tag_filters=local", "--build_tests_only");
- LoadingResult loadingResult = assertNoErrors(tester.loadTests("//tests:all", "//tests:t3"));
- assertThat(loadingResult.getTargets())
- .containsExactlyElementsIn(getTargets("//tests:t1", "//tests:t3"));
- assertThat(loadingResult.getTestsToRun())
- .containsExactlyElementsIn(getTargets("//tests:t1", "//tests:t3"));
- assertThinTargetsEqualToTargets(tester.getFilteredTargets(), getTargets());
- assertThinTargetsEqualToTargets(tester.getTestFilteredTargets(), getTargets("//tests:t2"));
+ TargetPatternPhaseValue result = assertNoErrors(tester.loadTests("//tests:all", "//tests:t3"));
+ assertThat(result.getTargetLabels())
+ .containsExactlyElementsIn(getLabels("//tests:t1", "//tests:t3"));
+ assertThat(result.getTestsToRunLabels())
+ .containsExactlyElementsIn(getLabels("//tests:t1", "//tests:t3"));
+ assertThat(tester.getFilteredTargets()).isEmpty();
+ assertThat(tester.getTestFilteredTargets()).containsExactlyElementsIn(getLabels("//tests:t2"));
}
@Test
@@ -370,11 +361,13 @@
tester.addFile("cc/BUILD",
"cc_test(name = 'my_test', srcs = ['test.cc'])",
"test_suite(name = 'tests', tests = [':my_test'])");
- LoadingResult loadingResult = assertNoErrors(tester.loadTests("//cc:tests"));
- assertThat(loadingResult.getTargets()).containsExactlyElementsIn(getTargets("//cc:my_test"));
- assertThat(loadingResult.getTestsToRun()).containsExactlyElementsIn(getTargets("//cc:my_test"));
- assertThinTargetsEqualToTargets(
- tester.getOriginalTargets(), getTargets("//cc:tests", "//cc:my_test"));
+ TargetPatternPhaseValue loadingResult = assertNoErrors(tester.loadTests("//cc:tests"));
+ assertThat(loadingResult.getTargetLabels())
+ .containsExactlyElementsIn(getLabels("//cc:my_test"));
+ assertThat(loadingResult.getTestsToRunLabels())
+ .containsExactlyElementsIn(getLabels("//cc:my_test"));
+ assertThat(tester.getOriginalTargets())
+ .containsExactlyElementsIn(getLabels("//cc:tests", "//cc:my_test"));
assertThat(tester.getTestSuiteTargets())
.containsExactly(Label.parseAbsoluteUnchecked("//cc:tests"));
}
@@ -384,9 +377,9 @@
tester.addFile("ts/BUILD",
"test_suite(name = 'tests', tests = ['//nonexistent:my_test'])");
tester.useLoadingOptions("--build_tests_only");
- LoadingResult loadingResult = tester.loadTestsKeepGoing("//ts:tests");
- assertThat(loadingResult.hasTargetPatternError()).isTrue();
- assertThat(loadingResult.hasLoadingError()).isFalse();
+ TargetPatternPhaseValue loadingResult = tester.loadTestsKeepGoing("//ts:tests");
+ assertThat(loadingResult.hasError()).isTrue();
+ assertThat(loadingResult.hasPostExpansionError()).isFalse();
tester.assertContainsError("no such package 'nonexistent'");
}
@@ -394,9 +387,9 @@
public void testTestSuiteExpansionFailsForBuild() throws Exception {
tester.addFile("ts/BUILD",
"test_suite(name = 'tests', tests = [':nonexistent_test'])");
- LoadingResult loadingResult = tester.loadKeepGoing("//ts:tests");
- assertThat(loadingResult.hasTargetPatternError()).isFalse();
- assertThat(loadingResult.hasLoadingError()).isTrue();
+ TargetPatternPhaseValue loadingResult = tester.loadKeepGoing("//ts:tests");
+ assertThat(loadingResult.hasError()).isFalse();
+ assertThat(loadingResult.hasPostExpansionError()).isTrue();
tester.assertContainsError(
"expecting a test or a test_suite rule but '//ts:nonexistent_test' is not one");
}
@@ -406,9 +399,9 @@
tester.addFile("other/BUILD", "");
tester.addFile("ts/BUILD",
"test_suite(name = 'tests', tests = ['//other:no_such_test'])");
- LoadingResult loadingResult = tester.loadTestsKeepGoing("//ts:tests");
- assertThat(loadingResult.hasTargetPatternError()).isTrue();
- assertThat(loadingResult.hasLoadingError()).isTrue();
+ TargetPatternPhaseValue result = tester.loadTestsKeepGoing("//ts:tests");
+ assertThat(result.hasError()).isTrue();
+ assertThat(result.hasPostExpansionError()).isTrue();
tester.assertContainsError("no such target '//other:no_such_test'");
}
@@ -418,9 +411,9 @@
tester.addFile("ts/BUILD",
"test_suite(name = 'a', tests = ['//other:no_such_test'])",
"test_suite(name = 'b', tests = [])");
- LoadingResult loadingResult = tester.loadTestsKeepGoing("//ts:all");
- assertThat(loadingResult.hasTargetPatternError()).isTrue();
- assertThat(loadingResult.hasLoadingError()).isTrue();
+ TargetPatternPhaseValue result = tester.loadTestsKeepGoing("//ts:all");
+ assertThat(result.hasError()).isTrue();
+ assertThat(result.hasPostExpansionError()).isTrue();
tester.assertContainsError("no such target '//other:no_such_test'");
}
@@ -432,13 +425,14 @@
"sh_test(name = 'baz', srcs = ['baz.sh'])",
"test_suite(name = 'foo_suite', tests = [':foo', ':baz'])");
tester.useLoadingOptions("--build_tests_only");
- LoadingResult loadingResult = assertNoErrors(tester.loadTests("//foo:all"));
- assertThat(loadingResult.getTargets())
- .containsExactlyElementsIn(getTargets("//foo:foo", "//foo:baz"));
- assertThat(loadingResult.getTestsToRun())
- .containsExactlyElementsIn(getTargets("//foo:foo", "//foo:baz"));
- assertThinTargetsEqualToTargets(tester.getFilteredTargets(), getTargets());
- assertThinTargetsEqualToTargets(tester.getTestFilteredTargets(), getTargets("//foo:foo_suite"));
+ TargetPatternPhaseValue result = assertNoErrors(tester.loadTests("//foo:all"));
+ assertThat(result.getTargetLabels())
+ .containsExactlyElementsIn(getLabels("//foo:foo", "//foo:baz"));
+ assertThat(result.getTestsToRunLabels())
+ .containsExactlyElementsIn(getLabels("//foo:foo", "//foo:baz"));
+ assertThat(tester.getFilteredTargets()).isEmpty();
+ assertThat(tester.getTestFilteredTargets())
+ .containsExactlyElementsIn(getLabels("//foo:foo_suite"));
}
/** Regression test for bug: "subtracting tests from test doesn't work" */
@@ -449,11 +443,12 @@
"cc_test(name = 'my_test', srcs = ['test.cc'])",
"cc_test(name = 'my_other_test', srcs = ['other_test.cc'])",
"test_suite(name = 'tests', tests = [':my_test', ':my_other_test'])");
- LoadingResult loadingResult = assertNoErrors(tester.loadTests("//cc:tests", "-//cc:my_test"));
- assertThat(loadingResult.getTargets())
- .containsExactlyElementsIn(getTargets("//cc:my_other_test", "//cc:my_test"));
- assertThat(loadingResult.getTestsToRun())
- .containsExactlyElementsIn(getTargets("//cc:my_other_test"));
+ TargetPatternPhaseValue result =
+ assertNoErrors(tester.loadTests("//cc:tests", "-//cc:my_test"));
+ assertThat(result.getTargetLabels())
+ .containsExactlyElementsIn(getLabels("//cc:my_other_test", "//cc:my_test"));
+ assertThat(result.getTestsToRunLabels())
+ .containsExactlyElementsIn(getLabels("//cc:my_other_test"));
}
/** Regression test for bug: "blaze doesn't seem to respect target subtractions" */
@@ -465,11 +460,12 @@
"cc_test(name = 'my_other_test', srcs = ['other_test.cc'])",
"test_suite(name = 'tests', tests = [':my_test'])",
"test_suite(name = 'all_tests', tests = ['my_other_test'])");
- LoadingResult loadingResult = assertNoErrors(tester.loadTests("//cc:all_tests", "-//cc:tests"));
- assertThat(loadingResult.getTargets())
- .containsExactlyElementsIn(getTargets("//cc:my_other_test"));
- assertThat(loadingResult.getTestsToRun())
- .containsExactlyElementsIn(getTargets("//cc:my_other_test"));
+ TargetPatternPhaseValue result =
+ assertNoErrors(tester.loadTests("//cc:all_tests", "-//cc:tests"));
+ assertThat(result.getTargetLabels())
+ .containsExactlyElementsIn(getLabels("//cc:my_other_test"));
+ assertThat(result.getTestsToRunLabels())
+ .containsExactlyElementsIn(getLabels("//cc:my_other_test"));
}
@Test
@@ -482,9 +478,9 @@
"test_suite(name = 'suite1', tests = ['empty', 'test1'])",
"test_suite(name = 'suite2', tests = ['test2'])",
"test_suite(name = 'all_tests', tests = ['suite1', 'suite2'])");
- LoadingResult loadingResult = assertNoErrors(tester.loadTests("//cc:all_tests"));
- assertThat(loadingResult.getTargets())
- .containsExactlyElementsIn(getTargets("//cc:test1", "//cc:test2"));
+ TargetPatternPhaseValue result = assertNoErrors(tester.loadTests("//cc:all_tests"));
+ assertThat(result.getTargetLabels())
+ .containsExactlyElementsIn(getLabels("//cc:test1", "//cc:test2"));
}
/**
@@ -496,19 +492,19 @@
tester.addFile("cc/BUILD",
"cc_test(name = 'my_test', srcs = ['test.cc'])",
"test_suite(name = 'tests', tests = [':my_test'])");
- LoadingResult loadingResult = tester.loadTests("//cc:tests", "-//cc:my_test");
+ TargetPatternPhaseValue result = tester.loadTests("//cc:tests", "-//cc:my_test");
tester.assertContainsWarning("All specified test targets were excluded by filters");
- assertThat(loadingResult.getTestsToRun()).containsExactlyElementsIn(getTargets());
+ assertThat(result.getTestsToRunLabels()).containsExactlyElementsIn(getLabels());
}
@Test
public void testRepeatedSameLoad() throws Exception {
tester.addFile("base/BUILD",
"filegroup(name = 'hello', srcs = ['foo.txt'])");
- LoadingResult firstResult = assertNoErrors(tester.load("//base:hello"));
- LoadingResult secondResult = assertNoErrors(tester.load("//base:hello"));
- assertThat(secondResult.getTargets()).isEqualTo(firstResult.getTargets());
- assertThat(secondResult.getTestsToRun()).isEqualTo(firstResult.getTestsToRun());
+ TargetPatternPhaseValue firstResult = assertNoErrors(tester.load("//base:hello"));
+ TargetPatternPhaseValue secondResult = assertNoErrors(tester.load("//base:hello"));
+ assertThat(secondResult.getTargetLabels()).isEqualTo(firstResult.getTargetLabels());
+ assertThat(secondResult.getTestsToRunLabels()).isEqualTo(firstResult.getTestsToRunLabels());
}
/**
@@ -521,14 +517,17 @@
public void testGlobPicksUpNewFile() throws Exception {
tester.addFile("foo/BUILD", "filegroup(name='x', srcs=glob(['*.y']))");
tester.addFile("foo/a.y");
- Target result = Iterables.getOnlyElement(assertNoErrors(tester.load("//foo:x")).getTargets());
+ Label label =
+ Iterables.getOnlyElement(assertNoErrors(tester.load("//foo:x")).getTargetLabels());
+ Target result = tester.getTarget(label.toString());
assertThat(
Iterables.transform(result.getAssociatedRule().getLabels(), Functions.toStringFunction()))
.containsExactly("//foo:a.y");
tester.addFile("foo/b.y");
tester.sync();
- result = Iterables.getOnlyElement(assertNoErrors(tester.load("//foo:x")).getTargets());
+ label = Iterables.getOnlyElement(assertNoErrors(tester.load("//foo:x")).getTargetLabels());
+ result = tester.getTarget(label.toString());
assertThat(
Iterables.transform(result.getAssociatedRule().getLabels(), Functions.toStringFunction()))
.containsExactly("//foo:a.y", "//foo:b.y");
@@ -579,8 +578,8 @@
"test_suite(name = 'a', tests = [':b'])",
"test_suite(name = 'b', tests = [':c'])",
"sh_test(name = 'c', srcs = ['test.cc'])");
- LoadingResult loadingResult = assertNoErrors(tester.load("//suite:a"));
- assertThat(loadingResult.getTargets()).containsExactlyElementsIn(getTargets("//suite:c"));
+ TargetPatternPhaseValue result = assertNoErrors(tester.load("//suite:a"));
+ assertThat(result.getTargetLabels()).containsExactlyElementsIn(getLabels("//suite:c"));
}
@Test
@@ -603,8 +602,8 @@
tester.addFile("bad/BUILD",
"sh_binary(name = 'bad', srcs = ['bad.sh'])",
"undefined_symbol");
- LoadingResult loadingResult = tester.loadKeepGoing("//bad");
- assertThat(loadingResult.hasTargetPatternError()).isTrue();
+ TargetPatternPhaseValue result = tester.loadKeepGoing("//bad");
+ assertThat(result.hasError()).isTrue();
tester.assertContainsEventWithFrequency("name 'undefined_symbol' is not defined", 1);
}
@@ -613,8 +612,8 @@
tester.addFile("base/BUILD",
"cc_library(name = 'hello', srcs = ['hello.cc'])");
tester.useLoadingOptions("--compile_one_dependency");
- LoadingResult loadingResult = assertNoErrors(tester.load("base/hello.cc"));
- assertThat(loadingResult.getTargets()).containsExactlyElementsIn(getTargets("//base:hello"));
+ TargetPatternPhaseValue result = assertNoErrors(tester.load("base/hello.cc"));
+ assertThat(result.getTargetLabels()).containsExactlyElementsIn(getLabels("//base:hello"));
}
@Test
@@ -623,8 +622,8 @@
"cc_library(name = 'hello', srcs = ['hello.cc', '//bad:bad.cc'])");
tester.useLoadingOptions("--compile_one_dependency");
try {
- LoadingResult loadingResult = tester.load("base/hello.cc");
- assertThat(loadingResult.hasLoadingError()).isFalse();
+ TargetPatternPhaseValue loadingResult = tester.load("base/hello.cc");
+ assertThat(loadingResult.hasPostExpansionError()).isFalse();
} catch (LoadingFailedException expected) {
tester.assertContainsError("no such package 'bad'");
}
@@ -635,8 +634,8 @@
tester.addFile("base/BUILD",
"cc_library(name = 'hello', srcs = ['hello.cc', '//bad:bad.cc'])");
tester.useLoadingOptions("--compile_one_dependency");
- LoadingResult loadingResult = tester.loadKeepGoing("base/hello.cc");
- assertThat(loadingResult.hasLoadingError()).isFalse();
+ TargetPatternPhaseValue loadingResult = tester.loadKeepGoing("base/hello.cc");
+ assertThat(loadingResult.hasPostExpansionError()).isFalse();
}
@Test
@@ -655,8 +654,8 @@
@Test
public void testParsingFailureReported() throws Exception {
- LoadingResult loadingResult = tester.loadKeepGoing("//does_not_exist");
- assertThat(loadingResult.hasTargetPatternError()).isTrue();
+ TargetPatternPhaseValue loadingResult = tester.loadKeepGoing("//does_not_exist");
+ assertThat(loadingResult.hasError()).isTrue();
ParsingFailedEvent event = tester.findPostOnce(ParsingFailedEvent.class);
assertThat(event.getPattern()).isEqualTo("//does_not_exist");
assertThat(event.getMessage()).contains("BUILD file not found on package path");
@@ -668,7 +667,7 @@
tester.addFile("test/cycle1.bzl", "load(':cycle2.bzl', 'make_cycle')");
tester.addFile("test/cycle2.bzl", "load(':cycle1.bzl', 'make_cycle')");
// The skyframe target pattern evaluator isn't able to provide partial results in the presence
- // of cycles, so it simply raises an exception rather than returning an empty LoadingResult.
+ // of cycles, so it simply raises an exception rather than returning an empty result.
try {
tester.load("//test:cycle1");
fail();
@@ -790,28 +789,27 @@
this.options = parser.getOptions(LoadingOptions.class);
}
- public LoadingResult load(String... patterns) throws Exception {
- return load(/*keepGoing=*/false, /*determineTests=*/false, patterns);
+ public TargetPatternPhaseValue load(String... patterns) throws Exception {
+ return loadWithFlags(/*keepGoing=*/false, /*determineTests=*/false, patterns);
}
- public LoadingResult loadKeepGoing(String... patterns) throws Exception {
- return load(/*keepGoing=*/true, /*determineTests=*/false, patterns);
+ public TargetPatternPhaseValue loadKeepGoing(String... patterns) throws Exception {
+ return loadWithFlags(/*keepGoing=*/true, /*determineTests=*/false, patterns);
}
- public LoadingResult loadTests(String... patterns) throws Exception {
- return load(/*keepGoing=*/false, /*determineTests=*/true, patterns);
+ public TargetPatternPhaseValue loadTests(String... patterns) throws Exception {
+ return loadWithFlags(/*keepGoing=*/false, /*determineTests=*/true, patterns);
}
- public LoadingResult loadTestsKeepGoing(String... patterns) throws Exception {
- return load(/*keepGoing=*/true, /*determineTests=*/true, patterns);
+ public TargetPatternPhaseValue loadTestsKeepGoing(String... patterns) throws Exception {
+ return loadWithFlags(/*keepGoing=*/true, /*determineTests=*/true, patterns);
}
- public LoadingResult load(boolean keepGoing, boolean determineTests, String... patterns)
- throws Exception {
+ public TargetPatternPhaseValue loadWithFlags(
+ boolean keepGoing, boolean determineTests, String... patterns) throws Exception {
sync();
storedErrors.clear();
- LoadingResult result;
- result =
+ TargetPatternPhaseValue result =
skyframeExecutor.loadTargetPatterns(
storedErrors,
ImmutableList.copyOf(patterns),
@@ -867,14 +865,6 @@
changes.clear();
}
- public List<Target> getTargets(String... targetNames) throws Exception {
- List<Target> result = new ArrayList<>();
- for (String targetName : targetNames) {
- result.add(getTarget(targetName));
- }
- return result;
- }
-
public Target getTarget(String targetName) throws Exception {
StoredEventHandler eventHandler = new StoredEventHandler();
Target target = getPkgManager().getTarget(
@@ -887,16 +877,16 @@
return skyframeExecutor.getPackageManager();
}
- public ImmutableSet<TargetParsingCompleteEvent.ThinTarget> getFilteredTargets() {
- return targetParsingCompleteEvent.getFilteredTargets();
+ public ImmutableSet<Label> getFilteredTargets() {
+ return ImmutableSet.copyOf(targetParsingCompleteEvent.getFilteredLabels());
}
- public ImmutableSet<TargetParsingCompleteEvent.ThinTarget> getTestFilteredTargets() {
- return targetParsingCompleteEvent.getTestFilteredTargets();
+ public ImmutableSet<Label> getTestFilteredTargets() {
+ return ImmutableSet.copyOf(targetParsingCompleteEvent.getTestFilteredLabels());
}
- public ImmutableSet<TargetParsingCompleteEvent.ThinTarget> getOriginalTargets() {
- return targetParsingCompleteEvent.getTargets();
+ public ImmutableSet<Label> getOriginalTargets() {
+ return ImmutableSet.copyOf(targetParsingCompleteEvent.getLabels());
}
public ImmutableSet<Label> getTestSuiteTargets() {
