Inject metadata when creating a filesystem symlink for a non-symlink …

…artifact.

A ctx.actions.symlink whose output is a declare_file or declare_directory
(as opposed to a declare_symlink) has "copy" semantics, i.e., the output
artifact is indistinguishable from the referent except for its name; the
symlink is just a filesystem-level optimization to avoid an actual copy,
and is transparently resolved when collecting the action output metadata.

When the symlink points to an artifact that was built remotely and without the
bytes, we currently must download it before executing the symlink action, so
that the output metadata can be constructed from the local filesystem. This
change short-circuits the filesystem traversal by injecting output metadata,
which is identical to the input plus a pointer to the original path. This is
used by the prefetcher to avoid downloading the same files multiple times when
they're symlinked more than once.

(An alternative would have been to teach all of the RemoteActionFileSystem
methods to resolve symlinks by patching together the local and remote metadata,
but that would have resulted in an awful lot of complexity.)

Fixes bazelbuild#15678.

src/main/java/com/google/devtools/build/lib/actions/ActionInputMap.java

@@ -84,6 +84,18 @@ TrieArtifact getOrPutSubFolder(String name) {
       return trieArtifact;
     }
 
+    @Nullable
+    TreeArtifactValue findTreeArtifactNode(PathFragment execPath) {
+      TrieArtifact current = this;
+      for (String segment : execPath.segments()) {
+        current = current.subFolders.get(segment);
+        if (current == null) {
+          return null;
+        }
+      }
+      return current.treeArtifactValue;
+    }
+
     @Nullable
     TreeArtifactValue findTreeArtifactNodeAtPrefix(PathFragment execPath) {
       TrieArtifact current = this;
@@ -244,6 +256,11 @@ private FileArtifactValue getMetadataFromTreeArtifacts(PathFragment execPath) {
     return entry != null ? entry.getValue() : null;
   }
 
+  @Nullable
+  public TreeArtifactValue getTreeMetadata(PathFragment execPath) {
+    return treeArtifactsRoot.findTreeArtifactNode(execPath);
+  }
+
   @Nullable
   @Override
   public ActionInput getInput(String execPathString) {

src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java

@@ -36,6 +36,7 @@
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.Objects;
+import java.util.Optional;
 import javax.annotation.Nullable;
 
 /**
@@ -52,6 +53,7 @@
  * <ul>
  *   <li>This is how dependent actions get hold of the output metadata of their generated inputs.
  *   <li>This is how {@code FileSystemValueChecker} figures out which actions need to be invalidated
+ *   <li>This is how {@code FileSystemValueChecker} figures out which actions need to be invalidated
  *       (just propagating the invalidation up from leaf nodes is not enough, because the output
  *       tree may have been changed while Blaze was not looking)
  * </ul>
@@ -172,6 +174,17 @@ protected boolean couldBeModifiedByMetadata(FileArtifactValue lastKnown) {
     return true;
   }
 
+  /**
+   * Optional symlink target path.
+   *
+   * <p>If present, this artifact is a copy of another artifact. It is materialized in the local
+   * filesystem as a symlink to the original artifact, whose contents live at this location. This is
+   * used to implement zero-cost copies of remotely stored artifacts.
+   */
+  public Optional<PathFragment> getSymlinkTargetExecPath() {
+    return Optional.empty();
+  }
+
   /**
    * Marker interface for singleton implementations of this class.
    *
@@ -311,6 +324,16 @@ public static FileArtifactValue createProxy(byte[] digest) {
     return createForNormalFile(digest, /*proxy=*/ null, /*size=*/ 0);
   }
 
+  /**
+   * Creates a FileArtifactValue used as a 'proxy' input for a TreeArtifactValue. These are used in
+   * {@link ActionCacheChecker}.
+   */
+  public static FileArtifactValue createProxyForTreeArtifact(
+      byte[] digest, boolean isRemote, @Nullable PathFragment symlinkTargetExecPath) {
+    Preconditions.checkNotNull(digest);
+    return new TreeArtifactProxyFileArtifactValue(digest, isRemote, symlinkTargetExecPath);
+  }
+
   private static String bytesToString(byte[] bytes) {
     return "0x" + BaseEncoding.base16().omitPadding().encode(bytes);
   }
