Include the full absolute paths of runfiles in action keys

Both `SourceManifestAction` and `SymlinkTreeAction`, the only users of `Runfiles#fingerprint`, emit absolute paths to runfiles artifacts in their output. This requires also including these paths in the action key computation to prevent incorrect incremental builds if `--output_base` changes.
bazelbuild · Aug 4, 2023 · e07fcfb · e07fcfb
1 parent 90ee766
commit e07fcfb
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 13 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/Runfiles.java b/src/main/java/com/google/devtools/build/lib/analysis/Runfiles.java
@@ -100,16 +100,17 @@ public void fingerprint(Fingerprint fp) {
         args.accept(symlink.getArtifact().getExecPathString());
       };
 
-  private static final CommandLineItem.ExceptionlessMapFn<Artifact> RUNFILES_AND_EXEC_PATH_MAP_FN =
-      (artifact, args) -> {
-        args.accept(artifact.getRunfilesPathString());
-        args.accept(artifact.getExecPathString());
-      };
+  private static final CommandLineItem.ExceptionlessMapFn<Artifact>
+      RUNFILES_AND_ABSOLUTE_PATH_MAP_FN =
+          (artifact, args) -> {
+            args.accept(artifact.getRunfilesPathString());
+            args.accept(artifact.getPath().getPathString());
+          };
 
   /**
    * The directory to put all runfiles under.
    *
-   * <p>Using "foo" will put runfiles under &lt;target&gt;.runfiles/foo.</p>
+   * <p>Using "foo" will put runfiles under &lt;target&gt;.runfiles/foo.
    *
    * <p>This is either set to the workspace name, or is empty.
    */
@@ -1112,23 +1113,26 @@ public Runfiles mergeAll(Sequence<?> sequence, StarlarkThread thread) throws Eva
     }
   }
 
-  /** Fingerprint this {@link Runfiles} tree. */
-  public void fingerprint(ActionKeyContext actionKeyContext, Fingerprint fp) {
+  /** Fingerprint this {@link Runfiles} tree, including the absolute paths of artifacts. */
+  public void fingerprintWithAbsolutePaths(ActionKeyContext actionKeyContext, Fingerprint fp) {
     fp.addInt(conflictPolicy.ordinal());
     fp.addBoolean(legacyExternalRunfiles);
     fp.addPath(suffix);
 
     actionKeyContext.addNestedSetToFingerprint(SYMLINK_ENTRY_MAP_FN, fp, symlinks);
     actionKeyContext.addNestedSetToFingerprint(SYMLINK_ENTRY_MAP_FN, fp, rootSymlinks);
-    actionKeyContext.addNestedSetToFingerprint(RUNFILES_AND_EXEC_PATH_MAP_FN, fp, artifacts);
+    actionKeyContext.addNestedSetToFingerprint(RUNFILES_AND_ABSOLUTE_PATH_MAP_FN, fp, artifacts);
 
     emptyFilesSupplier.fingerprint(fp);
 
     // extraMiddlemen does not affect the shape of the runfiles tree described by this instance and
     // thus does not need to be fingerprinted.
   }
 
-  /** Describes the inputs {@link #fingerprint} uses to aid describeKey() descriptions. */
+  /**
+   * Describes the inputs {@link #fingerprintWithAbsolutePaths} uses to aid describeKey()
+   * descriptions.
+   */
   public String describeFingerprint() {
     return String.format("conflictPolicy: %s\n", conflictPolicy)
         + String.format("legacyExternalRunfiles: %s\n", legacyExternalRunfiles)
@@ -1139,7 +1143,7 @@ public String describeFingerprint() {
             "rootSymlinks: %s\n", describeNestedSetFingerprint(SYMLINK_ENTRY_MAP_FN, rootSymlinks))
         + String.format(
             "artifacts: %s\n",
-            describeNestedSetFingerprint(RUNFILES_AND_EXEC_PATH_MAP_FN, artifacts))
+            describeNestedSetFingerprint(RUNFILES_AND_ABSOLUTE_PATH_MAP_FN, artifacts))
         + String.format("emptyFilesSupplier: %s\n", emptyFilesSupplier.getClass().getName());
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/SourceManifestAction.java b/src/main/java/com/google/devtools/build/lib/analysis/SourceManifestAction.java
@@ -254,7 +254,7 @@ protected void computeKey(
       Fingerprint fp) {
     fp.addString(GUID);
     fp.addBoolean(remotableSourceManifestActions);
-    runfiles.fingerprint(actionKeyContext, fp);
+    runfiles.fingerprintWithAbsolutePaths(actionKeyContext, fp);
     fp.addBoolean(repoMappingManifest != null);
     if (repoMappingManifest != null) {
       fp.addPath(repoMappingManifest.getExecPath());

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/SymlinkTreeAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/SymlinkTreeAction.java
@@ -215,7 +215,7 @@ protected void computeKey(
     // safe to add more fields in the future.
     fp.addBoolean(runfiles != null);
     if (runfiles != null) {
-      runfiles.fingerprint(actionKeyContext, fp);
+      runfiles.fingerprintWithAbsolutePaths(actionKeyContext, fp);
     }
     fp.addBoolean(repoMappingManifest != null);
     if (repoMappingManifest != null) {