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

7 files changed, 243 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));