@@ -533,12 +556,100 @@ protected boolean couldBeModifiedByMetadata(FileArtifactValue o) {
     }
   }
 
+  private static final class TreeArtifactProxyFileArtifactValue extends FileArtifactValue {
+    private final byte[] digest;
+    private final boolean isRemote;
+    @Nullable private final PathFragment symlinkTargetExecPath;
+
+    private TreeArtifactProxyFileArtifactValue(
+        byte[] digest, boolean isRemote, @Nullable PathFragment symlinkTargetExecPath) {
+      this.digest = digest;
+      this.isRemote = isRemote;
+      this.symlinkTargetExecPath = symlinkTargetExecPath;
+    }
+
+    @Override
+    public boolean equals(Object o) {
+      if (this == o) {
+        return true;
+      }
+      if (!(o instanceof TreeArtifactProxyFileArtifactValue)) {
+        return false;
+      }
+      TreeArtifactProxyFileArtifactValue that = (TreeArtifactProxyFileArtifactValue) o;
+      return Arrays.equals(digest, that.digest)
+          && Objects.equals(symlinkTargetExecPath, that.symlinkTargetExecPath);
+    }
+
+    @Override
+    public int hashCode() {
+      return Objects.hash(Arrays.hashCode(digest), symlinkTargetExecPath);
+    }
+
+    @Override
+    public FileStateType getType() {
+      // TODO(tjgq): Technically a directory, but I'm unsure about the implications of changing it.
+      return FileStateType.REGULAR_FILE;
+    }
+
+    @Override
+    public byte[] getDigest() {
+      return digest;
+    }
+
+    @Override
+    @Nullable
+    public FileContentsProxy getContentsProxy() {
+      return null;
+    }
+
+    @Override
+    public long getSize() {
+      return 0;
+    }
+
+    @Override
+    public boolean wasModifiedSinceDigest(Path path) {
+      return false;
+    }
+
+    @Override
+    public long getModifiedTime() {
+      throw new UnsupportedOperationException();
+    }
+
+    @Override
+    public String toString() {
+      return MoreObjects.toStringHelper(this)
+          .add(
+              "digest",
+              digest == null ? "(null)" : BaseEncoding.base16().lowerCase().encode(digest))
+          .add("symlinkTargetExecPath", symlinkTargetExecPath)
+          .toString();
+    }
+
+    @Override
+    protected boolean couldBeModifiedByMetadata(FileArtifactValue o) {
+      return false;
+    }
+
+    @Override
+    public boolean isRemote() {
+      return isRemote;
+    }
+
+    @Override
+    public Optional<PathFragment> getSymlinkTargetExecPath() {
+      return Optional.ofNullable(symlinkTargetExecPath);
+    }
+  }
+
   /** Metadata for remotely stored files. */
   public static class RemoteFileArtifactValue extends FileArtifactValue {
-    private final byte[] digest;
-    private final long size;
-    private final int locationIndex;
-    private final String actionId;
+    protected final byte[] digest;
+    protected final long size;
+    protected final int locationIndex;
+    protected final String actionId;
 
     private RemoteFileArtifactValue(byte[] digest, long size, int locationIndex, String actionId) {
       this.digest = Preconditions.checkNotNull(digest, actionId);
@@ -556,6 +667,19 @@ public static RemoteFileArtifactValue create(byte[] digest, long size, int locat
       return new RemoteFileArtifactValue(digest, size, locationIndex, /* actionId= */ "");
     }
 
+    public static RemoteFileArtifactValue create(
+        byte[] digest,
+        long size,
+        int locationIndex,
+        String actionId,
+        @Nullable PathFragment symlinkTargetExecPath) {
+      if (symlinkTargetExecPath != null) {
+        return new SymlinkToRemoteFileArtifactValue(
+            digest, size, locationIndex, actionId, symlinkTargetExecPath);
+      }
+      return new RemoteFileArtifactValue(digest, size, locationIndex, actionId);
+    }
+
     @Override
     public boolean equals(Object o) {
       if (!(o instanceof RemoteFileArtifactValue)) {
@@ -602,7 +726,7 @@ public String getActionId() {
     @Override
     public long getModifiedTime() {
       throw new UnsupportedOperationException(
-          "RemoteFileArifactValue doesn't support getModifiedTime");
+          "RemoteFileArtifactValue doesn't support getModifiedTime");
     }
 
     @Override
@@ -626,6 +750,58 @@ public String toString() {
           .add("digest", bytesToString(digest))
           .add("size", size)
           .add("locationIndex", locationIndex)
+          .add("actionId", actionId)
+          .toString();
+    }
+  }
+
+  /** An artifact that should be materialized as a symlink to another remote artifact. */
+  public static class SymlinkToRemoteFileArtifactValue extends RemoteFileArtifactValue {
+    private PathFragment symlinkTargetExecPath;
+
+    private SymlinkToRemoteFileArtifactValue(
+        byte[] digest,
+        long size,
+        int locationIndex,
+        String actionId,
+        PathFragment symlinkTargetExecPath) {
+      super(digest, size, locationIndex, actionId);
+      this.symlinkTargetExecPath = Preconditions.checkNotNull(symlinkTargetExecPath);
+    }
+
+    @Override
+    public Optional<PathFragment> getSymlinkTargetExecPath() {
+      return Optional.ofNullable(symlinkTargetExecPath);
+    }
+
+    @Override
+    public boolean equals(Object o) {
+      if (!(o instanceof SymlinkToRemoteFileArtifactValue)) {
+        return false;
+      }
+
+      SymlinkToRemoteFileArtifactValue that = (SymlinkToRemoteFileArtifactValue) o;
+      return Arrays.equals(digest, that.digest)
+          && size == that.size
+          && locationIndex == that.locationIndex
+          && Objects.equals(actionId, that.actionId)
+          && Objects.equals(symlinkTargetExecPath, that.symlinkTargetExecPath);
+    }
+
+    @Override
+    public int hashCode() {
+      return Objects.hash(
+          Arrays.hashCode(digest), size, locationIndex, actionId, symlinkTargetExecPath);
+    }
+
+    @Override
+    public String toString() {
+      return MoreObjects.toStringHelper(this)
+          .add("digest", bytesToString(digest))
+          .add("size", size)
+          .add("locationIndex", locationIndex)
+          .add("actionId", actionId)
+          .add("symlinkTargetExecPath", symlinkTargetExecPath)
           .toString();
     }
   }

src/main/java/com/google/devtools/build/lib/actions/cache/ActionCache.java

@@ -109,9 +109,10 @@ final class Entry {
     public abstract static class SerializableTreeArtifactValue {
       public static SerializableTreeArtifactValue create(
           ImmutableMap<String, RemoteFileArtifactValue> childValues,
-          Optional<RemoteFileArtifactValue> archivedFileValue) {
+          Optional<RemoteFileArtifactValue> archivedFileValue,
+          Optional<PathFragment> symlinkTargetExecPath) {
         return new AutoValue_ActionCache_Entry_SerializableTreeArtifactValue(
-            childValues, archivedFileValue);
+            childValues, archivedFileValue, symlinkTargetExecPath);
       }
 
       /**
@@ -138,17 +139,25 @@ public static Optional<SerializableTreeArtifactValue> createSerializable(
                 .filter(ar -> ar.archivedFileValue().isRemote())
                 .map(ar -> (RemoteFileArtifactValue) ar.archivedFileValue());
 
-        if (childValues.isEmpty() && archivedFileValue.isEmpty()) {
+        Optional<PathFragment> symlinkTargetExecPath = treeMetadata.getSymlinkTargetExecPath();
+
+        if (childValues.isEmpty()
+            && archivedFileValue.isEmpty()
+            && symlinkTargetExecPath.isEmpty()) {
           return Optional.empty();
         }
 
-        return Optional.of(SerializableTreeArtifactValue.create(childValues, archivedFileValue));
+        return Optional.of(
+            SerializableTreeArtifactValue.create(
+                childValues, archivedFileValue, symlinkTargetExecPath));
       }
 
       // A map from parentRelativePath to the file metadata
       public abstract ImmutableMap<String, RemoteFileArtifactValue> childValues();
 
       public abstract Optional<RemoteFileArtifactValue> archivedFileValue();
+
+      public abstract Optional<PathFragment> symlinkTargetExecPath();
     }
 
     public Entry(String key, Map<String, String> usedClientEnv, boolean discoversInputs) {

src/main/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCache.java

@@ -36,6 +36,7 @@
 import com.google.devtools.build.lib.util.VarInt;
 import com.google.devtools.build.lib.vfs.DigestUtils;
 import com.google.devtools.build.lib.vfs.Path;
+import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.lib.vfs.SyscallCache;
 import com.google.devtools.build.lib.vfs.UnixGlob;
 import java.io.ByteArrayOutputStream;
@@ -466,6 +467,14 @@ private static void encodeRemoteMetadata(
     VarInt.putVarInt(value.getLocationIndex(), sink);
 
     VarInt.putVarInt(indexer.getOrCreateIndex(value.getActionId()), sink);
+
+    Optional<PathFragment> symlinkTargetExecPath = value.getSymlinkTargetExecPath();
+    if (symlinkTargetExecPath.isPresent()) {
+      VarInt.putVarInt(1, sink);
+      VarInt.putVarInt(indexer.getOrCreateIndex(symlinkTargetExecPath.get().toString()), sink);
+    } else {
+      VarInt.putVarInt(0, sink);
+    }
   }
 
   private static final int MAX_REMOTE_METADATA_SIZE =
@@ -484,7 +493,18 @@ private static RemoteFileArtifactValue decodeRemoteMetadata(
 
     String actionId = getStringForIndex(indexer, VarInt.getVarInt(source));
 
-    return RemoteFileArtifactValue.create(digest, size, locationIndex, actionId);
+    PathFragment symlinkTargetExecPath = null;
+    int numSymlinkTargetExecPath = VarInt.getVarInt(source);
+    if (numSymlinkTargetExecPath > 0) {
+      if (numSymlinkTargetExecPath != 1) {
+        throw new IOException("Invalid number of symlink target paths");
+      }
+      symlinkTargetExecPath =
+          PathFragment.create(getStringForIndex(indexer, VarInt.getVarInt(source)));
+    }
+
+    return RemoteFileArtifactValue.create(
+        digest, size, locationIndex, actionId, symlinkTargetExecPath);
   }
 
   /** @return action data encoded as a byte[] array. */
@@ -578,6 +598,15 @@ private static byte[] encode(StringIndexer indexer, ActionCache.Entry entry) thr
       } else {
         VarInt.putVarInt(0, sink);
       }
+
+      Optional<PathFragment> symlinkTargetExecPath =
+          serializableTreeArtifactValue.symlinkTargetExecPath();
+      if (symlinkTargetExecPath.isPresent()) {
+        VarInt.putVarInt(1, sink);
+        VarInt.putVarInt(indexer.getOrCreateIndex(symlinkTargetExecPath.get().toString()), sink);
+      } else {
+        VarInt.putVarInt(0, sink);
+      }
     }
 
     return sink.toByteArray();
@@ -654,8 +683,20 @@ private static ActionCache.Entry decode(StringIndexer indexer, byte[] data) thro
           archivedFileValue = Optional.of(decodeRemoteMetadata(indexer, source));
         }
 
+        Optional<PathFragment> symlinkTargetPath = Optional.empty();
+        int numSymlinkTargetExecPath = VarInt.getVarInt(source);
+        if (numSymlinkTargetExecPath > 0) {
+          if (numSymlinkTargetExecPath != 1) {
+            throw new IOException("Invalid number of symlink target paths");
+          }
+          symlinkTargetPath =
+              Optional.of(
+                  PathFragment.create(getStringForIndex(indexer, VarInt.getVarInt(source))));
+        }
+
         SerializableTreeArtifactValue value =
-            SerializableTreeArtifactValue.create(childValues.buildOrThrow(), archivedFileValue);
+            SerializableTreeArtifactValue.create(
+                childValues.buildOrThrow(), archivedFileValue, symlinkTargetPath);
         outputTrees.put(treeKey, value);
       }