Incorporate evaluating version into FrontierNodeVersion fingerprint.

For Bazel, the default version is Long.MIN_VALUE, until we have a good story to handle external version control systems. PiperOrigin-RevId: 696008776 Change-Id: I9478be27fc91c2332dde5689f5ff16bccf0d7bbb
jesses-canva · Nov 13, 2024 · c1b510a · c1b510a
1 parent 3a31eb2
commit c1b510a
Show file tree

Hide file tree

Showing 6 changed files with 68 additions and 16 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java
@@ -116,6 +116,7 @@
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.lib.vfs.Root;
 import com.google.devtools.build.skyframe.EvaluationResult;
+import com.google.devtools.build.skyframe.IntVersion;
 import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.SkyValue;
 import com.google.devtools.common.options.RegexPatternOption;
@@ -1103,6 +1104,7 @@ private static final class RemoteAnalysisCachingDependenciesProviderImpl
     private final RemoteAnalysisCachingEventListener listener;
     private final HashCode blazeInstallMD5;
     private final Future<FingerprintValueService> fingerprintValueServiceFuture;
+    private final IntVersion evaluatingVersion;
 
     // Non-final because the top level BuildConfigurationValue is determined just before analysis
     // begins in BuildView for the download/deserialization pass, which is later than when this
@@ -1152,6 +1154,13 @@ private RemoteAnalysisCachingDependenciesProviderImpl(
       }
       this.blazeInstallMD5 = requireNonNull(env.getDirectories().getInstallMD5());
       this.diffFromEvaluatingVersion = env.getSkyframeExecutor().getDiffFromEvaluatingVersion();
