diff options
4 files changed, 134 insertions, 46 deletions
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 553c88c9db..6f58666fe3 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 @@ -1278,12 +1278,12 @@ public final class BuildConfiguration { // Shell environment variables specified via options take precedence over the // ones inherited from the fragments. In the long run, these fragments will // be replaced by appropriate default rc files anyway. - Map<String, String> shellEnv = new TreeMap(builder.build()); + Map<String, String> shellEnv = new TreeMap<>(builder.build()); for (Map.Entry<String, String> entry : options.actionEnvironment) { shellEnv.put(entry.getKey(), entry.getValue()); } - Map<String, String> fixedShellEnv = new TreeMap(shellEnv); - Set<String> variableShellEnv = new HashSet(); + Map<String, String> fixedShellEnv = new TreeMap<>(shellEnv); + Set<String> variableShellEnv = new HashSet<>(); for (Map.Entry<String, String> entry : shellEnv.entrySet()) { if (entry.getValue() == null) { String key = entry.getKey(); diff --git a/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java index 41004a7c6d..2f754a8e07 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java @@ -14,6 +14,7 @@ package com.google.devtools.build.lib.exec; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.actions.ActionExecutionContext; import com.google.devtools.build.lib.actions.Artifact; @@ -30,7 +31,6 @@ import com.google.devtools.build.lib.actions.SpawnActionContext; import com.google.devtools.build.lib.actions.TestExecException; import com.google.devtools.build.lib.analysis.RunfilesSupplierImpl; import com.google.devtools.build.lib.analysis.config.BinTools; -import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.EventKind; import com.google.devtools.build.lib.events.Reporter; @@ -41,6 +41,7 @@ import com.google.devtools.build.lib.rules.test.TestRunnerAction.ResolvedPaths; import com.google.devtools.build.lib.util.io.FileOutErr; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.view.test.TestStatus.BlazeTestStatus; import com.google.devtools.build.lib.view.test.TestStatus.TestCase; import com.google.devtools.build.lib.view.test.TestStatus.TestResultData; @@ -61,6 +62,20 @@ public class StandaloneTestStrategy extends TestStrategy { public static final String COLLECT_COVERAGE = "external/bazel_tools/tools/test/collect_coverage.sh"; + private static final ImmutableMap<String, String> ENV_VARS = + ImmutableMap.<String, String>builder() + .put("TZ", "UTC") + .put("USER", TestPolicy.SYSTEM_USER_NAME) + .put("TEST_SRCDIR", TestPolicy.RUNFILES_DIR) + // TODO(lberki): Remove JAVA_RUNFILES and PYTHON_RUNFILES. + .put("JAVA_RUNFILES", TestPolicy.RUNFILES_DIR) + .put("PYTHON_RUNFILES", TestPolicy.RUNFILES_DIR) + .put("RUNFILES_DIR", TestPolicy.RUNFILES_DIR) + .put("TEST_TMPDIR", TestPolicy.TEST_TMP_DIR) + .build(); + + public static final TestPolicy DEFAULT_LOCAL_POLICY = new TestPolicy(ENV_VARS); + protected final Path tmpDirRoot; public StandaloneTestStrategy( @@ -231,27 +246,18 @@ public class StandaloneTestStrategy extends TestStrategy { private Map<String, String> setupEnvironment( TestRunnerAction action, Path execRoot, Path runfilesDir, Path tmpDir) { - Map<String, String> env = getDefaultTestEnvironment(action); - BuildConfiguration config = action.getConfiguration(); - - env.putAll(config.getLocalShellEnvironment()); - env.putAll(action.getTestEnv()); - - String tmpDirString; + PathFragment relativeTmpDir; if (tmpDir.startsWith(execRoot)) { - tmpDirString = tmpDir.relativeTo(execRoot).getPathString(); + relativeTmpDir = tmpDir.relativeTo(execRoot); } else { - tmpDirString = tmpDir.getPathString(); + relativeTmpDir = tmpDir.asFragment(); } - - String testSrcDir = runfilesDir.relativeTo(execRoot).getPathString(); - env.put("JAVA_RUNFILES", testSrcDir); - env.put("PYTHON_RUNFILES", testSrcDir); - env.put("TEST_SRCDIR", testSrcDir); - env.put("TEST_TMPDIR", tmpDirString); - - action.setupEnvVariables(env, getTimeout(action)); - return env; + return DEFAULT_LOCAL_POLICY.computeTestEnvironment( + action, + clientEnv, + getTimeout(action), + runfilesDir.relativeTo(execRoot), + relativeTmpDir); } protected TestResultData executeTest( diff --git a/src/main/java/com/google/devtools/build/lib/exec/TestPolicy.java b/src/main/java/com/google/devtools/build/lib/exec/TestPolicy.java new file mode 100644 index 0000000000..a58c666129 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/exec/TestPolicy.java @@ -0,0 +1,106 @@ +// Copyright 2017 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.lib.exec; + +import com.google.common.collect.ImmutableMap; +import com.google.devtools.build.lib.rules.test.TestRunnerAction; +import com.google.devtools.build.lib.util.UserUtils; +import com.google.devtools.build.lib.vfs.PathFragment; +import java.util.HashMap; +import java.util.Map; + +/** + * A policy for running tests. It currently only encompasses the environment computation for the + * test. + */ +public class TestPolicy { + /** + * The user name of the user running Bazel; this may differ from ${USER} for tests that are run + * remotely. + */ + public static final String SYSTEM_USER_NAME = "${SYSTEM_USER_NAME}"; + + /** An absolute path to a writable directory that is reserved for the current test. */ + public static final String TEST_TMP_DIR = "${TEST_TMP_DIR}"; + + /** The path of the runfiles directory. */ + public static final String RUNFILES_DIR = "${RUNFILES_DIR}"; + + public static final String INHERITED = "${inherited}"; + + private final ImmutableMap<String, String> envVariables; + + /** + * Creates a new instance. The map's keys are the names of the environment variables, while the + * values can be either fixed values, or one of the constants in this class, specifically {@link + * #SYSTEM_USER_NAME}, {@link #TEST_TMP_DIR}, {@link #RUNFILES_DIR}, or {@link #INHERITED}. + */ + public TestPolicy(ImmutableMap<String, String> envVariables) { + this.envVariables = envVariables; + } + + /** + * Returns a mutable map of the environment variables for a specific test. This is intended to be + * the final, complete environment - callers should avoid relying on the mutability of the return + * value, and instead change the policy itself. + */ + public Map<String, String> computeTestEnvironment( + TestRunnerAction testAction, + ImmutableMap<String, String> clientEnv, + int timeoutInSeconds, + PathFragment relativeRunfilesDir, + PathFragment tmpDir) { + Map<String, String> env = new HashMap<>(); + + // Add all env variables, allow some string replacements and inheritance. + String userProp = UserUtils.getUserName(); + String tmpDirPath = tmpDir.getPathString(); + String runfilesDirPath = relativeRunfilesDir.getPathString(); + for (Map.Entry<String, String> entry : envVariables.entrySet()) { + String val = entry.getValue(); + if (val.contains("${")) { + if (val.equals(INHERITED)) { + if (!clientEnv.containsKey(entry.getKey())) { + continue; + } + val = clientEnv.get(entry.getKey()); + } else { + val = val.replace(SYSTEM_USER_NAME, userProp); + val = val.replace(TEST_TMP_DIR, tmpDirPath); + val = val.replace(RUNFILES_DIR, runfilesDirPath); + } + } + env.put(entry.getKey(), val); + } + + // Overwrite with the environment common to all actions, see --action_env. + // TODO(ulfjack): This also includes env variables from the configuration fragments, and it does + // not include the env variables which are supposed to be inherited, i.e., for with --action_env + // does not specify an explicit value. + env.putAll(testAction.getConfiguration().getLocalShellEnvironment()); + + // Overwrite with the environment common to all tests, see --test_env. + // TODO(ulfjack): This is handled differently from --action_env such that changing the + // --test_env flag (or any of the inherited env variables) requires a full re-analysis of + // everything, instead of triggering just the subset of actions that rely on inherited + // variables. Needless to say, that is not optimal, and we should fix it to use the same + // approach as --action_env. + env.putAll(testAction.getTestEnv()); + + // Setup any test-specific env variables; note that this does not overwrite existing values for + // TEST_RANDOM_SEED or TEST_SIZE if they're already set. + testAction.setupEnvVariables(env, timeoutInSeconds); + return env; + } +} diff --git a/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java index a0f371c631..f84754f57f 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java @@ -152,20 +152,6 @@ public abstract class TestStrategy implements TestActionContext { throws ExecException, InterruptedException; /** - * Returns mutable map of default testing shell environment. By itself it is incomplete and is - * modified further by the specific test strategy implementations (mostly due to the fact that - * environments used locally and remotely are different). - */ - protected Map<String, String> getDefaultTestEnvironment(TestRunnerAction action) { - Map<String, String> env = new HashMap<>(); - - env.putAll(action.getConfiguration().getLocalShellEnvironment()); - env.remove("LANG"); - env.put("TZ", "UTC"); - return env; - } - - /** * Generates a command line to run for the test action, taking into account coverage and {@code * --run_under} settings. * @@ -253,16 +239,6 @@ public abstract class TestStrategy implements TestActionContext { return executionOptions.testTimeout.get(testAction.getTestProperties().getTimeout()); } - /** - * Returns a subset of the environment from the current shell. - * - * <p>Warning: Since these variables are not part of the configuration's fingerprint, they MUST - * NOT be used by any rule or action in such a way as to affect the semantics of that build step. - */ - public Map<String, String> getAdmissibleShellEnvironment(Iterable<String> variables) { - return getMapping(variables, clientEnv); - } - /* * Finalize test run: persist the result, and post on the event bus. */ |