diff options
26 files changed, 360 insertions, 97 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(); diff --git a/src/test/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCacheTest.java b/src/test/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCacheTest.java index 41dfd5d4b3..479e152c17 100644 --- a/src/test/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCacheTest.java +++ b/src/test/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCacheTest.java @@ -22,6 +22,7 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.testutil.Scratch; import com.google.devtools.build.lib.util.Clock; import com.google.devtools.build.lib.vfs.Path; @@ -163,7 +164,8 @@ public class CompactPersistentActionCacheTest { // Mutations may result in IllegalStateException. @Test public void testEntryToStringIsIdempotent() throws Exception { - ActionCache.Entry entry = new ActionCache.Entry("actionKey", false); + ActionCache.Entry entry = + new ActionCache.Entry("actionKey", ImmutableMap.<String, String>of(), false); entry.toString(); entry.addFile(new PathFragment("foo/bar"), Metadata.CONSTANT_METADATA); entry.toString(); @@ -217,7 +219,8 @@ public class CompactPersistentActionCacheTest { } private void putKey(String key, ActionCache ac, boolean discoversInputs) { - ActionCache.Entry entry = new ActionCache.Entry(key, discoversInputs); + ActionCache.Entry entry = + new ActionCache.Entry(key, ImmutableMap.<String, String>of(), discoversInputs); entry.getFileDigest(); ac.put(key, entry); } diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java index e7d0655360..3cb49517e3 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java @@ -15,6 +15,7 @@ package com.google.devtools.build.lib.analysis.util; import com.google.common.base.Predicates; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSortedSet; import com.google.common.collect.Iterables; import com.google.common.eventbus.EventBus; @@ -182,6 +183,7 @@ public abstract class AnalysisTestCase extends FoundationTestCase { ruleClassProvider.getDefaultsPackageContent( analysisMock.getInvocationPolicyEnforcer().getInvocationPolicy()), UUID.randomUUID(), + ImmutableMap.of(), new TimestampGranularityMonitor(BlazeClock.instance())); packageManager = skyframeExecutor.getPackageManager(); loadingPhaseRunner = skyframeExecutor.getLoadingPhaseRunner( @@ -285,6 +287,7 @@ public abstract class AnalysisTestCase extends FoundationTestCase { ruleClassProvider.getDefaultsPackageContent( analysisMock.getInvocationPolicyEnforcer().getInvocationPolicy()), UUID.randomUUID(), + ImmutableMap.of(), new TimestampGranularityMonitor(BlazeClock.instance())); skyframeExecutor.invalidateFilesUnderPathForTesting(reporter, ModifiedFileSet.EVERYTHING_MODIFIED, rootDirectory); diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java index aac6fe6014..7da4ca5398 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java @@ -227,8 +227,13 @@ public abstract class BuildViewTestCase extends FoundationTestCase { analysisMock.getProductName()); skyframeExecutor.preparePackageLoading( new PathPackageLocator(outputBase, ImmutableList.of(rootDirectory)), - ConstantRuleVisibility.PUBLIC, true, 7, "", - UUID.randomUUID(), tsgm); + ConstantRuleVisibility.PUBLIC, + true, + 7, + "", + UUID.randomUUID(), + ImmutableMap.of(), + tsgm); useConfiguration(); setUpSkyframe(); // Also initializes ResourceManager. @@ -310,10 +315,15 @@ public abstract class BuildViewTestCase extends FoundationTestCase { private void setUpSkyframe() { PathPackageLocator pkgLocator = PathPackageLocator.create( outputBase, packageCacheOptions.packagePath, reporter, rootDirectory, rootDirectory); - skyframeExecutor.preparePackageLoading(pkgLocator, - packageCacheOptions.defaultVisibility, true, - 7, ruleClassProvider.getDefaultsPackageContent(optionsParser), - UUID.randomUUID(), tsgm); + skyframeExecutor.preparePackageLoading( + pkgLocator, + packageCacheOptions.defaultVisibility, + true, + 7, + ruleClassProvider.getDefaultsPackageContent(optionsParser), + UUID.randomUUID(), + ImmutableMap.<String, String>of(), + tsgm); skyframeExecutor.setDeletedPackages(ImmutableSet.copyOf(packageCacheOptions.getDeletedPackages())); } diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/ConfigurationTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/ConfigurationTestCase.java index af92ecfa76..b5ecba0643 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/ConfigurationTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/ConfigurationTestCase.java @@ -17,6 +17,7 @@ import static org.junit.Assert.fail; import com.google.common.base.Predicates; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSortedSet; import com.google.common.collect.Iterables; import com.google.devtools.build.lib.actions.Root; @@ -122,6 +123,7 @@ public abstract class ConfigurationTestCase extends FoundationTestCase { ruleClassProvider.getDefaultsPackageContent( analysisMock.getInvocationPolicyEnforcer().getInvocationPolicy()), UUID.randomUUID(), + ImmutableMap.of(), new TimestampGranularityMonitor(BlazeClock.instance())); mockToolsConfig = new MockToolsConfig(rootDirectory); diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/PackageLoadingTestCase.java b/src/test/java/com/google/devtools/build/lib/packages/util/PackageLoadingTestCase.java index 8b8d039055..6b1eebbc4a 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/util/PackageLoadingTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/packages/util/PackageLoadingTestCase.java @@ -49,15 +49,13 @@ import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.skyframe.SkyFunction; import com.google.devtools.build.skyframe.SkyFunctionName; import com.google.devtools.common.options.OptionsParser; - -import org.junit.Before; - import java.io.IOException; import java.util.Arrays; import java.util.HashSet; import java.util.List; import java.util.Set; import java.util.UUID; +import org.junit.Before; /** * This is a specialization of {@link FoundationTestCase} that's useful for @@ -131,7 +129,8 @@ public abstract class PackageLoadingTestCase extends FoundationTestCase { skyframeExecutor.preparePackageLoading( new PathPackageLocator(outputBase, ImmutableList.of(rootDirectory)), defaultVisibility, true, GLOBBING_THREADS, defaultsPackageContents, - UUID.randomUUID(), new TimestampGranularityMonitor(BlazeClock.instance())); + UUID.randomUUID(), ImmutableMap.of(), + new TimestampGranularityMonitor(BlazeClock.instance())); } private void setUpSkyframe(PackageCacheOptions packageCacheOptions) { @@ -144,8 +143,10 @@ public abstract class PackageLoadingTestCase extends FoundationTestCase { GLOBBING_THREADS, loadingMock.getDefaultsPackageContent(), UUID.randomUUID(), + ImmutableMap.of(), new TimestampGranularityMonitor(BlazeClock.instance())); - skyframeExecutor.setDeletedPackages(ImmutableSet.copyOf(packageCacheOptions.getDeletedPackages())); + skyframeExecutor.setDeletedPackages( + ImmutableSet.copyOf(packageCacheOptions.getDeletedPackages())); } private PackageCacheOptions parsePackageCacheOptions(String... options) throws Exception { diff --git a/src/test/java/com/google/devtools/build/lib/pkgcache/IncrementalLoadingTest.java b/src/test/java/com/google/devtools/build/lib/pkgcache/IncrementalLoadingTest.java index 3a2798f240..3069a1d12c 100644 --- a/src/test/java/com/google/devtools/build/lib/pkgcache/IncrementalLoadingTest.java +++ b/src/test/java/com/google/devtools/build/lib/pkgcache/IncrementalLoadingTest.java @@ -55,20 +55,17 @@ import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; import com.google.devtools.build.skyframe.SkyFunction; import com.google.devtools.build.skyframe.SkyFunctionName; - -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; - import java.io.FileNotFoundException; import java.io.IOException; import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.UUID; - import javax.annotation.Nullable; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; /** * Tests for incremental loading; these cover both normal operation and diff awareness, for which a @@ -488,7 +485,8 @@ public class IncrementalLoadingTest { skyframeExecutor.preparePackageLoading( new PathPackageLocator(outputBase, ImmutableList.of(workspace)), ConstantRuleVisibility.PUBLIC, true, 7, "", - UUID.randomUUID(), new TimestampGranularityMonitor(BlazeClock.instance())); + UUID.randomUUID(), ImmutableMap.of(), + new TimestampGranularityMonitor(BlazeClock.instance())); } Path addFile(String fileName, String... content) throws IOException { @@ -567,7 +565,8 @@ public class IncrementalLoadingTest { skyframeExecutor.preparePackageLoading( new PathPackageLocator(outputBase, ImmutableList.of(workspace)), ConstantRuleVisibility.PUBLIC, true, 7, "", - UUID.randomUUID(), new TimestampGranularityMonitor(BlazeClock.instance())); + UUID.randomUUID(), ImmutableMap.of(), + new TimestampGranularityMonitor(BlazeClock.instance())); skyframeExecutor.invalidateFilesUnderPathForTesting( new Reporter(), modifiedFileSet, workspace); ((SequencedSkyframeExecutor) skyframeExecutor).handleDiffs(new Reporter()); diff --git a/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java b/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java index afa781ae84..ceb26a1da3 100644 --- a/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java +++ b/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java @@ -26,6 +26,7 @@ import com.google.common.base.Predicate; import com.google.common.base.Predicates; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableListMultimap; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.eventbus.EventBus; @@ -620,9 +621,10 @@ public class LoadingPhaseRunnerTest { 7, analysisMock.getDefaultsPackageContent(), UUID.randomUUID(), + ImmutableMap.of(), new TimestampGranularityMonitor(clock)); - loadingPhaseRunner = skyframeExecutor.getLoadingPhaseRunner( - pkgFactory.getRuleClassNames(), useNewImpl); + loadingPhaseRunner = + skyframeExecutor.getLoadingPhaseRunner(pkgFactory.getRuleClassNames(), useNewImpl); this.options = Options.getDefaults(LoadingOptions.class); } diff --git a/src/test/java/com/google/devtools/build/lib/pkgcache/PackageCacheTest.java b/src/test/java/com/google/devtools/build/lib/pkgcache/PackageCacheTest.java index 9afcce146f..5735458b10 100644 --- a/src/test/java/com/google/devtools/build/lib/pkgcache/PackageCacheTest.java +++ b/src/test/java/com/google/devtools/build/lib/pkgcache/PackageCacheTest.java @@ -24,6 +24,7 @@ import static org.junit.Assert.fail; import com.google.common.base.Predicates; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider; @@ -55,17 +56,15 @@ import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.common.options.OptionsParser; import com.google.devtools.common.options.OptionsParsingException; - -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; - import java.io.IOException; import java.util.HashSet; import java.util.List; import java.util.Set; import java.util.UUID; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; /** * Tests for package loading. @@ -112,8 +111,10 @@ public class PackageCacheTest extends FoundationTestCase { 7, analysisMock.getDefaultsPackageContent(), UUID.randomUUID(), + ImmutableMap.of(), new TimestampGranularityMonitor(BlazeClock.instance())); - skyframeExecutor.setDeletedPackages(ImmutableSet.copyOf(packageCacheOptions.getDeletedPackages())); + skyframeExecutor.setDeletedPackages( + ImmutableSet.copyOf(packageCacheOptions.getDeletedPackages())); } private PackageCacheOptions parsePackageCacheOptions(String... options) throws Exception { diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java index 2aec5947e9..f6ee2ee57a 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java @@ -71,10 +71,16 @@ public class PackageFunctionTest extends BuildViewTestCase { private CustomInMemoryFs fs = new CustomInMemoryFs(new ManualClock()); private void preparePackageLoading(Path... roots) { - getSkyframeExecutor().preparePackageLoading( - new PathPackageLocator(outputBase, ImmutableList.copyOf(roots)), - ConstantRuleVisibility.PUBLIC, true, - 7, "", UUID.randomUUID(), new TimestampGranularityMonitor(BlazeClock.instance())); + getSkyframeExecutor() + .preparePackageLoading( + new PathPackageLocator(outputBase, ImmutableList.copyOf(roots)), + ConstantRuleVisibility.PUBLIC, + true, + 7, + "", + UUID.randomUUID(), + ImmutableMap.<String, String>of(), + new TimestampGranularityMonitor(BlazeClock.instance())); } @Override @@ -435,10 +441,16 @@ public class PackageFunctionTest extends BuildViewTestCase { Label.parseAbsoluteUnchecked("//foo:b.txt")) .inOrder(); getSkyframeExecutor().resetEvaluator(); - getSkyframeExecutor().preparePackageLoading( - new PathPackageLocator(outputBase, ImmutableList.of(rootDirectory)), - ConstantRuleVisibility.PUBLIC, true, 7, "", - UUID.randomUUID(), tsgm); + getSkyframeExecutor() + .preparePackageLoading( + new PathPackageLocator(outputBase, ImmutableList.<Path>of(rootDirectory)), + ConstantRuleVisibility.PUBLIC, + true, + 7, + "", + UUID.randomUUID(), + ImmutableMap.of(), + tsgm); value = validPackage(skyKey); assertThat( (Iterable<Label>) diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/SkyframeLabelVisitorTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/SkyframeLabelVisitorTest.java index b5a9494758..902c62f5a1 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/SkyframeLabelVisitorTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/SkyframeLabelVisitorTest.java @@ -18,6 +18,7 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.events.Event; @@ -33,14 +34,12 @@ import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.ModifiedFileSet; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; - -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; - import java.io.IOException; import java.util.Collection; import java.util.UUID; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; @RunWith(JUnit4.class) public class SkyframeLabelVisitorTest extends SkyframeLabelVisitorTestCase { @@ -410,6 +409,7 @@ public class SkyframeLabelVisitorTest extends SkyframeLabelVisitorTestCase { 7, loadingMock.getDefaultsPackageContent(), UUID.randomUUID(), + ImmutableMap.of(), new TimestampGranularityMonitor(BlazeClock.instance())); this.visitor = getSkyframeExecutor().pkgLoader(); scratch.file("pkg/BUILD", "sh_library(name = 'x', deps = ['z'])", "sh_library(name = 'z')"); @@ -454,6 +454,7 @@ public class SkyframeLabelVisitorTest extends SkyframeLabelVisitorTestCase { 7, loadingMock.getDefaultsPackageContent(), UUID.randomUUID(), + ImmutableMap.of(), new TimestampGranularityMonitor(BlazeClock.instance())); this.visitor = getSkyframeExecutor().pkgLoader(); scratch.file("a/BUILD", "subinclude('//b:c/d/foo')"); diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/SkylarkFileContentHashTests.java b/src/test/java/com/google/devtools/build/lib/skyframe/SkylarkFileContentHashTests.java index 089d6a4834..23f7171bf7 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/SkylarkFileContentHashTests.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/SkylarkFileContentHashTests.java @@ -17,6 +17,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; import com.google.devtools.build.lib.cmdline.PackageIdentifier; import com.google.devtools.build.lib.packages.ConstantRuleVisibility; @@ -28,15 +29,13 @@ import com.google.devtools.build.lib.util.BlazeClock; import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor; import com.google.devtools.build.skyframe.EvaluationResult; import com.google.devtools.build.skyframe.SkyKey; - +import java.util.Collection; +import java.util.UUID; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; -import java.util.Collection; -import java.util.UUID; - /** * Tests for the hash code calculated for Skylark RuleClasses based on the transitive closure * of the imports of their respective definition SkylarkEnvironments. @@ -164,6 +163,7 @@ public class SkylarkFileContentHashTests extends BuildViewTestCase { 7, "", UUID.randomUUID(), + ImmutableMap.of(), new TimestampGranularityMonitor(BlazeClock.instance())); SkyKey pkgLookupKey = PackageValue.key(PackageIdentifier.parse("@//" + pkg)); EvaluationResult<PackageValue> result = diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunctionTest.java index 838302ff57..78100f728e 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunctionTest.java @@ -19,6 +19,7 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.packages.ConstantRuleVisibility; @@ -30,14 +31,12 @@ import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.skyframe.ErrorInfo; import com.google.devtools.build.skyframe.EvaluationResult; import com.google.devtools.build.skyframe.SkyKey; - +import java.util.UUID; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; -import java.util.UUID; - /** * Tests for SkylarkImportLookupFunction. */ @@ -55,6 +54,7 @@ public class SkylarkImportLookupFunctionTest extends BuildViewTestCase { 7, "", UUID.randomUUID(), + ImmutableMap.of(), new TimestampGranularityMonitor(BlazeClock.instance())); } diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java b/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java index 0a2c7ddf06..a8ffdfea20 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java @@ -195,6 +195,7 @@ public abstract class TimestampBuilderTestCase extends FoundationTestCase { evaluationProgressReceiver); final SequentialBuildDriver driver = new SequentialBuildDriver(evaluator); PrecomputedValue.BUILD_ID.set(differencer, UUID.randomUUID()); + PrecomputedValue.CLIENT_ENV.set(differencer, ImmutableMap.<String, String>of()); PrecomputedValue.PATH_PACKAGE_LOCATOR.set(differencer, pkgLocator.get()); return new Builder() { diff --git a/src/test/shell/integration/action_env_test.sh b/src/test/shell/integration/action_env_test.sh index 24e87ee48e..156473b1ed 100755 --- a/src/test/shell/integration/action_env_test.sh +++ b/src/test/shell/integration/action_env_test.sh @@ -72,4 +72,52 @@ function test_client_env() { expect_log "FOO=client_foo" } +function test_redo_action() { + export FOO=initial_foo + export UNRELATED=some_value + bazel build --action_env=FOO pkg:showenv || fail "bazel build showenv failed" + cat `bazel info bazel-genfiles`/pkg/env.txt > $TEST_log + expect_log "FOO=initial_foo" + + # If an unrelated value changes, we expect the action not to be executed again + export UNRELATED=some_other_value + bazel build --action_env=FOO pkg:showenv 2> $TEST_log \ + || fail "bazel build showenv failed" + expect_log "Critical Path: 0.00s" + + # However, if a used variable changes, we expect the change to be propagated + export FOO=changed_foo + bazel build --action_env=FOO pkg:showenv || fail "bazel build showenv failed" + cat `bazel info bazel-genfiles`/pkg/env.txt > $TEST_log + expect_log "FOO=changed_foo" + + # But repeating the build with no further changes, no action should happen + bazel build --action_env=FOO pkg:showenv 2> $TEST_log \ + || fail "bazel build showenv failed" + expect_log "Critical Path: 0.00s" + +} + +function test_latest_wins_arg() { + export FOO=bar + export BAR=baz + bazel build --action_env=BAR --action_env=FOO --action_env=FOO=foo \ + pkg:showenv || fail "bazel build showenv failed" + cat `bazel info bazel-genfiles`/pkg/env.txt > $TEST_log + expect_log "FOO=foo" + expect_log "BAR=baz" + expect_not_log "FOO=bar" +} + +function test_latest_wins_env() { + export FOO=bar + export BAR=baz + bazel build --action_env=BAR --action_env=FOO=foo --action_env=FOO \ + pkg:showenv || fail "bazel build showenv failed" + cat `bazel info bazel-genfiles`/pkg/env.txt > $TEST_log + expect_log "FOO=bar" + expect_log "BAR=baz" + expect_not_log "FOO=foo" +} + run_suite "Tests for bazel's handling of environment variables in actions" |