From 4c9b72a936607781457bc0324b1b44b4c28fad89 Mon Sep 17 00:00:00 2001 From: Googler Date: Fri, 17 Jul 2020 10:35:58 -0700 Subject: [PATCH] Automated rollback of commit 5bb2239c4c835a55d22527c86702c0ded2028b82. *** Reason for rollback *** Manually rolled back on behalf of: michajlo. Reason Given: The build was fixed at HEAD, the autorollback https://github.com/bazelbuild/bazel/commit/5bb2239c4c835a55d22527c86702c0ded2028b82 wound up breaking it again. *** Original change description *** Automated rollback of commit 8a202670ede475e3cd34bbecee5ded4696c058de. *** Reason for rollback *** TAP has detected 10 or more targets failed to build at https://github.com/bazelbuild/bazel/commit/8a202670ede475e3cd34bbecee5ded4696c058de. TO ROLLFORWARD (without additional approval): Use[] To see all broken targets visit [] To prevent noise from flakes, TAP double-checked the following target fails to build: [] *** PiperOrigin-RevId: 321806550 --- .../packages/AbstractPackageLoader.java | 17 ++++--- .../build/lib/skyframe/packages/BUILD | 1 + .../lib/skyframe/packages/PackageLoader.java | 13 ++++- .../packages/AbstractPackageLoaderTest.java | 49 +++++++++++++++++-- 4 files changed, 70 insertions(+), 10 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java b/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java index 30e67c81572208..dfdb5931a67e4f 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java @@ -30,6 +30,7 @@ import com.google.devtools.build.lib.clock.BlazeClock; import com.google.devtools.build.lib.cmdline.PackageIdentifier; import com.google.devtools.build.lib.events.Reporter; +import com.google.devtools.build.lib.events.StoredEventHandler; import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException; import com.google.devtools.build.lib.packages.BuildFileName; import com.google.devtools.build.lib.packages.CachingPackageLocator; @@ -126,7 +127,7 @@ public Diff getDiff(WalkableGraph fromGraph, Version fromVersion, Version toVers return preinjectedDiff; } }; - private final Reporter reporter; + private final Reporter commonReporter; protected final ConfiguredRuleClassProvider ruleClassProvider; private final PackageFactory pkgFactory; protected StarlarkSemantics starlarkSemantics; @@ -149,7 +150,7 @@ public abstract static class Builder { protected ExternalFilesHelper externalFilesHelper; protected ConfiguredRuleClassProvider ruleClassProvider = getDefaultRuleClassProvider(); protected StarlarkSemantics starlarkSemantics; - protected Reporter reporter = new Reporter(new EventBus()); + protected Reporter commonReporter = new Reporter(new EventBus()); protected Map extraSkyFunctions = new HashMap<>(); List extraPrecomputedValues = new ArrayList<>(); int legacyGlobbingThreads = 1; @@ -194,8 +195,9 @@ public Builder useDefaultStarlarkSemantics() { return this; } - public Builder setReporter(Reporter reporter) { - this.reporter = reporter; + /** Sets the reporter used by all skyframe evaluations. */ + public Builder setCommonReporter(Reporter commonReporter) { + this.commonReporter = commonReporter; return this; } @@ -258,7 +260,7 @@ public final PackageLoader build() { AbstractPackageLoader(Builder builder) { this.ruleClassProvider = builder.ruleClassProvider; this.starlarkSemantics = builder.starlarkSemantics; - this.reporter = builder.reporter; + this.commonReporter = builder.commonReporter; this.extraSkyFunctions = ImmutableMap.copyOf(builder.extraSkyFunctions); this.pkgLocatorRef = builder.pkgLocatorRef; this.legacyGlobbingThreads = builder.legacyGlobbingThreads; @@ -322,6 +324,9 @@ public Result loadPackages(Iterable pkgIds) throws Interrupte keys.add(PackageValue.key(pkgId)); } + Reporter reporter = new Reporter(commonReporter); + StoredEventHandler storedEventHandler = new StoredEventHandler(); + reporter.addHandler(storedEventHandler); EvaluationContext evaluationContext = EvaluationContext.newBuilder() .setKeepGoing(true) @@ -344,7 +349,7 @@ public Result loadPackages(Iterable pkgIds) throws Interrupte : new PackageOrException(packageValue.getPackage(), null)); } - return new Result(result.build()); + return new Result(result.build(), storedEventHandler.getEvents()); } public ConfiguredRuleClassProvider getRuleClassProvider() { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/packages/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/packages/BUILD index bf66fc53f208cb..73d812a8e5bfc8 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/packages/BUILD +++ b/src/main/java/com/google/devtools/build/lib/skyframe/packages/BUILD @@ -16,6 +16,7 @@ java_library( srcs = ["PackageLoader.java"], deps = [ "//src/main/java/com/google/devtools/build/lib/cmdline", + "//src/main/java/com/google/devtools/build/lib/events", "//src/main/java/com/google/devtools/build/lib/packages", "//third_party:guava", "//third_party:jsr305", diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/packages/PackageLoader.java b/src/main/java/com/google/devtools/build/lib/skyframe/packages/PackageLoader.java index 97d7a313057365..627f6bb1fa0315 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/packages/PackageLoader.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/packages/PackageLoader.java @@ -15,8 +15,10 @@ import static com.google.common.base.Preconditions.checkState; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.cmdline.PackageIdentifier; +import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.packages.NoSuchPackageException; import com.google.devtools.build.lib.packages.Package; import javax.annotation.Nullable; @@ -38,14 +40,23 @@ public interface PackageLoader { /** Contains the result of package loading. */ class Result { private final ImmutableMap loadedPackages; + private final ImmutableList events; - Result(ImmutableMap loadedPackages) { + Result( + ImmutableMap loadedPackages, + ImmutableList events) { this.loadedPackages = loadedPackages; + this.events = events; } public ImmutableMap getLoadedPackages() { return loadedPackages; } + + /** Returns all events generated while loading the requested packages. */ + public ImmutableList getEvents() { + return events; + } } /** Contains a {@link Package} or the exception produced while loading it. */ diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoaderTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoaderTest.java index 6e2f62f6c7c85f..fcfd045aadca54 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoaderTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoaderTest.java @@ -59,7 +59,7 @@ public final void init() throws Exception { protected abstract AbstractPackageLoader.Builder newPackageLoaderBuilder(Root workspaceDir); private AbstractPackageLoader.Builder newPackageLoaderBuilder() { - return newPackageLoaderBuilder(root).useDefaultStarlarkSemantics().setReporter(reporter); + return newPackageLoaderBuilder(root).useDefaultStarlarkSemantics().setCommonReporter(reporter); } protected PackageLoader newPackageLoader() { @@ -67,7 +67,7 @@ protected PackageLoader newPackageLoader() { } @Test - public void simpleNoPackage() throws Exception { + public void simpleNoPackage() { PackageLoader pkgLoader = newPackageLoader(); PackageIdentifier pkgId = PackageIdentifier.createInMainRepo(PathFragment.create("nope")); NoSuchPackageException expected = @@ -109,8 +109,10 @@ public void simpleMultipleGoodPackage() throws Exception { file("good2/BUILD", "sh_library(name = 'good2')"); PackageIdentifier pkgId1 = PackageIdentifier.createInMainRepo(PathFragment.create("good1")); PackageIdentifier pkgId2 = PackageIdentifier.createInMainRepo(PathFragment.create("good2")); + PackageLoader.Result result = pkgLoader.loadPackages(ImmutableList.of(pkgId1, pkgId2)); ImmutableMap pkgs = - pkgLoader.loadPackages(ImmutableList.of(pkgId1, pkgId2)).getLoadedPackages(); + result.getLoadedPackages(); + assertThat(pkgs.get(pkgId1).get().containsErrors()).isFalse(); assertThat(pkgs.get(pkgId2).get().containsErrors()).isFalse(); assertThat(pkgs.get(pkgId1).get().getTarget("good1").getAssociatedRule().getRuleClass()) @@ -119,9 +121,38 @@ public void simpleMultipleGoodPackage() throws Exception { .isEqualTo("sh_library"); assertNoEvents(pkgs.get(pkgId1).get().getEvents()); assertNoEvents(pkgs.get(pkgId2).get().getEvents()); + assertNoEvents(result.getEvents()); assertNoEvents(handler.getEvents()); } + @Test + public void testGoodAndBadAndMissingPackages() throws Exception { + PackageLoader pkgLoader = newPackageLoader(); + + file("bad/BUILD", "invalidBUILDsyntax"); + PackageIdentifier badPkgId = PackageIdentifier.createInMainRepo(PathFragment.create("bad")); + + file("good/BUILD", "sh_library(name = 'good')"); + PackageIdentifier goodPkgId = PackageIdentifier.createInMainRepo(PathFragment.create("good")); + + PackageIdentifier missingPkgId = PackageIdentifier.createInMainRepo("missing"); + + PackageLoader.Result result = + pkgLoader.loadPackages(ImmutableList.of(badPkgId, goodPkgId, missingPkgId)); + + Package goodPkg = result.getLoadedPackages().get(goodPkgId).get(); + assertThat(goodPkg.containsErrors()).isFalse(); + + Package badPkg = result.getLoadedPackages().get(badPkgId).get(); + assertThat(badPkg.containsErrors()).isTrue(); + + assertThrows( + NoSuchPackageException.class, () -> result.getLoadedPackages().get(missingPkgId).get()); + + assertContainsEvent(result.getEvents(), "invalidBUILDsyntax"); + assertContainsEvent(handler.getEvents(), "invalidBUILDsyntax"); + } + @Test public void loadPackagesToleratesDuplicates() throws Exception { PackageLoader pkgLoader = newPackageLoader(); @@ -182,6 +213,18 @@ public void externalFile_AssumeNonExistentAndImmutable() throws Exception { assertThat(expected).hasMessageThat().contains("no such package 'foo': BUILD file not found"); } + @Test + public void testNonPackageEventsReported() throws Exception { + PackageLoader pkgLoader = newPackageLoader(); + path("foo").createDirectoryAndParents(); + symlink("foo/infinitesymlinkpkg", path("foo")); + + PackageIdentifier pkgId = PackageIdentifier.createInMainRepo("foo/infinitesymlinkpkg"); + PackageLoader.Result result = pkgLoader.loadPackages(ImmutableList.of(pkgId)); + assertThrows(NoSuchPackageException.class, () -> result.getLoadedPackages().get(pkgId).get()); + assertContainsEvent(result.getEvents(), "infinite symlink expansion detected"); + } + protected Path path(String rootRelativePath) { return workspaceDir.getRelative(PathFragment.create(rootRelativePath)); }