commit a4fff8785cf5adbbcf16384a10fda662350a5803
author Googler <noreply@google.com> Wed Jun 17 14:26:35 2020 -0700
committer Copybara-Service <copybara-worker@google.com> Wed Jun 17 14:27:33 2020 -0700
tree a6c8e1ba9e8bcceb068fbc2c1796efc555d648a0
parent 656cc0611b835a5fc2e549b7edf2e4501a4be0d3

Remove TODO about mixed remote/local tree artifacts. This must remain a supported use case. Rename isRemote to isEntirelyRemote to be more clear with respect to this case.

PiperOrigin-RevId: 316965014

src/main/java/com/google/devtools/build/lib/skyframe/FilesystemValueChecker.java

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/FilesystemValueChecker.java b/src/main/java/com/google/devtools/build/lib/skyframe/FilesystemValueChecker.java
index 9e3932e..b17d20b 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/FilesystemValueChecker.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/FilesystemValueChecker.java
@@ -429,7 +429,7 @@
     try {
       Set<PathFragment> currentDirectoryValue =
           TreeArtifactValue.explodeDirectory(artifact.getPath());
-      return !(currentDirectoryValue.isEmpty() && value.isRemote())
+      return !(currentDirectoryValue.isEmpty() && value.isEntirelyRemote())
           && !currentDirectoryValue.equals(value.getChildPaths());
     } catch (IOException e) {
       return true;
@@ -485,7 +485,7 @@
         actionValue.getAllTreeArtifactValues().entrySet()) {
       TreeArtifactValue tree = entry.getValue();
 
-      if (!tree.isRemote()) {
+      if (!tree.isEntirelyRemote()) {
         for (Map.Entry<TreeFileArtifact, FileArtifactValue> childEntry :
             tree.getChildValues().entrySet()) {
           if (artifactIsDirtyWithDirectSystemCalls(

src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java
index 01920ca..258a96c 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java
@@ -49,20 +49,22 @@
   @SerializationConstant @AutoCodec.VisibleForSerialization
   static final TreeArtifactValue EMPTY =
       new TreeArtifactValue(
-          DigestUtils.fromMetadata(ImmutableMap.of()), ImmutableSortedMap.of(), /*remote=*/ false);
+          DigestUtils.fromMetadata(ImmutableMap.of()),
+          ImmutableSortedMap.of(),
+          /*entirelyRemote=*/ false);
 
   private final byte[] digest;
   private final ImmutableSortedMap<TreeFileArtifact, FileArtifactValue> childData;
-  private final boolean remote;
+  private final boolean entirelyRemote;
 
   @AutoCodec.VisibleForSerialization
   TreeArtifactValue(
       byte[] digest,
       ImmutableSortedMap<TreeFileArtifact, FileArtifactValue> childData,
-      boolean remote) {
+      boolean entirelyRemote) {
     this.digest = digest;
     this.childData = childData;
-    this.remote = remote;
+    this.entirelyRemote = entirelyRemote;
   }
 
   /**
@@ -76,7 +78,7 @@
     }
     Map<String, FileArtifactValue> digestBuilder =
         Maps.newHashMapWithExpectedSize(childFileValues.size());
-    boolean remote = true;
+    boolean entirelyRemote = true;
     for (Map.Entry<TreeFileArtifact, ? extends FileArtifactValue> e : childFileValues.entrySet()) {
       TreeFileArtifact child = e.getKey();
       FileArtifactValue value = e.getValue();
@@ -84,15 +86,14 @@
           !FileArtifactValue.OMITTED_FILE_MARKER.equals(value),
           "Cannot construct TreeArtifactValue because child %s was omitted",
           child);
-      // TODO(buchgr): Enforce that all children in a tree artifact are either remote or local
-      // once b/70354083 is fixed.
-      remote = remote && value.isRemote();
+      // Tolerate a tree artifact having a mix of local and remote children (b/152496153#comment80).
+      entirelyRemote &= value.isRemote();
       digestBuilder.put(child.getParentRelativePath().getPathString(), value);
     }
     return new TreeArtifactValue(
         DigestUtils.fromMetadata(digestBuilder),
         ImmutableSortedMap.copyOf(childFileValues),
-        remote);
+        entirelyRemote);
   }
 
   FileArtifactValue getSelfData() {
@@ -124,8 +125,8 @@
   }
 
   /** Returns true if the {@link TreeFileArtifact}s are only stored remotely. */
-  public boolean isRemote() {
-    return remote;
+  public boolean isEntirelyRemote() {
+    return entirelyRemote;
   }
 
   @Override
@@ -173,7 +174,7 @@
   static final TreeArtifactValue MISSING_TREE_ARTIFACT = createMarker("MISSING_TREE_ARTIFACT");
 
   private static TreeArtifactValue createMarker(String toStringRepresentation) {
-    return new TreeArtifactValue(null, ImmutableSortedMap.of(), /*remote=*/ false) {
+    return new TreeArtifactValue(null, ImmutableSortedMap.of(), /*entirelyRemote=*/ false) {
       @Override
       FileArtifactValue getSelfData() {
         throw new UnsupportedOperationException(toString());