diff options
Diffstat (limited to 'src')
6 files changed, 92 insertions, 48 deletions
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 9b6b6841b3..7fda021118 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 @@ -281,7 +281,8 @@ public final class LegacyLoadingPhaseRunner extends LoadingPhaseRunner { private void postLoadingLogging(EventBus eventBus, ImmutableSet<Target> originalTargetsToLoad, ImmutableSet<Target> expandedTargetsToLoad, Stopwatch timer) { Set<Target> testSuiteTargets = Sets.difference(originalTargetsToLoad, expandedTargetsToLoad); - eventBus.post(new LoadingPhaseCompleteEvent(expandedTargetsToLoad, testSuiteTargets, + eventBus.post(new LoadingPhaseCompleteEvent( + expandedTargetsToLoad, ImmutableSet.copyOf(testSuiteTargets), packageManager.getStatistics(), timer.stop().elapsed(TimeUnit.MILLISECONDS))); LOG.info("Loading phase finished"); } diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseCompleteEvent.java b/src/main/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseCompleteEvent.java index 914ce3b530..4f42c0fe8d 100644 --- a/src/main/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseCompleteEvent.java +++ b/src/main/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseCompleteEvent.java @@ -14,18 +14,18 @@ package com.google.devtools.build.lib.pkgcache; import com.google.common.base.Function; +import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.packages.Target; -import java.util.Collection; - /** * This event is fired after the loading phase is complete. */ -public class LoadingPhaseCompleteEvent { - private final Collection<Target> targets; - private final Collection<Target> filteredTargets; +public final class LoadingPhaseCompleteEvent { + private final ImmutableSet<Target> targets; + private final ImmutableSet<Target> filteredTargets; private final PackageManager.PackageManagerStatistics pkgManagerStats; private final long timeInMs; @@ -35,11 +35,12 @@ public class LoadingPhaseCompleteEvent { * @param targets the set of active targets that remain * @param pkgManagerStats statistics about the package cache */ - public LoadingPhaseCompleteEvent(Collection<Target> targets, Collection<Target> filteredTargets, - PackageManager.PackageManagerStatistics pkgManagerStats, long timeInMs) { - this.targets = targets; - this.filteredTargets = filteredTargets; - this.pkgManagerStats = pkgManagerStats; + public LoadingPhaseCompleteEvent(ImmutableSet<Target> targets, + ImmutableSet<Target> filteredTargets, PackageManager.PackageManagerStatistics pkgManagerStats, + long timeInMs) { + this.targets = Preconditions.checkNotNull(targets); + this.filteredTargets = Preconditions.checkNotNull(filteredTargets); + this.pkgManagerStats = Preconditions.checkNotNull(pkgManagerStats); this.timeInMs = timeInMs; } @@ -47,14 +48,14 @@ public class LoadingPhaseCompleteEvent { * @return The set of active targets remaining, which is a subset of the * targets we attempted to load. */ - public Collection<Target> getTargets() { + public ImmutableSet<Target> getTargets() { return targets; } /** * @return The set of filtered targets. */ - public Collection<Target> getFilteredTargets() { + public ImmutableSet<Target> 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 12c38a8cba..92c680424f 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 @@ -1728,7 +1728,6 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { * Skyframe-based implementation of {@link LoadingPhaseRunner} based on {@link * TargetPatternPhaseFunction}. */ - // TODO(ulfjack): This is still incomplete. final class SkyframeLoadingPhaseRunner extends LoadingPhaseRunner { private final TargetPatternEvaluator targetPatternEvaluator; private final Set<String> ruleClassNames; @@ -1785,17 +1784,14 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { long time = timer.stop().elapsed(TimeUnit.MILLISECONDS); TargetPatternPhaseValue patternParsingValue = evalResult.get(key); - // TODO(ulfjack): The first event should be pre-test_suite expansion, the second post. - eventBus.post(new TargetParsingCompleteEvent(patternParsingValue.getTargets(), + eventBus.post(new TargetParsingCompleteEvent(patternParsingValue.getOriginalTargets(), patternParsingValue.getFilteredTargets(), patternParsingValue.getTestFilteredTargets(), time)); if (callback != null) { callback.notifyTargets(patternParsingValue.getTargets()); } eventBus.post(new LoadingPhaseCompleteEvent( - /*was expandedTargetsToLoad*/patternParsingValue.getTargets(), - // TODO(ulfjack): Should be: Sets.difference(originalTargetsToLoad, expandedTargetsToLoad) - /*was testSuiteTargets*/ImmutableSet.<Target>of(), + patternParsingValue.getTargets(), patternParsingValue.getTestSuiteTargets(), packageManager.getStatistics(), /*timeInMs=*/0)); return patternParsingValue.toLoadingResult(); } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseFunction.java index ea30c0584c..9608878f34 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseFunction.java @@ -159,9 +159,12 @@ final class TargetPatternPhaseFunction implements SkyFunction { expandedTargetsBuilder.add(target); } } - targets = expandedTargetsBuilder.build(); - return new TargetPatternPhaseValue(targets.getTargets(), testsToRun, preExpansionError, - targets.hasError(), filteredTargets, testFilteredTargets); + ResolvedTargets<Target> expandedTargets = expandedTargetsBuilder.build(); + Set<Target> testSuiteTargets = + Sets.difference(targets.getTargets(), expandedTargets.getTargets()); + return new TargetPatternPhaseValue(expandedTargets.getTargets(), testsToRun, preExpansionError, + expandedTargets.hasError(), filteredTargets, testFilteredTargets, + targets.getTargets(), ImmutableSet.copyOf(testSuiteTargets)); } /** 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 becc737239..4c864db3d8 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 @@ -52,15 +52,23 @@ public final class TargetPatternPhaseValue implements SkyValue { private final ImmutableSet<Target> filteredTargets; private final ImmutableSet<Target> testFilteredTargets; + // These two fields are only for the purposes of generating the TargetParsingCompleteEvent. + // TODO(ulfjack): Support EventBus event posting in Skyframe, and remove this code again. + private final ImmutableSet<Target> originalTargets; + private final ImmutableSet<Target> testSuiteTargets; + TargetPatternPhaseValue(ImmutableSet<Target> targets, @Nullable ImmutableSet<Target> testsToRun, boolean hasError, boolean hasPostExpansionError, ImmutableSet<Target> filteredTargets, - ImmutableSet<Target> testFilteredTargets) { + ImmutableSet<Target> testFilteredTargets, ImmutableSet<Target> originalTargets, + ImmutableSet<Target> testSuiteTargets) { this.targets = Preconditions.checkNotNull(targets); this.testsToRun = testsToRun; this.hasError = hasError; this.hasPostExpansionError = hasPostExpansionError; this.filteredTargets = Preconditions.checkNotNull(filteredTargets); this.testFilteredTargets = Preconditions.checkNotNull(testFilteredTargets); + this.originalTargets = Preconditions.checkNotNull(originalTargets); + this.testSuiteTargets = Preconditions.checkNotNull(testSuiteTargets); } public ImmutableSet<Target> getTargets() { @@ -88,6 +96,14 @@ public final class TargetPatternPhaseValue implements SkyValue { return testFilteredTargets; } + public ImmutableSet<Target> getOriginalTargets() { + return originalTargets; + } + + public ImmutableSet<Target> getTestSuiteTargets() { + return testSuiteTargets; + } + public LoadingResult toLoadingResult() { return new LoadingResult(hasError(), hasPostExpansionError(), getTargets(), getTestsToRun(), ImmutableMap.<PackageIdentifier, Path>of()); 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 1e0b234d89..65c10cb007 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 @@ -27,6 +27,7 @@ import com.google.common.base.Predicates; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableListMultimap; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.eventbus.EventBus; import com.google.common.eventbus.Subscribe; @@ -230,8 +231,8 @@ public class LoadingPhaseRunnerTest { tester.addFile("my_test/BUILD", "sh_test(name = 'my_test', srcs = ['test.cc'])"); assertNoErrors(tester.loadTests("-//my_test")); - assertThat(tester.filteredTargets).containsExactlyElementsIn(getTargets()); - assertThat(tester.testFilteredTargets).containsExactlyElementsIn(getTargets()); + assertThat(tester.getFilteredTargets()).containsExactlyElementsIn(getTargets()); + assertThat(tester.getTestFilteredTargets()).containsExactlyElementsIn(getTargets()); } @Test @@ -239,8 +240,8 @@ public class LoadingPhaseRunnerTest { tester.addFile("my_library/BUILD", "cc_library(name = 'my_library', srcs = ['test.cc'])"); assertNoErrors(tester.loadTests("-//my_library")); - assertThat(tester.filteredTargets).containsExactlyElementsIn(getTargets()); - assertThat(tester.testFilteredTargets).containsExactlyElementsIn(getTargets()); + assertThat(tester.getFilteredTargets()).containsExactlyElementsIn(getTargets()); + assertThat(tester.getTestFilteredTargets()).containsExactlyElementsIn(getTargets()); } private void writeBuildFilesForTestFiltering() throws Exception { @@ -258,8 +259,8 @@ public class LoadingPhaseRunnerTest { .containsExactlyElementsIn(getTargets("//tests:t1", "//tests:t2")); assertThat(loadingResult.getTestsToRun()) .containsExactlyElementsIn(getTargets("//tests:t1", "//tests:t2")); - assertThat(tester.filteredTargets).containsExactlyElementsIn(getTargets()); - assertThat(tester.testFilteredTargets).containsExactlyElementsIn(getTargets()); + assertThat(tester.getFilteredTargets()).containsExactlyElementsIn(getTargets()); + assertThat(tester.getTestFilteredTargets()).containsExactlyElementsIn(getTargets()); } @Test @@ -271,8 +272,8 @@ public class LoadingPhaseRunnerTest { .containsExactlyElementsIn(getTargets("//tests:t1", "//tests:t2")); assertThat(loadingResult.getTestsToRun()) .containsExactlyElementsIn(getTargets("//tests:t1", "//tests:t2")); - assertThat(tester.filteredTargets).containsExactlyElementsIn(getTargets()); - assertThat(tester.testFilteredTargets).containsExactlyElementsIn(getTargets()); + assertThat(tester.getFilteredTargets()).containsExactlyElementsIn(getTargets()); + assertThat(tester.getTestFilteredTargets()).containsExactlyElementsIn(getTargets()); } @Test @@ -283,8 +284,8 @@ public class LoadingPhaseRunnerTest { assertThat(loadingResult.getTargets()) .containsExactlyElementsIn(getTargets("//tests:t1", "//tests:t2")); assertThat(loadingResult.getTestsToRun()).containsExactlyElementsIn(getTargets("//tests:t1")); - assertThat(tester.filteredTargets).containsExactlyElementsIn(getTargets()); - assertThat(tester.testFilteredTargets).containsExactlyElementsIn(getTargets()); + assertThat(tester.getFilteredTargets()).containsExactlyElementsIn(getTargets()); + assertThat(tester.getTestFilteredTargets()).containsExactlyElementsIn(getTargets()); } @Test @@ -294,8 +295,8 @@ public class LoadingPhaseRunnerTest { LoadingResult loadingResult = assertNoErrors(tester.loadTests("//tests:all")); assertThat(loadingResult.getTargets()).containsExactlyElementsIn(getTargets("//tests:t1")); assertThat(loadingResult.getTestsToRun()).containsExactlyElementsIn(getTargets("//tests:t1")); - assertThat(tester.filteredTargets).containsExactlyElementsIn(getTargets()); - assertThat(tester.testFilteredTargets).containsExactlyElementsIn(getTargets("//tests:t2")); + assertThat(tester.getFilteredTargets()).containsExactlyElementsIn(getTargets()); + assertThat(tester.getTestFilteredTargets()).containsExactlyElementsIn(getTargets("//tests:t2")); } @Test @@ -307,8 +308,8 @@ public class LoadingPhaseRunnerTest { .containsExactlyElementsIn(getTargets("//tests:t1", "//tests:t3")); assertThat(loadingResult.getTestsToRun()) .containsExactlyElementsIn(getTargets("//tests:t1", "//tests:t3")); - assertThat(tester.filteredTargets).containsExactlyElementsIn(getTargets()); - assertThat(tester.testFilteredTargets).containsExactlyElementsIn(getTargets("//tests:t2")); + assertThat(tester.getFilteredTargets()).containsExactlyElementsIn(getTargets()); + assertThat(tester.getTestFilteredTargets()).containsExactlyElementsIn(getTargets("//tests:t2")); } @Test @@ -324,6 +325,10 @@ public class LoadingPhaseRunnerTest { assertThat(loadingResult.getPackageRoots().entrySet()) .contains(entryFor(PackageIdentifier.createInDefaultRepo("cc"), tester.getWorkspace())); } + assertThat(tester.getOriginalTargets()) + .containsExactlyElementsIn(getTargets("//cc:tests", "//cc:my_test")); + assertThat(tester.getTestSuiteTargets()) + .containsExactlyElementsIn(getTargets("//cc:tests")); } @Test @@ -384,8 +389,9 @@ public class LoadingPhaseRunnerTest { .containsExactlyElementsIn(getTargets("//foo:foo", "//foo:baz")); assertThat(loadingResult.getTestsToRun()) .containsExactlyElementsIn(getTargets("//foo:foo", "//foo:baz")); - assertThat(tester.filteredTargets).containsExactlyElementsIn(getTargets()); - assertThat(tester.testFilteredTargets).containsExactlyElementsIn(getTargets("//foo:foo_suite")); + assertThat(tester.getFilteredTargets()).containsExactlyElementsIn(getTargets()); + assertThat(tester.getTestFilteredTargets()) + .containsExactlyElementsIn(getTargets("//foo:foo_suite")); } /** Regression test for bug: "subtracting tests from test doesn't work" */ @@ -630,8 +636,8 @@ public class LoadingPhaseRunnerTest { private final StoredEventHandler storedErrors; private LoadingCallback loadingCallback; - private Set<Target> filteredTargets; - private Set<Target> testFilteredTargets; + private TargetParsingCompleteEvent targetParsingCompleteEvent; + private LoadingPhaseCompleteEvent loadingPhaseCompleteEvent; private MockToolsConfig mockToolsConfig; @@ -709,8 +715,8 @@ public class LoadingPhaseRunnerTest { result = loadingPhaseRunner.execute(storedErrors, eventBus, ImmutableList.copyOf(patterns), options, ImmutableListMultimap.<String, Label>of(), keepGoing, /*enableLoading=*/true, determineTests, loadingCallback); - this.filteredTargets = listener.filteredTargets; - this.testFilteredTargets = listener.testFilteredTargets; + this.targetParsingCompleteEvent = listener.targetParsingCompleteEvent; + this.loadingPhaseCompleteEvent = listener.loadingPhaseCompleteEvent; } catch (LoadingFailedException e) { System.err.println(storedErrors.getEvents()); throw e; @@ -780,6 +786,22 @@ public class LoadingPhaseRunnerTest { return skyframeExecutor.getPackageManager(); } + public ImmutableSet<Target> getFilteredTargets() { + return targetParsingCompleteEvent.getFilteredTargets(); + } + + public ImmutableSet<Target> getTestFilteredTargets() { + return targetParsingCompleteEvent.getTestFilteredTargets(); + } + + public ImmutableSet<Target> getOriginalTargets() { + return targetParsingCompleteEvent.getTargets(); + } + + public ImmutableSet<Target> getTestSuiteTargets() { + return loadingPhaseCompleteEvent.getFilteredTargets(); + } + private Iterable<Event> filteredEvents() { return Iterables.filter(storedErrors.getEvents(), new Predicate<Event>() { @Override @@ -808,12 +830,17 @@ public class LoadingPhaseRunnerTest { } public static class FilteredTargetListener { - private Set<Target> filteredTargets; - private Set<Target> testFilteredTargets; + private TargetParsingCompleteEvent targetParsingCompleteEvent; + private LoadingPhaseCompleteEvent loadingPhaseCompleteEvent; + + @Subscribe + public void targetParsingComplete(TargetParsingCompleteEvent event) { + this.targetParsingCompleteEvent = event; + } + @Subscribe - public void notifyFilteredTargets(TargetParsingCompleteEvent event) { - filteredTargets = event.getFilteredTargets(); - testFilteredTargets = event.getTestFilteredTargets(); + public void loadingPhaseComplete(LoadingPhaseCompleteEvent event) { + this.loadingPhaseCompleteEvent = event; } } } |