diff options
3 files changed, 27 insertions, 22 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/query2/BlazeQueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/BlazeQueryEnvironment.java index 3e4135b8f3..421c5b57eb 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/BlazeQueryEnvironment.java +++ b/src/main/java/com/google/devtools/build/lib/query2/BlazeQueryEnvironment.java @@ -50,6 +50,7 @@ import com.google.devtools.build.lib.query2.engine.QueryUtil.UniquifierImpl; import com.google.devtools.build.lib.query2.engine.SkyframeRestartQueryException; import com.google.devtools.build.lib.query2.engine.ThreadSafeOutputFormatterCallback; import com.google.devtools.build.lib.query2.engine.Uniquifier; +import com.google.devtools.build.lib.skyframe.SkyframeLabelVisitor; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import java.io.IOException; @@ -329,7 +330,8 @@ public class BlazeQueryEnvironment extends AbstractBlazeQueryEnvironment<Target> // Only do the full visitation if "maxDepth" is large enough. Otherwise, the benefits of // preloading will be outweighed by the cost of doing more work than necessary. Set<Label> labels = targets.stream().map(Target::getLabel).collect(toImmutableSet()); - transitivePackageLoader.sync(eventHandler, labels, keepGoing, loadingPhaseThreads); + ((SkyframeLabelVisitor) transitivePackageLoader) + .sync(eventHandler, labels, keepGoing, loadingPhaseThreads, false); } } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeLabelVisitor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeLabelVisitor.java index e8d25da219..c61c69ff9d 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeLabelVisitor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeLabelVisitor.java @@ -27,12 +27,13 @@ import com.google.devtools.build.skyframe.SkyKey; import java.util.Map.Entry; import java.util.Set; import java.util.concurrent.atomic.AtomicReference; +import java.util.stream.Collectors; import javax.annotation.Nullable; /** * Skyframe-based transitive package loader. */ -final class SkyframeLabelVisitor implements TransitivePackageLoader { +public final class SkyframeLabelVisitor implements TransitivePackageLoader { private final SkyframeTransitivePackageLoader transitivePackageLoader; private final AtomicReference<CyclesReporter> skyframeCyclesReporter; @@ -43,7 +44,6 @@ final class SkyframeLabelVisitor implements TransitivePackageLoader { this.skyframeCyclesReporter = skyframeCyclesReporter; } - // The only remaining non-test caller of this code is BlazeQueryEnvironment. @Override public boolean sync( ExtendedEventHandler eventHandler, @@ -51,6 +51,17 @@ final class SkyframeLabelVisitor implements TransitivePackageLoader { boolean keepGoing, int parallelThreads) throws InterruptedException { + return sync(eventHandler, labelsToVisit, keepGoing, parallelThreads, true); + } + + // The only remaining non-test caller of this code is BlazeQueryEnvironment. + public boolean sync( + ExtendedEventHandler eventHandler, + Set<Label> labelsToVisit, + boolean keepGoing, + int parallelThreads, + boolean errorOnCycles) + throws InterruptedException { EvaluationResult<TransitiveTargetValue> result = transitivePackageLoader.loadTransitiveTargets( eventHandler, labelsToVisit, keepGoing, parallelThreads); @@ -59,6 +70,17 @@ final class SkyframeLabelVisitor implements TransitivePackageLoader { } Set<Entry<SkyKey, ErrorInfo>> errors = result.errorMap().entrySet(); + if (!errorOnCycles) { + errors = + errors + .stream() + .filter(error -> Iterables.isEmpty(error.getValue().getCycleInfo())) + .collect(Collectors.toSet()); + if (errors.isEmpty()) { + return true; + } + } + if (!keepGoing) { // We may have multiple errors, but in non keep_going builds, we're obligated to print only // one of them. diff --git a/src/test/shell/integration/loading_phase_tests.sh b/src/test/shell/integration/loading_phase_tests.sh index f2ec8982f2..6b654ff349 100755 --- a/src/test/shell/integration/loading_phase_tests.sh +++ b/src/test/shell/integration/loading_phase_tests.sh @@ -265,25 +265,6 @@ function test_build_file_symlinks() { expect_log "Empty results" } -function test_visibility_edge_causes_cycle() { - mkdir -p a b || fail "mkdir failed" - echo 'sh_library(name="a", visibility=["//b"])' > a/BUILD - echo 'sh_library(name="b", deps=["//a"])' > b/BUILD - bazel query 'deps(//a)' >& $TEST_log && fail "Expected failure" - expect_log "cycle in dependency graph" - expect_log "The cycle is caused by a visibility edge" - bazel query 'deps(//b)' >& $TEST_log && fail "Expected failure" - expect_log "cycle in dependency graph" - expect_log "The cycle is caused by a visibility edge" - echo 'sh_library(name="a", visibility=["//b:__pkg__"])' > a/BUILD - bazel query 'deps(//a)' >& $TEST_log || fail "Expected success" - expect_log "//a:a" - expect_not_log "//b:b" - bazel query 'deps(//b)' >& $TEST_log || fail "Expected success" - expect_log "//a:a" - expect_log "//b:b" -} - # Regression test for bug "ASTFileLookupFunction has an unnoted # dependency on the PathPackageLocator". function test_incremental_deleting_package_roots() { |