Don't check direct deps when doing change pruning. Since dependents of a node don't have access to its dependencies ("grandparents don't know grandchildren", or vice versa, depending on your point of view), changes to a node's dependencies can't affect downstream nodes.

--
MOS_MIGRATED_REVID=114143894

5 files changed, 76 insertions, 10 deletions

diff --git a/src/main/java/com/google/devtools/build/skyframe/BuildingState.java b/src/main/java/com/google/devtools/build/skyframe/BuildingState.java
index 9de91be05d..2804ecd39d 100644
--- a/src/main/java/com/google/devtools/build/skyframe/BuildingState.java
+++ b/src/main/java/com/google/devtools/build/skyframe/BuildingState.java
@@ -300,16 +300,18 @@ public class BuildingState {
   }
 
   /**
-   * Returns true if {@code newValue}.equals the value from the last time this node was built, and
-   * the deps requested during this evaluation are exactly those requested the last time this node
-   * was built, in the same order. Should only be used by {@link NodeEntry#setValue}.
+   * Returns true if {@code newValue}.equals the value from the last time this node was built.
+   * Should only be used by {@link NodeEntry#setValue}.
+   *
+   * <p>Changes in direct deps do <i>not</i> force this to return false. Only the value is
+   * considered.
    */
   boolean unchangedFromLastBuild(SkyValue newValue) {
     checkFinishedBuildingWhenAboutToSetValue();
     if (newValue instanceof NotComparableSkyValue) {
       return false;
     }
-    return getLastBuildValue().equals(newValue) && lastBuildDirectDeps.equals(directDeps);
+    return getLastBuildValue().equals(newValue);
   }
 
   /**
diff --git a/src/main/java/com/google/devtools/build/skyframe/InMemoryNodeEntry.java b/src/main/java/com/google/devtools/build/skyframe/InMemoryNodeEntry.java
index c23b8222d7..7b885b8ebd 100644
--- a/src/main/java/com/google/devtools/build/skyframe/InMemoryNodeEntry.java
+++ b/src/main/java/com/google/devtools/build/skyframe/InMemoryNodeEntry.java
@@ -54,10 +54,10 @@ public class InMemoryNodeEntry implements NodeEntry {
   private SkyValue value = null;
 
   /**
-   * The last version of the graph at which this node entry was changed. In {@link #setValue} it
-   * may be determined that the data being written to the graph at a given version is the same as
-   * the already-stored data. In that case, the version will remain the same. The version can be
-   * thought of as the latest timestamp at which this entry was changed.
+   * The last version of the graph at which this node's value was changed. In {@link #setValue} it
+   * may be determined that the value being written to the graph at a given version is the same as
+   * the already-stored value. In that case, the version will remain the same. The version can be
+   * thought of as the latest timestamp at which this value was changed.
    */
   protected Version lastChangedVersion = MinimalVersion.INSTANCE;
 
diff --git a/src/test/java/com/google/devtools/build/skyframe/InMemoryNodeEntryTest.java b/src/test/java/com/google/devtools/build/skyframe/InMemoryNodeEntryTest.java
index f29537d032..1298c480f3 100644
--- a/src/test/java/com/google/devtools/build/skyframe/InMemoryNodeEntryTest.java
+++ b/src/test/java/com/google/devtools/build/skyframe/InMemoryNodeEntryTest.java
@@ -14,6 +14,7 @@
 package com.google.devtools.build.skyframe;
 
 import static com.google.common.truth.Truth.assertThat;
+import static com.google.devtools.build.skyframe.NodeEntrySubjectFactory.assertThatNodeEntry;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNull;
@@ -573,7 +574,7 @@ public class InMemoryNodeEntryTest {
   }
 
   @Test
-  public void noPruneWhenDepsChange() {
+  public void pruneWhenDepsChange() {
     NodeEntry entry = new InMemoryNodeEntry();
     entry.addReverseDepAndCheckIfDone(null); // Start evaluation.
     SkyKey dep = key("dep");
@@ -593,7 +594,7 @@ public class InMemoryNodeEntryTest {
     assertTrue(entry.signalDep(IntVersion.of(1L)));
     setValue(entry, new IntegerValue(5), /*errorInfo=*/null, /*graphVersion=*/1L);
     assertTrue(entry.isDone());
-    assertEquals("Version increments when deps change", IntVersion.of(1L), entry.getVersion());
+    assertThatNodeEntry(entry).hasVersionThat().isEqualTo(IntVersion.of(0L));
   }
 
   @Test
diff --git a/src/test/java/com/google/devtools/build/skyframe/NodeEntrySubject.java b/src/test/java/com/google/devtools/build/skyframe/NodeEntrySubject.java
new file mode 100644
index 0000000000..537d738c94
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/skyframe/NodeEntrySubject.java
@@ -0,0 +1,33 @@
+// Copyright 2016 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.skyframe;
+
+import com.google.common.truth.DefaultSubject;
+import com.google.common.truth.FailureStrategy;
+import com.google.common.truth.Subject;
+import com.google.common.truth.Truth;
+
+/**
+ * {@link Subject} for {@link NodeEntry}. Please add to this class if you need more
+ * functionality!
+ */
+public class NodeEntrySubject extends Subject<NodeEntrySubject, NodeEntry> {
+  public NodeEntrySubject(FailureStrategy failureStrategy, NodeEntry nodeEntry) {
+    super(failureStrategy, nodeEntry);
+  }
+
+  DefaultSubject hasVersionThat() {
+    return Truth.assertThat(getSubject().getVersion()).named("Version for " + getDisplaySubject());
+  }
+}
diff --git a/src/test/java/com/google/devtools/build/skyframe/NodeEntrySubjectFactory.java b/src/test/java/com/google/devtools/build/skyframe/NodeEntrySubjectFactory.java
new file mode 100644
index 0000000000..7278085a96
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/skyframe/NodeEntrySubjectFactory.java
@@ -0,0 +1,30 @@
+// Copyright 2016 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.skyframe;
+
+import com.google.common.truth.FailureStrategy;
+import com.google.common.truth.SubjectFactory;
+import com.google.common.truth.Truth;
+
+/** {@link SubjectFactory} for {@link NodeEntry} objects, providing {@link NodeEntrySubject}s. */
+public class NodeEntrySubjectFactory extends SubjectFactory<NodeEntrySubject, NodeEntry> {
+  public static NodeEntrySubject assertThatNodeEntry(NodeEntry nodeEntry) {
+    return Truth.assertAbout(new NodeEntrySubjectFactory()).that(nodeEntry);
+  }
+
+  @Override
+  public NodeEntrySubject getSubject(FailureStrategy failureStrategy, NodeEntry nodeEntry) {
+    return new NodeEntrySubject(failureStrategy, nodeEntry);
+  }
+}