Add execution platform information to the action key.

Needed so that actions are properly invalidated and re-run when the platform's remote execution properties change.

Fixes #4756.
Closes #4728.

Change-Id: Ia227d110c10c9aad2164cefa8a90361cc4359332
PiperOrigin-RevId: 187621349

5 files changed, 39 insertions, 0 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java b/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java
index 86fa2410b7..7e264670b8 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java
@@ -314,6 +314,15 @@ public abstract class AbstractAction implements Action, SkylarkValue {
       try {
         Fingerprint fp = new Fingerprint();
         computeKey(actionKeyContext, fp);
+
+        // Add a bool indicating whether the execution platform was set.
+        fp.addBoolean(getExecutionPlatform() != null);
+        if (getExecutionPlatform() != null) {
+          // Add the execution platform information.
+          getExecutionPlatform().addTo(fp);
+        }
+
+        // Compute the actual key and store it.
         cachedKey = fp.hexDigestAndReset();
       } catch (CommandLineExpansionException e) {
         cachedKey = KEY_ERROR;
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/platform/ConstraintSettingInfo.java b/src/main/java/com/google/devtools/build/lib/analysis/platform/ConstraintSettingInfo.java
index 16fa28af09..907a1b977c 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/platform/ConstraintSettingInfo.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/platform/ConstraintSettingInfo.java
@@ -28,6 +28,7 @@ import com.google.devtools.build.lib.skylarkinterface.SkylarkModuleCategory;
 import com.google.devtools.build.lib.syntax.EvalException;
 import com.google.devtools.build.lib.syntax.FunctionSignature;
 import com.google.devtools.build.lib.syntax.SkylarkType;
+import com.google.devtools.build.lib.util.Fingerprint;
 
 /** Provider for a platform constraint setting that is available to be fulfilled. */
 @SkylarkModule(
@@ -93,4 +94,9 @@ public class ConstraintSettingInfo extends NativeInfo {
   public static ConstraintSettingInfo create(Label constraintSetting, Location location) {
     return new ConstraintSettingInfo(constraintSetting, location);
   }
+
+  /** Add this constraint setting to the given fingerprint. */
+  public void addTo(Fingerprint fp) {
+    fp.addString(label.getCanonicalForm());
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/platform/ConstraintValueInfo.java b/src/main/java/com/google/devtools/build/lib/analysis/platform/ConstraintValueInfo.java
index febe3e7e84..7603f0820f 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/platform/ConstraintValueInfo.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/platform/ConstraintValueInfo.java
@@ -28,6 +28,7 @@ import com.google.devtools.build.lib.skylarkinterface.SkylarkModuleCategory;
 import com.google.devtools.build.lib.syntax.EvalException;
 import com.google.devtools.build.lib.syntax.FunctionSignature;
 import com.google.devtools.build.lib.syntax.SkylarkType;
+import com.google.devtools.build.lib.util.Fingerprint;
 
 /** Provider for a platform constraint value that fulfills a {@link ConstraintSettingInfo}. */
 @SkylarkModule(
@@ -111,4 +112,10 @@ public class ConstraintValueInfo extends NativeInfo {
       ConstraintSettingInfo constraint, Label value, Location location) {
     return new ConstraintValueInfo(constraint, value, location);
   }
+
+  /** Add this constraint value to the given fingerprint. */
+  public void addTo(Fingerprint fp) {
+    this.constraint.addTo(fp);
+    fp.addString(label.getCanonicalForm());
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/platform/PlatformInfo.java b/src/main/java/com/google/devtools/build/lib/analysis/platform/PlatformInfo.java
index c70690b7e2..6b57ddf786 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/platform/PlatformInfo.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/platform/PlatformInfo.java
@@ -38,6 +38,7 @@ import com.google.devtools.build.lib.syntax.EvalException;
 import com.google.devtools.build.lib.syntax.FunctionSignature;
 import com.google.devtools.build.lib.syntax.SkylarkList;
 import com.google.devtools.build.lib.syntax.SkylarkType;
+import com.google.devtools.build.lib.util.Fingerprint;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
@@ -173,6 +174,14 @@ public class PlatformInfo extends NativeInfo {
     return new Builder();
   }
 
+  /** Add this platform to the given fingerprint. */
+  public void addTo(Fingerprint fp) {
+    fp.addString(label.toString());
+    fp.addNullableString(remoteExecutionProperties);
+    fp.addInt(constraints.size());
+    constraints.values().forEach(constraintValue -> constraintValue.addTo(fp));
+  }
+
   /** Builder class to facilitate creating valid {@link PlatformInfo} instances. */
   public static class Builder {
     private Label label;
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/platform/PlatformInfoTest.java b/src/test/java/com/google/devtools/build/lib/analysis/platform/PlatformInfoTest.java
index 97fd4ef51e..6c7c91bd4d 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/platform/PlatformInfoTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/platform/PlatformInfoTest.java
@@ -116,6 +116,14 @@ public class PlatformInfoTest extends BuildViewTestCase {
                 .setLabel(makeLabel("//platform/plat1"))
                 .addConstraint(value1)
                 .build())
+        .addEqualityGroup(
+            // Different remote exec properties.
+            PlatformInfo.builder()
+                .setLabel(makeLabel("//platform/plat1"))
+                .addConstraint(value1)
+                .addConstraint(value2)
+                .setRemoteExecutionProperties("foo")
+                .build())
         .testEquals();
   }