Track client environment in Skyframe

...to determine which actions have to be recomputed based on changes
to the client environment. Note that this change does it the simple way
and reconsideres all actions on a changed client environment, while still
only reexecuting those, where the part that was inherited from the environment
actually did change.

--
Change-Id: Ie1116d094642165e5e959447a6fcf49d19b37d6e
Reviewed-on: https://bazel-review.googlesource.com/#/c/5431
MOS_MIGRATED_REVID=133010705

12 files changed, 226 insertions, 46 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionAnalysisMetadata.java b/src/main/java/com/google/devtools/build/lib/actions/ActionAnalysisMetadata.java
index 9a0edfd611..4b45965036 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionAnalysisMetadata.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionAnalysisMetadata.java
@@ -73,6 +73,11 @@ public interface ActionAnalysisMetadata {
   /**
    * Returns the environment variables from the client environment that this action depends on. May
    * be empty.
+   *
+   * <p>Warning: For optimization reasons, the available environment variables are restricted to
+   * those white-listed on the command line. If actions want to specify additional client
+   * environment variables to depend on, that restriction must be lifted in
+   * {@link com.google.devtools.build.lib.runtime.CommandEnvironment}.
    */
   Iterable<String> getClientEnvironmentVariables();
 
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java b/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java
index 5b913ad1c3..b97d32342b 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java
@@ -15,6 +15,7 @@ package com.google.devtools.build.lib.actions;
 
 import com.google.common.base.Predicate;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.actions.ActionAnalysisMetadata.MiddlemanType;
 import com.google.devtools.build.lib.actions.cache.ActionCache;
@@ -125,10 +126,41 @@ public class ActionCacheChecker {
     }
   }
 
+  private void reportClientEnv(EventHandler handler, Action action, Map<String, String> used) {
+    if (handler != null) {
+      if (verboseExplanations) {
+        StringBuilder message = new StringBuilder();
+        message.append("Effective client environment has changed. Now using\n");
+        for (Map.Entry<String, String> entry : used.entrySet()) {
+          message.append("  ").append(entry.getKey()).append("=").append(entry.getValue())
+              .append("\n");
+        }
+        reportRebuild(handler, action, message.toString());
+      } else {
+        reportRebuild(
+            handler,
+            action,
+            "Effective client environment has changed (try --verbose_explanations for more info)");
+      }
+    }
+  }
+
   protected boolean unconditionalExecution(Action action) {
     return !isActionExecutionProhibited(action) && action.executeUnconditionally();
   }
 
+  private static Map<String, String> computeUsedClientEnv(
+      Action action, Map<String, String> clientEnv) {
+    Map<String, String> used = new HashMap<>();
+    for (String var : action.getClientEnvironmentVariables()) {
+      String value = clientEnv.get(var);
+      if (value != null) {
+        used.put(var, value);
+      }
+    }
+    return used;
+  }
+
   /**
    * Checks whether {@code action} needs to be executed and returns a non-null Token if so.
    *
@@ -141,8 +173,12 @@ public class ActionCacheChecker {
    */
   // Note: the handler should only be used for DEPCHECKER events; there's no
   // guarantee it will be available for other events.