+      if (env.getWorkspaceInfoFromDiff() == null) {
+        // If there is no workspace info, we cannot confidently version the nodes. Use the min
+        // version as a sentinel.
+        this.evaluatingVersion = IntVersion.of(Long.MIN_VALUE);
+      } else {
+        this.evaluatingVersion = env.getWorkspaceInfoFromDiff().getEvaluatingVersion();
+      }
     }
 
     private static ObjectCodecs initAnalysisObjectCodecs(
@@ -1196,7 +1205,10 @@ public FrontierNodeVersion getSkyValueVersion() {
           if (frontierNodeVersionSingleton == null) {
             frontierNodeVersionSingleton =
                 new FrontierNodeVersion(
-                    topLevelConfigChecksum, activeDirectoriesMatcher.toString(), blazeInstallMD5);
+                    topLevelConfigChecksum,
+                    activeDirectoriesMatcher.toString(),
+                    blazeInstallMD5,
+                    evaluatingVersion);
             logger.atInfo().log(
                 "Remote analysis caching SkyValue version: %s", frontierNodeVersionSingleton);
             listener.recordSkyValueVersion(frontierNodeVersionSingleton);

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
@@ -3145,6 +3145,9 @@ java_library(
 java_library(
     name = "workspace_info",
     srcs = ["WorkspaceInfoFromDiff.java"],
+    deps = [
+        "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
+    ],
 )
 
 java_library(

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceInfoFromDiff.java b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceInfoFromDiff.java
@@ -13,5 +13,13 @@
 // limitations under the License.
 package com.google.devtools.build.lib.skyframe;
 
+import com.google.devtools.build.skyframe.IntVersion;
+
 /** Information for a workspace computed at the time of collecting diff. */
-public interface WorkspaceInfoFromDiff {}
+public interface WorkspaceInfoFromDiff {
+
+  default IntVersion getEvaluatingVersion() {
+    // TODO: b/367284400 - handle this for external version control systems.
+    return IntVersion.of(Long.MIN_VALUE);
+  }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/SkyValueRetriever.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/SkyValueRetriever.java
@@ -23,9 +23,11 @@
 import com.google.common.base.MoreObjects;
 import com.google.common.hash.HashCode;
 import com.google.common.primitives.Bytes;
+import com.google.common.primitives.Longs;
 import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.devtools.build.lib.skyframe.serialization.FingerprintValueStore.MissingFingerprintValueException;
+import com.google.devtools.build.skyframe.IntVersion;
 import com.google.devtools.build.skyframe.SkyFunction;
 import com.google.devtools.build.skyframe.SkyFunction.Environment;
 import com.google.devtools.build.skyframe.SkyFunction.Environment.SkyKeyComputeState;
@@ -329,25 +331,34 @@ private SkyValueRetriever() {}
   /** A tuple representing the version of a cached SkyValue in the frontier. */
   public static final class FrontierNodeVersion {
     public static final FrontierNodeVersion CONSTANT_FOR_TESTING =
-        new FrontierNodeVersion("123", "string_for_testing", HashCode.fromInt(42));
+        new FrontierNodeVersion(
+            "123", "string_for_testing", HashCode.fromInt(42), IntVersion.of(9000));
+
+    // Fingerprints of version components.
     private final byte[] topLevelConfigFingerprint;
     private final byte[] directoryMatcherFingerprint;
     private final byte[] blazeInstallMD5Fingerprint;
+    private final byte[] evaluatingVersionFingerprint;
+
+    // Fingerprint of the full version.
     private final byte[] precomputedFingerprint;
 
     public FrontierNodeVersion(
         String topLevelConfigChecksum,
         String directoryMatcherStringRepr,
-        HashCode blazeInstallMD5) {
+        HashCode blazeInstallMD5,
+        IntVersion evaluatingVersion) {
       // TODO: b/364831651 - add more fields like source and blaze versions.
       this.topLevelConfigFingerprint = topLevelConfigChecksum.getBytes(UTF_8);
       this.directoryMatcherFingerprint = directoryMatcherStringRepr.getBytes(UTF_8);
       this.blazeInstallMD5Fingerprint = blazeInstallMD5.asBytes();
+      this.evaluatingVersionFingerprint = Longs.toByteArray(evaluatingVersion.getVal());
       this.precomputedFingerprint =
           Bytes.concat(
               this.topLevelConfigFingerprint,
               this.directoryMatcherFingerprint,
-              this.blazeInstallMD5Fingerprint);
+              this.blazeInstallMD5Fingerprint,
+              this.evaluatingVersionFingerprint);
     }
 
     public byte[] getTopLevelConfigFingerprint() {
@@ -368,6 +379,7 @@ public String toString() {
           .add("topLevelConfig", Arrays.hashCode(topLevelConfigFingerprint))
           .add("directoryMatcher", Arrays.hashCode(directoryMatcherFingerprint))
           .add("blazeInstall", Arrays.hashCode(blazeInstallMD5Fingerprint))
+          .add("evaluatingVersion", Arrays.hashCode(evaluatingVersionFingerprint))
           .add("precomputed", hashCode())
           .toString();
     }

diff --git a/src/main/java/com/google/devtools/build/skyframe/BUILD b/src/main/java/com/google/devtools/build/skyframe/BUILD
@@ -12,6 +12,7 @@ SKYFRAME_OBJECT_SRCS = [
     "AbstractSkyKey.java",
     "FunctionHermeticity.java",
     "GroupedDeps.java",
+    "IntVersion.java",
     "NodeVersion.java",
     "NotComparableSkyValue.java",
     "SkyframeLookupResult.java",

diff --git a/...test/java/com/google/devtools/build/lib/skyframe/serialization/SkyValueRetrieverTest.java b/...test/java/com/google/devtools/build/lib/skyframe/serialization/SkyValueRetrieverTest.java
@@ -36,6 +36,7 @@
 import com.google.devtools.build.lib.skyframe.serialization.SkyValueRetriever.WaitingForLookupContinuation;
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import com.google.devtools.build.lib.skyframe.serialization.testutils.GetRecordingStore;
+import com.google.devtools.build.skyframe.IntVersion;
 import com.google.devtools.build.skyframe.SkyFunctionName;
 import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.SkyValue;
@@ -168,7 +169,8 @@ public void waitingForFutureValueBytes_withMatchingDistinguisher_returnsImmediat
         new FrontierNodeVersion(
             /* topLevelConfigChecksum= */ "42",
             /* directoryMatcherStringRepr= */ "some_string",
-            /* blazeInstallMD5= */ HashCode.fromInt(42));
+            /* blazeInstallMD5= */ HashCode.fromInt(42),
+            /* evaluatingVersion= */ IntVersion.of(9000));
     uploadKeyValuePair(key, version, value, fingerprintValueService);
 
     RetrievalResult result =
@@ -195,7 +197,8 @@ public void waitingForFutureValueBytes_withNonMatchingDistinguisher_returnsNoCac
         new FrontierNodeVersion(
             /* topLevelConfigChecksum= */ "42",
             /* directoryMatcherStringRepr= */ "some_string",
-            /* blazeInstallMD5= */ HashCode.fromInt(42));
+            /* blazeInstallMD5= */ HashCode.fromInt(42),
+            /* evaluatingVersion= */ IntVersion.of(1234));
     uploadKeyValuePair(key, version, value, fingerprintValueService);
 
     RetrievalResult result =
@@ -209,7 +212,8 @@ public void waitingForFutureValueBytes_withNonMatchingDistinguisher_returnsNoCac
             /* frontierNodeVersion= */ new FrontierNodeVersion(
                 /* topLevelConfigChecksum= */ "9000",
                 /* directoryMatcherStringRepr= */ "another_string",
-                /* blazeInstallMD5= */ HashCode.fromInt(9000)));
+                /* blazeInstallMD5= */ HashCode.fromInt(9000),
+                /* evaluatingVersion= */ IntVersion.of(5678)));
 
     assertThat(result).isSameInstanceAs(NO_CACHED_DATA);
   }
@@ -546,35 +550,47 @@ public void exceptionWhileWaitingForResult_throwsException() throws Exception {
 
   @Test
   public void frontierNodeVersions_areEqual_ifTupleComponentsAreEqual() {
-    var first = new FrontierNodeVersion("foo", "bar", HashCode.fromInt(42));
-    var second = new FrontierNodeVersion("foo", "bar", HashCode.fromInt(42));
+    var first = new FrontierNodeVersion("foo", "bar", HashCode.fromInt(42), IntVersion.of(9000));
+    var second = new FrontierNodeVersion("foo", "bar", HashCode.fromInt(42), IntVersion.of(9000));
 
     assertThat(first.getPrecomputedFingerprint()).isEqualTo(second.getPrecomputedFingerprint());
     assertThat(first).isEqualTo(second);
   }
 
   @Test
   public void frontierNodeVersions_areNotEqual_ifTopLevelConfigChecksumIsDifferent() {
-    var first = new FrontierNodeVersion("foo", "bar", HashCode.fromInt(42));
-    var second = new FrontierNodeVersion("CHANGED", "bar", HashCode.fromInt(42));
+    var first = new FrontierNodeVersion("foo", "bar", HashCode.fromInt(42), IntVersion.of(9000));
+    var second =
+        new FrontierNodeVersion("CHANGED", "bar", HashCode.fromInt(42), IntVersion.of(9000));
 
     assertThat(first.getPrecomputedFingerprint()).isNotEqualTo(second.getPrecomputedFingerprint());
     assertThat(first).isNotEqualTo(second);
   }
 
   @Test
   public void frontierNodeVersions_areNotEqual_ifActiveDirectoriesAreDifferent() {
-    var first = new FrontierNodeVersion("foo", "bar", HashCode.fromInt(42));
-    var second = new FrontierNodeVersion("foo", "CHANGED", HashCode.fromInt(42));
+    var first = new FrontierNodeVersion("foo", "bar", HashCode.fromInt(42), IntVersion.of(9000));
+    var second =
+        new FrontierNodeVersion("foo", "CHANGED", HashCode.fromInt(42), IntVersion.of(9000));
 
     assertThat(first.getPrecomputedFingerprint()).isNotEqualTo(second.getPrecomputedFingerprint());
     assertThat(first).isNotEqualTo(second);
   }
 
   @Test
   public void frontierNodeVersions_areNotEqual_ifBlazeInstallMD5IsDifferent() {
-    var first = new FrontierNodeVersion("foo", "bar", HashCode.fromInt(42));
-    var second = new FrontierNodeVersion("foo", "bar", HashCode.fromInt(9000));
+    var first = new FrontierNodeVersion("foo", "bar", HashCode.fromInt(42), IntVersion.of(9000));
+    var second = new FrontierNodeVersion("foo", "bar", HashCode.fromInt(9000), IntVersion.of(9000));
+
+    assertThat(first.getPrecomputedFingerprint()).isNotEqualTo(second.getPrecomputedFingerprint());
+    assertThat(first).isNotEqualTo(second);
+  }
+
+  @Test
+  public void frontierNodeVersions_areNotEqual_ifEvaluatingVersionIsDifferent() {
+    var first = new FrontierNodeVersion("foo", "bar", HashCode.fromInt(42), IntVersion.of(9000));
+    var second =
+        new FrontierNodeVersion("foo", "bar", HashCode.fromInt(9000), IntVersion.of(10000));
 
     assertThat(first.getPrecomputedFingerprint()).isNotEqualTo(second.getPrecomputedFingerprint());
     assertThat(first).isNotEqualTo(second);