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
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 5ccc56b..55b73d2 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 901b225..bc52055 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 @@
}
@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 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 8341301..fd548b6 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 @@
}
@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 22242de..fced368 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 @@
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 564460e..3857783 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 @@
}
@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 6963b01..3a59d9f 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 @@
* 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 e1156e9..4448ea8 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 @@
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 deb987b..dc52388 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 @@
* 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 @@
*/
@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 @@
*/
@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 @@
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 b5f4b92..b9a055b 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.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.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 @@
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 @@
}
/**
- * 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 @@
* 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 @@
}
}
+ @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 @@
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 a0f87d4..14802ac 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 @@
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 85d1d1d6..e251cd6 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 @@
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 6ec33404..702b9d6 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 @@
}
/**
- * 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 8dae278..6c4b0a8 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 @@
@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 1f6af91..97fc677 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.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 @@
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 aee3897..a16497f 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.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 @@
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 53173e4..ee496e8 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.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 @@
@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 94d1c11..58906fd 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.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 @@
/**
* 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 @@
.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 0000000..3baee79
--- /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 6d49de9..eb49366 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 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 @@
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 342680b..d2497ff 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.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.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 @@
}
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 @@
// 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"));