-  public Token getTokenIfNeedToExecute(Action action, Iterable<Artifact> resolvedCacheArtifacts,
-      EventHandler handler, MetadataHandler metadataHandler) {
+  public Token getTokenIfNeedToExecute(
+      Action action,
+      Iterable<Artifact> resolvedCacheArtifacts,
+      Map<String, String> clientEnv,
+      EventHandler handler,
+      MetadataHandler metadataHandler) {
     // TODO(bazel-team): (2010) For RunfilesAction/SymlinkAction and similar actions that
     // produce only symlinks we should not check whether inputs are valid at all - all that matters
     // that inputs and outputs are still exist (and new inputs have not appeared). All other checks
@@ -168,7 +204,7 @@ public class ActionCacheChecker {
       actionInputs = resolvedCacheArtifacts;
     }
     ActionCache.Entry entry = getCacheEntry(action);
-    if (mustExecute(action, entry, handler, metadataHandler, actionInputs)) {
+    if (mustExecute(action, entry, handler, metadataHandler, actionInputs, clientEnv)) {
       if (entry != null) {
         removeCacheEntry(action);
       }
@@ -181,8 +217,13 @@ public class ActionCacheChecker {
     return null;
   }
 
-  protected boolean mustExecute(Action action, @Nullable ActionCache.Entry entry,
-      EventHandler handler, MetadataHandler metadataHandler, Iterable<Artifact> actionInputs) {
+  protected boolean mustExecute(
+      Action action,
+      @Nullable ActionCache.Entry entry,
+      EventHandler handler,
+      MetadataHandler metadataHandler,
+      Iterable<Artifact> actionInputs,
+      Map<String, String> clientEnv) {
     // Unconditional execution can be applied only for actions that are allowed to be executed.
     if (unconditionalExecution(action)) {
       Preconditions.checkState(action.isVolatile());
@@ -204,12 +245,19 @@ public class ActionCacheChecker {
       reportCommand(handler, action);
       return true; // must execute -- action key is different
     }
+    Map<String, String> usedClientEnv = computeUsedClientEnv(action, clientEnv);
+    if (!entry.getUsedClientEnvDigest().equals(DigestUtils.fromEnv(usedClientEnv))) {
+      reportClientEnv(handler, action, usedClientEnv);
+      return true; // different values taken from the environment -- must execute
+    }
+
 
     entry.getFileDigest();
     return false; // cache hit
   }
 
-  public void afterExecution(Action action, Token token, MetadataHandler metadataHandler)
+  public void afterExecution(
+      Action action, Token token, MetadataHandler metadataHandler, Map<String, String> clientEnv)
       throws IOException {
     Preconditions.checkArgument(token != null);
     String key = token.cacheKey;
@@ -217,7 +265,9 @@ public class ActionCacheChecker {
       // This cache entry has already been updated by a shared action. We don't need to do it again.
       return;
     }
-    ActionCache.Entry entry = new ActionCache.Entry(action.getKey(), action.discoversInputs());
+    Map<String, String> usedClientEnv = computeUsedClientEnv(action, clientEnv);
+    ActionCache.Entry entry =
+        new ActionCache.Entry(action.getKey(), usedClientEnv, action.discoversInputs());
     for (Artifact output : action.getOutputs()) {
       // Remove old records from the cache if they used different key.
       String execPath = output.getExecPathString();
@@ -295,7 +345,7 @@ public class ActionCacheChecker {
       // Compute the aggregated middleman digest.
       // Since we never validate action key for middlemen, we should not store
       // it in the cache entry and just use empty string instead.
-      entry = new ActionCache.Entry("", false);
+      entry = new ActionCache.Entry("", ImmutableMap.<String, String>of(), false);
       for (Artifact input : action.getInputs()) {
         entry.addFile(input.getExecPath(), metadataHandler.getMetadataMaybe(input));
       }
diff --git a/src/main/java/com/google/devtools/build/lib/actions/cache/ActionCache.java b/src/main/java/com/google/devtools/build/lib/actions/cache/ActionCache.java
index 5dc88cf0ff..1e123e5f93 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/cache/ActionCache.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/cache/ActionCache.java
@@ -74,15 +74,22 @@ public interface ActionCache {
     // If null, md5Digest is non-null and the entry is immutable.
     private Map<String, Metadata> mdMap;
     private Md5Digest md5Digest;
+    private final Md5Digest usedClientEnvDigest;
 
-    public Entry(String key, boolean discoversInputs) {
+    public Entry(String key, Map<String, String> usedClientEnv, boolean discoversInputs) {
       actionKey = key;
+      this.usedClientEnvDigest = DigestUtils.fromEnv(usedClientEnv);
       files = discoversInputs ? new ArrayList<String>() : null;
       mdMap = new HashMap<>();
     }
 
-    public Entry(String key, @Nullable List<String> files, Md5Digest md5Digest) {
+    public Entry(
+        String key,
+        Md5Digest usedClientEnvDigest,
+        @Nullable List<String> files,
+        Md5Digest md5Digest) {
       actionKey = key;
+      this.usedClientEnvDigest = usedClientEnvDigest;
       this.files = files;
       this.md5Digest = md5Digest;
       mdMap = null;
@@ -111,6 +118,11 @@ public interface ActionCache {
       return actionKey;
     }
 
+    /** @return the effectively used client environment */
+    public Md5Digest getUsedClientEnvDigest() {
+      return usedClientEnvDigest;
+    }
+
     /**
      * Returns the combined md5Digest of the action's inputs and outputs.
      *
diff --git a/src/main/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCache.java b/src/main/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCache.java
index f0b47f857d..a644000cc3 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCache.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCache.java
@@ -16,6 +16,7 @@ package com.google.devtools.build.lib.actions.cache;
 import static java.nio.charset.StandardCharsets.ISO_8859_1;
 
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ConditionallyThreadSafe;
 import com.google.devtools.build.lib.profiler.AutoProfiler;
 import com.google.devtools.build.lib.util.Clock;
@@ -62,7 +63,7 @@ public class CompactPersistentActionCache implements ActionCache {
 
   private static final int NO_INPUT_DISCOVERY_COUNT = -1;
 
-  private static final int VERSION = 11;
+  private static final int VERSION = 12;
 
   private static final Logger LOG = Logger.getLogger(CompactPersistentActionCache.class.getName());
 
@@ -152,7 +153,8 @@ public class CompactPersistentActionCache implements ActionCache {
 
   private final PersistentMap<Integer, byte[]> map;
   private final PersistentStringIndexer indexer;
-  static final ActionCache.Entry CORRUPTED = new ActionCache.Entry(null, false);
+  static final ActionCache.Entry CORRUPTED =
+      new ActionCache.Entry(null, ImmutableMap.<String, String>of(), false);
 
   public CompactPersistentActionCache(Path cacheRoot, Clock clock) throws IOException {
     Path cacheFile = cacheFile(cacheRoot);
@@ -351,12 +353,14 @@ public class CompactPersistentActionCache implements ActionCache {
       // + 16 bytes for the digest
       // + 5 bytes max for the file list length
       // + 5 bytes max for each file id
+      // + 16 bytes for the environment digest
       int maxSize =
           VarInt.MAX_VARINT_SIZE
               + actionKeyBytes.length
               + Md5Digest.MD5_SIZE
               + VarInt.MAX_VARINT_SIZE
-              + files.size() * VarInt.MAX_VARINT_SIZE;
+              + files.size() * VarInt.MAX_VARINT_SIZE
+              + Md5Digest.MD5_SIZE;
       ByteArrayOutputStream sink = new ByteArrayOutputStream(maxSize);
 
       VarInt.putVarInt(actionKeyBytes.length, sink);
@@ -368,6 +372,9 @@ public class CompactPersistentActionCache implements ActionCache {
       for (String file : files) {
         VarInt.putVarInt(indexer.getOrCreateIndex(file), sink);
       }
+
+      DigestUtils.write(entry.getUsedClientEnvDigest(), sink);
+
       return sink.toByteArray();
     } catch (IOException e) {
       // This Exception can never be thrown by ByteArrayOutputStream.
@@ -400,11 +407,17 @@ public class CompactPersistentActionCache implements ActionCache {
         }
         builder.add(filename);
       }
+
+      Md5Digest usedClientEnvDigest = DigestUtils.read(source);
+
       if (source.remaining() > 0) {
         throw new IOException("serialized entry data has not been fully decoded");
       }
       return new Entry(
-          actionKey, count == NO_INPUT_DISCOVERY_COUNT ? null : builder.build(), md5Digest);
+          actionKey,
+          usedClientEnvDigest,
+          count == NO_INPUT_DISCOVERY_COUNT ? null : builder.build(),
+          md5Digest);
     } catch (BufferUnderflowException e) {
       throw new IOException("encoded entry data is incomplete", e);
     }
diff --git a/src/main/java/com/google/devtools/build/lib/actions/cache/DigestUtils.java b/src/main/java/com/google/devtools/build/lib/actions/cache/DigestUtils.java
index ef4d9dc4d9..a35eed6a95 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/cache/DigestUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/cache/DigestUtils.java
@@ -171,6 +171,21 @@ public class DigestUtils {
     return new Md5Digest(result);
   }
 
+  /**
+   * @param env A collection of (String, String) pairs.
+   * @return an order-independent digest of the given set of pairs.
+   */
+  public static Md5Digest fromEnv(Map<String, String> env) {
+    byte[] result = new byte[Md5Digest.MD5_SIZE];
+    Fingerprint fp = new Fingerprint();
+    for (Map.Entry<String, String> entry : env.entrySet()) {
+      fp.addStringLatin1(entry.getKey());
+      fp.addStringLatin1(entry.getValue());
+      xorWith(result, fp.digestAndReset());
+    }
+    return new Md5Digest(result);
+  }
+
   private static byte[] getDigest(Fingerprint fp, String execPath, Metadata md) {
     fp.addStringLatin1(execPath);
 
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
index 59e99885d7..f51a41eeda 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
@@ -599,6 +599,9 @@ public final class BuildConfiguration {
     )
     public List<Map.Entry<String, String>> testEnvironment;
 
+    // TODO(bazel-team): The set of available variables from the client environment for actions
+    // is computed independently in CommandEnvironment to inject a more restricted client
+    // environment to skyframe.
     @Option(
       name = "action_env",
       converter = Converters.OptionalAssignmentConverter.class,
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java b/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java
index 1ace4adb91..bb6fb831a6 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java
@@ -57,10 +57,11 @@ import com.google.devtools.common.options.OptionsProvider;
 import java.io.IOException;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.TreeMap;
+import java.util.TreeSet;
 import java.util.UUID;
 import java.util.concurrent.atomic.AtomicReference;
 
@@ -77,7 +78,8 @@ public final class CommandEnvironment {
   private final Reporter reporter;
   private final EventBus eventBus;
   private final BlazeModule.ModuleEnvironment blazeModuleEnvironment;
-  private final Map<String, String> clientEnv = new HashMap<>();
+  private final Map<String, String> clientEnv = new TreeMap<>();
+  private final Set<String> visibleClientEnv = new TreeSet<>();
   private final TimestampGranularityMonitor timestampGranularityMonitor;
 
   private String[] crashData;
@@ -163,6 +165,21 @@ public final class CommandEnvironment {
     return Collections.unmodifiableMap(clientEnv);
   }
 
+  /**
+   * Return an ordered version of the client environment restricted to those variables
+   * whitelisted by the command-line options to be inheritable by actions.
+   */
+  private Map<String, String> getCommandlineWhitelistedClientEnv() {
+    Map<String, String> visibleEnv = new TreeMap<>();
+    for (String var : visibleClientEnv) {
+      String value = clientEnv.get(var);
+      if (value != null) {
+        visibleEnv.put(var, value);
+      }
+    }
+    return Collections.unmodifiableMap(visibleEnv);
+  }
+
   @VisibleForTesting
   void updateClientEnv(List<Map.Entry<String, String>> clientEnvList, boolean ignoreClientEnv) {
     Preconditions.checkState(clientEnv.isEmpty());
@@ -407,8 +424,16 @@ public final class CommandEnvironment {
     if (!skyframeExecutor.hasIncrementalState()) {
       skyframeExecutor.resetEvaluator();
     }
-    skyframeExecutor.sync(reporter, packageCacheOptions, getOutputBase(),
-        getWorkingDirectory(), defaultsPackageContents, getCommandId(),
+    skyframeExecutor.sync(
+        reporter,
+        packageCacheOptions,
+        getOutputBase(),
+        getWorkingDirectory(),
+        defaultsPackageContents,
+        getCommandId(),
+        // TODO(bazel-team): this optimization disallows rule-specified additional dependencies
+        // on the client environment!
+        getCommandlineWhitelistedClientEnv(),
         timestampGranularityMonitor);
   }
 
@@ -499,6 +524,17 @@ public final class CommandEnvironment {
         testEnv.put(entry.getKey(), entry.getValue());
       }
 
+      // Compute the set of environment variables that are whitelisted on the commandline
+      // for inheritence.
+      for (Map.Entry<String, String> entry :
+             optionsParser.getOptions(BuildConfiguration.Options.class).actionEnvironment) {
+        if (entry.getValue() == null) {
+          visibleClientEnv.add(entry.getKey());
+        } else {
+          visibleClientEnv.remove(entry.getKey());
+        }
+      }
+
       try {
         for (Map.Entry<String, String> entry : testEnv.entrySet()) {
           if (entry.getValue() == null) {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
index f73d2475fc..0808b7c06e 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
@@ -102,6 +102,9 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver
       // Depending on the buildID ensure that these actions have a chance to execute.
       PrecomputedValue.BUILD_ID.get(env);
     }
+    // The client environment might influence the action.
+    Map<String, String> clientEnv = PrecomputedValue.CLIENT_ENV.get(env);
+
     // For restarts of this ActionExecutionFunction we use a ContinuationState variable, below, to
     // avoid redoing work. However, if two actions are shared and the first one executes, when the
     // second one goes to execute, we should detect that and short-circuit, even without taking
@@ -170,7 +173,7 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver
 
     ActionExecutionValue result;
     try {
-      result = checkCacheAndExecuteIfNeeded(action, state, env);
+      result = checkCacheAndExecuteIfNeeded(action, state, env, clientEnv);
     } catch (ActionExecutionException e) {
       // Remove action from state map in case it's there (won't be unless it discovers inputs).
       stateMap.remove(action);
@@ -326,9 +329,8 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver
   }
 
   private ActionExecutionValue checkCacheAndExecuteIfNeeded(
-      Action action,
-      ContinuationState state,
-      Environment env) throws ActionExecutionException, InterruptedException {
+      Action action, ContinuationState state, Environment env, Map<String, String> clientEnv)
+      throws ActionExecutionException, InterruptedException {
     // If this is a shared action and the other action is the one that executed, we must use that
     // other action's value, provided here, since it is populated with metadata for the outputs.
     if (!state.hasArtifactData()) {
@@ -340,8 +342,13 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver
     long actionStartTime = System.nanoTime();
     // We only need to check the action cache if we haven't done it on a previous run.
     if (!state.hasCheckedActionCache()) {
-      state.token = skyframeActionExecutor.checkActionCache(action, metadataHandler,
-          actionStartTime, state.allInputs.actionCacheInputs);
+      state.token =
+          skyframeActionExecutor.checkActionCache(
+              action,
+              metadataHandler,
+              actionStartTime,
+              state.allInputs.actionCacheInputs,
+              clientEnv);
     }
 
     if (state.token == null) {
@@ -451,7 +458,7 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver
       }
     }
     Preconditions.checkState(!env.valuesMissing(), action);
-    skyframeActionExecutor.afterExecution(action, metadataHandler, state.token);
+    skyframeActionExecutor.afterExecution(action, metadataHandler, state.token, clientEnv);
     return state.value;
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PrecomputedValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/PrecomputedValue.java
index bc9977e4c9..d572058eac 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PrecomputedValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PrecomputedValue.java
@@ -83,6 +83,9 @@ public final class PrecomputedValue implements SkyValue {
   static final Precomputed<UUID> BUILD_ID =
       new Precomputed<>(SkyKey.create(SkyFunctions.PRECOMPUTED, "build_id"));
 
+  static final Precomputed<Map<String, String>> CLIENT_ENV =
+      new Precomputed<>(SkyKey.create(SkyFunctions.PRECOMPUTED, "client_env"));
+
   static final Precomputed<WorkspaceStatusAction> WORKSPACE_STATUS_KEY =
       new Precomputed<>(SkyKey.create(SkyFunctions.PRECOMPUTED, "workspace_status_action"));
 
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
index 18903e8c8b..bda49c2284 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
@@ -249,12 +249,18 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor {
   }
 
   @Override
-  public void sync(EventHandler eventHandler, PackageCacheOptions packageCacheOptions,
-      Path outputBase, Path workingDirectory, String defaultsPackageContents, UUID commandId,
+  public void sync(
+      EventHandler eventHandler,
+      PackageCacheOptions packageCacheOptions,
+      Path outputBase,
+      Path workingDirectory,
+      String defaultsPackageContents,
+      UUID commandId,
+      Map<String, String> clientEnv,
       TimestampGranularityMonitor tsgm)
-          throws InterruptedException, AbruptExitException {
+      throws InterruptedException, AbruptExitException {
     super.sync(eventHandler, packageCacheOptions, outputBase, workingDirectory,
-        defaultsPackageContents, commandId, tsgm);
+        defaultsPackageContents, commandId, clientEnv, tsgm);
     handleDiffs(eventHandler, packageCacheOptions.checkOutputFiles);
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
index f6c6742d76..d69e6476fe 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
@@ -459,11 +459,16 @@ public final class SkyframeActionExecutor implements ActionExecutionContextFacto
    * if the action is up to date, and non-null if it needs to be executed, in which case that token
    * should be provided to the ActionCacheChecker after execution.
    */
-  Token checkActionCache(Action action, MetadataHandler metadataHandler,
-      long actionStartTime, Iterable<Artifact> resolvedCacheArtifacts) {
+  Token checkActionCache(
+      Action action,
+      MetadataHandler metadataHandler,
+      long actionStartTime,
+      Iterable<Artifact> resolvedCacheArtifacts,
+      Map<String, String> clientEnv) {
     profiler.startTask(ProfilerTask.ACTION_CHECK, action);
-    Token token = actionCacheChecker.getTokenIfNeedToExecute(
-        action, resolvedCacheArtifacts, explain ? reporter : null, metadataHandler);
+    Token token =
+        actionCacheChecker.getTokenIfNeedToExecute(
+            action, resolvedCacheArtifacts, clientEnv, explain ? reporter : null, metadataHandler);
     profiler.completeTask(ProfilerTask.ACTION_CHECK);
     if (token == null) {
       boolean eventPosted = false;
@@ -487,7 +492,8 @@ public final class SkyframeActionExecutor implements ActionExecutionContextFacto
     return token;
   }
 
-  void afterExecution(Action action, MetadataHandler metadataHandler, Token token) {
+  void afterExecution(
+      Action action, MetadataHandler metadataHandler, Token token, Map<String, String> clientEnv) {
     if (!actionReallyExecuted(action)) {
       // If an action shared with this one executed, then we need not update the action cache, since
       // the other action will do it. Moreover, this action is not aware of metadata acquired
@@ -495,7 +501,7 @@ public final class SkyframeActionExecutor implements ActionExecutionContextFacto
       return;
     }
     try {
-      actionCacheChecker.afterExecution(action, token, metadataHandler);
+      actionCacheChecker.afterExecution(action, token, metadataHandler, clientEnv);
     } catch (IOException e) {
       // Skyframe has already done all the filesystem access needed for outputs and swallows
       // IOExceptions for inputs. So an IOException is impossible here.
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
index 3110f50033..65011b7e4e 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -685,6 +685,10 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
     buildId.set(commandId);
   }
 
+  protected void setPrecomputedClientEnv(Map<String, String> clientEnv) {
+    PrecomputedValue.CLIENT_ENV.set(injectable(), clientEnv);
+  }
+
   /** Returns the build-info.txt and build-changelist.txt artifacts. */
   public Collection<Artifact> getWorkspaceStatusArtifacts(EventHandler eventHandler)
       throws InterruptedException {
@@ -891,11 +895,16 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
    *
    * <p>MUST be run before every incremental build.
    */
-  @VisibleForTesting  // productionVisibility = Visibility.PRIVATE
-  public void preparePackageLoading(PathPackageLocator pkgLocator, RuleVisibility defaultVisibility,
-                                    boolean showLoadingProgress, int globbingThreads,
-                                    String defaultsPackageContents, UUID commandId,
-                                    TimestampGranularityMonitor tsgm) {
+  @VisibleForTesting // productionVisibility = Visibility.PRIVATE
+  public void preparePackageLoading(
+      PathPackageLocator pkgLocator,
+      RuleVisibility defaultVisibility,
+      boolean showLoadingProgress,
+      int globbingThreads,
+      String defaultsPackageContents,
+      UUID commandId,
+      Map<String, String> clientEnv,
+      TimestampGranularityMonitor tsgm) {
     Preconditions.checkNotNull(pkgLocator);
     Preconditions.checkNotNull(tsgm);
     setActive(true);
@@ -903,6 +912,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
     this.tsgm.set(tsgm);
     maybeInjectPrecomputedValuesForAnalysis();
     setCommandId(commandId);
+    setPrecomputedClientEnv(clientEnv);
     setBlacklistedPackagePrefixesFile(getBlacklistedPackagePrefixesFile());
     setShowLoadingProgress(showLoadingProgress);
     setDefaultVisibility(defaultVisibility);
@@ -1627,16 +1637,30 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
     return memoizingEvaluator;
   }
 
-  public void sync(EventHandler eventHandler, PackageCacheOptions packageCacheOptions,
-      Path outputBase, Path workingDirectory, String defaultsPackageContents, UUID commandId,
+  public void sync(
+      EventHandler eventHandler,
+      PackageCacheOptions packageCacheOptions,
+      Path outputBase,
+      Path workingDirectory,
+      String defaultsPackageContents,
+      UUID commandId,
+      Map<String, String> clientEnv,
       TimestampGranularityMonitor tsgm)
-          throws InterruptedException, AbruptExitException{
+      throws InterruptedException, AbruptExitException {
     preparePackageLoading(
         createPackageLocator(
-            eventHandler, packageCacheOptions, outputBase, directories.getWorkspace(),
+            eventHandler,
+            packageCacheOptions,
+            outputBase,
+            directories.getWorkspace(),
             workingDirectory),
-        packageCacheOptions.defaultVisibility, packageCacheOptions.showLoadingProgress,
-        packageCacheOptions.globbingThreads, defaultsPackageContents, commandId, tsgm);
+        packageCacheOptions.defaultVisibility,
+        packageCacheOptions.showLoadingProgress,
+        packageCacheOptions.globbingThreads,
+        defaultsPackageContents,
+        commandId,
+        clientEnv,
+        tsgm);
     setDeletedPackages(packageCacheOptions.getDeletedPackages());
 
     incrementalBuildMonitor = new SkyframeIncrementalBuildMonitor();