aboutsummaryrefslogtreecommitdiffhomepage
path: root/src
diff options
context:
space:
mode:
authorGravatar ulfjack <ulfjack@google.com>2017-11-29 03:37:04 -0800
committerGravatar Copybara-Service <copybara-piper@google.com>2017-11-29 03:38:22 -0800
commit4d7f8f7846960ffc111cf1aef2a5efb094114442 (patch)
treed91dbe1ea83226eadb96eda6562c9a70fa0cd18d /src
parent91420101cf977990166acec2a0e46e1787389512 (diff)
Clean up ExecutionRequirements
- remove BaseSpawn.Local; instead, all callers pass in the full set of execution requirements they want to set - disable caching and sandboxing for the symlink tree action - it does not declare outputs, so it can't be cached or sandboxed (fixes #4041) - centralize the existing execution requirements in the ExecutionRequirements class - centralize checking for execution requirements in the Spawn class (it's possible that we may need a more decentralized, extensible design in the future, but for now having them in a single place is simple and effective) - update the documentation - forward the relevant tags to execution requirements in TargetUtils (progress on #3960) - this also contributes to #4153 PiperOrigin-RevId: 177288598
Diffstat (limited to 'src')
-rw-r--r--src/main/java/com/google/devtools/build/docgen/templates/attributes/common/tags.html17
-rw-r--r--src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java36
-rw-r--r--src/main/java/com/google/devtools/build/lib/actions/DelegateSpawn.java10
-rw-r--r--src/main/java/com/google/devtools/build/lib/actions/ExecutionRequirements.java36
-rw-r--r--src/main/java/com/google/devtools/build/lib/actions/SimpleSpawn.java10
-rw-r--r--src/main/java/com/google/devtools/build/lib/actions/Spawn.java11
-rw-r--r--src/main/java/com/google/devtools/build/lib/actions/Spawns.java15
-rw-r--r--src/main/java/com/google/devtools/build/lib/analysis/config/BinTools.java36
-rw-r--r--src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java88
-rw-r--r--src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategy.java17
-rw-r--r--src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java7
-rw-r--r--src/main/java/com/google/devtools/build/lib/packages/TargetUtils.java11
-rw-r--r--src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java2
-rw-r--r--src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedSpawnRunner.java3
-rw-r--r--src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java6
-rw-r--r--src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextProvider.java3
-rw-r--r--src/main/java/com/google/devtools/build/lib/sandbox/SandboxHelpers.java14
-rw-r--r--src/test/java/com/google/devtools/build/lib/exec/SymlinkTreeHelperTest.java67
-rw-r--r--src/test/java/com/google/devtools/build/lib/packages/TargetUtilsTest.java21
-rw-r--r--src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java21
20 files changed, 286 insertions, 145 deletions
diff --git a/src/main/java/com/google/devtools/build/docgen/templates/attributes/common/tags.html b/src/main/java/com/google/devtools/build/docgen/templates/attributes/common/tags.html
index 5ccc56ba33..55b73d2560 100644
--- a/src/main/java/com/google/devtools/build/docgen/templates/attributes/common/tags.html
+++ b/src/main/java/com/google/devtools/build/docgen/templates/attributes/common/tags.html
@@ -21,6 +21,20 @@
</p>
<ul>
+ <li><code>no-sandbox</code> keyword results in the action or test never being
+ <a href="../user-manual.html#sandboxing">sandboxed</a>; it can still be
+ cached or run remotely - use <code>no-cache</code> or <code>no-remote</code>
+ to prevent either or both of those.
+ </li>
+
+ <li><code>no-cache</code> keyword results in the action or test never being
+ cached.
+ </li>
+
+ <li><code>no-remote</code> keyword results in the action or test never being
+ executed remotely (but it may be cached remotely).
+ </li>
+
<li><code>local</code> keyword results in the action or test never being
run remotely or inside the
<a href="../user-manual.html#sandboxing">sandbox</a>.
@@ -30,7 +44,8 @@
<li><code>block-network</code> keyword blocks access to the external
network from inside the sandbox. In this case, only communication
- with localhost is allowed.
+ with localhost is allowed. This tag only has an effect if sandboxing is
+ enabled.
</li>
<li><code>requires-fakeroot</code> runs the test or action as uid and gid 0 (i.e., the root
diff --git a/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java b/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java
index 901b225afe..bc52055207 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java
@@ -69,16 +69,6 @@ public class BaseSpawn implements Spawn {
}
@Override
- public boolean hasNoSandbox() {
- return executionInfo.containsKey("nosandbox");
- }
-
- @Override
- public boolean isRemotable() {
- return !executionInfo.containsKey("local");
- }
-
- @Override
public final ImmutableMap<String, String> getExecutionInfo() {
return executionInfo;
}
@@ -157,30 +147,4 @@ public class BaseSpawn implements Spawn {
public String getMnemonic() {
return action.getMnemonic();
}
-
- /** A local spawn. */
- public static class Local extends BaseSpawn {
- public Local(
- List<String> arguments, Map<String, String> environment, ActionExecutionMetadata action,
- ResourceSet localResources) {
- this(arguments, environment, ImmutableMap.<String, String>of(), action, localResources);
- }
-
- public Local(
- List<String> arguments,
- Map<String, String> environment,
- Map<String, String> executionInfo,
- ActionExecutionMetadata action,
- ResourceSet localResources) {
- super(arguments, environment, buildExecutionInfo(executionInfo), action, localResources);
- }
-
- private static ImmutableMap<String, String> buildExecutionInfo(
- Map<String, String> additionalExecutionInfo) {
- ImmutableMap.Builder<String, String> executionInfo = ImmutableMap.builder();
- executionInfo.putAll(additionalExecutionInfo);
- executionInfo.put("local", "");
- return executionInfo.build();
- }
- }
}
diff --git a/src/main/java/com/google/devtools/build/lib/actions/DelegateSpawn.java b/src/main/java/com/google/devtools/build/lib/actions/DelegateSpawn.java
index 8341301053..fd548b66cd 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/DelegateSpawn.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/DelegateSpawn.java
@@ -36,16 +36,6 @@ public class DelegateSpawn implements Spawn {
}
@Override
- public boolean isRemotable() {
- return spawn.isRemotable();
- }
-
- @Override
- public boolean hasNoSandbox() {
- return spawn.hasNoSandbox();
- }
-
- @Override
public ImmutableList<Artifact> getFilesetManifests() {
return spawn.getFilesetManifests();
}
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ExecutionRequirements.java b/src/main/java/com/google/devtools/build/lib/actions/ExecutionRequirements.java
index 22242de04c..fced3688b0 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ExecutionRequirements.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ExecutionRequirements.java
@@ -145,8 +145,40 @@ public class ExecutionRequirements {
ImmutableMap.of(SUPPORTS_WORKERS, "1");
/**
- * Whether we should disable remote caching of an action. This can be set to force a rerun of an
- * action even if there is a cache entry for it.
+ * Requires local execution without sandboxing for a spawn.
+ *
+ * <p>This tag is deprecated; use no-cache, no-remote, or no-sandbox instead.
+ */
+ public static final String LOCAL = "local";
+
+ /**
+ * Disables local and remote caching for a spawn, but note that the local action cache may still
+ * apply.
+ *
+ * <p>This tag can also be set on an action, in which case it completely disables all caching for
+ * that action, but note that action-generated spawns may still be cached, unless they also carry
+ * this tag.
*/
public static final String NO_CACHE = "no-cache";
+
+ /** Disables local sandboxing of a spawn. */
+ public static final String LEGACY_NOSANDBOX = "nosandbox";
+
+ /** Disables local sandboxing of a spawn. */
+ public static final String NO_SANDBOX = "no-sandbox";
+
+ /** Disables remote execution of a spawn. */
+ public static final String NO_REMOTE = "no-remote";
+
+ /**
+ * Disables networking for a spawn if possible (only if sandboxing is enabled and if the sandbox
+ * supports it).
+ */
+ public static final String BLOCK_NETWORK = "block-network";
+
+ /**
+ * On linux, if sandboxing is enabled, ensures that a spawn is run with uid 0, i.e., root. Has no
+ * effect otherwise.
+ */
+ public static final String REQUIRES_FAKEROOT = "requires-fakeroot";
}
diff --git a/src/main/java/com/google/devtools/build/lib/actions/SimpleSpawn.java b/src/main/java/com/google/devtools/build/lib/actions/SimpleSpawn.java
index 564460e726..3857783cb5 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/SimpleSpawn.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/SimpleSpawn.java
@@ -82,16 +82,6 @@ public final class SimpleSpawn implements Spawn {
}
@Override
- public boolean hasNoSandbox() {
- return executionInfo.containsKey("nosandbox");
- }
-
- @Override
- public boolean isRemotable() {
- return !executionInfo.containsKey("local");
- }
-
- @Override
public final ImmutableMap<String, String> getExecutionInfo() {
return executionInfo;
}
diff --git a/src/main/java/com/google/devtools/build/lib/actions/Spawn.java b/src/main/java/com/google/devtools/build/lib/actions/Spawn.java
index 6963b016d9..3a59d9ffb3 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/Spawn.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/Spawn.java
@@ -25,17 +25,6 @@ import java.util.Collection;
* of files it is expected to read and write.
*/
public interface Spawn {
-
- /**
- * Returns true iff this command may be executed remotely.
- */
- boolean isRemotable();
-
- /**
- * Returns true iff this command should be executed without a sandbox.
- */
- boolean hasNoSandbox();
-
/**
* Out-of-band data for this spawn. This can be used to signal hints (hardware requirements,
* local vs. remote) to the execution subsystem.
diff --git a/src/main/java/com/google/devtools/build/lib/actions/Spawns.java b/src/main/java/com/google/devtools/build/lib/actions/Spawns.java
index e1156e9745..4448ea8f7b 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/Spawns.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/Spawns.java
@@ -32,6 +32,21 @@ public final class Spawns {
return !spawn.getExecutionInfo().containsKey(ExecutionRequirements.NO_CACHE);
}
+ public static boolean mayBeSandboxed(Spawn spawn) {
+ return !spawn.getExecutionInfo().containsKey(ExecutionRequirements.LEGACY_NOSANDBOX)
+ && !spawn.getExecutionInfo().containsKey(ExecutionRequirements.NO_SANDBOX)
+ && !spawn.getExecutionInfo().containsKey(ExecutionRequirements.LOCAL);
+ }
+
+ public static boolean requiresNetwork(Spawn spawn) {
+ return !spawn.getExecutionInfo().containsKey(ExecutionRequirements.BLOCK_NETWORK);
+ }
+
+ public static boolean mayBeExecutedRemotely(Spawn spawn) {
+ return !spawn.getExecutionInfo().containsKey(ExecutionRequirements.LOCAL)
+ && !spawn.getExecutionInfo().containsKey(ExecutionRequirements.NO_REMOTE);
+ }
+
/**
* Parse the timeout key in the spawn execution info, if it exists. Otherwise, return -1.
*/
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BinTools.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BinTools.java
index deb987b771..dc52388d83 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/BinTools.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BinTools.java
@@ -33,15 +33,22 @@ import java.io.IOException;
* using relative paths from the execution root.
*/
public final class BinTools {
- private final BlazeDirectories directories;
+ private final Path embeddedBinariesRoot;
private final Path execrootParent;
private final ImmutableList<String> embeddedTools;
private Path binDir; // the working bin directory under execRoot
private BinTools(BlazeDirectories directories, ImmutableList<String> tools) {
- this.directories = directories;
- this.execrootParent = directories.getExecRoot().getParentDirectory();
+ this(
+ directories.getEmbeddedBinariesRoot(),
+ directories.getExecRoot().getParentDirectory(),
+ tools);
+ }
+
+ private BinTools(Path embeddedBinariesRoot, Path execrootParent, ImmutableList<String> tools) {
+ this.embeddedBinariesRoot = embeddedBinariesRoot;
+ this.execrootParent = execrootParent;
ImmutableList.Builder<String> builder = ImmutableList.builder();
// Files under embedded_tools shouldn't be copied to under _bin dir
// They won't be used during action execution time.
@@ -69,8 +76,8 @@ public final class BinTools {
*/
@VisibleForTesting
public static BinTools empty(BlazeDirectories directories) {
- return new BinTools(directories, ImmutableList.<String>of()).setBinDir(
- directories.getWorkspace().getBaseName());
+ return new BinTools(directories, ImmutableList.<String>of())
+ .setBinDir(directories.getWorkspace().getBaseName());
}
/**
@@ -80,8 +87,21 @@ public final class BinTools {
*/
@VisibleForTesting
public static BinTools forUnitTesting(BlazeDirectories directories, Iterable<String> tools) {
- return new BinTools(directories, ImmutableList.copyOf(tools)).setBinDir(
- directories.getWorkspace().getBaseName());
+ return new BinTools(directories, ImmutableList.copyOf(tools))
+ .setBinDir(directories.getWorkspace().getBaseName());
+ }
+
+ /**
+ * Creates an instance for testing without actually symlinking the tools.
+ *
+ * <p>Used for tests that need a set of embedded tools to be present, but not the actual files.
+ */
+ @VisibleForTesting
+ public static BinTools forUnitTesting(Path execroot, Iterable<String> tools) {
+ return new BinTools(
+ execroot.getRelative("/fake/embedded/tools"),
+ execroot.getParentDirectory(),
+ ImmutableList.copyOf(tools)).setBinDir(execroot.getBaseName());
}
/**
@@ -168,7 +188,7 @@ public final class BinTools {
private void setupTool(String embeddedPath) throws ExecException {
Preconditions.checkNotNull(binDir);
- Path sourcePath = directories.getEmbeddedBinariesRoot().getRelative(embeddedPath);
+ Path sourcePath = embeddedBinariesRoot.getRelative(embeddedPath);
Path linkPath = binDir.getRelative(PathFragment.create(embeddedPath).getBaseName());
linkTool(sourcePath, linkPath);
}
diff --git a/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java b/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java
index b5f4b92024..b9a055b238 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java
@@ -18,11 +18,15 @@ import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Lists;
-import com.google.devtools.build.lib.actions.AbstractAction;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
-import com.google.devtools.build.lib.actions.BaseSpawn;
+import com.google.devtools.build.lib.actions.ActionExecutionMetadata;
+import com.google.devtools.build.lib.actions.ActionInput;
+import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.ExecException;
+import com.google.devtools.build.lib.actions.ExecutionRequirements;
import com.google.devtools.build.lib.actions.ResourceSet;
+import com.google.devtools.build.lib.actions.SimpleSpawn;
+import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.SpawnResult;
import com.google.devtools.build.lib.actions.UserExecException;
import com.google.devtools.build.lib.analysis.config.BinTools;
@@ -37,18 +41,17 @@ import java.io.IOException;
import java.util.List;
/**
- * Helper class responsible for the symlink tree creation.
- * Used to generate runfiles and fileset symlink farms.
+ * Helper class responsible for the symlink tree creation. Used to generate runfiles and fileset
+ * symlink farms.
*/
public final class SymlinkTreeHelper {
@VisibleForTesting
public static final String BUILD_RUNFILES = "build-runfiles" + OsUtils.executableExtension();
/**
- * These actions run faster overall when serialized, because most of their
- * cost is in the ext2 block allocator, and there's less seeking required if
- * their directory creations get non-interleaved allocations. So we give them
- * a huge resource cost.
+ * These actions run faster overall when serialized, because most of their cost is in the ext2
+ * block allocator, and there's less seeking required if their directory creations get
+ * non-interleaved allocations. So we give them a huge resource cost.
*/
public static final ResourceSet RESOURCE_SET = ResourceSet.createWithRamCpuIo(1000, 0.5, 0.75);
@@ -57,16 +60,14 @@ public final class SymlinkTreeHelper {
private final boolean filesetTree;
/**
- * Creates SymlinkTreeHelper instance. Can be used independently of
- * SymlinkTreeAction.
+ * Creates SymlinkTreeHelper instance. Can be used independently of SymlinkTreeAction.
*
* @param inputManifest exec path to the input runfiles manifest
* @param symlinkTreeRoot the root of the symlink tree to be created
* @param filesetTree true if this is fileset symlink tree,
* false if this is a runfiles symlink tree.
*/
- public SymlinkTreeHelper(Path inputManifest, Path symlinkTreeRoot,
- boolean filesetTree) {
+ public SymlinkTreeHelper(Path inputManifest, Path symlinkTreeRoot, boolean filesetTree) {
this.inputManifest = inputManifest;
this.symlinkTreeRoot = symlinkTreeRoot;
this.filesetTree = filesetTree;
@@ -77,20 +78,19 @@ public final class SymlinkTreeHelper {
}
/**
- * Creates a symlink tree using a CommandBuilder. This means that the symlink
- * tree will always be present on the developer's workstation. Useful when
- * running commands locally.
+ * Creates a symlink tree using a CommandBuilder. This means that the symlink tree will always be
+ * present on the developer's workstation. Useful when running commands locally.
*
- * Warning: this method REALLY executes the command on the box Blaze was
- * run on, without any kind of synchronization, locking, or anything else.
+ * Warning: this method REALLY executes the command on the box Bazel is running on, without any
+ * kind of synchronization, locking, or anything else.
*
* @param config the configuration that is used for creating the symlink tree.
* @throws CommandException
*/
- public void createSymlinksUsingCommand(Path execRoot,
- BuildConfiguration config, BinTools binTools) throws CommandException {
+ public void createSymlinksUsingCommand(
+ Path execRoot, BuildConfiguration config, BinTools binTools)
+ throws CommandException {
List<String> argv = getSpawnArgumentList(execRoot, binTools);
-
CommandBuilder builder = new CommandBuilder();
builder.addArgs(argv);
builder.setWorkingDir(execRoot);
@@ -101,30 +101,30 @@ public final class SymlinkTreeHelper {
* Creates symlink tree using appropriate method. At this time tree always created using
* build-runfiles helper application.
*
- * <p>Note: method may try to acquire resources - meaning that it would block for undetermined
- * period of time. If it is interrupted during that wait, ExecException will be thrown but
- * interrupted bit will be preserved.
- *
- * @param action action instance that requested symlink tree creation
+ * @param owner action instance that requested symlink tree creation
* @param actionExecutionContext Services that are in the scope of the action.
* @param enableRunfiles
* @return a list of SpawnResults created during symlink creation, if any
*/
public List<SpawnResult> createSymlinks(
- AbstractAction action,
+ ActionExecutionMetadata owner,
ActionExecutionContext actionExecutionContext,
BinTools binTools,
ImmutableMap<String, String> shellEnvironment,
+ Artifact inputManifestArtifact,
boolean enableRunfiles)
- throws ExecException, InterruptedException {
+ throws ExecException, InterruptedException {
+ Preconditions.checkState(inputManifestArtifact.getPath().equals(inputManifest));
if (enableRunfiles) {
- List<String> args =
- getSpawnArgumentList(
- actionExecutionContext.getExecRoot(), binTools);
return actionExecutionContext
- .getSpawnActionContext(action.getMnemonic())
+ .getSpawnActionContext(owner.getMnemonic())
.exec(
- new BaseSpawn.Local(args, shellEnvironment, action, RESOURCE_SET),
+ createSpawn(
+ owner,
+ actionExecutionContext.getExecRoot(),
+ binTools,
+ shellEnvironment,
+ inputManifestArtifact),
actionExecutionContext);
} else {
// Pretend we created the runfiles tree by copying the manifest
@@ -138,10 +138,30 @@ public final class SymlinkTreeHelper {
}
}
+ @VisibleForTesting
+ Spawn createSpawn(
+ ActionExecutionMetadata owner,
+ Path execRoot,
+ BinTools binTools,
+ ImmutableMap<String, String> environment,
+ ActionInput inputManifestArtifact) {
+ return new SimpleSpawn(
+ owner,
+ getSpawnArgumentList(execRoot, binTools),
+ environment,
+ ImmutableMap.of(
+ ExecutionRequirements.LOCAL, "",
+ ExecutionRequirements.NO_CACHE, "",
+ ExecutionRequirements.NO_SANDBOX, ""),
+ ImmutableList.of(inputManifestArtifact),
+ /*outputs=*/ ImmutableList.of(),
+ RESOURCE_SET);
+ }
+
/**
* Returns the complete argument list build-runfiles has to be called with.
*/
- private List<String> getSpawnArgumentList(Path execRoot, BinTools binTools) {
+ private ImmutableList<String> getSpawnArgumentList(Path execRoot, BinTools binTools) {
PathFragment path = binTools.getExecPath(BUILD_RUNFILES);
Preconditions.checkNotNull(path, BUILD_RUNFILES + " not found in embedded tools");
@@ -156,6 +176,6 @@ public final class SymlinkTreeHelper {
args.add(inputManifest.relativeTo(execRoot).getPathString());
args.add(symlinkTreeRoot.relativeTo(execRoot).getPathString());
- return args;
+ return ImmutableList.copyOf(args);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategy.java
index a0f87d4da0..14802acd99 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategy.java
@@ -55,18 +55,25 @@ public final class SymlinkTreeStrategy implements SymlinkTreeActionContext {
AutoProfiler.logged(
"running " + action.prettyPrint(), logger, /*minTimeForLoggingInMilliseconds=*/ 100)) {
try {
- SymlinkTreeHelper helper = new SymlinkTreeHelper(
- action.getInputManifest().getPath(),
- action.getOutputManifest().getPath().getParentDirectory(), action.isFilesetTree());
if (outputService != null && outputService.canCreateSymlinkTree()) {
- outputService.createSymlinkTree(action.getInputManifest().getPath(),
+ outputService.createSymlinkTree(
+ action.getInputManifest().getPath(),
action.getOutputManifest().getPath(),
action.isFilesetTree(),
action.getOutputManifest().getExecPath().getParentDirectory());
return ImmutableList.of();
} else {
+ SymlinkTreeHelper helper = new SymlinkTreeHelper(
+ action.getInputManifest().getPath(),
+ action.getOutputManifest().getPath().getParentDirectory(),
+ action.isFilesetTree());
return helper.createSymlinks(
- action, actionExecutionContext, binTools, shellEnvironment, enableRunfiles);
+ action,
+ actionExecutionContext,
+ binTools,
+ shellEnvironment,
+ action.getInputManifest(),
+ enableRunfiles);
}
} catch (ExecException e) {
throw e.toActionExecutionException(
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 85d1d1d60d..e251cd60a0 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
@@ -410,7 +410,12 @@ public abstract class TestStrategy implements TestActionContext {
new SymlinkTreeHelper(execSettings.getInputManifest().getPath(), runfilesDir, false)
.createSymlinks(
- testAction, actionExecutionContext, binTools, shellEnvironment, enableRunfiles);
+ testAction,
+ actionExecutionContext,
+ binTools,
+ shellEnvironment,
+ execSettings.getInputManifest(),
+ enableRunfiles);
actionExecutionContext.getEventHandler()
.handle(Event.progress(testAction.getProgressMessage()));
diff --git a/src/main/java/com/google/devtools/build/lib/packages/TargetUtils.java b/src/main/java/com/google/devtools/build/lib/packages/TargetUtils.java
index 6ec3340465..702b9d6902 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/TargetUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/TargetUtils.java
@@ -203,16 +203,23 @@ public final class TargetUtils {
}
/**
- * Returns the execution info. These include execution requirement
- * tags ('requires-*' as well as "local") as keys with empty values.
+ * Returns the execution info. These include execution requirement tags ('block-*', 'requires-*',
+ * 'no-*', 'supports-*', 'disable-*', 'local', and 'cpu:*') as keys with empty values.
*/
public static Map<String, String> getExecutionInfo(Rule rule) {
// tags may contain duplicate values.
Map<String, String> map = new HashMap<>();
for (String tag :
NonconfigurableAttributeMapper.of(rule).get(CONSTRAINTS_ATTR, Type.STRING_LIST)) {
+ // We don't want to pollute the execution info with random things, and we also need to reserve
+ // some internal tags that we don't allow to be set on targets. We also don't want to
+ // exhaustively enumerate all the legal values here. Right now, only a ~small set of tags is
+ // recognized by Bazel.
if (tag.startsWith("block-")
|| tag.startsWith("requires-")
+ || tag.startsWith("no-")
+ || tag.startsWith("supports-")
+ || tag.startsWith("disable-")
|| tag.equals("local")
|| tag.startsWith("cpu:")) {
map.put(tag, "");
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java
index 8dae278958..6c4b0a8eda 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java
@@ -108,7 +108,7 @@ class RemoteSpawnRunner implements SpawnRunner {
@Override
public SpawnResult exec(Spawn spawn, SpawnExecutionPolicy policy)
throws ExecException, InterruptedException, IOException {
- if (!spawn.isRemotable() || remoteCache == null) {
+ if (!Spawns.mayBeExecutedRemotely(spawn) || remoteCache == null) {
return fallbackRunner.exec(spawn, policy);
}
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedSpawnRunner.java
index 1f6af91075..97fc677503 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedSpawnRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedSpawnRunner.java
@@ -24,6 +24,7 @@ import com.google.devtools.build.lib.actions.ExecutionStrategy;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.SpawnActionContext;
import com.google.devtools.build.lib.actions.SpawnResult;
+import com.google.devtools.build.lib.actions.Spawns;
import com.google.devtools.build.lib.exec.apple.XCodeLocalEnvProvider;
import com.google.devtools.build.lib.exec.local.LocalEnvProvider;
import com.google.devtools.build.lib.runtime.CommandEnvironment;
@@ -197,7 +198,7 @@ final class DarwinSandboxedSpawnRunner extends AbstractSandboxSpawnRunner {
Map<String, String> environment =
localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), execRoot, tmpDir, productName);
- boolean allowNetworkForThisSpawn = allowNetwork || SandboxHelpers.shouldAllowNetwork(spawn);
+ boolean allowNetworkForThisSpawn = allowNetwork || Spawns.requiresNetwork(spawn);
SandboxedSpawn sandbox = new SymlinkedSandboxedSpawn(
sandboxPath,
sandboxExecRoot,
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java
index aee389713a..a16497febc 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java
@@ -19,8 +19,10 @@ import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Maps;
import com.google.common.io.ByteStreams;
import com.google.devtools.build.lib.actions.ExecException;
+import com.google.devtools.build.lib.actions.ExecutionRequirements;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.SpawnResult;
+import com.google.devtools.build.lib.actions.Spawns;
import com.google.devtools.build.lib.actions.UserExecException;
import com.google.devtools.build.lib.analysis.BlazeDirectories;
import com.google.devtools.build.lib.exec.local.LocalEnvProvider;
@@ -134,8 +136,8 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner {
writableDirs,
getTmpfsPaths(),
getReadOnlyBindMounts(blazeDirs, sandboxExecRoot),
- allowNetwork || SandboxHelpers.shouldAllowNetwork(spawn),
- spawn.getExecutionInfo().containsKey("requires-fakeroot"));
+ allowNetwork || Spawns.requiresNetwork(spawn),
+ spawn.getExecutionInfo().containsKey(ExecutionRequirements.REQUIRES_FAKEROOT));
Map<String, String> environment =
localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), execRoot, tmpDir, productName);
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextProvider.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextProvider.java
index 53173e4bea..ee496e8e29 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextProvider.java
@@ -20,6 +20,7 @@ import com.google.devtools.build.lib.actions.ExecException;
import com.google.devtools.build.lib.actions.ResourceManager;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.SpawnResult;
+import com.google.devtools.build.lib.actions.Spawns;
import com.google.devtools.build.lib.exec.ActionContextProvider;
import com.google.devtools.build.lib.exec.SpawnRunner;
import com.google.devtools.build.lib.exec.apple.XCodeLocalEnvProvider;
@@ -121,7 +122,7 @@ final class SandboxActionContextProvider extends ActionContextProvider {
@Override
public SpawnResult exec(Spawn spawn, SpawnExecutionPolicy policy)
throws InterruptedException, IOException, ExecException {
- if (!spawn.isRemotable() || spawn.hasNoSandbox()) {
+ if (!Spawns.mayBeSandboxed(spawn)) {
return fallbackSpawnRunner.exec(spawn, policy);
} else {
return sandboxSpawnRunner.exec(spawn, policy);
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxHelpers.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxHelpers.java
index 94d1c1111c..58906fddf2 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxHelpers.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxHelpers.java
@@ -21,6 +21,7 @@ import com.google.devtools.build.lib.actions.ActionInput;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander;
import com.google.devtools.build.lib.actions.Spawn;
+import com.google.devtools.build.lib.actions.Spawns;
import com.google.devtools.build.lib.analysis.test.TestConfiguration;
import com.google.devtools.build.lib.exec.SpawnInputExpander;
import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionPolicy;
@@ -115,7 +116,7 @@ public final class SandboxHelpers {
/**
* Returns true if the build options are set in a way that requires network access for all
- * actions. This is separate from {@link #shouldAllowNetwork(Spawn)} to avoid having to keep a
+ * actions. This is separate from {@link Spawns#requiresNetwork} to avoid having to keep a
* reference to the full set of build options (and also for performance, since this only needs to
* be checked once-per-build).
*/
@@ -128,15 +129,4 @@ public final class SandboxHelpers {
.testArguments
.contains("--wrapper_script_flag=--debug");
}
-
- /** Returns true if this specific spawn requires network access. */
- static boolean shouldAllowNetwork(Spawn spawn) {
- // If the Spawn requests to block network access, do so.
- if (spawn.getExecutionInfo().containsKey("block-network")) {
- return false;
- }
-
- // Network access is allowed by default.
- return true;
- }
}
diff --git a/src/test/java/com/google/devtools/build/lib/exec/SymlinkTreeHelperTest.java b/src/test/java/com/google/devtools/build/lib/exec/SymlinkTreeHelperTest.java
new file mode 100644
index 0000000000..3baee79ccd
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/exec/SymlinkTreeHelperTest.java
@@ -0,0 +1,67 @@
+// 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 static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.devtools.build.lib.actions.ActionExecutionMetadata;
+import com.google.devtools.build.lib.actions.ActionInput;
+import com.google.devtools.build.lib.actions.ActionInputHelper;
+import com.google.devtools.build.lib.actions.ExecutionRequirements;
+import com.google.devtools.build.lib.actions.Spawn;
+import com.google.devtools.build.lib.analysis.config.BinTools;
+import com.google.devtools.build.lib.exec.util.FakeOwner;
+import com.google.devtools.build.lib.vfs.FileSystem;
+import com.google.devtools.build.lib.vfs.Path;
+import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/** Unit tests for {@link SymlinkTreeHelper}. */
+@RunWith(JUnit4.class)
+public final class SymlinkTreeHelperTest {
+ private final FileSystem fs = new InMemoryFileSystem();
+
+ @Test
+ public void checkCreatedSpawn() {
+ ActionExecutionMetadata owner = new FakeOwner("SymlinkTree", "Creating it");
+ Path execRoot = fs.getPath("/my/workspace");
+ Path inputManifestPath = execRoot.getRelative("input_manifest");
+ ActionInput inputManifest = ActionInputHelper.fromPath(inputManifestPath.asFragment());
+ Spawn spawn =
+ new SymlinkTreeHelper(
+ inputManifestPath,
+ fs.getPath("/my/workspace/output/MANIFEST"),
+ false)
+ .createSpawn(
+ owner,
+ execRoot,
+ BinTools.forUnitTesting(execRoot, ImmutableList.of(SymlinkTreeHelper.BUILD_RUNFILES)),
+ ImmutableMap.of(),
+ inputManifest);
+ assertThat(spawn.getResourceOwner()).isSameAs(owner);
+ assertThat(spawn.getEnvironment()).isEmpty();
+ assertThat(spawn.getExecutionInfo()).containsExactly(
+ ExecutionRequirements.LOCAL, "",
+ ExecutionRequirements.NO_CACHE, "",
+ ExecutionRequirements.NO_SANDBOX, "");
+ assertThat(spawn.getInputFiles()).containsExactly(inputManifest);
+ // At this time, the spawn does not declare any output files.
+ assertThat(spawn.getOutputFiles()).isEmpty();
+ }
+}
diff --git a/src/test/java/com/google/devtools/build/lib/packages/TargetUtilsTest.java b/src/test/java/com/google/devtools/build/lib/packages/TargetUtilsTest.java
index 6d49de9024..eb49366cf2 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/TargetUtilsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/TargetUtilsTest.java
@@ -18,6 +18,7 @@ import static com.google.common.truth.Truth.assertThat;
import com.google.common.base.Predicate;
import com.google.common.collect.Lists;
import com.google.devtools.build.lib.packages.util.PackageLoadingTestCase;
+import java.util.Map;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
@@ -75,4 +76,24 @@ public class TargetUtilsTest extends PackageLoadingTestCase {
assertThat(tagFilter.apply(tag2)).isTrue();
assertThat(tagFilter.apply(tag1b)).isFalse();
}
+
+ @Test
+ public void testExecutionInfo() throws Exception {
+ scratch.file(
+ "tests/BUILD",
+ "sh_binary(name = 'tag1', srcs=['sh.sh'], tags=['supports-workers', 'no-cache'])",
+ "sh_binary(name = 'tag2', srcs=['sh.sh'], tags=['disable-local-prefetch'])",
+ "sh_binary(name = 'tag1b', srcs=['sh.sh'], tags=['local', 'cpu:4'])");
+
+ Rule tag1 = (Rule) getTarget("//tests:tag1");
+ Rule tag2 = (Rule) getTarget("//tests:tag2");
+ Rule tag1b = (Rule) getTarget("//tests:tag1b");
+
+ Map<String, String> execInfo = TargetUtils.getExecutionInfo(tag1);
+ assertThat(execInfo).containsExactly("supports-workers", "", "no-cache", "");
+ execInfo = TargetUtils.getExecutionInfo(tag2);
+ assertThat(execInfo).containsExactly("disable-local-prefetch", "");
+ execInfo = TargetUtils.getExecutionInfo(tag1b);
+ assertThat(execInfo).containsExactly("local", "", "cpu:4", "");
+ }
}
diff --git a/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java b/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java
index 342680b700..d2497ff313 100644
--- a/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java
@@ -26,10 +26,10 @@ import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.ActionInputPrefetcher;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander;
-import com.google.devtools.build.lib.actions.BaseSpawn;
import com.google.devtools.build.lib.actions.ExecException;
import com.google.devtools.build.lib.actions.ResourceManager;
import com.google.devtools.build.lib.actions.ResourceSet;
+import com.google.devtools.build.lib.actions.SimpleSpawn;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.SpawnActionContext;
import com.google.devtools.build.lib.actions.SpawnResult;
@@ -58,7 +58,6 @@ import com.google.devtools.build.lib.vfs.util.FileSystems;
import com.google.devtools.common.options.Options;
import com.google.devtools.common.options.OptionsParser;
import java.io.IOException;
-import java.util.Arrays;
import java.util.Collection;
import java.util.List;
import org.junit.Before;
@@ -148,10 +147,13 @@ public class StandaloneSpawnStrategyTest {
}
private Spawn createSpawn(String... arguments) {
- return new BaseSpawn.Local(
- Arrays.asList(arguments),
- ImmutableMap.<String, String>of(),
+ return new SimpleSpawn(
new ActionsTestUtil.NullAction(),
+ ImmutableList.copyOf(arguments),
+ /*environment=*/ ImmutableMap.of(),
+ /*executionInfo=*/ ImmutableMap.of(),
+ /*inputs=*/ ImmutableList.of(),
+ /*outputs=*/ ImmutableList.of(),
ResourceSet.ZERO);
}
@@ -230,10 +232,13 @@ public class StandaloneSpawnStrategyTest {
// down where that env var is coming from.
return;
}
- Spawn spawn = new BaseSpawn.Local(
- Arrays.asList("/usr/bin/env"),
- ImmutableMap.of("foo", "bar", "baz", "boo"),
+ Spawn spawn = new SimpleSpawn(
new ActionsTestUtil.NullAction(),
+ ImmutableList.of("/usr/bin/env"),
+ /*environment=*/ ImmutableMap.of("foo", "bar", "baz", "boo"),
+ /*executionInfo=*/ ImmutableMap.of(),
+ /*inputs=*/ ImmutableList.of(),
+ /*outputs=*/ ImmutableList.of(),
ResourceSet.ZERO);
run(spawn);
assertThat(Sets.newHashSet(out().split("\n"))).isEqualTo(Sets.newHashSet("foo=bar", "baz=boo"));