Inject the changes in the client environment for the ClientEnvironmentFunction

Using a precomputed values prevent us from being able to depends on any
environment variable (or we would invalidate the whole graph each time
the environment change, that is each time we run a command).

--
Change-Id: If322df4240271d04b9278e1222f936156531580c
Reviewed-on: https://cr.bazel.build/8400
PiperOrigin-RevId: 146298592
MOS_MIGRATED_REVID=146298592

4 files changed, 38 insertions, 12 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ClientEnvironmentFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ClientEnvironmentFunction.java
index cc2c37ad59..20de7df60a 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ClientEnvironmentFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ClientEnvironmentFunction.java
@@ -18,11 +18,18 @@ import com.google.devtools.build.skyframe.SkyFunction;
 import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.SkyValue;
 import java.util.Map;
+import java.util.concurrent.atomic.AtomicReference;
 import javax.annotation.Nullable;
 
 /** The Skyframe function that generates values for variables of the client environment. */
 public final class ClientEnvironmentFunction implements SkyFunction {
 
+  private final AtomicReference<Map<String, String>> clientEnv;
+
+  ClientEnvironmentFunction(AtomicReference<Map<String, String>> clientEnv) {
+    this.clientEnv = clientEnv;
+  }
+
   @Nullable
   @Override
   public String extractTag(SkyKey skyKey) {
@@ -31,8 +38,7 @@ public final class ClientEnvironmentFunction implements SkyFunction {
 
   @Nullable
   @Override
-  public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedException {
-    Map<String, String> clientEnv = PrecomputedValue.CLIENT_ENV.get(env);
-    return new ClientEnvironmentValue(clientEnv.get((String) skyKey.argument()));
+  public SkyValue compute(SkyKey key, Environment env) throws InterruptedException {
+    return new ClientEnvironmentValue(clientEnv.get().get((String) key.argument()));
   }
 }
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 8901745c00..05b9277789 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,9 +83,6 @@ 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 55fac95b51..069c61799c 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
@@ -15,6 +15,7 @@ package com.google.devtools.build.lib.skyframe;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Joiner;
+import com.google.common.base.Objects;
 import com.google.common.base.Predicate;
 import com.google.common.base.Predicates;
 import com.google.common.collect.ImmutableList;
@@ -98,6 +99,7 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor {
   private RecordingDifferencer recordingDiffer;
   private final DiffAwarenessManager diffAwarenessManager;
   private final Iterable<SkyValueDirtinessChecker> customDirtinessCheckers;
+  private Set<String> previousClientEnvironment = null;
 
   private SequencedSkyframeExecutor(
       EvaluatorSupplier evaluatorSupplier,
@@ -366,6 +368,30 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor {
     handleDiffsWithCompleteDiffInformation(tsgm, modifiedFilesByPathEntry);
     handleDiffsWithMissingDiffInformation(eventHandler, tsgm, pathEntriesWithoutDiffInformation,
         checkOutputFiles);
+    handleClientEnvironmentChanges();
+  }
+
+  /** Invalidates entries in the client environment that have changed since last sync. */
+  private void handleClientEnvironmentChanges() {
+    Map<SkyKey, SkyValue> values = memoizingEvaluator.getValues();
+    ImmutableMap.Builder<SkyKey, SkyValue> newValuesBuilder = ImmutableMap.builder();
+    HashSet<String> envToCheck = new HashSet<>();
+    if (previousClientEnvironment != null) {
+      envToCheck.addAll(previousClientEnvironment);
+    }
+    envToCheck.addAll(clientEnv.get().keySet());
+    previousClientEnvironment = clientEnv.get().keySet();
+    for (String env : envToCheck) {
+      SkyKey key = SkyKey.create(SkyFunctions.CLIENT_ENVIRONMENT_VARIABLE, env);
+      if (values.containsKey(key)) {
+        String value = ((ClientEnvironmentValue) values.get(key)).getValue();
+        String newValue = clientEnv.get().get(env);
+        if (!Objects.equal(newValue, value)) {
+          newValuesBuilder.put(key, new ClientEnvironmentValue(newValue));
+        }
+      }
+    }
+    recordingDiffer.inject(newValuesBuilder.build());
   }
 
   /**
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 a93ee6b5a4..a14e8b6cc2 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
@@ -222,6 +222,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
   private final AtomicReference<EventBus> eventBus = new AtomicReference<>();
   protected final AtomicReference<TimestampGranularityMonitor> tsgm =
       new AtomicReference<>();
+  protected final AtomicReference<Map<String, String>> clientEnv = new AtomicReference<>();
 
   private final ImmutableList<BuildInfoFactory> buildInfoFactories;
   // Under normal circumstances, the artifact factory persists for the life of a Blaze server, but
@@ -349,7 +350,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
     // is inserted.
     ImmutableMap.Builder<SkyFunctionName, SkyFunction> map = ImmutableMap.builder();
     map.put(SkyFunctions.PRECOMPUTED, new PrecomputedFunction());
-    map.put(SkyFunctions.CLIENT_ENVIRONMENT_VARIABLE, new ClientEnvironmentFunction());
+    map.put(SkyFunctions.CLIENT_ENVIRONMENT_VARIABLE, new ClientEnvironmentFunction(clientEnv));
     map.put(SkyFunctions.FILE_STATE, new FileStateFunction(tsgm, externalFilesHelper));
     map.put(SkyFunctions.DIRECTORY_LISTING_STATE,
         new DirectoryListingStateFunction(externalFilesHelper));
@@ -727,10 +728,6 @@ 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 {
@@ -952,7 +949,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
     this.tsgm.set(tsgm);
     maybeInjectPrecomputedValuesForAnalysis();
     setCommandId(commandId);
-    setPrecomputedClientEnv(clientEnv);
+    this.clientEnv.set(clientEnv);
     setBlacklistedPackagePrefixesFile(getBlacklistedPackagePrefixesFile());
     setShowLoadingProgress(packageCacheOptions.showLoadingProgress);
     setDefaultVisibility(packageCacheOptions.defaultVisibility);