Skip to content

Commit

Permalink
Automated rollback of commit 5bb2239.
Browse files Browse the repository at this point in the history
*** Reason for rollback ***

Manually rolled back on behalf of: michajlo.
Reason Given: The build was fixed at HEAD, the autorollback bazelbuild@5bb2239 wound up breaking it again.

*** Original change description ***

Automated rollback of commit 8a20267.

*** Reason for rollback ***

TAP has detected 10 or more targets failed to build at bazelbuild@8a20267.

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
  • Loading branch information
Googler authored and copybara-github committed Jul 17, 2020
1 parent 5bb2239 commit 4c9b72a
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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<SkyFunctionName, SkyFunction> extraSkyFunctions = new HashMap<>();
List<PrecomputedValue.Injected> extraPrecomputedValues = new ArrayList<>();
int legacyGlobbingThreads = 1;
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -322,6 +324,9 @@ public Result loadPackages(Iterable<PackageIdentifier> 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)
Expand All @@ -344,7 +349,7 @@ public Result loadPackages(Iterable<PackageIdentifier> pkgIds) throws Interrupte
: new PackageOrException(packageValue.getPackage(), null));
}

return new Result(result.build());
return new Result(result.build(), storedEventHandler.getEvents());
}

public ConfiguredRuleClassProvider getRuleClassProvider() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -38,14 +40,23 @@ public interface PackageLoader {
/** Contains the result of package loading. */
class Result {
private final ImmutableMap<PackageIdentifier, PackageOrException> loadedPackages;
private final ImmutableList<Event> events;

Result(ImmutableMap<PackageIdentifier, PackageOrException> loadedPackages) {
Result(
ImmutableMap<PackageIdentifier, PackageOrException> loadedPackages,
ImmutableList<Event> events) {
this.loadedPackages = loadedPackages;
this.events = events;
}

public ImmutableMap<PackageIdentifier, PackageOrException> getLoadedPackages() {
return loadedPackages;
}

/** Returns all events generated while loading the requested packages. */
public ImmutableList<Event> getEvents() {
return events;
}
}

/** Contains a {@link Package} or the exception produced while loading it. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,15 +59,15 @@ 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() {
return newPackageLoaderBuilder().build();
}

@Test
public void simpleNoPackage() throws Exception {
public void simpleNoPackage() {
PackageLoader pkgLoader = newPackageLoader();
PackageIdentifier pkgId = PackageIdentifier.createInMainRepo(PathFragment.create("nope"));
NoSuchPackageException expected =
Expand Down Expand Up @@ -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<PackageIdentifier, PackageLoader.PackageOrException> 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())
Expand All @@ -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();
Expand Down Expand Up @@ -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));
}
Expand Down

0 comments on commit 4c9b72a

Please sign in to comment.