diff --git a/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java b/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java index 1afcd70e2b533b..be99bbd147220b 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java +++ b/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java @@ -169,11 +169,19 @@ protected boolean couldBeModifiedByMetadata(FileArtifactValue lastKnown) { /** * Optional materialization path. * - *

If present, this artifact is a copy of another artifact. It is still tracked as a - * non-symlink by Bazel, but materialized in the local filesystem as a symlink to the original - * artifact, whose contents live at this location. This is used by {@link - * com.google.devtools.build.lib.remote.AbstractActionInputPrefetcher} to implement zero-cost - * copies of remotely stored artifacts. + *

If present, this artifact is a copy of another artifact whose contents live at this path. + * This can happen when it is declared as a file and not as an unresolved symlink but the action + * that creates it materializes it in the filesystem as a symlink to another output artifact. This + * information is useful in two situations: + * + *

    + *
  1. When the symlink target is a remotely stored artifact, we can avoid downloading it + * multiple times when building without the bytes (see AbstractActionInputPrefetcher). + *
  2. When the symlink target is inaccessible from the sandboxed environment an action runs + * under, we can rewrite it accordingly (see SandboxHelpers). + *
+ * + * @see com.google.devtools.build.lib.skyframe.TreeArtifactValue#getMaterializationExecPath(). */ public Optional getMaterializationExecPath() { return Optional.empty(); @@ -214,6 +222,12 @@ public static FileArtifactValue createForSourceArtifact( xattrProvider); } + public static FileArtifactValue createForResolvedSymlink( + PathFragment realPath, FileArtifactValue metadata, @Nullable byte[] digest) { + return new ResolvedSymlinkFileArtifactValue( + realPath, digest, metadata.getContentsProxy(), metadata.getSize()); + } + public static FileArtifactValue createFromInjectedDigest( FileArtifactValue metadata, @Nullable byte[] digest) { return createForNormalFile(digest, metadata.getContentsProxy(), metadata.getSize()); @@ -439,7 +453,25 @@ public String toString() { } } - private static final class RegularFileArtifactValue extends FileArtifactValue { + private static final class ResolvedSymlinkFileArtifactValue extends RegularFileArtifactValue { + private final PathFragment realPath; + + private ResolvedSymlinkFileArtifactValue( + PathFragment realPath, + @Nullable byte[] digest, + @Nullable FileContentsProxy proxy, + long size) { + super(digest, proxy, size); + this.realPath = realPath; + } + + @Override + public Optional getMaterializationExecPath() { + return Optional.of(realPath); + } + } + + private static class RegularFileArtifactValue extends FileArtifactValue { private final byte[] digest; @Nullable private final FileContentsProxy proxy; private final long size; @@ -462,7 +494,8 @@ public boolean equals(Object o) { RegularFileArtifactValue that = (RegularFileArtifactValue) o; return Arrays.equals(digest, that.digest) && Objects.equals(proxy, that.proxy) - && size == that.size; + && size == that.size + && Objects.equals(getMaterializationExecPath(), that.getMaterializationExecPath()); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java index 2a0fa882d93725..61868714abc722 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java @@ -15,6 +15,7 @@ package com.google.devtools.build.lib.sandbox; import static java.nio.charset.StandardCharsets.UTF_8; +import static java.util.stream.Collectors.joining; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; @@ -59,7 +60,8 @@ import java.time.Duration; import java.time.Instant; import java.util.Map; -import javax.annotation.Nullable; +import java.util.Optional; +import java.util.stream.Stream; /** Abstract common ancestor for sandbox spawn runners implementing the common parts. */ abstract class AbstractSandboxSpawnRunner implements SpawnRunner { @@ -224,7 +226,7 @@ private final SpawnResult run( StringBuilder msg = new StringBuilder("Action failed to execute: java.io.IOException: "); msg.append(exceptionMsg); msg.append("\n"); - if (sandboxDebugOutput != null) { + if (!sandboxDebugOutput.isEmpty()) { msg.append("Sandbox debug output:\n"); msg.append(sandboxDebugOutput); msg.append("\n"); @@ -294,12 +296,12 @@ private final SpawnResult run( } String sandboxDebugOutput = getSandboxDebugOutput(sandbox); - if (sandboxDebugOutput != null) { + if (!sandboxDebugOutput.isEmpty()) { reporter.handle( Event.of( EventKind.DEBUG, String.format( - "Sandbox debug output for %s %s: %s", + "Sandbox debug output for %s %s:\n%s", originalSpawn.getMnemonic(), originalSpawn.getTargetLabel(), sandboxDebugOutput))); @@ -334,18 +336,21 @@ private final SpawnResult run( return spawnResultBuilder.build(); } - @Nullable private String getSandboxDebugOutput(SandboxedSpawn sandbox) throws IOException { + Optional sandboxDebugOutput = Optional.empty(); Path sandboxDebugPath = sandbox.getSandboxDebugPath(); if (sandboxDebugPath != null && sandboxDebugPath.exists()) { try (InputStream inputStream = sandboxDebugPath.getInputStream()) { String msg = new String(inputStream.readAllBytes(), UTF_8); if (!msg.isEmpty()) { - return msg; + sandboxDebugOutput = Optional.of(msg); } } } - return null; + Optional interactiveDebugInstructions = sandbox.getInteractiveDebugInstructions(); + return Stream.of(sandboxDebugOutput, interactiveDebugInstructions) + .flatMap(Optional::stream) + .collect(joining("\n")); } private boolean wasTimeout(Duration timeout, Duration wallTime) { diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/BUILD b/src/main/java/com/google/devtools/build/lib/sandbox/BUILD index 87ac37c3644eb0..9e92eb4750b833 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/BUILD +++ b/src/main/java/com/google/devtools/build/lib/sandbox/BUILD @@ -17,6 +17,7 @@ java_library( deps = [ "//src/main/java/com/google/devtools/build/lib/actions", "//src/main/java/com/google/devtools/build/lib/actions:artifacts", + "//src/main/java/com/google/devtools/build/lib/actions:file_metadata", "//src/main/java/com/google/devtools/build/lib/analysis:test/test_configuration", "//src/main/java/com/google/devtools/build/lib/cmdline", "//src/main/java/com/google/devtools/build/lib/vfs", @@ -72,6 +73,7 @@ java_library( ":sandbox_helpers", ":sandbox_options", "//src/main/java/com/google/devtools/build/lib/exec:tree_deleter", + "//src/main/java/com/google/devtools/build/lib/util:command", "//src/main/java/com/google/devtools/build/lib/util:describable_execution_unit", "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", 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 fad5cfa108b11e..3ad9c32531e0f0 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 @@ -228,6 +228,7 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context SandboxInputs inputs = helpers.processInputFiles( context.getInputMapping(PathFragment.EMPTY_FRAGMENT, /* willAccessRepeatedly= */ true), + context.getInputMetadataProvider(), execRoot, execRoot, packageRoots, @@ -258,29 +259,30 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context allowNetwork || Spawns.requiresNetwork(spawn, getSandboxOptions().defaultSandboxAllowNetwork); - return new SymlinkedSandboxedSpawn( - sandboxPath, - sandboxExecRoot, - commandLine, - environment, - inputs, - outputs, - writableDirs, - treeDeleter, - /* sandboxDebugPath= */ null, - statisticsPath, - spawn.getMnemonic()) { - @Override - public void createFileSystem() throws IOException, InterruptedException { - super.createFileSystem(); - writeConfig( - sandboxConfigPath, - writableDirs, - getInaccessiblePaths(), - allowNetworkForThisSpawn, - statisticsPath); - } - }; + return new SymlinkedSandboxedSpawn( + sandboxPath, + sandboxExecRoot, + commandLine, + environment, + inputs, + outputs, + writableDirs, + treeDeleter, + /* sandboxDebugPath= */ null, + statisticsPath, + /* interactiveDebugArguments= */ null, + spawn.getMnemonic()) { + @Override + public void createFileSystem() throws IOException, InterruptedException { + super.createFileSystem(); + writeConfig( + sandboxConfigPath, + writableDirs, + getInaccessiblePaths(), + allowNetworkForThisSpawn, + statisticsPath); + } + }; } private void writeConfig( diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/DockerSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/DockerSandboxedSpawnRunner.java index 3dacaeaa05ee9a..971b05cd11b99b 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/DockerSandboxedSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/DockerSandboxedSpawnRunner.java @@ -226,6 +226,7 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context SandboxInputs inputs = helpers.processInputFiles( context.getInputMapping(PathFragment.EMPTY_FRAGMENT, /* willAccessRepeatedly= */ true), + context.getInputMetadataProvider(), execRoot, execRoot, packageRoots, diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxCommandLineBuilder.java b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxCommandLineBuilder.java index cf80265a85e6f0..967c08b5d6ce59 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxCommandLineBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxCommandLineBuilder.java @@ -51,7 +51,6 @@ public static BindMount of(Path mountPoint, Path source) { } private final Path linuxSandboxPath; - private final List commandArguments; private Path hermeticSandboxPath; private Path workingDirectory; private Duration timeout; @@ -72,15 +71,13 @@ public static BindMount of(Path mountPoint, Path source) { private boolean sigintSendsSigterm = false; private String cgroupsDir; - private LinuxSandboxCommandLineBuilder(Path linuxSandboxPath, List commandArguments) { + private LinuxSandboxCommandLineBuilder(Path linuxSandboxPath) { this.linuxSandboxPath = linuxSandboxPath; - this.commandArguments = commandArguments; } /** Returns a new command line builder for the {@code linux-sandbox} tool. */ - public static LinuxSandboxCommandLineBuilder commandLineBuilder( - Path linuxSandboxPath, List commandArguments) { - return new LinuxSandboxCommandLineBuilder(linuxSandboxPath, commandArguments); + public static LinuxSandboxCommandLineBuilder commandLineBuilder(Path linuxSandboxPath) { + return new LinuxSandboxCommandLineBuilder(linuxSandboxPath); } /** @@ -247,7 +244,7 @@ public LinuxSandboxCommandLineBuilder addExecutionInfo(Map execu } /** Builds the command line to invoke a specific command using the {@code linux-sandbox} tool. */ - public ImmutableList build() { + public ImmutableList buildForCommand(List commandArguments) { Preconditions.checkState( !(this.useFakeUsername && this.useFakeRoot), "useFakeUsername and useFakeRoot are exclusive"); 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 2d593e7a28003e..5e5e0a825d9cbf 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 @@ -106,10 +106,9 @@ private static boolean computeIsSupported(CommandEnvironment cmdEnv, Path linuxS throws InterruptedException { LocalExecutionOptions options = cmdEnv.getOptions().getOptions(LocalExecutionOptions.class); ImmutableList linuxSandboxArgv = - LinuxSandboxCommandLineBuilder.commandLineBuilder( - linuxSandbox, ImmutableList.of("/bin/true")) + LinuxSandboxCommandLineBuilder.commandLineBuilder(linuxSandbox) .setTimeout(options.getLocalSigkillGraceSeconds()) - .build(); + .buildForCommand(ImmutableList.of("/bin/true")); ImmutableMap env = ImmutableMap.of(); Path execRoot = cmdEnv.getExecRoot(); File cwd = execRoot.getPathFile(); @@ -281,6 +280,7 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context SandboxInputs inputs = helpers.processInputFiles( context.getInputMapping(PathFragment.EMPTY_FRAGMENT, /* willAccessRepeatedly= */ true), + context.getInputMetadataProvider(), execRoot, withinSandboxExecRoot, packageRoots, @@ -309,7 +309,7 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context boolean createNetworkNamespace = !(allowNetwork || Spawns.requiresNetwork(spawn, sandboxOptions.defaultSandboxAllowNetwork)); LinuxSandboxCommandLineBuilder commandLineBuilder = - LinuxSandboxCommandLineBuilder.commandLineBuilder(linuxSandbox, spawn.getArguments()) + LinuxSandboxCommandLineBuilder.commandLineBuilder(linuxSandbox) .addExecutionInfo(spawn.getExecutionInfo()) .setWritableFilesAndDirectories(writableDirs) .setTmpfsDirectories(ImmutableSet.copyOf(getSandboxOptions().sandboxTmpfsPath)) @@ -354,7 +354,7 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context return new HardlinkedSandboxedSpawn( sandboxPath, sandboxExecRoot, - commandLineBuilder.build(), + commandLineBuilder.buildForCommand(spawn.getArguments()), environment, inputs, outputs, @@ -368,7 +368,7 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context return new SymlinkedSandboxedSpawn( sandboxPath, sandboxExecRoot, - commandLineBuilder.build(), + commandLineBuilder.buildForCommand(spawn.getArguments()), environment, inputs, outputs, @@ -376,6 +376,7 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context treeDeleter, sandboxDebugPath, statisticsPath, + makeInteractiveDebugArguments(commandLineBuilder, sandboxOptions), spawn.getMnemonic()); } } @@ -538,4 +539,13 @@ public void cleanupSandboxBase(Path sandboxBase, TreeDeleter treeDeleter) throws super.cleanupSandboxBase(sandboxBase, treeDeleter); } + + @Nullable + private ImmutableList makeInteractiveDebugArguments( + LinuxSandboxCommandLineBuilder commandLineBuilder, SandboxOptions sandboxOptions) { + if (!sandboxOptions.sandboxDebug) { + return null; + } + return commandLineBuilder.buildForCommand(ImmutableList.of("/bin/sh", "-i")); + } } diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java index 2f7608dcc8b857..14d2a63545a734 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java @@ -100,24 +100,26 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context SandboxInputs inputs = helpers.processInputFiles( context.getInputMapping(PathFragment.EMPTY_FRAGMENT, /* willAccessRepeatedly= */ true), + context.getInputMetadataProvider(), execRoot, execRoot, packageRoots, null); SandboxOutputs outputs = helpers.getOutputs(spawn); - return new SymlinkedSandboxedSpawn( - sandboxPath, - sandboxExecRoot, - commandLineBuilder.build(), - environment, - inputs, - outputs, - getWritableDirs(sandboxExecRoot, sandboxExecRoot, environment), - treeDeleter, - /* sandboxDebugPath= */ null, - statisticsPath, - spawn.getMnemonic()); + return new SymlinkedSandboxedSpawn( + sandboxPath, + sandboxExecRoot, + commandLineBuilder.build(), + environment, + inputs, + outputs, + getWritableDirs(sandboxExecRoot, sandboxExecRoot, environment), + treeDeleter, + /* sandboxDebugPath= */ null, + statisticsPath, + /* interactiveDebugArguments= */ null, + spawn.getMnemonic()); } @Override 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 fb72d591e0781e..dfc18bda2c839a 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 @@ -20,6 +20,7 @@ import static com.google.devtools.build.lib.vfs.Dirent.Type.SYMLINK; import com.google.auto.value.AutoValue; +import com.google.common.base.Function; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -29,6 +30,8 @@ import com.google.common.flogger.GoogleLogger; import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.FileArtifactValue; +import com.google.devtools.build.lib.actions.InputMetadataProvider; import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.actions.UserExecException; import com.google.devtools.build.lib.actions.cache.VirtualActionInput; @@ -444,6 +447,29 @@ private static RootedPath processFilesetSymlink( symlink.getPathString())); } + private static RootedPath processResolvedSymlink( + Root absoluteRoot, + PathFragment execRootRelativeSymlinkTarget, + Root execRootWithinSandbox, + PathFragment execRootFragment, + ImmutableList packageRoots, + Function sourceRooWithinSandbox) { + PathFragment symlinkTarget = execRootFragment.getRelative(execRootRelativeSymlinkTarget); + for (Root packageRoot : packageRoots) { + if (packageRoot.contains(symlinkTarget)) { + return RootedPath.toRootedPath( + sourceRooWithinSandbox.apply(packageRoot), packageRoot.relativize(symlinkTarget)); + } + } + + if (symlinkTarget.startsWith(execRootFragment)) { + return RootedPath.toRootedPath( + execRootWithinSandbox, symlinkTarget.relativeTo(execRootFragment)); + } + + return RootedPath.toRootedPath(absoluteRoot, symlinkTarget); + } + /** * Returns the inputs of a Spawn as a map of PathFragments relative to an execRoot to paths in the * host filesystem where the input files can be found. @@ -458,6 +484,7 @@ private static RootedPath processFilesetSymlink( */ public SandboxInputs processInputFiles( Map inputMap, + InputMetadataProvider inputMetadataProvider, Path execRootPath, Path withinSandboxExecRootPath, ImmutableList packageRoots, @@ -468,12 +495,24 @@ public SandboxInputs processInputFiles( withinSandboxExecRootPath.equals(execRootPath) ? withinSandboxExecRoot : Root.fromPath(execRootPath); + Root absoluteRoot = Root.absoluteRoot(execRootPath.getFileSystem()); Map inputFiles = new TreeMap<>(); Map inputSymlinks = new TreeMap<>(); Map virtualInputs = new HashMap<>(); Map sourceRootToSandboxSourceRoot = new TreeMap<>(); + Function sourceRootWithinSandbox = + r -> { + if (!sourceRootToSandboxSourceRoot.containsKey(r)) { + int next = sourceRootToSandboxSourceRoot.size(); + sourceRootToSandboxSourceRoot.put( + r, Root.fromPath(sandboxSourceRoots.getRelative(Integer.toString(next)))); + } + + return sourceRootToSandboxSourceRoot.get(r); + }; + for (Map.Entry e : inputMap.entrySet()) { if (Thread.interrupted()) { throw new InterruptedException(); @@ -503,23 +542,63 @@ public SandboxInputs processInputFiles( if (actionInput instanceof EmptyActionInput) { inputPath = null; + } else if (actionInput instanceof VirtualActionInput) { + inputPath = RootedPath.toRootedPath(withinSandboxExecRoot, actionInput.getExecPath()); } else if (actionInput instanceof Artifact) { Artifact inputArtifact = (Artifact) actionInput; - if (inputArtifact.isSourceArtifact() && sandboxSourceRoots != null) { - Root sourceRoot = inputArtifact.getRoot().getRoot(); - if (!sourceRootToSandboxSourceRoot.containsKey(sourceRoot)) { - int next = sourceRootToSandboxSourceRoot.size(); - sourceRootToSandboxSourceRoot.put( - sourceRoot, - Root.fromPath(sandboxSourceRoots.getRelative(Integer.toString(next)))); - } - - inputPath = - RootedPath.toRootedPath( - sourceRootToSandboxSourceRoot.get(sourceRoot), - inputArtifact.getRootRelativePath()); - } else { + if (sandboxSourceRoots == null) { inputPath = RootedPath.toRootedPath(withinSandboxExecRoot, inputArtifact.getExecPath()); + } else { + if (inputArtifact.isSourceArtifact()) { + Root sourceRoot = inputArtifact.getRoot().getRoot(); + inputPath = + RootedPath.toRootedPath( + sourceRootWithinSandbox.apply(sourceRoot), + inputArtifact.getRootRelativePath()); + } else { + PathFragment materializationExecPath = null; + if (inputArtifact.isChildOfDeclaredDirectory()) { + FileArtifactValue parentMetadata = + inputMetadataProvider.getInputMetadata(inputArtifact.getParent()); + if (parentMetadata.getMaterializationExecPath().isPresent()) { + materializationExecPath = + parentMetadata + .getMaterializationExecPath() + .get() + .getRelative(inputArtifact.getParentRelativePath()); + } + } else if (!inputArtifact.isTreeArtifact()) { + // Normally, one would not see tree artifacts here because they have already been + // expanded by the time the code gets here. However, there is one very special case: + // when an action has an archived tree artifact on its output and is executed on the + // local branch of the dynamic execution strategy, the tree artifact is zipped up + // in a little extra spawn, which has direct tree artifact on its inputs. Sadly, + // it's not easy to fix this because there isn't an easy way to inject this new + // tree artifact into the artifact expander being used. + // + // The best would be to not rely on spawn strategies for executing that little + // command: it's entirely under the control of Bazel so we can guarantee that it + // does not cause mischief. + FileArtifactValue metadata = inputMetadataProvider.getInputMetadata(actionInput); + if (metadata.getMaterializationExecPath().isPresent()) { + materializationExecPath = metadata.getMaterializationExecPath().get(); + } + } + + if (materializationExecPath != null) { + inputPath = + processResolvedSymlink( + absoluteRoot, + materializationExecPath, + withinSandboxExecRoot, + execRootPath.asFragment(), + packageRoots, + sourceRootWithinSandbox); + } else { + inputPath = + RootedPath.toRootedPath(withinSandboxExecRoot, inputArtifact.getExecPath()); + } + } } } else { PathFragment execPath = actionInput.getExecPath(); @@ -544,6 +623,7 @@ public SandboxInputs processInputFiles( return new SandboxInputs(inputFiles, virtualInputs, inputSymlinks, sandboxRootToSourceRoot); } + /** The file and directory outputs of a sandboxed spawn. */ @AutoValue public abstract static class SandboxOutputs { diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxedSpawn.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxedSpawn.java index 1a355bc90b6733..44657780bbb446 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxedSpawn.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxedSpawn.java @@ -17,6 +17,7 @@ import com.google.devtools.build.lib.util.DescribableExecutionUnit; import com.google.devtools.build.lib.vfs.Path; import java.io.IOException; +import java.util.Optional; import javax.annotation.Nullable; /** @@ -61,4 +62,12 @@ default boolean useSubprocessTimeout() { /** Deletes the sandbox directory. */ void delete(); + + /** + * Returns user-facing instructions for starting an interactive sandboxed environment identical to + * the one in which this spawn is executed. + */ + default Optional getInteractiveDebugInstructions() { + return Optional.empty(); + } } diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SymlinkedSandboxedSpawn.java b/src/main/java/com/google/devtools/build/lib/sandbox/SymlinkedSandboxedSpawn.java index 57a09356f92cb2..4462d8a17df739 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/SymlinkedSandboxedSpawn.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/SymlinkedSandboxedSpawn.java @@ -21,10 +21,13 @@ import com.google.devtools.build.lib.exec.TreeDeleter; import com.google.devtools.build.lib.sandbox.SandboxHelpers.SandboxInputs; import com.google.devtools.build.lib.sandbox.SandboxHelpers.SandboxOutputs; +import com.google.devtools.build.lib.util.CommandDescriptionForm; +import com.google.devtools.build.lib.util.CommandFailureUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import java.io.IOException; import java.util.LinkedHashSet; +import java.util.Optional; import java.util.Set; import javax.annotation.Nullable; @@ -37,6 +40,8 @@ public class SymlinkedSandboxedSpawn extends AbstractContainerizingSandboxedSpaw /** Mnemonic of the action running in this spawn. */ private final String mnemonic; + @Nullable private final ImmutableList interactiveDebugArguments; + public SymlinkedSandboxedSpawn( Path sandboxPath, Path sandboxExecRoot, @@ -48,6 +53,7 @@ public SymlinkedSandboxedSpawn( TreeDeleter treeDeleter, @Nullable Path sandboxDebugPath, @Nullable Path statisticsPath, + @Nullable ImmutableList interactiveDebugArguments, String mnemonic) { super( sandboxPath, @@ -62,6 +68,7 @@ public SymlinkedSandboxedSpawn( statisticsPath, mnemonic); this.mnemonic = isNullOrEmpty(mnemonic) ? "_NoMnemonic_" : mnemonic; + this.interactiveDebugArguments = interactiveDebugArguments; } @Override @@ -96,4 +103,23 @@ public void delete() { SandboxStash.stashSandbox(sandboxPath, mnemonic); super.delete(); } + + @Nullable + @Override + public Optional getInteractiveDebugInstructions() { + if (interactiveDebugArguments == null) { + return Optional.empty(); + } + return Optional.of( + "Run this command to start an interactive shell in an identical sandboxed environment:\n" + + CommandFailureUtils.describeCommand( + CommandDescriptionForm.COMPLETE, + /* prettyPrintArgs= */ false, + interactiveDebugArguments, + getEnvironment(), + /* environmentVariablesToClear= */ null, + /* cwd= */ null, + /* configurationChecksum= */ null, + /* executionPlatformLabel= */ null)); + } } diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/WindowsSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/WindowsSandboxedSpawnRunner.java index c7996e38582fa1..505e2417850161 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/WindowsSandboxedSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/WindowsSandboxedSpawnRunner.java @@ -76,6 +76,7 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context SandboxInputs readablePaths = helpers.processInputFiles( context.getInputMapping(PathFragment.EMPTY_FRAGMENT, /* willAccessRepeatedly= */ true), + context.getInputMetadataProvider(), execRoot, execRoot, packageRoots, diff --git a/src/main/java/com/google/devtools/build/lib/server/GrpcServerImpl.java b/src/main/java/com/google/devtools/build/lib/server/GrpcServerImpl.java index 8efb5da16d3473..c7c21960e63f6e 100644 --- a/src/main/java/com/google/devtools/build/lib/server/GrpcServerImpl.java +++ b/src/main/java/com/google/devtools/build/lib/server/GrpcServerImpl.java @@ -46,6 +46,7 @@ import com.google.devtools.build.lib.util.DetailedExitCode; import com.google.devtools.build.lib.util.ExitCode; import com.google.devtools.build.lib.util.InterruptedFailureDetails; +import com.google.devtools.build.lib.util.OS; import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.util.io.OutErr; import com.google.devtools.build.lib.vfs.FileSystemUtils; @@ -376,6 +377,26 @@ public void interrupt() { commandManager.interruptInflightCommands(); } + private Server bindIpv6WithRetries(InetSocketAddress address, int maxRetries) throws IOException { + Server server = null; + for (int attempt = 1; attempt <= maxRetries; attempt++) { + try { + server = + NettyServerBuilder.forAddress(address) + .addService(this) + .directExecutor() + .build() + .start(); + break; + } catch (IOException e) { + if (attempt == maxRetries) { + throw e; + } + } + } + return server; + } + @Override public void serve() throws AbruptExitException { Preconditions.checkState(!serving); @@ -391,8 +412,10 @@ public void serve() throws AbruptExitException { if (Epoll.isAvailable() && !Socket.isIPv6Preferred()) { throw new IOException("ipv6 is not preferred on the system."); } - server = - NettyServerBuilder.forAddress(address).addService(this).directExecutor().build().start(); + // For some strange reasons, Bazel server sometimes fails to bind to IPv6 localhost when + // running in macOS sandbox-exec with internet blocked. Retrying seems to help. + // See https://github.com/bazelbuild/bazel/issues/20743 + server = bindIpv6WithRetries(address, OS.getCurrent() == OS.DARWIN ? 3 : 1); } catch (IOException ipv6Exception) { address = new InetSocketAddress("127.0.0.1", port); try { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStore.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStore.java index a1b1a77d3d0cbb..1bc2f4c7c0ac6a 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStore.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStore.java @@ -264,7 +264,15 @@ private TreeArtifactValue constructTreeArtifactValueFromFilesystem(SpecialArtifa Path treeDir = artifactPathResolver.toPath(parent); boolean chmod = executionMode.get(); - FileStatus stat = treeDir.statIfFound(Symlinks.FOLLOW); + FileStatus lstat = treeDir.statIfFound(Symlinks.NOFOLLOW); + FileStatus stat; + if (lstat == null) { + stat = null; + } else if (!lstat.isSymbolicLink()) { + stat = lstat; + } else { + stat = treeDir.statIfFound(Symlinks.FOLLOW); + } // Make sure the tree artifact root exists and is a regular directory. Note that this is how the // action is initialized, so this should hold unless the action itself has deleted the root. @@ -332,12 +340,18 @@ private TreeArtifactValue constructTreeArtifactValueFromFilesystem(SpecialArtifa } // Same rationale as for constructFileArtifactValue. - if (anyRemote.get() && treeDir.isSymbolicLink() && stat instanceof FileStatusWithMetadata) { - FileArtifactValue metadata = ((FileStatusWithMetadata) stat).getMetadata(); - tree.setMaterializationExecPath( - metadata - .getMaterializationExecPath() - .orElse(treeDir.resolveSymbolicLinks().asFragment().relativeTo(execRoot))); + if (lstat.isSymbolicLink()) { + PathFragment materializationExecPath = null; + if (stat instanceof FileStatusWithMetadata) { + materializationExecPath = + ((FileStatusWithMetadata) stat).getMetadata().getMaterializationExecPath().orElse(null); + } + + if (materializationExecPath == null) { + materializationExecPath = treeDir.resolveSymbolicLinks().asFragment().relativeTo(execRoot); + } + + tree.setMaterializationExecPath(materializationExecPath); } return tree.build(); @@ -509,7 +523,13 @@ private FileArtifactValue constructFileArtifactValue( return value; } - if (type.isFile() && fileDigest != null) { + boolean isResolvedSymlink = + statAndValue.statNoFollow() != null + && statAndValue.statNoFollow().isSymbolicLink() + && statAndValue.realPath() != null + && !value.isRemote(); + + if (type.isFile() && !isResolvedSymlink && fileDigest != null) { // The digest is in the file value and that is all that is needed for this file's metadata. return value; } @@ -525,22 +545,30 @@ private FileArtifactValue constructFileArtifactValue( artifactPathResolver.toPath(artifact).getLastModifiedTime()); } - if (injectedDigest == null && type.isFile()) { + byte[] actualDigest = fileDigest != null ? fileDigest : injectedDigest; + + if (actualDigest == null && type.isFile()) { // We don't have an injected digest and there is no digest in the file value (which attempts a // fast digest). Manually compute the digest instead. - Path path = statAndValue.pathNoFollow(); - if (statAndValue.statNoFollow() != null - && statAndValue.statNoFollow().isSymbolicLink() - && statAndValue.realPath() != null) { - // If the file is a symlink, we compute the digest using the target path so that it's - // possible to hit the digest cache - we probably already computed the digest for the - // target during previous action execution. - path = statAndValue.realPath(); - } - injectedDigest = DigestUtils.manuallyComputeDigest(path, value.getSize()); + // If the file is a symlink, we compute the digest using the target path so that it's + // possible to hit the digest cache - we probably already computed the digest for the + // target during previous action execution. + Path pathToDigest = isResolvedSymlink ? statAndValue.realPath() : statAndValue.pathNoFollow(); + actualDigest = DigestUtils.manuallyComputeDigest(pathToDigest, value.getSize()); } - return FileArtifactValue.createFromInjectedDigest(value, injectedDigest); + + if (!isResolvedSymlink) { + return FileArtifactValue.createFromInjectedDigest(value, actualDigest); + } + + PathFragment realPathAsFragment = statAndValue.realPath().asFragment(); + PathFragment execRootRelativeRealPath = + realPathAsFragment.startsWith(execRoot) + ? realPathAsFragment.relativeTo(execRoot) + : realPathAsFragment; + return FileArtifactValue.createForResolvedSymlink( + execRootRelativeRealPath, value, actualDigest); } /** diff --git a/src/main/java/com/google/devtools/build/lib/worker/SandboxedWorker.java b/src/main/java/com/google/devtools/build/lib/worker/SandboxedWorker.java index 9d0509181a8b53..06453e46c6a233 100644 --- a/src/main/java/com/google/devtools/build/lib/worker/SandboxedWorker.java +++ b/src/main/java/com/google/devtools/build/lib/worker/SandboxedWorker.java @@ -181,7 +181,7 @@ protected Subprocess createProcess() throws IOException, UserExecException { // Mostly tests require network, and some blaze run commands, but no workers. LinuxSandboxCommandLineBuilder commandLineBuilder = LinuxSandboxCommandLineBuilder.commandLineBuilder( - this.hardenedSandboxOptions.sandboxBinary(), args) + this.hardenedSandboxOptions.sandboxBinary()) .setWritableFilesAndDirectories(getWritableDirs(workDir)) .setTmpfsDirectories(ImmutableSet.copyOf(this.hardenedSandboxOptions.tmpfsPath())) .setPersistentProcess(true) @@ -202,7 +202,7 @@ protected Subprocess createProcess() throws IOException, UserExecException { commandLineBuilder.setUseFakeUsername(true); } - args = commandLineBuilder.build(); + args = commandLineBuilder.buildForCommand(args); } return createProcessBuilder(args).start(); } diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java index 6718b7e709a561..c21eb8e6bdc6b2 100644 --- a/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java @@ -188,6 +188,7 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context) helpers.processInputFiles( context.getInputMapping( PathFragment.EMPTY_FRAGMENT, /* willAccessRepeatedly= */ true), + context.getInputMetadataProvider(), execRoot, execRoot, packageRoots, diff --git a/src/test/java/com/google/devtools/build/lib/sandbox/BUILD b/src/test/java/com/google/devtools/build/lib/sandbox/BUILD index 74873ef72d3311..671997db568c43 100644 --- a/src/test/java/com/google/devtools/build/lib/sandbox/BUILD +++ b/src/test/java/com/google/devtools/build/lib/sandbox/BUILD @@ -59,12 +59,14 @@ java_test( deps = [ "//src/main/java/com/google/devtools/build/lib/actions", "//src/main/java/com/google/devtools/build/lib/actions:artifacts", + "//src/main/java/com/google/devtools/build/lib/actions:file_metadata", "//src/main/java/com/google/devtools/build/lib/exec:bin_tools", "//src/main/java/com/google/devtools/build/lib/exec:tree_deleter", "//src/main/java/com/google/devtools/build/lib/sandbox:sandbox_helpers", "//src/main/java/com/google/devtools/build/lib/sandbox:sandbox_options", "//src/main/java/com/google/devtools/build/lib/sandbox:sandboxed_spawns", "//src/main/java/com/google/devtools/build/lib/sandbox:tree_deleter", + "//src/main/java/com/google/devtools/build/lib/skyframe:tree_artifact_value", "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", "//src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs", diff --git a/src/test/java/com/google/devtools/build/lib/sandbox/LinuxSandboxCommandLineBuilderTest.java b/src/test/java/com/google/devtools/build/lib/sandbox/LinuxSandboxCommandLineBuilderTest.java index a8caa07f8836d2..9f8df6d9c30997 100644 --- a/src/test/java/com/google/devtools/build/lib/sandbox/LinuxSandboxCommandLineBuilderTest.java +++ b/src/test/java/com/google/devtools/build/lib/sandbox/LinuxSandboxCommandLineBuilderTest.java @@ -53,11 +53,10 @@ public void testLinuxSandboxCommandLineBuilder_fakeRootAndFakeUsernameAreExclusi assertThrows( IllegalStateException.class, () -> - LinuxSandboxCommandLineBuilder.commandLineBuilder( - linuxSandboxPath, commandArguments) + LinuxSandboxCommandLineBuilder.commandLineBuilder(linuxSandboxPath) .setUseFakeRoot(true) .setUseFakeUsername(true) - .build()); + .buildForCommand(commandArguments)); assertThat(e).hasMessageThat().contains("exclusive"); } @@ -75,8 +74,8 @@ public void testLinuxSandboxCommandLineBuilder_buildsWithoutOptionalArguments() .build(); List commandLine = - LinuxSandboxCommandLineBuilder.commandLineBuilder(linuxSandboxPath, commandArguments) - .build(); + LinuxSandboxCommandLineBuilder.commandLineBuilder(linuxSandboxPath) + .buildForCommand(commandArguments); assertThat(commandLine).containsExactlyElementsIn(expectedCommandLine).inOrder(); } @@ -163,7 +162,7 @@ public void testLinuxSandboxCommandLineBuilder_buildsWithOptionalArguments() { .build(); List commandLine = - LinuxSandboxCommandLineBuilder.commandLineBuilder(linuxSandboxPath, commandArguments) + LinuxSandboxCommandLineBuilder.commandLineBuilder(linuxSandboxPath) .setWorkingDirectory(workingDirectory) .setStdoutPath(stdoutPath) .setStderrPath(stderrPath) @@ -180,7 +179,7 @@ public void testLinuxSandboxCommandLineBuilder_buildsWithOptionalArguments() { .setSandboxDebugPath(sandboxDebugPath.getPathString()) .setPersistentProcess(true) .setCgroupsDir(cgroupsDir) - .build(); + .buildForCommand(commandArguments); assertThat(commandLine).containsExactlyElementsIn(expectedCommandLine).inOrder(); } diff --git a/src/test/java/com/google/devtools/build/lib/sandbox/SandboxHelpersTest.java b/src/test/java/com/google/devtools/build/lib/sandbox/SandboxHelpersTest.java index 8df844739de17e..78fe5c244ee282 100644 --- a/src/test/java/com/google/devtools/build/lib/sandbox/SandboxHelpersTest.java +++ b/src/test/java/com/google/devtools/build/lib/sandbox/SandboxHelpersTest.java @@ -24,17 +24,23 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.devtools.build.lib.actions.ActionInput; +import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact; +import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact; import com.google.devtools.build.lib.actions.ArtifactRoot; import com.google.devtools.build.lib.actions.CommandLines.ParamFileActionInput; +import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType; import com.google.devtools.build.lib.actions.PathMapper; import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.actions.cache.VirtualActionInput; import com.google.devtools.build.lib.actions.util.ActionsTestUtil; import com.google.devtools.build.lib.exec.BinTools; +import com.google.devtools.build.lib.exec.util.FakeActionInputFileCache; import com.google.devtools.build.lib.exec.util.SpawnBuilder; import com.google.devtools.build.lib.sandbox.SandboxHelpers.SandboxInputs; import com.google.devtools.build.lib.sandbox.SandboxHelpers.SandboxOutputs; +import com.google.devtools.build.lib.skyframe.TreeArtifactValue; import com.google.devtools.build.lib.testutil.Scratch; import com.google.devtools.build.lib.testutil.TestUtils; import com.google.devtools.build.lib.vfs.DigestHashFunction; @@ -68,6 +74,7 @@ /** Tests for {@link SandboxHelpers}. */ @RunWith(JUnit4.class) public class SandboxHelpersTest { + private static final byte[] FAKE_DIGEST = new byte[] {1}; private final Scratch scratch = new Scratch(); private Path execRootPath; @@ -94,6 +101,79 @@ private RootedPath execRootedPath(String execPath) { return RootedPath.toRootedPath(execRoot, PathFragment.create(execPath)); } + @Test + public void processInputFiles_resolvesMaterializationPath_fileArtifact() throws Exception { + ArtifactRoot outputRoot = + ArtifactRoot.asDerivedRoot(execRootPath, ArtifactRoot.RootType.Output, "outputs"); + Path sandboxSourceRoot = scratch.dir("/faketmp/sandbox-source-roots"); + + Artifact input = ActionsTestUtil.createArtifact(outputRoot, "a/a"); + FileArtifactValue symlinkTargetMetadata = + FileArtifactValue.createForNormalFile(FAKE_DIGEST, null, 0L); + FileArtifactValue inputMetadata = + FileArtifactValue.createForResolvedSymlink( + PathFragment.create("b/b"), symlinkTargetMetadata, FAKE_DIGEST); + + FakeActionInputFileCache inputMetadataProvider = new FakeActionInputFileCache(); + inputMetadataProvider.put(input, inputMetadata); + + SandboxHelpers sandboxHelpers = new SandboxHelpers(); + SandboxInputs inputs = + sandboxHelpers.processInputFiles( + inputMap(input), + inputMetadataProvider, + execRootPath, + execRootPath, + ImmutableList.of(), + sandboxSourceRoot); + + assertThat(inputs.getFiles()) + .containsEntry( + input.getExecPath(), RootedPath.toRootedPath(execRoot, PathFragment.create("b/b"))); + } + + @Test + public void processInputFiles_resolvesMaterializationPath_treeArtifact() throws Exception { + ArtifactRoot outputRoot = + ArtifactRoot.asDerivedRoot(execRootPath, ArtifactRoot.RootType.Output, "outputs"); + Path sandboxSourceRoot = scratch.dir("/faketmp/sandbox-source-roots"); + SpecialArtifact parent = + ActionsTestUtil.createTreeArtifactWithGeneratingAction( + outputRoot, "bin/config/other_dir/subdir"); + + TreeFileArtifact childA = TreeFileArtifact.createTreeOutput(parent, "a/a"); + TreeFileArtifact childB = TreeFileArtifact.createTreeOutput(parent, "b/b"); + FileArtifactValue childMetadata = FileArtifactValue.createForNormalFile(FAKE_DIGEST, null, 0L); + TreeArtifactValue parentMetadata = + TreeArtifactValue.newBuilder(parent) + .putChild(childA, childMetadata) + .putChild(childB, childMetadata) + .setMaterializationExecPath(PathFragment.create("materialized")) + .build(); + + FakeActionInputFileCache inputMetadataProvider = new FakeActionInputFileCache(); + inputMetadataProvider.put(parent, parentMetadata.getMetadata()); + + SandboxHelpers sandboxHelpers = new SandboxHelpers(); + SandboxInputs inputs = + sandboxHelpers.processInputFiles( + inputMap(childA, childB), + inputMetadataProvider, + execRootPath, + execRootPath, + ImmutableList.of(), + sandboxSourceRoot); + + assertThat(inputs.getFiles()) + .containsEntry( + childA.getExecPath(), + RootedPath.toRootedPath(execRoot, PathFragment.create("materialized/a/a"))); + assertThat(inputs.getFiles()) + .containsEntry( + childB.getExecPath(), + RootedPath.toRootedPath(execRoot, PathFragment.create("materialized/b/b"))); + } + @Test public void processInputFiles_materializesParamFile() throws Exception { SandboxHelpers sandboxHelpers = new SandboxHelpers(); @@ -106,7 +186,12 @@ public void processInputFiles_materializesParamFile() throws Exception { SandboxInputs inputs = sandboxHelpers.processInputFiles( - inputMap(paramFile), execRootPath, execRootPath, ImmutableList.of(), null); + inputMap(paramFile), + new FakeActionInputFileCache(), + execRootPath, + execRootPath, + ImmutableList.of(), + null); assertThat(inputs.getFiles()) .containsExactly(PathFragment.create("paramFile"), execRootedPath("paramFile")); @@ -127,7 +212,12 @@ public void processInputFiles_materializesBinToolsFile() throws Exception { SandboxInputs inputs = sandboxHelpers.processInputFiles( - inputMap(tool), execRootPath, execRootPath, ImmutableList.of(), null); + inputMap(tool), + new FakeActionInputFileCache(), + execRootPath, + execRootPath, + ImmutableList.of(), + null); assertThat(inputs.getFiles()) .containsExactly(PathFragment.create("_bin/say_hello"), execRootedPath("_bin/say_hello")); @@ -173,7 +263,12 @@ protected void setExecutable(PathFragment path, boolean executable) throws IOExc try { var unused = sandboxHelpers.processInputFiles( - inputMap(input), customExecRoot, customExecRoot, ImmutableList.of(), null); + inputMap(input), + new FakeActionInputFileCache(), + customExecRoot, + customExecRoot, + ImmutableList.of(), + null); finishProcessingSemaphore.release(); } catch (IOException | InterruptedException e) { throw new IllegalArgumentException(e); @@ -181,7 +276,12 @@ protected void setExecutable(PathFragment path, boolean executable) throws IOExc }); var unused = sandboxHelpers.processInputFiles( - inputMap(input), customExecRoot, customExecRoot, ImmutableList.of(), null); + inputMap(input), + new FakeActionInputFileCache(), + customExecRoot, + customExecRoot, + ImmutableList.of(), + null); finishProcessingSemaphore.release(); future.get(); diff --git a/src/test/java/com/google/devtools/build/lib/sandbox/SymlinkedSandboxedSpawnTest.java b/src/test/java/com/google/devtools/build/lib/sandbox/SymlinkedSandboxedSpawnTest.java index 1dd8e66f6640a9..dfc659bcb6d3ec 100644 --- a/src/test/java/com/google/devtools/build/lib/sandbox/SymlinkedSandboxedSpawnTest.java +++ b/src/test/java/com/google/devtools/build/lib/sandbox/SymlinkedSandboxedSpawnTest.java @@ -83,6 +83,7 @@ public void createFileSystem() throws Exception { new SynchronousTreeDeleter(), /* sandboxDebugPath= */ null, /* statisticsPath= */ null, + /* interactiveDebugArguments= */ null, "SomeMnemonic"); symlinkedExecRoot.createFileSystem(); @@ -114,6 +115,7 @@ public void copyOutputs() throws Exception { new SynchronousTreeDeleter(), /* sandboxDebugPath= */ null, /* statisticsPath= */ null, + /* interactiveDebugArguments= */ null, "SomeMnemonic"); symlinkedExecRoot.createFileSystem(); diff --git a/src/test/java/com/google/devtools/build/lib/shell/CommandUsingLinuxSandboxTest.java b/src/test/java/com/google/devtools/build/lib/shell/CommandUsingLinuxSandboxTest.java index 0a36c62c3d615f..f4278c5e6406e5 100644 --- a/src/test/java/com/google/devtools/build/lib/shell/CommandUsingLinuxSandboxTest.java +++ b/src/test/java/com/google/devtools/build/lib/shell/CommandUsingLinuxSandboxTest.java @@ -76,8 +76,8 @@ public void testLinuxSandboxedCommand_echo() throws Exception { ImmutableList commandArguments = ImmutableList.of("echo", "sleep furiously"); List fullCommandLine = - LinuxSandboxCommandLineBuilder.commandLineBuilder(getLinuxSandboxPath(), commandArguments) - .build(); + LinuxSandboxCommandLineBuilder.commandLineBuilder(getLinuxSandboxPath()) + .buildForCommand(commandArguments); Command command = new Command(fullCommandLine.toArray(new String[0])); CommandResult commandResult = command.execute(); @@ -98,9 +98,9 @@ private void checkLinuxSandboxStatistics(Duration userTimeToSpend, Duration syst Path statisticsFilePath = outputDir.getRelative("stats.out"); List fullCommandLine = - LinuxSandboxCommandLineBuilder.commandLineBuilder(getLinuxSandboxPath(), commandArguments) + LinuxSandboxCommandLineBuilder.commandLineBuilder(getLinuxSandboxPath()) .setStatisticsPath(statisticsFilePath) - .build(); + .buildForCommand(commandArguments); ExecutionStatisticsTestUtil.executeCommandAndCheckStatisticsAboutCpuTimeSpent( userTimeToSpend, systemTimeToSpend, fullCommandLine, statisticsFilePath); diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStoreTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStoreTest.java index 430e809fb50f31..afcf8281bb36b8 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStoreTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStoreTest.java @@ -82,10 +82,6 @@ private enum TreeComposition { FULLY_LOCAL, FULLY_REMOTE, MIXED; - - boolean isPartiallyRemote() { - return this == FULLY_REMOTE || this == MIXED; - } } private final Map chmodCalls = Maps.newConcurrentMap(); @@ -422,11 +418,10 @@ public void fileArtifactMaterializedAsSymlink( .getPath(outputArtifact.getPath().getPathString()) .createSymbolicLink(targetArtifact.getPath().asFragment()); - PathFragment expectedMaterializationExecPath = null; - if (location == FileLocation.REMOTE) { - expectedMaterializationExecPath = - preexistingPath != null ? preexistingPath : targetArtifact.getExecPath(); - } + PathFragment expectedMaterializationExecPath = + location == FileLocation.REMOTE && preexistingPath != null + ? preexistingPath + : targetArtifact.getExecPath(); assertThat(store.getOutputMetadata(outputArtifact)) .isEqualTo(createFileMetadataForSymlinkTest(location, expectedMaterializationExecPath)); @@ -436,7 +431,12 @@ private FileArtifactValue createFileMetadataForSymlinkTest( FileLocation location, @Nullable PathFragment materializationExecPath) { switch (location) { case LOCAL: - return FileArtifactValue.createForNormalFile(new byte[] {1, 2, 3}, /* proxy= */ null, 10); + FileArtifactValue target = + FileArtifactValue.createForNormalFile(new byte[] {1, 2, 3}, /* proxy= */ null, 10); + return materializationExecPath == null + ? target + : FileArtifactValue.createForResolvedSymlink( + materializationExecPath, target, target.getDigest()); case REMOTE: return RemoteFileArtifactValue.create( new byte[] {1, 2, 3}, 10, 1, -1, materializationExecPath); @@ -481,11 +481,8 @@ public void treeArtifactMaterializedAsSymlink( .getPath(outputArtifact.getPath().getPathString()) .createSymbolicLink(targetArtifact.getPath().asFragment()); - PathFragment expectedMaterializationExecPath = null; - if (composition.isPartiallyRemote()) { - expectedMaterializationExecPath = - preexistingPath != null ? preexistingPath : targetArtifact.getExecPath(); - } + PathFragment expectedMaterializationExecPath = + preexistingPath != null ? preexistingPath : targetArtifact.getExecPath(); assertThat(store.getTreeArtifactValue(outputArtifact)) .isEqualTo( @@ -814,7 +811,7 @@ public void fileArtifactValueForSymlink_readFromCache() throws Exception { var symlinkMetadata = store.getOutputMetadata(symlink); - assertThat(symlinkMetadata).isEqualTo(targetMetadata); + assertThat(symlinkMetadata.getDigest()).isEqualTo(targetMetadata.getDigest()); assertThat(DigestUtils.getCacheStats().hitCount()).isEqualTo(1); } } diff --git a/src/test/shell/bazel/bazel_sandboxing_test.sh b/src/test/shell/bazel/bazel_sandboxing_test.sh index 3e65184291777a..86fa7ad1807cd4 100755 --- a/src/test/shell/bazel/bazel_sandboxing_test.sh +++ b/src/test/shell/bazel/bazel_sandboxing_test.sh @@ -334,6 +334,95 @@ EOF assert_contains GOOD bazel-bin/pkg/gen.txt } +function test_symlink_with_output_base_under_tmp() { + if [[ "$PLATFORM" == "darwin" ]]; then + # Tests Linux-specific functionality + return 0 + fi + + create_workspace_with_default_repos WORKSPACE + + sed -i.bak '/sandbox_tmpfs_path/d' $TEST_TMPDIR/bazelrc + + mkdir -p pkg + cat > pkg/BUILD <<'EOF' +load(":r.bzl", "symlink_rule") + +genrule(name="r1", srcs=[], outs=[":r1"], cmd="echo CONTENT > $@") +symlink_rule(name="r2", input=":r1") +genrule(name="r3", srcs=[":r2"], outs=[":r3"], cmd="cp $< $@") +EOF + + cat > pkg/r.bzl <<'EOF' +def _symlink_impl(ctx): + output = ctx.actions.declare_file(ctx.label.name) + ctx.actions.symlink(output = output, target_file = ctx.file.input) + return [DefaultInfo(files = depset([output]))] + +symlink_rule = rule( + implementation = _symlink_impl, + attrs = {"input": attr.label(allow_single_file=True)}) +EOF + + local tmp_output_base=$(mktemp -d "/tmp/bazel_output_base.XXXXXXXX") + trap "chmod -R u+w $tmp_output_base && rm -fr $tmp_output_base" EXIT + + bazel --output_base="$tmp_output_base" build //pkg:r3 + assert_contains CONTENT bazel-bin/pkg/r3 + bazel --output_base="$tmp_output_base" shutdown +} + +function test_symlink_to_directory_with_output_base_under_tmp() { + if [[ "$PLATFORM" == "darwin" ]]; then + # Tests Linux-specific functionality + return 0 + fi + + create_workspace_with_default_repos WORKSPACE + + sed -i.bak '/sandbox_tmpfs_path/d' $TEST_TMPDIR/bazelrc + + mkdir -p pkg + cat > pkg/BUILD <<'EOF' +load(":r.bzl", "symlink_rule", "tree_rule") + +tree_rule(name="t1") +symlink_rule(name="t2", input=":t1") +genrule(name="t3", srcs=[":t2"], outs=[":t3"], cmd=";\n".join( + ["cat $(location :t2)/{a/a,b/b} > $@"])) +EOF + + cat > pkg/r.bzl <<'EOF' +def _symlink_impl(ctx): + output = ctx.actions.declare_directory(ctx.label.name) + ctx.actions.symlink(output = output, target_file = ctx.file.input) + return [DefaultInfo(files = depset([output]))] + +symlink_rule = rule( + implementation = _symlink_impl, + attrs = {"input": attr.label(allow_single_file=True)}) + +def _tree_impl(ctx): + output = ctx.actions.declare_directory(ctx.label.name) + ctx.actions.run_shell( + outputs = [output], + command = "export TREE=%s && mkdir $TREE/a $TREE/b && echo -n A > $TREE/a/a && echo -n B > $TREE/b/b" % output.path) + return [DefaultInfo(files = depset([output]))] + +tree_rule = rule( + implementation = _tree_impl, + attrs = {}) + +EOF + + local tmp_output_base=$(mktemp -d "/tmp/bazel_output_base.XXXXXXXX") + trap "chmod -R u+w $tmp_output_base && rm -fr $tmp_output_base" EXIT + + bazel --output_base="$tmp_output_base" build //pkg:t3 + assert_contains AB bazel-bin/pkg/t3 + bazel --output_base="$tmp_output_base" shutdown +} + # The test shouldn't fail if the environment doesn't support running it. check_sandbox_allowed || exit 0 diff --git a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/RemoteWorker.java b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/RemoteWorker.java index 54955ff5fdc6a6..7336879f246fe0 100644 --- a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/RemoteWorker.java +++ b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/RemoteWorker.java @@ -383,8 +383,8 @@ private static Path prepareSandboxRunner(FileSystem fs, RemoteWorkerOptions remo CommandResult cmdResult = null; Command cmd = new Command( - LinuxSandboxCommandLineBuilder.commandLineBuilder(sandboxPath, ImmutableList.of("true")) - .build() + LinuxSandboxCommandLineBuilder.commandLineBuilder(sandboxPath) + .buildForCommand(ImmutableList.of("true")) .toArray(new String[0]), ImmutableMap.of(), sandboxPath.getParentDirectory().getPathFile());