diff options
author | mschaller <mschaller@google.com> | 2018-04-19 11:40:16 -0700 |
---|---|---|
committer | Copybara-Service <copybara-piper@google.com> | 2018-04-19 11:42:04 -0700 |
commit | 1673cf67fc8be563ff3541795c42c44de1f70705 (patch) | |
tree | a376126c32bcddac31273793263b74d85a7dc335 /src | |
parent | 913b66c552a6e6b6c4bb496d4ccfd8f3fd518e7c (diff) |
Introduce PackageErrorMessageValue
This CL introduces a new intermediate SkyValue type,
PackageErrorMessageValue, to be used by RecursivePkgFunction.
RecursivePkgValue will now have a direct dep on
PackageErrorMessageValue rather than PackageValue.
RELNOTES: None
PiperOrigin-RevId: 193549158
Diffstat (limited to 'src')
8 files changed, 331 insertions, 42 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/CollectPackagesUnderDirectoryFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/CollectPackagesUnderDirectoryFunction.java index 2efd41b5a0..5d10a86fae 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/CollectPackagesUnderDirectoryFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/CollectPackagesUnderDirectoryFunction.java @@ -18,7 +18,6 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.cmdline.RepositoryName; -import com.google.devtools.build.lib.packages.NoSuchPackageException; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.RootedPath; import com.google.devtools.build.skyframe.SkyFunction; @@ -113,8 +112,8 @@ public class CollectPackagesUnderDirectoryFunction implements SkyFunction { } @Override - public void notePackageError(NoSuchPackageException e) { - errorMessage = e.getMessage(); + public void notePackageError(String noSuchPackageExceptionErrorMessage) { + this.errorMessage = noSuchPackageExceptionErrorMessage; } boolean isDirectoryPackage() { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageErrorMessageFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageErrorMessageFunction.java new file mode 100644 index 0000000000..ff4d1451eb --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageErrorMessageFunction.java @@ -0,0 +1,51 @@ +// Copyright 2018 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.skyframe; + +import com.google.devtools.build.lib.cmdline.PackageIdentifier; +import com.google.devtools.build.lib.packages.NoSuchPackageException; +import com.google.devtools.build.lib.packages.Package; +import com.google.devtools.build.skyframe.SkyFunction; +import com.google.devtools.build.skyframe.SkyKey; +import com.google.devtools.build.skyframe.SkyValue; +import javax.annotation.Nullable; + +/** {@link SkyFunction} for {@link PackageErrorMessageValue}. */ +public class PackageErrorMessageFunction implements SkyFunction { + @Nullable + @Override + public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedException { + PackageIdentifier pkgId = (PackageIdentifier) skyKey.argument(); + PackageValue pkgValue; + try { + pkgValue = + (PackageValue) env.getValueOrThrow(PackageValue.key(pkgId), NoSuchPackageException.class); + } catch (NoSuchPackageException e) { + return PackageErrorMessageValue.ofNoSuchPackageException(e.getMessage()); + } + if (pkgValue == null) { + return null; + } + Package pkg = pkgValue.getPackage(); + return pkg.containsErrors() + ? PackageErrorMessageValue.ofPackageWithErrors() + : PackageErrorMessageValue.ofPackageWithNoErrors(); + } + + @Nullable + @Override + public String extractTag(SkyKey skyKey) { + return null; + } +} diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageErrorMessageValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageErrorMessageValue.java new file mode 100644 index 0000000000..9cd9d7961b --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageErrorMessageValue.java @@ -0,0 +1,159 @@ +// Copyright 2018 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.skyframe; + +import com.google.common.collect.Interner; +import com.google.devtools.build.lib.cmdline.PackageIdentifier; +import com.google.devtools.build.lib.concurrent.BlazeInterners; +import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; +import com.google.devtools.build.skyframe.AbstractSkyKey; +import com.google.devtools.build.skyframe.SkyFunctionName; +import com.google.devtools.build.skyframe.SkyKey; +import com.google.devtools.build.skyframe.SkyValue; + +/** + * Encapsulates the errors, if any, encountered when loading a specific package. + * + * <p>This is a change-pruning-friendly convenience {@link SkyValue} for use cases where {@link + * Result} is sufficient. + */ +public abstract class PackageErrorMessageValue implements SkyValue { + /** Tri-state result of loading the package. */ + enum Result { + /** + * There was no error loading the package and {@link + * com.google.devtools.build.lib.packages.Package#containsErrors} returned {@code false}. + */ + NO_ERROR, + + /** + * There was no error loading the package and {@link + * com.google.devtools.build.lib.packages.Package#containsErrors} returned {@code true}. + */ + ERROR, + + /** + * There was a {@link com.google.devtools.build.lib.packages.NoSuchPackageException} loading the + * package. + */ + NO_SUCH_PACKAGE_EXCEPTION, + } + + /** Returns the {@link Result} from loading the package. */ + abstract Result getResult(); + + /** + * If {@code getResult().equals(NO_SUCH_PACKAGE_EXCEPTION)}, returns the error message from the + * {@link com.google.devtools.build.lib.packages.NoSuchPackageException} encountered. + */ + abstract String getNoSuchPackageExceptionMessage(); + + static PackageErrorMessageValue ofPackageWithNoErrors() { + return NO_ERROR_VALUE; + } + + static PackageErrorMessageValue ofPackageWithErrors() { + return ERROR_VALUE; + } + + static PackageErrorMessageValue ofNoSuchPackageException(String errorMessage) { + return new NoSuchPackageExceptionValue(errorMessage); + } + + static SkyKey key(PackageIdentifier pkgId) { + return Key.create(pkgId); + } + + @AutoCodec.VisibleForSerialization + @AutoCodec + static class Key extends AbstractSkyKey<PackageIdentifier> { + private static final Interner<Key> interner = BlazeInterners.newWeakInterner(); + + private Key(PackageIdentifier arg) { + super(arg); + } + + @AutoCodec.VisibleForSerialization + @AutoCodec.Instantiator + static Key create(PackageIdentifier arg) { + return interner.intern(new Key(arg)); + } + + @Override + public SkyFunctionName functionName() { + return SkyFunctions.PACKAGE_ERROR_MESSAGE; + } + } + + @AutoCodec + static final PackageErrorMessageValue NO_ERROR_VALUE = + new PackageErrorMessageValue() { + @Override + Result getResult() { + return Result.NO_ERROR; + } + + @Override + String getNoSuchPackageExceptionMessage() { + throw new IllegalStateException(); + } + }; + + @AutoCodec + static final PackageErrorMessageValue ERROR_VALUE = + new PackageErrorMessageValue() { + @Override + Result getResult() { + return Result.ERROR; + } + + @Override + String getNoSuchPackageExceptionMessage() { + throw new IllegalStateException(); + } + }; + + @AutoCodec + static class NoSuchPackageExceptionValue extends PackageErrorMessageValue { + private final String errorMessage; + + public NoSuchPackageExceptionValue(String errorMessage) { + this.errorMessage = errorMessage; + } + + @Override + Result getResult() { + return Result.NO_SUCH_PACKAGE_EXCEPTION; + } + + @Override + String getNoSuchPackageExceptionMessage() { + return errorMessage; + } + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof NoSuchPackageExceptionValue)) { + return false; + } + NoSuchPackageExceptionValue other = (NoSuchPackageExceptionValue) obj; + return errorMessage.equals(other.errorMessage); + } + + @Override + public int hashCode() { + return errorMessage.hashCode(); + } + } +} diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveDirectoryTraversalFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveDirectoryTraversalFunction.java index 4de261913d..faccffda57 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveDirectoryTraversalFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveDirectoryTraversalFunction.java @@ -23,14 +23,11 @@ import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.cmdline.PackageIdentifier; import com.google.devtools.build.lib.cmdline.RepositoryName; 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 com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.RootedPath; import com.google.devtools.build.skyframe.SkyFunction.Environment; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; -import com.google.devtools.build.skyframe.ValueOrException; import java.util.Map; /** @@ -106,7 +103,7 @@ abstract class RecursiveDirectoryTraversalFunction< * cycle * </ol> */ - void notePackageError(NoSuchPackageException e); + void notePackageError(String noSuchPackageExceptionErrorMessage); } /** @@ -137,47 +134,39 @@ abstract class RecursiveDirectoryTraversalFunction< Map<SkyKey, SkyValue> subdirectorySkyValues; if (packageExistenceAndSubdirDeps.packageExists()) { PathFragment rootRelativePath = rootedPath.getRootRelativePath(); - SkyKey packageKey = - PackageValue.key( + SkyKey packageErrorMessageKey = + PackageErrorMessageValue.key( PackageIdentifier.create(recursivePkgKey.getRepository(), rootRelativePath)); - Map<SkyKey, ValueOrException<NoSuchPackageException>> dependentSkyValues = - env.getValuesOrThrow( - Iterables.concat(childDeps, ImmutableList.of(packageKey)), - NoSuchPackageException.class); + Map<SkyKey, SkyValue> dependentSkyValues = + env.getValues(Iterables.concat(childDeps, ImmutableList.of(packageErrorMessageKey))); if (env.valuesMissing()) { return null; } - try { - PackageValue pkgValue = (PackageValue) dependentSkyValues.get(packageKey).get(); - if (pkgValue == null) { - return null; - } - Package pkg = pkgValue.getPackage(); - if (pkg.containsErrors()) { + PackageErrorMessageValue pkgErrorMessageValue = + (PackageErrorMessageValue) dependentSkyValues.get(packageErrorMessageKey); + switch (pkgErrorMessageValue.getResult()) { + case NO_ERROR: + consumer.notePackage(rootRelativePath); + break; + case ERROR: env.getListener() .handle(Event.error("package contains errors: " + rootRelativePath.getPathString())); - } - consumer.notePackage(rootRelativePath); - } catch (NoSuchPackageException e) { - // The package had errors, but don't fail-fast as there might be subpackages below the - // current directory. - env.getListener().handle(Event.error(e.getMessage())); - consumer.notePackageError(e); - if (env.valuesMissing()) { - return null; - } + consumer.notePackage(rootRelativePath); + break; + case NO_SUCH_PACKAGE_EXCEPTION: + // The package had errors, but don't fail-fast as there might be subpackages below the + // current directory. + String msg = pkgErrorMessageValue.getNoSuchPackageExceptionMessage(); + env.getListener().handle(Event.error(msg)); + consumer.notePackageError(msg); + break; + default: + throw new IllegalStateException(pkgErrorMessageValue.getResult().toString()); } - ImmutableMap.Builder<SkyKey, SkyValue> subdirectoryBuilder = ImmutableMap.builder(); - for (Map.Entry<SkyKey, ValueOrException<NoSuchPackageException>> entry : - Maps.filterKeys(dependentSkyValues, Predicates.not(Predicates.equalTo(packageKey))) - .entrySet()) { - try { - subdirectoryBuilder.put(entry.getKey(), entry.getValue().get()); - } catch (NoSuchPackageException e) { - // ignored. - } - } - subdirectorySkyValues = subdirectoryBuilder.build(); + subdirectorySkyValues = + ImmutableMap.copyOf( + Maps.filterKeys( + dependentSkyValues, Predicates.not(Predicates.equalTo(packageErrorMessageKey)))); } else { subdirectorySkyValues = env.getValues(childDeps); } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RecursivePkgFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/RecursivePkgFunction.java index 64c27dc97d..2de3bce9cb 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/RecursivePkgFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/RecursivePkgFunction.java @@ -89,7 +89,7 @@ public class RecursivePkgFunction implements SkyFunction { } @Override - public void notePackageError(NoSuchPackageException e) { + public void notePackageError(String noSuchPackageExceptionErrorMessage) { // Nothing to do because the RecursiveDirectoryTraversalFunction has already emitted an error // event. } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java index a80d7616d8..4d347e8783 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java @@ -46,6 +46,8 @@ public final class SkyFunctions { public static final SkyFunctionName GLOB = SkyFunctionName.create("GLOB"); public static final SkyFunctionName PACKAGE = SkyFunctionName.create("PACKAGE"); public static final SkyFunctionName PACKAGE_ERROR = SkyFunctionName.create("PACKAGE_ERROR"); + public static final SkyFunctionName PACKAGE_ERROR_MESSAGE = + SkyFunctionName.create("PACKAGE_ERROR_MESSAGE"); public static final SkyFunctionName TARGET_MARKER = SkyFunctionName.create("TARGET_MARKER"); public static final SkyFunctionName TARGET_PATTERN = SkyFunctionName.create("TARGET_PATTERN"); public static final SkyFunctionName TARGET_PATTERN_ERROR = 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 512996e2ff..63eb72b138 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 @@ -431,6 +431,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { actionOnIOExceptionReadingBuildFile, IncrementalityIntent.INCREMENTAL)); map.put(SkyFunctions.PACKAGE_ERROR, new PackageErrorFunction()); + map.put(SkyFunctions.PACKAGE_ERROR_MESSAGE, new PackageErrorMessageFunction()); map.put(SkyFunctions.TARGET_MARKER, new TargetMarkerFunction()); map.put(SkyFunctions.TARGET_PATTERN_ERROR, new TargetPatternErrorFunction()); map.put(SkyFunctions.TRANSITIVE_TARGET, new TransitiveTargetFunction(ruleClassProvider)); diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/PackageErrorMessageFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/PackageErrorMessageFunctionTest.java new file mode 100644 index 0000000000..411381de5f --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/skyframe/PackageErrorMessageFunctionTest.java @@ -0,0 +1,88 @@ +// Copyright 2018 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.skyframe; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.common.collect.ImmutableList; +import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; +import com.google.devtools.build.lib.cmdline.PackageIdentifier; +import com.google.devtools.build.lib.skyframe.PackageErrorMessageValue.Result; +import com.google.devtools.build.skyframe.EvaluationResult; +import com.google.devtools.build.skyframe.SkyKey; +import com.google.devtools.build.skyframe.SkyValue; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Tests for {@link PackageErrorMessageFunction}. */ +@RunWith(JUnit4.class) +public class PackageErrorMessageFunctionTest extends BuildViewTestCase { + + private SkyframeExecutor skyframeExecutor; + + @Before + public final void createSkyframeExecutor() { + skyframeExecutor = getSkyframeExecutor(); + } + + @Test + public void testNoErrorMessage() throws Exception { + scratch.file("a/BUILD"); + assertThat(getPackageErrorMessageValue(/*keepGoing=*/ false).getResult()) + .isEqualTo(Result.NO_ERROR); + } + + @Test + public void testPackageWithErrors() throws Exception { + // Opt out of failing fast on an error event. + reporter.removeHandler(failFastHandler); + + scratch.file("a/BUILD", "imaginary_macro(name = 'this macro is not defined')"); + + assertThat(getPackageErrorMessageValue(/*keepGoing=*/ false).getResult()) + .isEqualTo(Result.ERROR); + } + + @Test + public void testNoSuchPackageException() throws Exception { + scratch.file("a/BUILD", "load('//a:does_not_exist.bzl', 'imaginary_macro')"); + + PackageErrorMessageValue packageErrorMessageValue = + getPackageErrorMessageValue(/*keepGoing=*/ true); + assertThat(packageErrorMessageValue.getResult()).isEqualTo(Result.NO_SUCH_PACKAGE_EXCEPTION); + assertThat(packageErrorMessageValue.getNoSuchPackageExceptionMessage()) + .isEqualTo( + "error loading package 'a': Extension file not found. Unable to load file " + + "'//a:does_not_exist.bzl': file doesn't exist or isn't a file"); + } + + private PackageErrorMessageValue getPackageErrorMessageValue(boolean keepGoing) + throws InterruptedException { + SkyKey key = PackageErrorMessageValue.key(PackageIdentifier.createInMainRepo("a")); + EvaluationResult<SkyValue> result = + skyframeExecutor + .getDriverForTesting() + .evaluate( + ImmutableList.of(key), + /*keepGoing=*/ keepGoing, + SequencedSkyframeExecutor.DEFAULT_THREAD_COUNT, + reporter); + assertThat(result.hasError()).isFalse(); + SkyValue value = result.get(key); + assertThat(value).isInstanceOf(PackageErrorMessageValue.class); + return (PackageErrorMessageValue) value; + } +} |