Let Starlark tests inherit env variables (#15217)
Adds an inherited_environment field to testing.TestEnvironment to allow
Starlark rules to implement the equivalent of the native rules'
`env_inherit` attribute.
Work towards #7364. To fully resolve that issue, it remains to handle
executable non-test rules.
RELNOTES: Starlark test rules can use the new inherited_environment
parameter of testing.TestEnvironment to specify environment variables
whose values should be inherited from the shell environment.
Closes #14849.
PiperOrigin-RevId: 439277689
Cherry-pick contains parts of:
Delete non-interning, non-singleton @AutoCodec.
PiperOrigin-RevId: 411683398
Cherry-pick makes the following additional changes:
- Replace use of ImmutableMap.buildKeepingLast with .copyOf
Co-authored-by: Chenchu Kolli <ckolli@google.com>
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 3dc5903..8978789 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
@@ -701,7 +701,7 @@
@Override
public Dict<String, String> getEnv() {
- return Dict.immutableCopyOf(env.getFixedEnv().toMap());
+ return Dict.immutableCopyOf(env.getFixedEnv());
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionEnvironment.java b/src/main/java/com/google/devtools/build/lib/actions/ActionEnvironment.java
index f1ff0a0..8a9e3d1 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionEnvironment.java
@@ -17,7 +17,6 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
-import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.util.Fingerprint;
import java.util.LinkedHashMap;
import java.util.Map;
@@ -44,26 +43,36 @@
* action cache), such that Bazel knows exactly which actions it needs to rerun, and does not have
* to reanalyze the entire dependency graph.
*/
-@AutoCodec
public final class ActionEnvironment {
- /** A map of environment variables. */
+ /**
+ * A map of environment variables together with a list of variables to inherit from the shell
+ * environment.
+ */
public interface EnvironmentVariables {
/**
- * Returns the environment variables as a map.
+ * Returns the fixed environment variables as a map.
*
- * <p>WARNING: this allocations additional objects if the underlying implementation is a {@link
+ * <p>WARNING: this allocates additional objects if the underlying implementation is a {@link
* CompoundEnvironmentVariables}; use sparingly.
*/
- ImmutableMap<String, String> toMap();
+ ImmutableMap<String, String> getFixedEnvironment();
+
+ /**
+ * Returns the inherited environment variables as a set.
+ *
+ * <p>WARNING: this allocates additional objects if the underlying implementation is a {@link
+ * CompoundEnvironmentVariables}; use sparingly.
+ */
+ ImmutableSet<String> getInheritedEnvironment();
default boolean isEmpty() {
- return toMap().isEmpty();
+ return getFixedEnvironment().isEmpty() && getInheritedEnvironment().isEmpty();
}
default int size() {
- return toMap().size();
+ return getFixedEnvironment().size() + getInheritedEnvironment().size();
}
}
@@ -72,11 +81,21 @@
* allocation a new map.
*/
static class CompoundEnvironmentVariables implements EnvironmentVariables {
+
+ static EnvironmentVariables create(
+ Map<String, String> fixedVars, Set<String> inheritedVars, EnvironmentVariables base) {
+ if (fixedVars.isEmpty() && inheritedVars.isEmpty() && base.isEmpty()) {
+ return EMPTY_ENVIRONMENT_VARIABLES;
+ }
+ return new CompoundEnvironmentVariables(fixedVars, inheritedVars, base);
+ }
+
private final EnvironmentVariables current;
private final EnvironmentVariables base;
- CompoundEnvironmentVariables(Map<String, String> vars, EnvironmentVariables base) {
- this.current = new SimpleEnvironmentVariables(vars);
+ private CompoundEnvironmentVariables(
+ Map<String, String> fixedVars, Set<String> inheritedVars, EnvironmentVariables base) {
+ this.current = SimpleEnvironmentVariables.create(fixedVars, inheritedVars);
this.base = base;
}
@@ -86,39 +105,54 @@
}
@Override
- public ImmutableMap<String, String> toMap() {
- Map<String, String> result = new LinkedHashMap<>();
- result.putAll(base.toMap());
- result.putAll(current.toMap());
+ public ImmutableMap<String, String> getFixedEnvironment() {
+ LinkedHashMap<String, String> result = new LinkedHashMap<>();
+ result.putAll(base.getFixedEnvironment());
+ result.putAll(current.getFixedEnvironment());
return ImmutableMap.copyOf(result);
}
+
+ @Override
+ public ImmutableSet<String> getInheritedEnvironment() {
+ ImmutableSet.Builder<String> result = new ImmutableSet.Builder<>();
+ result.addAll(base.getInheritedEnvironment());
+ result.addAll(current.getInheritedEnvironment());
+ return result.build();
+ }
}
/** A simple {@link EnvironmentVariables}. */
static class SimpleEnvironmentVariables implements EnvironmentVariables {
- static EnvironmentVariables create(Map<String, String> vars) {
- if (vars.isEmpty()) {
+ static EnvironmentVariables create(Map<String, String> fixedVars, Set<String> inheritedVars) {
+ if (fixedVars.isEmpty() && inheritedVars.isEmpty()) {
return EMPTY_ENVIRONMENT_VARIABLES;
}
- return new SimpleEnvironmentVariables(vars);
+ return new SimpleEnvironmentVariables(fixedVars, inheritedVars);
}
- private final ImmutableMap<String, String> vars;
+ private final ImmutableMap<String, String> fixedVars;
+ private final ImmutableSet<String> inheritedVars;
- private SimpleEnvironmentVariables(Map<String, String> vars) {
- this.vars = ImmutableMap.copyOf(vars);
+ private SimpleEnvironmentVariables(Map<String, String> fixedVars, Set<String> inheritedVars) {
+ this.fixedVars = ImmutableMap.copyOf(fixedVars);
+ this.inheritedVars = ImmutableSet.copyOf(inheritedVars);
}
@Override
- public ImmutableMap<String, String> toMap() {
- return vars;
+ public ImmutableMap<String, String> getFixedEnvironment() {
+ return fixedVars;
+ }
+
+ @Override
+ public ImmutableSet<String> getInheritedEnvironment() {
+ return inheritedVars;
}
}
/** An empty {@link EnvironmentVariables}. */
public static final EnvironmentVariables EMPTY_ENVIRONMENT_VARIABLES =
- new SimpleEnvironmentVariables(ImmutableMap.of());
+ new SimpleEnvironmentVariables(ImmutableMap.of(), ImmutableSet.of());
/**
* An empty environment, mainly for testing. Production code should never use this, but instead
@@ -126,8 +160,7 @@
*/
// TODO(ulfjack): Migrate all production code to use the proper action environment, and then make
// this @VisibleForTesting or rename it to clarify.
- public static final ActionEnvironment EMPTY =
- new ActionEnvironment(EMPTY_ENVIRONMENT_VARIABLES, ImmutableSet.of());
+ public static final ActionEnvironment EMPTY = new ActionEnvironment(EMPTY_ENVIRONMENT_VARIABLES);
/**
* Splits the given map into a map of variables with a fixed value, and a set of variables that
@@ -147,15 +180,13 @@
inheritedEnv.add(key);
}
}
- return create(new SimpleEnvironmentVariables(fixedEnv), ImmutableSet.copyOf(inheritedEnv));
+ return create(SimpleEnvironmentVariables.create(fixedEnv, inheritedEnv));
}
- private final EnvironmentVariables fixedEnv;
- private final ImmutableSet<String> inheritedEnv;
+ private final EnvironmentVariables vars;
- private ActionEnvironment(EnvironmentVariables fixedEnv, ImmutableSet<String> inheritedEnv) {
- this.fixedEnv = fixedEnv;
- this.inheritedEnv = inheritedEnv;
+ private ActionEnvironment(EnvironmentVariables vars) {
+ this.vars = vars;
}
/**
@@ -163,35 +194,43 @@
* undefined, so callers need to take care that the key set of the {@code fixedEnv} map and the
* set of {@code inheritedEnv} elements are disjoint.
*/
- @AutoCodec.Instantiator
- public static ActionEnvironment create(
- EnvironmentVariables fixedEnv, ImmutableSet<String> inheritedEnv) {
- if (fixedEnv.isEmpty() && inheritedEnv.isEmpty()) {
+ private static ActionEnvironment create(EnvironmentVariables vars) {
+ if (vars.isEmpty()) {
return EMPTY;
}
- return new ActionEnvironment(fixedEnv, inheritedEnv);
+ return new ActionEnvironment(vars);
}
public static ActionEnvironment create(
Map<String, String> fixedEnv, ImmutableSet<String> inheritedEnv) {
- return new ActionEnvironment(SimpleEnvironmentVariables.create(fixedEnv), inheritedEnv);
+ return ActionEnvironment.create(SimpleEnvironmentVariables.create(fixedEnv, inheritedEnv));
}
public static ActionEnvironment create(Map<String, String> fixedEnv) {
- return new ActionEnvironment(new SimpleEnvironmentVariables(fixedEnv), ImmutableSet.of());
+ return ActionEnvironment.create(SimpleEnvironmentVariables.create(fixedEnv, ImmutableSet.of()));
}
/**
* Returns a copy of the environment with the given fixed variables added to it, <em>overwriting
* any existing occurrences of those variables</em>.
*/
- public ActionEnvironment addFixedVariables(Map<String, String> vars) {
- return new ActionEnvironment(new CompoundEnvironmentVariables(vars, fixedEnv), inheritedEnv);
+ public ActionEnvironment addFixedVariables(Map<String, String> fixedVars) {
+ return ActionEnvironment.create(
+ CompoundEnvironmentVariables.create(fixedVars, ImmutableSet.of(), vars));
+ }
+
+ /**
+ * Returns a copy of the environment with the given fixed and inherited variables added to it,
+ * <em>overwriting any existing occurrences of those variables</em>.
+ */
+ public ActionEnvironment addVariables(Map<String, String> fixedVars, Set<String> inheritedVars) {
+ return ActionEnvironment.create(
+ CompoundEnvironmentVariables.create(fixedVars, inheritedVars, vars));
}
/** Returns the combined size of the fixed and inherited environments. */
public int size() {
- return fixedEnv.size() + inheritedEnv.size();
+ return vars.size();
}
/**
@@ -199,8 +238,8 @@
* fixed values and their values. This should only be used for testing and to compute the cache
* keys of actions. Use {@link #resolve} instead to get the complete environment.
*/
- public EnvironmentVariables getFixedEnv() {
- return fixedEnv;
+ public ImmutableMap<String, String> getFixedEnv() {
+ return vars.getFixedEnvironment();
}
/**
@@ -210,7 +249,7 @@
* get the complete environment.
*/
public ImmutableSet<String> getInheritedEnv() {
- return inheritedEnv;
+ return vars.getInheritedEnvironment();
}
/**
@@ -221,8 +260,8 @@
*/
public void resolve(Map<String, String> result, Map<String, String> clientEnv) {
checkNotNull(clientEnv);
- result.putAll(fixedEnv.toMap());
- for (String var : inheritedEnv) {
+ result.putAll(vars.getFixedEnvironment());
+ for (String var : vars.getInheritedEnvironment()) {
String value = clientEnv.get(var);
if (value != null) {
result.put(var, value);
@@ -231,7 +270,7 @@
}
public void addTo(Fingerprint f) {
- f.addStringMap(fixedEnv.toMap());
- f.addStrings(inheritedEnv);
+ f.addStringMap(vars.getFixedEnvironment());
+ f.addStrings(vars.getInheritedEnvironment());
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
index f7e147a..59cde31 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
@@ -470,6 +470,7 @@
providersBuilder.getProvider(TestEnvironmentInfo.PROVIDER.getKey());
if (environmentProvider != null) {
testActionBuilder.addExtraEnv(environmentProvider.getEnvironment());
+ testActionBuilder.addExtraInheritedEnv(environmentProvider.getInheritedEnvironment());
}
TestParams testParams =
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java
index 14c42dd..ca3c553 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java
@@ -442,7 +442,7 @@
StringBuilder message = new StringBuilder();
message.append(getProgressMessage());
message.append('\n');
- for (Map.Entry<String, String> entry : env.getFixedEnv().toMap().entrySet()) {
+ for (Map.Entry<String, String> entry : env.getFixedEnv().entrySet()) {
message.append(" Environment variable: ");
message.append(ShellEscaper.escapeString(entry.getKey()));
message.append('=');
@@ -551,7 +551,7 @@
// ActionEnvironment to avoid developers misunderstanding the purpose of this method. That
// requires first updating all subclasses and callers to actually handle environments correctly,
// so it's not a small change.
- return env.getFixedEnv().toMap();
+ return env.getFixedEnv();
}
/**
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 34be910..c10bcca 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
@@ -442,7 +442,7 @@
*/
@Override
public ImmutableMap<String, String> getLocalShellEnvironment() {
- return actionEnv.getFixedEnv().toMap();
+ return actionEnv.getFixedEnv();
}
/**
@@ -579,7 +579,7 @@
*/
@Override
public ImmutableMap<String, String> getTestEnv() {
- return testEnv.getFixedEnv().toMap();
+ return testEnv.getFixedEnv();
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionBuilder.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionBuilder.java
index b1b3eb2..d545812 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionBuilder.java
@@ -55,7 +55,9 @@
import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.List;
import java.util.Map;
+import java.util.Set;
import java.util.TreeMap;
+import java.util.TreeSet;
import javax.annotation.Nullable;
/**
@@ -96,10 +98,12 @@
private InstrumentedFilesInfo instrumentedFiles;
private int explicitShardCount;
private final Map<String, String> extraEnv;
+ private final Set<String> extraInheritedEnv;
public TestActionBuilder(RuleContext ruleContext) {
this.ruleContext = ruleContext;
this.extraEnv = new TreeMap<>();
+ this.extraInheritedEnv = new TreeSet<>();
this.additionalTools = new ImmutableList.Builder<>();
}
@@ -154,6 +158,11 @@
return this;
}
+ public TestActionBuilder addExtraInheritedEnv(List<String> extraInheritedEnv) {
+ this.extraInheritedEnv.addAll(extraInheritedEnv);
+ return this;
+ }
+
/**
* Set the explicit shard count. Note that this may be overridden by the sharding strategy.
*/
@@ -442,7 +451,9 @@
coverageArtifact,
coverageDirectory,
testProperties,
- runfilesSupport.getActionEnvironment().addFixedVariables(extraTestEnv),
+ runfilesSupport
+ .getActionEnvironment()
+ .addVariables(extraTestEnv, extraInheritedEnv),
executionSettings,
shard,
run,
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestEnvironmentInfo.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestEnvironmentInfo.java
index f7e4100..d524533 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestEnvironmentInfo.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestEnvironmentInfo.java
@@ -15,10 +15,13 @@
package com.google.devtools.build.lib.analysis.test;
import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.packages.BuiltinProvider;
import com.google.devtools.build.lib.packages.NativeInfo;
import com.google.devtools.build.lib.starlarkbuildapi.test.TestEnvironmentInfoApi;
+import java.util.List;
import java.util.Map;
/** Provider containing any additional environment variables for use in the test action. */
@@ -29,11 +32,14 @@
public static final BuiltinProvider<TestEnvironmentInfo> PROVIDER =
new BuiltinProvider<TestEnvironmentInfo>("TestEnvironment", TestEnvironmentInfo.class) {};
- private final Map<String, String> environment;
+ private final ImmutableMap<String, String> environment;
+ private final ImmutableList<String> inheritedEnvironment;
- /** Constructs a new provider with the given variable name to variable value mapping. */
- public TestEnvironmentInfo(Map<String, String> environment) {
- this.environment = Preconditions.checkNotNull(environment);
+ /** Constructs a new provider with the given fixed and inherited environment variables. */
+ public TestEnvironmentInfo(Map<String, String> environment, List<String> inheritedEnvironment) {
+ this.environment = ImmutableMap.copyOf(Preconditions.checkNotNull(environment));
+ this.inheritedEnvironment =
+ ImmutableList.copyOf(Preconditions.checkNotNull(inheritedEnvironment));
}
@Override
@@ -48,4 +54,10 @@
public Map<String, String> getEnvironment() {
return environment;
}
+
+ /** Returns names of environment variables whose value should be obtained from the environment. */
+ @Override
+ public ImmutableList<String> getInheritedEnvironment() {
+ return inheritedEnvironment;
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java
index 83385b4..01795eb 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java
@@ -997,7 +997,7 @@
@Override
public ImmutableMap<String, String> getIncompleteEnvironmentForTesting()
throws ActionExecutionException {
- return getEnvironment().getFixedEnv().toMap();
+ return getEnvironment().getFixedEnv();
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphTextOutputFormatterCallback.java b/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphTextOutputFormatterCallback.java
index 78fe828..5976c40 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphTextOutputFormatterCallback.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphTextOutputFormatterCallback.java
@@ -218,7 +218,7 @@
// TODO(twerth): This handles the fixed environment. We probably want to output the inherited
// environment as well.
Iterable<Map.Entry<String, String>> fixedEnvironment =
- abstractAction.getEnvironment().getFixedEnv().toMap().entrySet();
+ abstractAction.getEnvironment().getFixedEnv().entrySet();
if (!Iterables.isEmpty(fixedEnvironment)) {
stringBuilder
.append(" Environment: [")
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java
index 31f97c2..e34e51d 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java
@@ -575,7 +575,7 @@
// ActionEnvironment to avoid developers misunderstanding the purpose of this method. That
// requires first updating all subclasses and callers to actually handle environments correctly,
// so it's not a small change.
- return env.getFixedEnv().toMap();
+ return env.getFixedEnv();
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/rules/test/StarlarkTestingModule.java b/src/main/java/com/google/devtools/build/lib/rules/test/StarlarkTestingModule.java
index 08f938c..1c6fcb6 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/test/StarlarkTestingModule.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/test/StarlarkTestingModule.java
@@ -18,6 +18,8 @@
import com.google.devtools.build.lib.starlarkbuildapi.test.TestingModuleApi;
import net.starlark.java.eval.Dict;
import net.starlark.java.eval.EvalException;
+import net.starlark.java.eval.Sequence;
+import net.starlark.java.eval.StarlarkList;
/** A class that exposes testing infrastructure to Starlark. */
public class StarlarkTestingModule implements TestingModuleApi {
@@ -29,9 +31,13 @@
}
@Override
- public TestEnvironmentInfo testEnvironment(Dict<?, ?> environment /* <String, String> */)
+ public TestEnvironmentInfo testEnvironment(
+ Dict<?, ?> environment /* <String, String> */,
+ Sequence<?> inheritedEnvironment /* <String> */)
throws EvalException {
return new TestEnvironmentInfo(
- Dict.cast(environment, String.class, String.class, "environment"));
+ Dict.cast(environment, String.class, String.class, "environment"),
+ StarlarkList.immutableCopyOf(
+ Sequence.cast(inheritedEnvironment, String.class, "inherited_environment")));
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/ActionGraphDump.java b/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/ActionGraphDump.java
index 5cc56c9..1646a7b 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/ActionGraphDump.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/ActionGraphDump.java
@@ -15,6 +15,7 @@
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
@@ -159,7 +160,7 @@
SpawnAction spawnAction = (SpawnAction) action;
// TODO(twerth): This handles the fixed environment. We probably want to output the inherited
// environment as well.
- Map<String, String> fixedEnvironment = spawnAction.getEnvironment().getFixedEnv().toMap();
+ ImmutableMap<String, String> fixedEnvironment = spawnAction.getEnvironment().getFixedEnv();
for (Map.Entry<String, String> environmentVariable : fixedEnvironment.entrySet()) {
actionBuilder.addEnvironmentVariables(
AnalysisProtosV2.KeyValuePair.newBuilder()
diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/test/TestEnvironmentInfoApi.java b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/test/TestEnvironmentInfoApi.java
index c843097..ed11b24 100644
--- a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/test/TestEnvironmentInfoApi.java
+++ b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/test/TestEnvironmentInfoApi.java
@@ -15,6 +15,7 @@
package com.google.devtools.build.lib.starlarkbuildapi.test;
import com.google.devtools.build.lib.starlarkbuildapi.core.StructApi;
+import java.util.List;
import java.util.Map;
import net.starlark.java.annot.StarlarkBuiltin;
import net.starlark.java.annot.StarlarkMethod;
@@ -28,4 +29,10 @@
doc = "A dict containing environment variables which should be set on the test action.",
structField = true)
Map<String, String> getEnvironment();
+
+ @StarlarkMethod(
+ name = "inherited_environment",
+ doc = "A list of variables that the test action should inherit from the shell environment.",
+ structField = true)
+ List<String> getInheritedEnvironment();
}
diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/test/TestingModuleApi.java b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/test/TestingModuleApi.java
index a6f243a..1570f80 100644
--- a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/test/TestingModuleApi.java
+++ b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/test/TestingModuleApi.java
@@ -15,10 +15,12 @@
package com.google.devtools.build.lib.starlarkbuildapi.test;
import net.starlark.java.annot.Param;
+import net.starlark.java.annot.ParamType;
import net.starlark.java.annot.StarlarkBuiltin;
import net.starlark.java.annot.StarlarkMethod;
import net.starlark.java.eval.Dict;
import net.starlark.java.eval.EvalException;
+import net.starlark.java.eval.Sequence;
import net.starlark.java.eval.StarlarkValue;
/** Helper module for accessing test infrastructure. */
@@ -56,12 +58,26 @@
parameters = {
@Param(
name = "environment",
- named = false,
+ named = true,
positional = true,
doc =
"A map of string keys and values that represent environment variables and their"
- + " values. These will be made available during the test execution.")
+ + " values. These will be made available during the test execution."),
+ @Param(
+ name = "inherited_environment",
+ allowedTypes = {@ParamType(type = Sequence.class, generic1 = String.class)},
+ defaultValue = "[]",
+ named = true,
+ positional = true,
+ doc =
+ "A sequence of names of environment variables. These variables are made available"
+ + " during the test execution with their current value taken from the shell"
+ + " environment. If a variable is contained in both <code>environment</code>"
+ + " and <code>inherited_environment</code>, the value inherited from the"
+ + " shell environment will take precedence if set.")
})
- TestEnvironmentInfoApi testEnvironment(Dict<?, ?> environment // <String, String> expected
- ) throws EvalException;
+ TestEnvironmentInfoApi testEnvironment(
+ Dict<?, ?> environment, // <String, String> expected
+ Sequence<?> inheritedEnvironment /* <String> expected */)
+ throws EvalException;
}
diff --git a/src/test/java/com/google/devtools/build/lib/actions/ActionEnvironmentTest.java b/src/test/java/com/google/devtools/build/lib/actions/ActionEnvironmentTest.java
index 8ff8e44..d48d266 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/ActionEnvironmentTest.java
+++ b/src/test/java/com/google/devtools/build/lib/actions/ActionEnvironmentTest.java
@@ -34,10 +34,10 @@
// entries added by env2 override the existing entries
ActionEnvironment env2 = env1.addFixedVariables(ImmutableMap.of("FOO", "foo2"));
- assertThat(env1.getFixedEnv().toMap()).containsExactly("FOO", "foo1", "BAR", "bar");
+ assertThat(env1.getFixedEnv()).containsExactly("FOO", "foo1", "BAR", "bar");
assertThat(env1.getInheritedEnv()).containsExactly("baz");
- assertThat(env2.getFixedEnv().toMap()).containsExactly("FOO", "foo2", "BAR", "bar");
+ assertThat(env2.getFixedEnv()).containsExactly("FOO", "foo2", "BAR", "bar");
assertThat(env2.getInheritedEnv()).containsExactly("baz");
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProviderTest.java b/src/test/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProviderTest.java
index 4e339f7..a40319b 100644
--- a/src/test/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProviderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProviderTest.java
@@ -175,8 +175,8 @@
"--action_env=FOO=bar");
ActionEnvironment env = BazelRuleClassProvider.SHELL_ACTION_ENV.getActionEnvironment(options);
- assertThat(env.getFixedEnv().toMap()).containsEntry("PATH", "/bin:/usr/bin:/usr/local/bin");
- assertThat(env.getFixedEnv().toMap()).containsEntry("FOO", "bar");
+ assertThat(env.getFixedEnv()).containsEntry("PATH", "/bin:/usr/bin:/usr/local/bin");
+ assertThat(env.getFixedEnv()).containsEntry("FOO", "bar");
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/rules/test/StarlarkTestingModuleTest.java b/src/test/java/com/google/devtools/build/lib/rules/test/StarlarkTestingModuleTest.java
index 051c4ad..77943fe 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/test/StarlarkTestingModuleTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/test/StarlarkTestingModuleTest.java
@@ -80,6 +80,39 @@
}
@Test
+ public void testStarlarkRulePropagatesTestEnvironmentProviderWithInheritedEnv() throws Exception {
+ scratch.file("examples/rule/BUILD");
+ scratch.file(
+ "examples/rule/apple_rules.bzl",
+ "def my_rule_impl(ctx):",
+ " test_env = testing.TestEnvironment(",
+ " {'XCODE_VERSION_OVERRIDE': '7.3.1'},",
+ " [",
+ " 'DEVELOPER_DIR',",
+ " 'XCODE_VERSION_OVERRIDE',",
+ " ]",
+ ")",
+ " return [test_env]",
+ "my_rule = rule(implementation = my_rule_impl,",
+ " attrs = {},",
+ ")");
+ scratch.file(
+ "examples/apple_starlark/BUILD",
+ "package(default_visibility = ['//visibility:public'])",
+ "load('//examples/rule:apple_rules.bzl', 'my_rule')",
+ "my_rule(",
+ " name = 'my_target',",
+ ")");
+
+ ConfiguredTarget starlarkTarget = getConfiguredTarget("//examples/apple_starlark:my_target");
+ TestEnvironmentInfo provider = starlarkTarget.get(TestEnvironmentInfo.PROVIDER);
+
+ assertThat(provider.getEnvironment()).containsEntry("XCODE_VERSION_OVERRIDE", "7.3.1");
+ assertThat(provider.getInheritedEnvironment()).contains("DEVELOPER_DIR");
+ assertThat(provider.getInheritedEnvironment()).contains("XCODE_VERSION_OVERRIDE");
+ }
+
+ @Test
public void testExecutionInfoProviderCanMarkTestAsLocal() throws Exception {
scratch.file("examples/rule/BUILD");
scratch.file(
diff --git a/src/test/shell/bazel/bazel_rules_test.sh b/src/test/shell/bazel/bazel_rules_test.sh
index ef70918..5650f0d 100755
--- a/src/test/shell/bazel/bazel_rules_test.sh
+++ b/src/test/shell/bazel/bazel_rules_test.sh
@@ -593,4 +593,75 @@
assert_contains "hello world" bazel-bin/pkg/hello_gen.txt
}
+function test_starlark_test_with_test_environment() {
+ mkdir pkg
+ cat >pkg/BUILD <<'EOF'
+load(":rules.bzl", "my_test")
+my_test(
+ name = "my_test",
+)
+EOF
+
+ # On Windows this file needs to be acceptable by CreateProcessW(), rather
+ # than a Bourne script.
+ if "$is_windows"; then
+ cat >pkg/rules.bzl <<'EOF'
+_SCRIPT_EXT = ".bat"
+_SCRIPT_CONTENT = """@ECHO OFF
+if not "%FIXED_ONLY%" == "fixed" exit /B 1
+if not "%FIXED_AND_INHERITED%" == "inherited" exit /B 1
+if not "%FIXED_AND_INHERITED_BUT_NOT_SET%" == "fixed" exit /B 1
+if not "%INHERITED_ONLY%" == "inherited" exit /B 1
+"""
+EOF
+ else
+ cat >pkg/rules.bzl <<'EOF'
+_SCRIPT_EXT = ".sh"
+_SCRIPT_CONTENT = """#!/bin/bash
+[[ "$FIXED_ONLY" == "fixed" \
+ && "$FIXED_AND_INHERITED" == "inherited" \
+ && "$FIXED_AND_INHERITED_BUT_NOT_SET" == "fixed" \
+ && "$INHERITED_ONLY" == "inherited" ]]
+"""
+EOF
+ fi
+
+ cat >>pkg/rules.bzl <<'EOF'
+def _my_test_impl(ctx):
+ test_sh = ctx.actions.declare_file(ctx.attr.name + _SCRIPT_EXT)
+ ctx.actions.write(
+ output = test_sh,
+ content = _SCRIPT_CONTENT,
+ is_executable = True,
+ )
+ test_env = testing.TestEnvironment(
+ {
+ "FIXED_AND_INHERITED": "fixed",
+ "FIXED_AND_INHERITED_BUT_NOT_SET": "fixed",
+ "FIXED_ONLY": "fixed",
+ },
+ [
+ "FIXED_AND_INHERITED",
+ "FIXED_AND_INHERITED_BUT_NOT_SET",
+ "INHERITED_ONLY",
+ ]
+ )
+ return [
+ DefaultInfo(
+ executable = test_sh,
+ ),
+ test_env
+ ]
+
+my_test = rule(
+ implementation = _my_test_impl,
+ attrs = {},
+ test = True,
+)
+EOF
+
+ FIXED_AND_INHERITED=inherited INHERITED_ONLY=inherited \
+ bazel test //pkg:my_test &> /dev/null || fail "Test should pass"
+}
+
run_suite "rules test"
