From eac6c09288a0f39d80b5caab06086aee2dd8689c Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Thu, 16 May 2024 07:23:25 -0700 Subject: [PATCH] [7.2.0] Preserve paths under hermetic `/tmp` in the sandbox The bind mounting scheme used with the Linux sandbox' hermetic `/tmp` feature is modified to preserve all paths as they are outside the sandbox, which removes the need to rewrite paths when staging inputs into and, crucially, moving outputs out of the sandbox. Source roots and output base paths under `/tmp` are now treated just like any user-specified bind mount under `/tmp`: They are mounted under the hermetic tmp directory with their path relativized against `/tmp` before the hermetic tmp directory is mounted as `/tmp` as the final step. There is one caveat compared to user-specified mounts: Source roots, which may themselves not lie under `/tmp`, can be symlinks to directories under `/tmp` (e.g., when they arise from a `local_repository`). To handle this situation in the common case, all parent directories of package path entries (up to direct children of `/tmp`) are mounted into the sandbox. If users use `local_repository`s with fixed target paths under `/tmp`, they will need to specify `--sandbox_add_mount_pair`. Overlayfs has been considered as an alternative to this approach, but ultimately doesn't seem to work for this use case since its `lowerpath`, which would be `/tmp`, is not allowed to have child mounts from a different user namespace (see https://unix.stackexchange.com/questions/776030/mounting-overlayfs-in-a-user-namespace-with-child-mounts). However, this is exactly the situation created by a Bazel-in-Bazel test and can also arise if the user has existing mounts under `/tmp` when using Bazel (e.g. the JetBrains toolbox on Linux uses such mounts). This replaces and mostly reverts the following commits, but keeps their tests: * https://github.com/bazelbuild/bazel/commit/bf6ebe9f7c428e15b7c4d7e86a762b7470f97d5b * https://github.com/bazelbuild/bazel/commit/fb6658c86164b81205a056a9a54975a62f2f957a * https://github.com/bazelbuild/bazel/commit/bc1d9d38bea907beea8fd82c362c9882ddf48cfd * https://github.com/bazelbuild/bazel/commit/182988347ef02399bc8fbc31a0ad972f0ea72210 * https://github.com/bazelbuild/bazel/commit/70691f2b76be0b9530e49c3df18356925843b465 * https://github.com/bazelbuild/bazel/commit/a556969326107423738fbf8a80bad9aaf9935578 * https://github.com/bazelbuild/bazel/commit/8e32f44a279c5c4e9c24fb84a08276ffe4fe90c0 (had its test lost in an incorrect merge conflict resolution, this PR adds it back) Fixes #20533 Work towards #20753 Fixes #21215 Fixes #22117 Fixes #22226 Fixes #22290 RELNOTES: Paths in the Linux sandbox are now again identical to those outside the sandbox, even with `--incompatible_sandbox_hermetic_tmp`. Closes #22001. PiperOrigin-RevId: 634381503 Change-Id: I9f7f3948c705be120c55c9b0c51204e5bea45f61 --- .../build/lib/actions/FileArtifactValue.java | 47 +-- .../AbstractContainerizingSandboxedSpawn.java | 5 +- .../sandbox/AbstractSandboxSpawnRunner.java | 8 +- .../sandbox/DarwinSandboxedSpawnRunner.java | 12 +- .../sandbox/DockerSandboxedSpawnRunner.java | 9 +- .../LinuxSandboxCommandLineBuilder.java | 29 +- .../sandbox/LinuxSandboxedSpawnRunner.java | 226 +++++-------- .../ProcessWrapperSandboxedSpawnRunner.java | 12 +- .../build/lib/sandbox/SandboxHelpers.java | 225 ++---------- .../build/lib/sandbox/WindowsSandboxUtil.java | 9 +- .../sandbox/WindowsSandboxedSpawnRunner.java | 12 +- .../skyframe/ActionOutputMetadataStore.java | 68 ++-- .../build/lib/worker/SandboxedWorker.java | 7 +- .../build/lib/worker/WorkerExecRoot.java | 5 +- .../build/lib/worker/WorkerModule.java | 1 - .../build/lib/worker/WorkerSpawnRunner.java | 19 +- ...tractContainerizingSandboxedSpawnTest.java | 6 +- .../LinuxSandboxCommandLineBuilderTest.java | 14 +- .../build/lib/sandbox/SandboxHelpersTest.java | 179 ++-------- .../sandbox/SymlinkedSandboxedSpawnTest.java | 14 +- .../ActionOutputMetadataStoreTest.java | 29 +- .../build/lib/worker/SandboxHelper.java | 27 +- .../lib/worker/WorkerSpawnRunnerTest.java | 49 +-- .../lib/worker/WorkerSpawnStrategyTest.java | 11 +- src/test/shell/bazel/bazel_sandboxing_test.sh | 320 ++++++++++++++++-- 25 files changed, 549 insertions(+), 794 deletions(-) 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 0f900b14b8bc72..d88614b7e953d6 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 @@ -170,19 +170,11 @@ protected boolean couldBeModifiedByMetadata(FileArtifactValue lastKnown) { /** * Optional materialization path. * - *

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(). + *

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. */ public Optional getMaterializationExecPath() { return Optional.empty(); @@ -223,12 +215,6 @@ 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()); @@ -459,25 +445,7 @@ public String toString() { } } - 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 static final class RegularFileArtifactValue extends FileArtifactValue { private final byte[] digest; @Nullable private final FileContentsProxy proxy; private final long size; @@ -500,8 +468,7 @@ public boolean equals(Object o) { RegularFileArtifactValue that = (RegularFileArtifactValue) o; return Arrays.equals(digest, that.digest) && Objects.equals(proxy, that.proxy) - && size == that.size - && Objects.equals(getMaterializationExecPath(), that.getMaterializationExecPath()); + && size == that.size; } @Override diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/AbstractContainerizingSandboxedSpawn.java b/src/main/java/com/google/devtools/build/lib/sandbox/AbstractContainerizingSandboxedSpawn.java index 19f1bbc01b334f..6825942ac7ac08 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/AbstractContainerizingSandboxedSpawn.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/AbstractContainerizingSandboxedSpawn.java @@ -27,7 +27,6 @@ import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; -import com.google.devtools.build.lib.vfs.RootedPath; import java.io.IOException; import java.util.LinkedHashSet; import java.util.Set; @@ -163,9 +162,9 @@ void createInputs(Iterable inputsToCreate, SandboxInputs inputs) } Path key = sandboxExecRoot.getRelative(fragment); if (inputs.getFiles().containsKey(fragment)) { - RootedPath fileDest = inputs.getFiles().get(fragment); + Path fileDest = inputs.getFiles().get(fragment); if (fileDest != null) { - copyFile(fileDest.asPath(), key); + copyFile(fileDest, key); } else { FileSystemUtils.createEmptyFile(key); } 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 61868714abc722..cb6fcfed42000f 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 @@ -360,13 +360,11 @@ private boolean wasTimeout(Duration timeout, Duration wallTime) { /** * Gets the list of directories that the spawn will assume to be writable. * - * @param sandboxExecRoot the exec root of the sandbox from the point of view of the Bazel process - * @param withinSandboxExecRoot the exec root from the point of view of the sandboxed processes + * @param sandboxExecRoot the exec root of the sandbox * @param env the environment of the sandboxed processes * @throws IOException because we might resolve symlinks, which throws {@link IOException}. */ - protected ImmutableSet getWritableDirs( - Path sandboxExecRoot, Path withinSandboxExecRoot, Map env) + protected ImmutableSet getWritableDirs(Path sandboxExecRoot, Map env) throws IOException { // We have to make the TEST_TMPDIR directory writable if it is specified. ImmutableSet.Builder writablePaths = ImmutableSet.builder(); @@ -374,7 +372,7 @@ protected ImmutableSet getWritableDirs( // On Windows, sandboxExecRoot is actually the main execroot. We will specify // exactly which output path is writable. if (OS.getCurrent() != OS.WINDOWS) { - writablePaths.add(withinSandboxExecRoot); + writablePaths.add(execRoot); } String testTmpdir = env.get("TEST_TMPDIR"); 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 3ad9c32531e0f0..932423ff1c80bb 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 @@ -37,7 +37,6 @@ import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; -import com.google.devtools.build.lib.vfs.Root; import java.io.BufferedWriter; import java.io.File; import java.io.IOException; @@ -103,7 +102,6 @@ private static boolean computeIsSupported() throws InterruptedException { private final SandboxHelpers helpers; private final Path execRoot; - private final ImmutableList packageRoots; private final boolean allowNetwork; private final ProcessWrapper processWrapper; private final Path sandboxBase; @@ -134,7 +132,6 @@ private static boolean computeIsSupported() throws InterruptedException { super(cmdEnv); this.helpers = helpers; this.execRoot = cmdEnv.getExecRoot(); - this.packageRoots = cmdEnv.getPackageLocator().getPathEntries(); this.allowNetwork = helpers.shouldAllowNetwork(cmdEnv.getOptions()); this.alwaysWritableDirs = getAlwaysWritableDirs(cmdEnv.getRuntime().getFileSystem()); this.processWrapper = ProcessWrapper.fromCommandEnvironment(cmdEnv); @@ -221,18 +218,13 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), binTools, "/tmp"); final HashSet writableDirs = new HashSet<>(alwaysWritableDirs); - ImmutableSet extraWritableDirs = - getWritableDirs(sandboxExecRoot, sandboxExecRoot, environment); + ImmutableSet extraWritableDirs = getWritableDirs(sandboxExecRoot, environment); writableDirs.addAll(extraWritableDirs); SandboxInputs inputs = helpers.processInputFiles( context.getInputMapping(PathFragment.EMPTY_FRAGMENT, /* willAccessRepeatedly= */ true), - context.getInputMetadataProvider(), - execRoot, - execRoot, - packageRoots, - null); + execRoot); SandboxOutputs outputs = helpers.getOutputs(spawn); final Path sandboxConfigPath = sandboxPath.getRelative("sandbox.sb"); 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 971b05cd11b99b..9e85cbcbd87b8c 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 @@ -45,7 +45,6 @@ import com.google.devtools.build.lib.util.ProcessUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; -import com.google.devtools.build.lib.vfs.Root; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.IOException; @@ -143,7 +142,6 @@ public static boolean isSupported(CommandEnvironment cmdEnv, Path dockerClient) private final SandboxHelpers helpers; private final Path execRoot; - private final ImmutableList packageRoots; private final boolean allowNetwork; private final Path dockerClient; private final ProcessWrapper processWrapper; @@ -181,7 +179,6 @@ public static boolean isSupported(CommandEnvironment cmdEnv, Path dockerClient) super(cmdEnv); this.helpers = helpers; this.execRoot = cmdEnv.getExecRoot(); - this.packageRoots = cmdEnv.getPackageLocator().getPathEntries(); this.allowNetwork = helpers.shouldAllowNetwork(cmdEnv.getOptions()); this.dockerClient = dockerClient; this.processWrapper = ProcessWrapper.fromCommandEnvironment(cmdEnv); @@ -226,11 +223,7 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context SandboxInputs inputs = helpers.processInputFiles( context.getInputMapping(PathFragment.EMPTY_FRAGMENT, /* willAccessRepeatedly= */ true), - context.getInputMetadataProvider(), - execRoot, - execRoot, - packageRoots, - null); + execRoot); SandboxOutputs outputs = helpers.getOutputs(spawn); Duration timeout = context.getTimeout(); 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 967c08b5d6ce59..ac9d0ddcd9f688 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 @@ -17,9 +17,9 @@ import static com.google.devtools.build.lib.sandbox.LinuxSandboxCommandLineBuilder.NetworkNamespace.NETNS; import static com.google.devtools.build.lib.sandbox.LinuxSandboxCommandLineBuilder.NetworkNamespace.NETNS_WITH_LOOPBACK; -import com.google.auto.value.AutoValue; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.actions.ExecutionRequirements; import com.google.devtools.build.lib.vfs.Path; @@ -36,20 +36,6 @@ * linux-sandbox} tool. */ public class LinuxSandboxCommandLineBuilder { - /** A bind mount that needs to be present when the sandboxed command runs. */ - @AutoValue - public abstract static class BindMount { - public static BindMount of(Path mountPoint, Path source) { - return new AutoValue_LinuxSandboxCommandLineBuilder_BindMount(mountPoint, source); - } - - /** "target" in mount(2) */ - public abstract Path getMountPoint(); - - /** "source" in mount(2) */ - public abstract Path getContent(); - } - private final Path linuxSandboxPath; private Path hermeticSandboxPath; private Path workingDirectory; @@ -60,7 +46,7 @@ public static BindMount of(Path mountPoint, Path source) { private Path stderrPath; private Set writableFilesAndDirectories = ImmutableSet.of(); private ImmutableSet tmpfsDirectories = ImmutableSet.of(); - private List bindMounts = ImmutableList.of(); + private Map bindMounts = ImmutableMap.of(); private Path statisticsPath; private boolean useFakeHostname = false; private NetworkNamespace createNetworkNamespace = NetworkNamespace.NO_NETNS; @@ -164,7 +150,7 @@ public LinuxSandboxCommandLineBuilder setTmpfsDirectories( * if any. */ @CanIgnoreReturnValue - public LinuxSandboxCommandLineBuilder setBindMounts(List bindMounts) { + public LinuxSandboxCommandLineBuilder setBindMounts(Map bindMounts) { this.bindMounts = bindMounts; return this; } @@ -273,11 +259,12 @@ public ImmutableList buildForCommand(List commandArguments) { for (PathFragment tmpfsPath : tmpfsDirectories) { commandLineBuilder.add("-e", tmpfsPath.getPathString()); } - for (BindMount bindMount : bindMounts) { - commandLineBuilder.add("-M", bindMount.getContent().getPathString()); + for (Path bindMountTarget : bindMounts.keySet()) { + Path bindMountSource = bindMounts.get(bindMountTarget); + commandLineBuilder.add("-M", bindMountSource.getPathString()); // The file is mounted in a custom location inside the sandbox. - if (!bindMount.getContent().equals(bindMount.getMountPoint())) { - commandLineBuilder.add("-m", bindMount.getMountPoint().getPathString()); + if (!bindMountSource.equals(bindMountTarget)) { + commandLineBuilder.add("-m", bindMountTarget.getPathString()); } } if (statisticsPath != null) { 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 eadf84125162bf..5d6274f1625e92 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 @@ -14,7 +14,6 @@ package com.google.devtools.build.lib.sandbox; -import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.collect.ImmutableSet.toImmutableSet; import static com.google.devtools.build.lib.sandbox.LinuxSandboxCommandLineBuilder.NetworkNamespace.NETNS_WITH_LOOPBACK; import static com.google.devtools.build.lib.sandbox.LinuxSandboxCommandLineBuilder.NetworkNamespace.NO_NETNS; @@ -22,6 +21,8 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; +import com.google.common.collect.Maps; import com.google.common.io.ByteStreams; import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.ExecException; @@ -33,7 +34,6 @@ import com.google.devtools.build.lib.actions.Spawns; import com.google.devtools.build.lib.actions.UserExecException; import com.google.devtools.build.lib.actions.cache.VirtualActionInput; -import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.Reporter; import com.google.devtools.build.lib.exec.TreeDeleter; @@ -43,7 +43,6 @@ import com.google.devtools.build.lib.profiler.Profiler; import com.google.devtools.build.lib.profiler.SilentCloseable; import com.google.devtools.build.lib.runtime.CommandEnvironment; -import com.google.devtools.build.lib.sandbox.LinuxSandboxCommandLineBuilder.BindMount; import com.google.devtools.build.lib.sandbox.SandboxHelpers.SandboxInputs; import com.google.devtools.build.lib.sandbox.SandboxHelpers.SandboxOutputs; import com.google.devtools.build.lib.shell.Command; @@ -65,16 +64,11 @@ import java.util.TreeMap; import java.util.TreeSet; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.stream.Stream; import javax.annotation.Nullable; /** Spawn runner that uses linux sandboxing APIs to execute a local subprocess. */ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { - private static final PathFragment SLASH_TMP = PathFragment.create("/tmp"); - private static final PathFragment BAZEL_EXECROOT = PathFragment.create("bazel-execroot"); - private static final PathFragment BAZEL_WORKING_DIRECTORY = - PathFragment.create("bazel-working-directory"); - private static final PathFragment BAZEL_SOURCE_ROOTS = PathFragment.create("bazel-source-roots"); - // Since checking if sandbox is supported is expensive, we remember what we've checked. private static final Map isSupportedMap = new HashMap<>(); @@ -127,7 +121,6 @@ private static boolean computeIsSupported(CommandEnvironment cmdEnv, Path linuxS private final SandboxHelpers helpers; private final FileSystem fileSystem; - private final BlazeDirectories blazeDirs; private final Path execRoot; private final boolean allowNetwork; private final Path linuxSandbox; @@ -138,7 +131,8 @@ private static boolean computeIsSupported(CommandEnvironment cmdEnv, Path linuxS private final Duration timeoutKillDelay; private final TreeDeleter treeDeleter; private final Reporter reporter; - private final ImmutableList packageRoots; + private final Path slashTmp; + private final ImmutableSet knownPathsToMountUnderHermeticTmp; private String cgroupsDir; /** @@ -162,7 +156,6 @@ private static boolean computeIsSupported(CommandEnvironment cmdEnv, Path linuxS super(cmdEnv); this.helpers = helpers; this.fileSystem = cmdEnv.getRuntime().getFileSystem(); - this.blazeDirs = cmdEnv.getDirectories(); this.execRoot = cmdEnv.getExecRoot(); this.allowNetwork = helpers.shouldAllowNetwork(cmdEnv.getOptions()); this.linuxSandbox = LinuxSandboxUtil.getLinuxSandbox(cmdEnv.getBlazeWorkspace()); @@ -173,13 +166,42 @@ private static boolean computeIsSupported(CommandEnvironment cmdEnv, Path linuxS this.localEnvProvider = new PosixLocalEnvProvider(cmdEnv.getClientEnv()); this.treeDeleter = treeDeleter; this.reporter = cmdEnv.getReporter(); - this.packageRoots = cmdEnv.getPackageLocator().getPathEntries(); + this.slashTmp = cmdEnv.getRuntime().getFileSystem().getPath("/tmp"); + this.knownPathsToMountUnderHermeticTmp = collectPathsToMountUnderHermeticTmp(cmdEnv); } - private void createDirectoryWithinSandboxTmp(Path sandboxTmp, Path withinSandboxDirectory) - throws IOException { - PathFragment withinTmp = withinSandboxDirectory.asFragment().relativeTo(SLASH_TMP); - sandboxTmp.getRelative(withinTmp).createDirectoryAndParents(); + private ImmutableSet collectPathsToMountUnderHermeticTmp(CommandEnvironment cmdEnv) { + // If any path managed or tracked by Bazel is under /tmp, it needs to be explicitly mounted + // into the sandbox when using hermetic /tmp. We attempt to collect an over-approximation of + // these paths, as the main goal of hermetic /tmp is to avoid inheriting any direct + // or well-known children of /tmp from the host. + return Stream.concat( + Stream.of(cmdEnv.getOutputBase()), + cmdEnv.getPackageLocator().getPathEntries().stream().map(Root::asPath)) + .filter(p -> p.startsWith(slashTmp)) + // For any path /tmp/dir1/dir2 we encounter, we instead mount /tmp/dir1 (first two + // path segments). This is necessary to gracefully handle an edge case: + // - A workspace contains a subdirectory (e.g. examples) that is itself a workspace. + // - The child workspace brings in the parent workspace as a local_repository with + // an up-level reference. + // - The parent workspace is checked out under /tmp. + // In this scenario, the parent workspace's external source root points to the parent + // workspace's source directory under /tmp, but this directory is neither under the + // output base nor on the package path. While it would be possible to track the + // external roots of all inputs and mount their entire symlink chain, this would be + // very invasive to do in the face of resolved symlink artifacts (and impossible with + // unresolved symlinks). + // Instead, by mounting the direct children of /tmp that are parents of the source + // roots, we attempt to cover all reasonable cases in which repositories symlink + // paths relative to themselves and workspaces are checked out into subdirectories of + // /tmp. All explicit references to paths under /tmp must be handled by the user via + // --sandbox_add_mount_pair. + .map( + p -> + p.getFileSystem() + .getPath( + p.asFragment().subFragment(0, Math.min(2, p.asFragment().segmentCount())))) + .collect(toImmutableSet()); } private boolean useHermeticTmp() { @@ -202,7 +224,14 @@ private boolean useHermeticTmp() { return false; } - if (getSandboxOptions().sandboxTmpfsPath.contains(SLASH_TMP)) { + if (knownPathsToMountUnderHermeticTmp.contains(slashTmp)) { + // /tmp as a package path entry or output base seems very unlikely to work, but the bind + // mounting logic is not prepared for it and we don't want to crash, so just disable hermetic + // tmp in this case. + return false; + } + + if (getSandboxOptions().sandboxTmpfsPath.contains(slashTmp.asFragment())) { // A tmpfs path under /tmp is as hermetic as "hermetic /tmp". return false; } @@ -213,9 +242,6 @@ private boolean useHermeticTmp() { @Override protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context) throws IOException, ForbiddenActionInputException, ExecException, InterruptedException { - // b/64689608: The execroot of the sandboxed process must end with the workspace name, just like - // the normal execroot does. - String workspaceName = execRoot.getBaseName(); // Each invocation of "exec" gets its own sandbox base. // Note that the value returned by context.getId() is only unique inside one given SpawnRunner, @@ -223,72 +249,35 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context Path sandboxPath = sandboxBase.getRelative(getName()).getRelative(Integer.toString(context.getId())); - // The exec root base and the exec root of the sandbox from the point of view of the Bazel - // process (can be different from where the exec root appears within the sandbox due to file - // system namespace shenanigans). - Path sandboxExecRootBase = sandboxPath.getRelative("execroot"); - Path sandboxExecRoot = sandboxExecRootBase.getRelative(workspaceName); - - // The directory that will be mounted as the hermetic /tmp, if any (otherwise null) - Path sandboxTmp = null; - - // These paths are paths that are visible for the processes running inside the sandbox. They - // can be different from paths from the point of view of the Bazel server because if we use - // hermetic /tmp and either the output base or a source root are under /tmp, they would be - // hidden by the newly mounted hermetic /tmp . So in that case, we make the sandboxed processes - // see the exec root, the source roots and the working directory of the action at constant - // locations under /tmp . - - // Base directory for source roots; each source root is a sequentially numbered subdirectory. - Path withinSandboxSourceRoots = null; - - // Working directory of the action; this is where the inputs (and only the inputs) of the action - // are visible. - Path withinSandboxWorkingDirectory = null; - - // The exec root. Necessary because the working directory contains symlinks to the execroot. - Path withinSandboxExecRoot = execRoot; - - boolean useHermeticTmp = useHermeticTmp(); - - if (useHermeticTmp) { - // The directory which will be mounted at /tmp in the sandbox - sandboxTmp = sandboxPath.getRelative("_hermetic_tmp"); - withinSandboxSourceRoots = fileSystem.getPath(SLASH_TMP.getRelative(BAZEL_SOURCE_ROOTS)); - withinSandboxWorkingDirectory = - fileSystem - .getPath(SLASH_TMP.getRelative(BAZEL_WORKING_DIRECTORY)) - .getRelative(workspaceName); - withinSandboxExecRoot = - fileSystem.getPath(SLASH_TMP.getRelative(BAZEL_EXECROOT)).getRelative(workspaceName); - } + // b/64689608: The execroot of the sandboxed process must end with the workspace name, just like + // the normal execroot does. + String workspaceName = execRoot.getBaseName(); + Path sandboxExecRoot = sandboxPath.getRelative("execroot").getRelative(workspaceName); + sandboxExecRoot.createDirectoryAndParents(); SandboxInputs inputs = helpers.processInputFiles( context.getInputMapping(PathFragment.EMPTY_FRAGMENT, /* willAccessRepeatedly= */ true), - context.getInputMetadataProvider(), - execRoot, - withinSandboxExecRoot, - packageRoots, - withinSandboxSourceRoots); - - sandboxExecRoot.createDirectoryAndParents(); + execRoot); - if (useHermeticTmp) { - for (Root root : inputs.getSourceRootBindMounts().keySet()) { - createDirectoryWithinSandboxTmp(sandboxTmp, root.asPath()); - } + Path sandboxTmp = null; + ImmutableSet pathsUnderTmpToMount = ImmutableSet.of(); + if (useHermeticTmp()) { + // Special paths under /tmp are treated exactly like a user mount under /tmp to ensure that + // they are visible at the same path after mounting the hermetic tmp. + pathsUnderTmpToMount = knownPathsToMountUnderHermeticTmp; - createDirectoryWithinSandboxTmp(sandboxTmp, withinSandboxExecRoot); - createDirectoryWithinSandboxTmp(sandboxTmp, withinSandboxWorkingDirectory); + // The initially empty directory that will be mounted as /tmp in the sandbox. + sandboxTmp = sandboxPath.getRelative("_hermetic_tmp"); + sandboxTmp.createDirectoryAndParents(); for (PathFragment pathFragment : getSandboxOptions().sandboxTmpfsPath) { Path path = fileSystem.getPath(pathFragment); - if (path.startsWith(SLASH_TMP)) { + if (path.startsWith(slashTmp)) { // tmpfs mount points must exist, which is usually the user's responsibility. But if the // user requests a tmpfs mount under /tmp, we have to create it under the sandbox tmp // directory. - createDirectoryWithinSandboxTmp(sandboxTmp, path); + sandboxTmp.getRelative(path.relativeTo(slashTmp)).createDirectoryAndParents(); } } } @@ -296,9 +285,7 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context SandboxOutputs outputs = helpers.getOutputs(spawn); ImmutableMap environment = localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), binTools, "/tmp"); - ImmutableSet writableDirs = - getWritableDirs( - sandboxExecRoot, useHermeticTmp ? withinSandboxExecRoot : sandboxExecRoot, environment); + ImmutableSet writableDirs = getWritableDirs(sandboxExecRoot, environment); Duration timeout = context.getTimeout(); SandboxOptions sandboxOptions = getSandboxOptions(); @@ -310,7 +297,7 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context .setWritableFilesAndDirectories(writableDirs) .setTmpfsDirectories(ImmutableSet.copyOf(getSandboxOptions().sandboxTmpfsPath)) .setBindMounts( - prepareAndGetBindMounts(blazeDirs, inputs, sandboxExecRootBase, sandboxTmp)) + prepareAndGetBindMounts(sandboxExecRoot, sandboxTmp, pathsUnderTmpToMount)) .setUseFakeHostname(getSandboxOptions().sandboxFakeHostname) .setEnablePseudoterminal(getSandboxOptions().sandboxExplicitPseudoterminal) .setCreateNetworkNamespace(createNetworkNamespace ? NETNS_WITH_LOOPBACK : NO_NETNS) @@ -332,10 +319,6 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context commandLineBuilder.setCgroupsDir(cgroupsDir); } - if (useHermeticTmp) { - commandLineBuilder.setWorkingDirectory(withinSandboxWorkingDirectory); - } - if (!timeout.isZero()) { commandLineBuilder.setTimeout(timeout); } @@ -384,11 +367,10 @@ public String getName() { } @Override - protected ImmutableSet getWritableDirs( - Path sandboxExecRoot, Path withinSandboxExecRoot, Map env) + protected ImmutableSet getWritableDirs(Path sandboxExecRoot, Map env) throws IOException { Set writableDirs = new TreeSet<>(); - writableDirs.addAll(super.getWritableDirs(sandboxExecRoot, withinSandboxExecRoot, env)); + writableDirs.addAll(super.getWritableDirs(sandboxExecRoot, env)); if (getSandboxOptions().memoryLimitMb > 0) { CgroupsInfo cgroupsInfo = CgroupsInfo.getInstance(); writableDirs.add(fileSystem.getPath(cgroupsInfo.getMountPoint().getAbsolutePath())); @@ -396,36 +378,15 @@ protected ImmutableSet getWritableDirs( FileSystem fs = sandboxExecRoot.getFileSystem(); writableDirs.add(fs.getPath("/dev/shm").resolveSymbolicLinks()); writableDirs.add(fs.getPath("/tmp")); - - if (sandboxExecRoot.equals(withinSandboxExecRoot)) { - return ImmutableSet.copyOf(writableDirs); - } - - // If a writable directory is under the sandbox exec root, transform it so that its path will - // be the one that it will be available at after processing the bind mounts (this is how the - // sandbox interprets the corresponding arguments) - // - // Notably, this is usually the case for $TEST_TMPDIR because its default value is under the - // execroot. - return writableDirs.stream() - .map( - d -> - d.startsWith(sandboxExecRoot) - ? withinSandboxExecRoot.getRelative(d.relativeTo(sandboxExecRoot)) - : d) - .collect(toImmutableSet()); + return ImmutableSet.copyOf(writableDirs); } - private ImmutableList prepareAndGetBindMounts( - BlazeDirectories blazeDirs, - SandboxInputs inputs, - Path sandboxExecRootBase, - @Nullable Path sandboxTmp) + private ImmutableMap prepareAndGetBindMounts( + Path sandboxExecRoot, @Nullable Path sandboxTmp, ImmutableSet pathsUnderTmpToMount) throws UserExecException, IOException { - Path tmpPath = fileSystem.getPath(SLASH_TMP); final SortedMap userBindMounts = new TreeMap<>(); SandboxHelpers.mountAdditionalPaths( - getSandboxOptions().sandboxAdditionalMounts, sandboxExecRootBase, userBindMounts); + getSandboxOptions().sandboxAdditionalMounts, sandboxExecRoot, userBindMounts); for (Path inaccessiblePath : getInaccessiblePaths()) { if (inaccessiblePath.isDirectory(Symlinks.NOFOLLOW)) { @@ -438,52 +399,35 @@ private ImmutableList prepareAndGetBindMounts( LinuxSandboxUtil.validateBindMounts(userBindMounts); if (sandboxTmp == null) { - return userBindMounts.entrySet().stream() - .map(e -> BindMount.of(e.getKey(), e.getValue())) - .collect(toImmutableList()); + return ImmutableMap.copyOf(userBindMounts); } SortedMap bindMounts = new TreeMap<>(); - for (var entry : userBindMounts.entrySet()) { + for (var entry : + Iterables.concat( + userBindMounts.entrySet(), Maps.asMap(pathsUnderTmpToMount, p -> p).entrySet())) { Path mountPoint = entry.getKey(); Path content = entry.getValue(); - if (mountPoint.startsWith(tmpPath)) { - // sandboxTmp should be null if /tmp is an explicit mount point since useHermeticTmp() - // returns false in that case. - if (mountPoint.equals(tmpPath)) { + if (mountPoint.startsWith(slashTmp)) { + // sandboxTmp is null if /tmp is an explicit mount point. + if (mountPoint.equals(slashTmp)) { throw new IOException( "Cannot mount /tmp explicitly with hermetic /tmp. Please file a bug at" + " https://github.com/bazelbuild/bazel/issues/new/choose."); } // We need to rewrite the mount point to be under the sandbox tmp directory, which will be // mounted onto /tmp as the final mount. - mountPoint = sandboxTmp.getRelative(mountPoint.relativeTo(tmpPath)); + mountPoint = sandboxTmp.getRelative(mountPoint.relativeTo(slashTmp)); mountPoint.createDirectoryAndParents(); } bindMounts.put(mountPoint, content); } - ImmutableList.Builder result = ImmutableList.builder(); - bindMounts.forEach((k, v) -> result.add(BindMount.of(k, v))); - - // First mount the real exec root and the empty directory created as the working dir of the - // action under $SANDBOX/_tmp - result.add(BindMount.of(sandboxTmp.getRelative(BAZEL_EXECROOT), blazeDirs.getExecRootBase())); - result.add(BindMount.of(sandboxTmp.getRelative(BAZEL_WORKING_DIRECTORY), sandboxExecRootBase)); - - // Then mount the individual package roots under $SANDBOX/_tmp/bazel-source-roots - inputs - .getSourceRootBindMounts() - .forEach( - (withinSandbox, real) -> { - PathFragment sandboxTmpSourceRoot = withinSandbox.asPath().relativeTo(tmpPath); - result.add(BindMount.of(sandboxTmp.getRelative(sandboxTmpSourceRoot), real)); - }); - - // Then mount $SANDBOX/_tmp at /tmp. At this point, even if the output base (and execroot) and - // individual source roots are under /tmp, they are accessible at /tmp/bazel-* - result.add(BindMount.of(tmpPath, sandboxTmp)); - return result.build(); + // Mount $SANDBOX/_hermetic_tmp at /tmp as the final mount. + return ImmutableMap.builder() + .putAll(bindMounts) + .put(slashTmp, sandboxTmp) + .buildOrThrow(); } @Override 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 14d2a63545a734..f19a5ea361862e 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 @@ -14,7 +14,6 @@ package com.google.devtools.build.lib.sandbox; -import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.actions.ForbiddenActionInputException; import com.google.devtools.build.lib.actions.Spawn; @@ -27,7 +26,6 @@ import com.google.devtools.build.lib.util.OS; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; -import com.google.devtools.build.lib.vfs.Root; import java.io.IOException; import java.time.Duration; @@ -41,7 +39,6 @@ public static boolean isSupported(CommandEnvironment cmdEnv) { private final SandboxHelpers helpers; private final ProcessWrapper processWrapper; private final Path execRoot; - private final ImmutableList packageRoots; private final Path sandboxBase; private final LocalEnvProvider localEnvProvider; private final TreeDeleter treeDeleter; @@ -62,7 +59,6 @@ public static boolean isSupported(CommandEnvironment cmdEnv) { this.helpers = helpers; this.processWrapper = ProcessWrapper.fromCommandEnvironment(cmdEnv); this.execRoot = cmdEnv.getExecRoot(); - this.packageRoots = cmdEnv.getPackageLocator().getPathEntries(); this.localEnvProvider = LocalEnvProvider.forCurrentOs(cmdEnv.getClientEnv()); this.sandboxBase = sandboxBase; this.treeDeleter = treeDeleter; @@ -100,11 +96,7 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context SandboxInputs inputs = helpers.processInputFiles( context.getInputMapping(PathFragment.EMPTY_FRAGMENT, /* willAccessRepeatedly= */ true), - context.getInputMetadataProvider(), - execRoot, - execRoot, - packageRoots, - null); + execRoot); SandboxOutputs outputs = helpers.getOutputs(spawn); return new SymlinkedSandboxedSpawn( @@ -114,7 +106,7 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context environment, inputs, outputs, - getWritableDirs(sandboxExecRoot, sandboxExecRoot, environment), + getWritableDirs(sandboxExecRoot, environment), treeDeleter, /* sandboxDebugPath= */ null, statisticsPath, 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 aa80db5ca280a0..52e4ea18b6d588 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,9 +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; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; @@ -30,8 +28,6 @@ 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; @@ -48,10 +44,9 @@ import com.google.devtools.build.lib.vfs.FileSystemUtils.MoveResult; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; -import com.google.devtools.build.lib.vfs.Root; -import com.google.devtools.build.lib.vfs.RootedPath; import com.google.devtools.build.lib.vfs.Symlinks; import com.google.devtools.common.options.OptionsParsingResult; +import com.google.errorprone.annotations.CanIgnoreReturnValue; import java.io.IOException; import java.util.HashMap; import java.util.HashSet; @@ -252,9 +247,9 @@ private static void cleanRecursively( */ static Optional getExpectedSymlinkDestination( PathFragment fragment, SandboxInputs inputs) { - RootedPath file = inputs.getFiles().get(fragment); + Path file = inputs.getFiles().get(fragment); if (file != null) { - return Optional.of(file.asPath().asFragment()); + return Optional.of(file.asFragment()); } return Optional.ofNullable(inputs.getSymlinks().get(fragment)); } @@ -390,31 +385,27 @@ public static void mountAdditionalPaths( /** Wrapper class for the inputs of a sandbox. */ public static final class SandboxInputs { - private final Map files; + private final Map files; private final Map virtualInputs; private final Map symlinks; - private final Map sourceRootBindMounts; private static final SandboxInputs EMPTY_INPUTS = - new SandboxInputs( - ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of()); + new SandboxInputs(ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of()); public SandboxInputs( - Map files, + Map files, Map virtualInputs, - Map symlinks, - Map sourceRootBindMounts) { + Map symlinks) { this.files = files; this.virtualInputs = virtualInputs; this.symlinks = symlinks; - this.sourceRootBindMounts = sourceRootBindMounts; } public static SandboxInputs getEmptyInputs() { return EMPTY_INPUTS; } - public Map getFiles() { + public Map getFiles() { return files; } @@ -422,10 +413,6 @@ public Map getSymlinks() { return symlinks; } - public Map getSourceRootBindMounts() { - return sourceRootBindMounts; - } - public ImmutableMap getVirtualInputDigests() { return ImmutableMap.copyOf(virtualInputs); } @@ -435,16 +422,10 @@ public ImmutableMap getVirtualInputDigests() { * included. */ public SandboxInputs limitedCopy(Set allowed) { - Map limitedFiles = Maps.filterKeys(files, allowed::contains); - Map limitedSymlinks = - Maps.filterKeys(symlinks, allowed::contains); - Set usedRoots = - new HashSet<>(Maps.transformValues(limitedFiles, RootedPath::getRoot).values()); - Map limitedSourceRoots = - Maps.filterKeys(sourceRootBindMounts, usedRoots::contains); - return new SandboxInputs( - limitedFiles, ImmutableMap.of(), limitedSymlinks, limitedSourceRoots); + Maps.filterKeys(files, allowed::contains), + ImmutableMap.of(), + Maps.filterKeys(symlinks, allowed::contains)); } @Override @@ -453,105 +434,20 @@ public String toString() { } } - /** - * Returns the appropriate {@link RootedPath} for a Fileset symlink. - * - *

Filesets are weird because sometimes exec paths of the {@link ActionInput}s in them are not - * relative, as exec paths should be, but absolute and point to under one of the package roots or - * the execroot. In order to handle this, if we find such an absolute exec path, we iterate over - * possible base directories. - * - *

The inputs to this function should be symlinks that are contained within Filesets; in - * particular, this is different from "unresolved symlinks" in that Fileset contents are regular - * files (but implemented by symlinks in the output tree) whose contents matter and unresolved - * symlinks are symlinks for which the important content is the result of {@code readlink()} - */ - private static RootedPath processFilesetSymlink( - PathFragment symlink, - Root execRootWithinSandbox, - PathFragment execRootFragment, - ImmutableList packageRoots) { - for (Root packageRoot : packageRoots) { - if (packageRoot.contains(symlink)) { - return RootedPath.toRootedPath(packageRoot, packageRoot.relativize(symlink)); - } - } - - if (symlink.startsWith(execRootFragment)) { - return RootedPath.toRootedPath(execRootWithinSandbox, symlink.relativeTo(execRootFragment)); - } - - throw new IllegalStateException( - String.format( - "absolute action input path '%s' not found under package roots", - 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. * * @param inputMap the map of action inputs and where they should be visible in the action - * @param execRootPath the exec root from the point of view of the Bazel server - * @param withinSandboxExecRootPath the exec root from within the sandbox (different from {@code - * execRootPath} because the sandbox does magic with fiile system namespaces) - * @param packageRoots the package path entries during this build - * @param sandboxSourceRoots the directory where source roots are mapped within the sandbox + * @param execRoot the exec root * @throws IOException if processing symlinks fails */ - public SandboxInputs processInputFiles( - Map inputMap, - InputMetadataProvider inputMetadataProvider, - Path execRootPath, - Path withinSandboxExecRootPath, - ImmutableList packageRoots, - Path sandboxSourceRoots) + @CanIgnoreReturnValue + public SandboxInputs processInputFiles(Map inputMap, Path execRoot) throws IOException, InterruptedException { - Root withinSandboxExecRoot = Root.fromPath(withinSandboxExecRootPath); - Root execRoot = - withinSandboxExecRootPath.equals(execRootPath) - ? withinSandboxExecRoot - : Root.fromPath(execRootPath); - Root absoluteRoot = Root.absoluteRoot(execRootPath.getFileSystem()); - - Map inputFiles = new TreeMap<>(); + 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()) { @@ -559,12 +455,11 @@ public SandboxInputs processInputFiles( } PathFragment pathFragment = e.getKey(); ActionInput actionInput = e.getValue(); - if (actionInput instanceof VirtualActionInput) { + if (actionInput instanceof VirtualActionInput input) { // TODO(larsrc): Figure out which VAIs actually require atomicity, maybe avoid it. - VirtualActionInput input = (VirtualActionInput) actionInput; byte[] digest = input.atomicallyWriteRelativeTo( - execRootPath, + execRoot, // When 2 actions try to atomically create the same virtual input, they need to have // a different suffix for the temporary file in order to avoid racy write to the // same one. @@ -578,92 +473,16 @@ public SandboxInputs processInputFiles( Path inputPath = execRoot.getRelative(actionInput.getExecPath()); inputSymlinks.put(pathFragment, inputPath.readSymbolicLink()); } else { - RootedPath inputPath; - - 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 (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(); - if (execPath.isAbsolute()) { - // This happens for ActionInputs that are part of Filesets (see the Javadoc on - // processFilesetSymlink()) - inputPath = - processFilesetSymlink( - actionInput.getExecPath(), execRoot, execRootPath.asFragment(), packageRoots); - } else { - inputPath = RootedPath.toRootedPath(execRoot, actionInput.getExecPath()); - } - } - + Path inputPath = + actionInput instanceof EmptyActionInput + ? null + : execRoot.getRelative(actionInput.getExecPath()); inputFiles.put(pathFragment, inputPath); } } - - Map sandboxRootToSourceRoot = new TreeMap<>(); - sourceRootToSandboxSourceRoot.forEach((k, v) -> sandboxRootToSourceRoot.put(v, k.asPath())); - - return new SandboxInputs(inputFiles, virtualInputs, inputSymlinks, sandboxRootToSourceRoot); + return new SandboxInputs(inputFiles, virtualInputs, inputSymlinks); } - /** 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/WindowsSandboxUtil.java b/src/main/java/com/google/devtools/build/lib/sandbox/WindowsSandboxUtil.java index 707f541e98f7e8..c582ecb3df3da5 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/WindowsSandboxUtil.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/WindowsSandboxUtil.java @@ -24,7 +24,6 @@ import com.google.devtools.build.lib.shell.SubprocessBuilder.StreamAction; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; -import com.google.devtools.build.lib.vfs.RootedPath; import com.google.errorprone.annotations.CanIgnoreReturnValue; import java.io.ByteArrayOutputStream; import java.io.File; @@ -107,7 +106,7 @@ public static class CommandLineBuilder { private Path stdoutPath; private Path stderrPath; private Set writableFilesAndDirectories = ImmutableSet.of(); - private Map readableFilesAndDirectories = new TreeMap<>(); + private Map readableFilesAndDirectories = new TreeMap<>(); private Set inaccessiblePaths = ImmutableSet.of(); private boolean useDebugMode = false; private List commandArguments = ImmutableList.of(); @@ -166,7 +165,7 @@ public CommandLineBuilder setWritableFilesAndDirectories( /** Sets the files or directories to make readable for the sandboxed process, if any. */ @CanIgnoreReturnValue public CommandLineBuilder setReadableFilesAndDirectories( - Map readableFilesAndDirectories) { + Map readableFilesAndDirectories) { this.readableFilesAndDirectories = readableFilesAndDirectories; return this; } @@ -213,8 +212,8 @@ public ImmutableList build() { for (Path writablePath : writableFilesAndDirectories) { commandLineBuilder.add("-w", writablePath.getPathString()); } - for (RootedPath readablePath : readableFilesAndDirectories.values()) { - commandLineBuilder.add("-r", readablePath.asPath().getPathString()); + for (Path readablePath : readableFilesAndDirectories.values()) { + commandLineBuilder.add("-r", readablePath.getPathString()); } for (Path writablePath : inaccessiblePaths) { commandLineBuilder.add("-b", writablePath.getPathString()); 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 505e2417850161..4c866b77a3aea9 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 @@ -15,7 +15,6 @@ package com.google.devtools.build.lib.sandbox; import com.google.common.base.Joiner; -import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.actions.ActionInput; @@ -27,7 +26,6 @@ import com.google.devtools.build.lib.sandbox.SandboxHelpers.SandboxInputs; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; -import com.google.devtools.build.lib.vfs.Root; import java.io.IOException; import java.time.Duration; @@ -36,7 +34,6 @@ final class WindowsSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { private final SandboxHelpers helpers; private final Path execRoot; - private final ImmutableList packageRoots; private final PathFragment windowsSandbox; private final LocalEnvProvider localEnvProvider; private final Duration timeoutKillDelay; @@ -57,7 +54,6 @@ final class WindowsSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { super(cmdEnv); this.helpers = helpers; this.execRoot = cmdEnv.getExecRoot(); - this.packageRoots = cmdEnv.getPackageLocator().getPathEntries(); this.windowsSandbox = windowsSandboxPath; this.timeoutKillDelay = timeoutKillDelay; this.localEnvProvider = new WindowsLocalEnvProvider(cmdEnv.getClientEnv()); @@ -76,14 +72,10 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context SandboxInputs readablePaths = helpers.processInputFiles( context.getInputMapping(PathFragment.EMPTY_FRAGMENT, /* willAccessRepeatedly= */ true), - context.getInputMetadataProvider(), - execRoot, - execRoot, - packageRoots, - null); + execRoot); ImmutableSet.Builder writablePaths = ImmutableSet.builder(); - writablePaths.addAll(getWritableDirs(execRoot, execRoot, environment)); + writablePaths.addAll(getWritableDirs(execRoot, environment)); for (ActionInput output : spawn.getOutputFiles()) { writablePaths.add(execRoot.getRelative(output.getExecPath())); } 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 35a60742ef7aa6..01ef8b835dfabe 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,15 +264,7 @@ private TreeArtifactValue constructTreeArtifactValueFromFilesystem(SpecialArtifa Path treeDir = artifactPathResolver.toPath(parent); boolean chmod = executionMode.get(); - 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); - } + FileStatus 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. @@ -329,18 +321,12 @@ private TreeArtifactValue constructTreeArtifactValueFromFilesystem(SpecialArtifa } // Same rationale as for constructFileArtifactValue. - 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); + if (anyRemote.get() && treeDir.isSymbolicLink() && stat instanceof FileStatusWithMetadata) { + FileArtifactValue metadata = ((FileStatusWithMetadata) stat).getMetadata(); + tree.setMaterializationExecPath( + metadata + .getMaterializationExecPath() + .orElse(treeDir.resolveSymbolicLinks().asFragment().relativeTo(execRoot))); } return tree.build(); @@ -512,13 +498,7 @@ private FileArtifactValue constructFileArtifactValue( return value; } - boolean isResolvedSymlink = - statAndValue.statNoFollow() != null - && statAndValue.statNoFollow().isSymbolicLink() - && statAndValue.realPath() != null - && !value.isRemote(); - - if (type.isFile() && !isResolvedSymlink && fileDigest != null) { + if (type.isFile() && fileDigest != null) { // The digest is in the file value and that is all that is needed for this file's metadata. return value; } @@ -534,30 +514,22 @@ private FileArtifactValue constructFileArtifactValue( artifactPathResolver.toPath(artifact).getLastModifiedTime()); } - byte[] actualDigest = fileDigest != null ? fileDigest : injectedDigest; - - if (actualDigest == null && type.isFile()) { + if (injectedDigest == 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(); + } - // 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); - } - - if (!isResolvedSymlink) { - return FileArtifactValue.createFromInjectedDigest(value, actualDigest); + injectedDigest = DigestUtils.manuallyComputeDigest(path); } - - PathFragment realPathAsFragment = statAndValue.realPath().asFragment(); - PathFragment execRootRelativeRealPath = - realPathAsFragment.startsWith(execRoot) - ? realPathAsFragment.relativeTo(execRoot) - : realPathAsFragment; - return FileArtifactValue.createForResolvedSymlink( - execRootRelativeRealPath, value, actualDigest); + return FileArtifactValue.createFromInjectedDigest(value, injectedDigest); } /** 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 b43846b70e2f49..befba2576a290a 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 @@ -25,7 +25,6 @@ import com.google.devtools.build.lib.exec.TreeDeleter; import com.google.devtools.build.lib.sandbox.CgroupsInfo; import com.google.devtools.build.lib.sandbox.LinuxSandboxCommandLineBuilder; -import com.google.devtools.build.lib.sandbox.LinuxSandboxCommandLineBuilder.BindMount; import com.google.devtools.build.lib.sandbox.LinuxSandboxUtil; import com.google.devtools.build.lib.sandbox.SandboxHelpers; import com.google.devtools.build.lib.sandbox.SandboxHelpers.SandboxInputs; @@ -145,12 +144,11 @@ private ImmutableSet getWritableDirs(Path sandboxExecRoot) throws IOExcept return writableDirs.build(); } - private ImmutableList getBindMounts(Path sandboxExecRoot, @Nullable Path sandboxTmp) + private SortedMap getBindMounts(Path sandboxExecRoot, @Nullable Path sandboxTmp) throws UserExecException, IOException { FileSystem fs = sandboxExecRoot.getFileSystem(); Path tmpPath = fs.getPath("/tmp"); final SortedMap bindMounts = Maps.newTreeMap(); - ImmutableList.Builder result = ImmutableList.builder(); // Mount a fresh, empty temporary directory as /tmp for each sandbox rather than reusing the // host filesystem's /tmp. Since we're in a worker, we clean this dir between requests. bindMounts.put(tmpPath, sandboxTmp); @@ -167,9 +165,8 @@ private ImmutableList getBindMounts(Path sandboxExecRoot, @Nullable P } } // TODO(larsrc): Handle hermetic tmp - bindMounts.forEach((k, v) -> result.add(BindMount.of(k, v))); LinuxSandboxUtil.validateBindMounts(bindMounts); - return result.build(); + return bindMounts; } @Override diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerExecRoot.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerExecRoot.java index d8802d06f7b66b..f45f4f47eb26c6 100644 --- a/src/main/java/com/google/devtools/build/lib/worker/WorkerExecRoot.java +++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerExecRoot.java @@ -22,7 +22,6 @@ import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; -import com.google.devtools.build.lib.vfs.RootedPath; import java.io.IOException; import java.util.LinkedHashSet; import java.util.List; @@ -87,9 +86,9 @@ static void createInputs(Iterable inputsToCreate, SandboxInputs in } Path key = dir.getRelative(fragment); if (inputs.getFiles().containsKey(fragment)) { - RootedPath fileDest = inputs.getFiles().get(fragment); + Path fileDest = inputs.getFiles().get(fragment); if (fileDest != null) { - key.createSymbolicLink(fileDest.asPath()); + key.createSymbolicLink(fileDest); } else { FileSystemUtils.createEmptyFile(key); } diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerModule.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerModule.java index 5defc2f9457c50..509907e3596e5a 100644 --- a/src/main/java/com/google/devtools/build/lib/worker/WorkerModule.java +++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerModule.java @@ -219,7 +219,6 @@ public void registerSpawnStrategies( new WorkerSpawnRunner( new SandboxHelpers(), env.getExecRoot(), - env.getPackageLocator().getPathEntries(), workerPool, env.getReporter(), localEnvProvider, 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 7adf6eaf188203..1cb92581f32c2c 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 @@ -18,7 +18,6 @@ import static java.nio.charset.StandardCharsets.UTF_8; import com.google.common.base.Stopwatch; -import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.hash.HashCode; import com.google.devtools.build.lib.actions.ActionExecutionMetadata; @@ -60,8 +59,6 @@ import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; -import com.google.devtools.build.lib.vfs.Root; -import com.google.devtools.build.lib.vfs.RootedPath; import com.google.devtools.build.lib.worker.WorkerProtocol.WorkRequest; import com.google.devtools.build.lib.worker.WorkerProtocol.WorkResponse; import com.google.protobuf.ByteString; @@ -90,7 +87,6 @@ final class WorkerSpawnRunner implements SpawnRunner { private final SandboxHelpers helpers; private final Path execRoot; - private final ImmutableList packageRoots; private final ExtendedEventHandler reporter; private final ResourceManager resourceManager; private final RunfilesTreeUpdater runfilesTreeUpdater; @@ -102,7 +98,6 @@ final class WorkerSpawnRunner implements SpawnRunner { public WorkerSpawnRunner( SandboxHelpers helpers, Path execRoot, - ImmutableList packageRoots, WorkerPool workers, ExtendedEventHandler reporter, LocalEnvProvider localEnvProvider, @@ -114,7 +109,6 @@ public WorkerSpawnRunner( Clock clock) { this.helpers = helpers; this.execRoot = execRoot; - this.packageRoots = packageRoots; this.reporter = reporter; this.resourceManager = resourceManager; this.runfilesTreeUpdater = runfilesTreeUpdater; @@ -190,11 +184,7 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context) helpers.processInputFiles( context.getInputMapping( PathFragment.EMPTY_FRAGMENT, /* willAccessRepeatedly= */ true), - context.getInputMetadataProvider(), - execRoot, - execRoot, - packageRoots, - null); + execRoot); } SandboxOutputs outputs = helpers.getOutputs(spawn); @@ -303,21 +293,20 @@ static void expandArgument(SandboxInputs inputs, String arg, WorkRequest.Builder throw new InterruptedException(); } String argValue = arg.substring(1); - RootedPath path = inputs.getFiles().get(PathFragment.create(argValue)); + Path path = inputs.getFiles().get(PathFragment.create(argValue)); if (path == null) { throw new IOException( String.format( "Failed to read @-argument '%s': file is not a declared input", argValue)); } try { - for (String line : FileSystemUtils.readLines(path.asPath(), UTF_8)) { + for (String line : FileSystemUtils.readLines(path, UTF_8)) { expandArgument(inputs, line, requestBuilder); } } catch (IOException e) { throw new IOException( String.format( - "Failed to read @-argument '%s' from file '%s'.", - argValue, path.asPath().getPathString()), + "Failed to read @-argument '%s' from file '%s'.", argValue, path.getPathString()), e); } } else { diff --git a/src/test/java/com/google/devtools/build/lib/sandbox/AbstractContainerizingSandboxedSpawnTest.java b/src/test/java/com/google/devtools/build/lib/sandbox/AbstractContainerizingSandboxedSpawnTest.java index f1904ef4202776..aa1614ed66d4ce 100644 --- a/src/test/java/com/google/devtools/build/lib/sandbox/AbstractContainerizingSandboxedSpawnTest.java +++ b/src/test/java/com/google/devtools/build/lib/sandbox/AbstractContainerizingSandboxedSpawnTest.java @@ -35,7 +35,6 @@ import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; -import com.google.devtools.build.lib.vfs.RootedPath; import com.google.devtools.build.lib.vfs.Symlinks; import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; import java.io.IOException; @@ -374,7 +373,7 @@ private static SandboxInputs createSandboxInputs( private static SandboxInputs createSandboxInputs( ImmutableList files, ImmutableMap symlinks) { - Map filesMap = Maps.newHashMapWithExpectedSize(files.size()); + Map filesMap = Maps.newHashMapWithExpectedSize(files.size()); for (String file : files) { filesMap.put(PathFragment.create(file), null); } @@ -384,8 +383,7 @@ private static SandboxInputs createSandboxInputs( symlinks.entrySet().stream() .collect( toImmutableMap( - e -> PathFragment.create(e.getKey()), e -> PathFragment.create(e.getValue()))), - ImmutableMap.of()); + e -> PathFragment.create(e.getKey()), e -> PathFragment.create(e.getValue())))); } /** Return a list of all entries under the provided directory recursively. */ 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 9f8df6d9c30997..d1790e620f1c1d 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 @@ -20,8 +20,9 @@ import static org.junit.Assert.assertThrows; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; -import com.google.devtools.build.lib.sandbox.LinuxSandboxCommandLineBuilder.BindMount; +import com.google.common.collect.ImmutableSortedMap; import com.google.devtools.build.lib.vfs.DigestHashFunction; import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.Path; @@ -125,11 +126,12 @@ public void testLinuxSandboxCommandLineBuilder_buildsWithOptionalArguments() { ImmutableSet tmpfsDirectories = ImmutableSet.of(tmpfsDir1, tmpfsDir2); - ImmutableList bindMounts = - ImmutableList.of( - BindMount.of(bindMountSameSourceAndTarget, bindMountSameSourceAndTarget), - BindMount.of(bindMountTarget1, bindMountSource1), - BindMount.of(bindMountTarget2, bindMountSource2)); + ImmutableMap bindMounts = + ImmutableSortedMap.naturalOrder() + .put(bindMountSameSourceAndTarget, bindMountSameSourceAndTarget) + .put(bindMountTarget1, bindMountSource1) + .put(bindMountTarget2, bindMountSource2) + .buildOrThrow(); String cgroupsDir = "/sys/fs/cgroups/something"; 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 78fe5c244ee282..f3cddd126bf433 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,23 +24,17 @@ 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; @@ -49,8 +43,6 @@ import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; -import com.google.devtools.build.lib.vfs.Root; -import com.google.devtools.build.lib.vfs.RootedPath; import com.google.devtools.build.lib.vfs.Symlinks; import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; import java.io.IOException; @@ -74,17 +66,14 @@ /** 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; - private Root execRoot; + private Path execRoot; @Nullable private ExecutorService executorToCleanup; @Before public void createExecRoot() throws IOException { - execRootPath = scratch.dir("/execRoot"); - execRoot = Root.fromPath(execRootPath); + execRoot = scratch.dir("/execRoot"); } @After @@ -97,83 +86,6 @@ public void shutdownExecutor() throws InterruptedException { executorToCleanup.awaitTermination(TestUtils.WAIT_TIMEOUT_SECONDS, SECONDS); } - 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(); @@ -184,22 +96,15 @@ public void processInputFiles_materializesParamFile() throws Exception { ParameterFileType.UNQUOTED, UTF_8); - SandboxInputs inputs = - sandboxHelpers.processInputFiles( - inputMap(paramFile), - new FakeActionInputFileCache(), - execRootPath, - execRootPath, - ImmutableList.of(), - null); + SandboxInputs inputs = sandboxHelpers.processInputFiles(inputMap(paramFile), execRoot); assertThat(inputs.getFiles()) - .containsExactly(PathFragment.create("paramFile"), execRootedPath("paramFile")); + .containsExactly(PathFragment.create("paramFile"), execRoot.getChild("paramFile")); assertThat(inputs.getSymlinks()).isEmpty(); - assertThat(FileSystemUtils.readLines(execRootPath.getChild("paramFile"), UTF_8)) + assertThat(FileSystemUtils.readLines(execRoot.getChild("paramFile"), UTF_8)) .containsExactly("-a", "-b") .inOrder(); - assertThat(execRootPath.getChild("paramFile").isExecutable()).isTrue(); + assertThat(execRoot.getChild("paramFile").isExecutable()).isTrue(); } @Test @@ -210,22 +115,16 @@ public void processInputFiles_materializesBinToolsFile() throws Exception { scratch.file("tool", "#!/bin/bash", "echo hello"), PathFragment.create("_bin/say_hello")); - SandboxInputs inputs = - sandboxHelpers.processInputFiles( - inputMap(tool), - new FakeActionInputFileCache(), - execRootPath, - execRootPath, - ImmutableList.of(), - null); + SandboxInputs inputs = sandboxHelpers.processInputFiles(inputMap(tool), execRoot); assertThat(inputs.getFiles()) - .containsExactly(PathFragment.create("_bin/say_hello"), execRootedPath("_bin/say_hello")); + .containsExactly( + PathFragment.create("_bin/say_hello"), execRoot.getRelative("_bin/say_hello")); assertThat(inputs.getSymlinks()).isEmpty(); - assertThat(FileSystemUtils.readLines(execRootPath.getRelative("_bin/say_hello"), UTF_8)) + assertThat(FileSystemUtils.readLines(execRoot.getRelative("_bin/say_hello"), UTF_8)) .containsExactly("#!/bin/bash", "echo hello") .inOrder(); - assertThat(execRootPath.getRelative("_bin/say_hello").isExecutable()).isTrue(); + assertThat(execRoot.getRelative("_bin/say_hello").isExecutable()).isTrue(); } /** @@ -261,27 +160,13 @@ protected void setExecutable(PathFragment path, boolean executable) throws IOExc executorToCleanup.submit( () -> { try { - var unused = - sandboxHelpers.processInputFiles( - inputMap(input), - new FakeActionInputFileCache(), - customExecRoot, - customExecRoot, - ImmutableList.of(), - null); + sandboxHelpers.processInputFiles(inputMap(input), customExecRoot); finishProcessingSemaphore.release(); } catch (IOException | InterruptedException e) { throw new IllegalArgumentException(e); } }); - var unused = - sandboxHelpers.processInputFiles( - inputMap(input), - new FakeActionInputFileCache(), - customExecRoot, - customExecRoot, - ImmutableList.of(), - null); + sandboxHelpers.processInputFiles(inputMap(input), customExecRoot); finishProcessingSemaphore.release(); future.get(); @@ -350,10 +235,8 @@ public void atomicallyWriteVirtualInput_writesArbitraryVirtualInput() throws Exc @Test public void cleanExisting_updatesDirs() throws IOException, InterruptedException { - RootedPath inputTxt = - RootedPath.toRootedPath( - Root.fromPath(scratch.getFileSystem().getPath("/")), PathFragment.create("hello.txt")); - Path rootDir = execRootPath.getParentDirectory(); + Path inputTxt = scratch.getFileSystem().getPath(PathFragment.create("/hello.txt")); + Path rootDir = execRoot.getParentDirectory(); PathFragment input1 = PathFragment.create("existing/directory/with/input1.txt"); PathFragment input2 = PathFragment.create("partial/directory/input2.txt"); PathFragment input3 = PathFragment.create("new/directory/input3.txt"); @@ -361,7 +244,6 @@ public void cleanExisting_updatesDirs() throws IOException, InterruptedException new SandboxInputs( ImmutableMap.of(input1, inputTxt, input2, inputTxt, input3, inputTxt), ImmutableMap.of(), - ImmutableMap.of(), ImmutableMap.of()); Set inputsToCreate = new LinkedHashSet<>(); LinkedHashSet dirsToCreate = new LinkedHashSet<>(); @@ -382,17 +264,17 @@ public void cleanExisting_updatesDirs() throws IOException, InterruptedException assertThat(inputsToCreate).containsExactly(input1, input2, input3); // inputdir1 exists fully - execRootPath.getRelative(inputDir1).createDirectoryAndParents(); + execRoot.getRelative(inputDir1).createDirectoryAndParents(); // inputdir2 exists partially, should be kept nonetheless. - execRootPath + execRoot .getRelative(inputDir2) .getParentDirectory() .getRelative("doomedSubdir") .createDirectoryAndParents(); // inputDir3 just doesn't exist // outputDir only exists partially - execRootPath.getRelative(outputDir).getParentDirectory().createDirectoryAndParents(); - execRootPath.getRelative("justSomeDir/thatIsDoomed").createDirectoryAndParents(); + execRoot.getRelative(outputDir).getParentDirectory().createDirectoryAndParents(); + execRoot.getRelative("justSomeDir/thatIsDoomed").createDirectoryAndParents(); // `thiswillbeafile/output` simulates a directory that was in the stashed dir but whose same // path is used later for a regular file. scratch.dir("/execRoot/thiswillbeafile/output"); @@ -403,23 +285,22 @@ public void cleanExisting_updatesDirs() throws IOException, InterruptedException new SandboxInputs( ImmutableMap.of(input1, inputTxt, input2, inputTxt, input3, inputTxt, input4, inputTxt), ImmutableMap.of(), - ImmutableMap.of(), ImmutableMap.of()); - SandboxHelpers.cleanExisting(rootDir, inputs2, inputsToCreate, dirsToCreate, execRootPath); + SandboxHelpers.cleanExisting(rootDir, inputs2, inputsToCreate, dirsToCreate, execRoot); assertThat(dirsToCreate).containsExactly(inputDir2, inputDir3, outputDir); - assertThat(execRootPath.getRelative("existing/directory/with").exists()).isTrue(); - assertThat(execRootPath.getRelative("partial").exists()).isTrue(); - assertThat(execRootPath.getRelative("partial/doomedSubdir").exists()).isFalse(); - assertThat(execRootPath.getRelative("partial/directory").exists()).isFalse(); - assertThat(execRootPath.getRelative("justSomeDir/thatIsDoomed").exists()).isFalse(); - assertThat(execRootPath.getRelative("out").exists()).isTrue(); - assertThat(execRootPath.getRelative("out/dir").exists()).isFalse(); + assertThat(execRoot.getRelative("existing/directory/with").exists()).isTrue(); + assertThat(execRoot.getRelative("partial").exists()).isTrue(); + assertThat(execRoot.getRelative("partial/doomedSubdir").exists()).isFalse(); + assertThat(execRoot.getRelative("partial/directory").exists()).isFalse(); + assertThat(execRoot.getRelative("justSomeDir/thatIsDoomed").exists()).isFalse(); + assertThat(execRoot.getRelative("out").exists()).isTrue(); + assertThat(execRoot.getRelative("out/dir").exists()).isFalse(); } @Test public void populateInputsAndDirsToCreate_createsMappedDirectories() { ArtifactRoot outputRoot = - ArtifactRoot.asDerivedRoot(execRootPath, ArtifactRoot.RootType.Output, "outputs"); + ArtifactRoot.asDerivedRoot(execRoot, ArtifactRoot.RootType.Output, "outputs"); ActionInput outputFile = ActionsTestUtil.createArtifact(outputRoot, "bin/config/dir/file"); ActionInput outputDir = ActionsTestUtil.createTreeArtifactWithGeneratingAction( @@ -459,13 +340,13 @@ public void moveOutputs_mappedPathMovedToUnmappedPath() throws Exception { .setPathMapper(pathMapper) .build(); var sandboxHelpers = new SandboxHelpers(); - Path sandboxBase = execRootPath.getRelative("sandbox"); + Path sandboxBase = execRoot.getRelative("sandbox"); PathFragment mappedOutputPath = PathFragment.create("bin/output"); sandboxBase.getRelative(mappedOutputPath).getParentDirectory().createDirectoryAndParents(); FileSystemUtils.writeLinesAs( sandboxBase.getRelative(mappedOutputPath), UTF_8, "hello", "pathmapper"); - Path realBase = execRootPath.getRelative("real"); + Path realBase = execRoot.getRelative("real"); SandboxHelpers.moveOutputs(sandboxHelpers.getOutputs(spawn), sandboxBase, realBase); assertThat( 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 dfc659bcb6d3ec..9c26026eeafe61 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 @@ -26,8 +26,6 @@ import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; -import com.google.devtools.build.lib.vfs.Root; -import com.google.devtools.build.lib.vfs.RootedPath; import com.google.devtools.build.lib.vfs.Symlinks; import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; import java.io.IOException; @@ -62,9 +60,8 @@ public final void setupTestDirs() throws IOException { @Test public void createFileSystem() throws Exception { - RootedPath helloTxt = - RootedPath.toRootedPath(Root.fromPath(workspaceDir), PathFragment.create("hello.txt")); - FileSystemUtils.createEmptyFile(helloTxt.asPath()); + Path helloTxt = workspaceDir.getRelative("hello.txt"); + FileSystemUtils.createEmptyFile(helloTxt); SymlinkedSandboxedSpawn symlinkedExecRoot = new SymlinkedSandboxedSpawn( @@ -75,7 +72,6 @@ public void createFileSystem() throws Exception { new SandboxInputs( ImmutableMap.of(PathFragment.create("such/input.txt"), helloTxt), ImmutableMap.of(), - ImmutableMap.of(), ImmutableMap.of()), SandboxOutputs.create( ImmutableSet.of(PathFragment.create("very/output.txt")), ImmutableSet.of()), @@ -89,8 +85,7 @@ public void createFileSystem() throws Exception { symlinkedExecRoot.createFileSystem(); assertThat(execRoot.getRelative("such/input.txt").isSymbolicLink()).isTrue(); - assertThat(execRoot.getRelative("such/input.txt").resolveSymbolicLinks()) - .isEqualTo(helloTxt.asPath()); + assertThat(execRoot.getRelative("such/input.txt").resolveSymbolicLinks()).isEqualTo(helloTxt); assertThat(execRoot.getRelative("very").isDirectory()).isTrue(); assertThat(execRoot.getRelative("wow/writable").isDirectory()).isTrue(); } @@ -107,8 +102,7 @@ public void copyOutputs() throws Exception { execRoot, ImmutableList.of("/bin/true"), ImmutableMap.of(), - new SandboxInputs( - ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of()), + new SandboxInputs(ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of()), SandboxOutputs.create( ImmutableSet.of(outputFile.relativeTo(execRoot)), ImmutableSet.of()), ImmutableSet.of(), 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 d594263d912043..9eddd2e1c19c9c 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,6 +82,10 @@ private enum TreeComposition { FULLY_LOCAL, FULLY_REMOTE, MIXED; + + boolean isPartiallyRemote() { + return this == FULLY_REMOTE || this == MIXED; + } } private final Map chmodCalls = Maps.newConcurrentMap(); @@ -432,10 +436,11 @@ public void fileArtifactMaterializedAsSymlink( .getPath(outputArtifact.getPath().getPathString()) .createSymbolicLink(targetArtifact.getPath().asFragment()); - PathFragment expectedMaterializationExecPath = - location == FileLocation.REMOTE && preexistingPath != null - ? preexistingPath - : targetArtifact.getExecPath(); + PathFragment expectedMaterializationExecPath = null; + if (location == FileLocation.REMOTE) { + expectedMaterializationExecPath = + preexistingPath != null ? preexistingPath : targetArtifact.getExecPath(); + } assertThat(store.getOutputMetadata(outputArtifact)) .isEqualTo(createFileMetadataForSymlinkTest(location, expectedMaterializationExecPath)); @@ -445,12 +450,7 @@ private FileArtifactValue createFileMetadataForSymlinkTest( FileLocation location, @Nullable PathFragment materializationExecPath) { switch (location) { case LOCAL: - FileArtifactValue target = - FileArtifactValue.createForNormalFile(new byte[] {1, 2, 3}, /* proxy= */ null, 10); - return materializationExecPath == null - ? target - : FileArtifactValue.createForResolvedSymlink( - materializationExecPath, target, target.getDigest()); + return FileArtifactValue.createForNormalFile(new byte[] {1, 2, 3}, /* proxy= */ null, 10); case REMOTE: return RemoteFileArtifactValue.create( new byte[] {1, 2, 3}, 10, 1, -1, materializationExecPath); @@ -495,8 +495,11 @@ public void treeArtifactMaterializedAsSymlink( .getPath(outputArtifact.getPath().getPathString()) .createSymbolicLink(targetArtifact.getPath().asFragment()); - PathFragment expectedMaterializationExecPath = - preexistingPath != null ? preexistingPath : targetArtifact.getExecPath(); + PathFragment expectedMaterializationExecPath = null; + if (composition.isPartiallyRemote()) { + expectedMaterializationExecPath = + preexistingPath != null ? preexistingPath : targetArtifact.getExecPath(); + } assertThat(store.getTreeArtifactValue(outputArtifact)) .isEqualTo( @@ -825,7 +828,7 @@ public void fileArtifactValueForSymlink_readFromCache() throws Exception { var symlinkMetadata = store.getOutputMetadata(symlink); - assertThat(symlinkMetadata.getDigest()).isEqualTo(targetMetadata.getDigest()); + assertThat(symlinkMetadata).isEqualTo(targetMetadata); assertThat(DigestUtils.getCacheStats().hitCount()).isEqualTo(1); } } diff --git a/src/test/java/com/google/devtools/build/lib/worker/SandboxHelper.java b/src/test/java/com/google/devtools/build/lib/worker/SandboxHelper.java index 745e59b90c71c4..32941228ec56e0 100644 --- a/src/test/java/com/google/devtools/build/lib/worker/SandboxHelper.java +++ b/src/test/java/com/google/devtools/build/lib/worker/SandboxHelper.java @@ -16,7 +16,6 @@ import static java.nio.charset.StandardCharsets.UTF_8; -import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.actions.cache.VirtualActionInput; import com.google.devtools.build.lib.actions.util.ActionsTestUtil; @@ -25,8 +24,6 @@ import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; -import com.google.devtools.build.lib.vfs.Root; -import com.google.devtools.build.lib.vfs.RootedPath; import com.google.errorprone.annotations.CanIgnoreReturnValue; import java.io.IOException; import java.util.ArrayList; @@ -41,7 +38,7 @@ class SandboxHelper { /** Map from workdir-relative input path to optional real file path. */ - private final Map inputs = new HashMap<>(); + private final Map inputs = new HashMap<>(); private final Map virtualInputs = new HashMap<>(); private final Map symlinks = new HashMap<>(); @@ -50,15 +47,13 @@ class SandboxHelper { private final List outputDirs = new ArrayList<>(); /** The global execRoot. */ - final Path execRootPath; + final Path execRoot; - final Root execRoot; /** The worker process's sandbox root. */ final Path workDir; public SandboxHelper(Path execRoot, Path workDir) { - this.execRootPath = execRoot; - this.execRoot = Root.fromPath(execRoot); + this.execRoot = execRoot; this.workDir = workDir; } @@ -70,9 +65,7 @@ public SandboxHelper(Path execRoot, Path workDir) { public SandboxHelper addInputFile(String relativePath, String workspacePath) { inputs.put( PathFragment.create(relativePath), - workspacePath != null - ? RootedPath.toRootedPath(execRoot, PathFragment.create(workspacePath)) - : null); + workspacePath != null ? execRoot.getRelative(workspacePath) : null); return this; } @@ -85,7 +78,7 @@ public SandboxHelper addInputFile(String relativePath, String workspacePath) { public SandboxHelper addAndCreateInputFile( String relativePath, String workspacePath, String contents) throws IOException { addInputFile(relativePath, workspacePath); - Path absPath = execRootPath.getRelative(workspacePath); + Path absPath = execRoot.getRelative(workspacePath); absPath.getParentDirectory().createDirectoryAndParents(); FileSystemUtils.writeContentAsLatin1(absPath, contents); return this; @@ -96,7 +89,7 @@ public SandboxHelper addAndCreateInputFile( public SandboxHelper addAndCreateVirtualInput(String relativePath, String contents) { VirtualActionInput input = ActionsTestUtil.createVirtualActionInput(relativePath, contents); byte[] digest = - execRootPath + execRoot .getRelative(relativePath) .getFileSystem() .getDigestFunction() @@ -135,7 +128,7 @@ public SandboxHelper addOutputDir(String relativePath) { */ @CanIgnoreReturnValue public SandboxHelper addWorkerFile(String relativePath) { - Path absPath = execRootPath.getRelative(relativePath); + Path absPath = execRoot.getRelative(relativePath); workerFiles.put(PathFragment.create(relativePath), absPath); return this; } @@ -148,7 +141,7 @@ public SandboxHelper addWorkerFile(String relativePath) { public SandboxHelper addAndCreateWorkerFile(String relativePath, String contents) throws IOException { addWorkerFile(relativePath); - Path absPath = execRootPath.getRelative(relativePath); + Path absPath = execRoot.getRelative(relativePath); absPath.getParentDirectory().createDirectoryAndParents(); FileSystemUtils.writeContentAsLatin1(absPath, contents); return this; @@ -173,7 +166,7 @@ public SandboxHelper createExecRootFile(String relativePath, String contents) th @CanIgnoreReturnValue public SandboxHelper createWorkspaceDirFile(String workspaceDirPath, String contents) throws IOException { - Path absPath = execRootPath.getRelative(workspaceDirPath); + Path absPath = execRoot.getRelative(workspaceDirPath); absPath.getParentDirectory().createDirectoryAndParents(); FileSystemUtils.writeContentAsLatin1(absPath, contents); return this; @@ -192,7 +185,7 @@ public SandboxHelper createSymlink(String relativePath, String relativeDestinati } public SandboxInputs getSandboxInputs() { - return new SandboxInputs(inputs, virtualInputs, symlinks, ImmutableMap.of()); + return new SandboxInputs(inputs, virtualInputs, symlinks); } public SandboxOutputs getSandboxOutputs() { diff --git a/src/test/java/com/google/devtools/build/lib/worker/WorkerSpawnRunnerTest.java b/src/test/java/com/google/devtools/build/lib/worker/WorkerSpawnRunnerTest.java index b7b55eb054e6bd..def0e8d919cc33 100644 --- a/src/test/java/com/google/devtools/build/lib/worker/WorkerSpawnRunnerTest.java +++ b/src/test/java/com/google/devtools/build/lib/worker/WorkerSpawnRunnerTest.java @@ -62,8 +62,6 @@ import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; -import com.google.devtools.build.lib.vfs.Root; -import com.google.devtools.build.lib.vfs.RootedPath; import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; import com.google.devtools.build.lib.worker.WorkerPoolImpl.WorkerPoolConfig; import com.google.devtools.build.lib.worker.WorkerProtocol.WorkRequest; @@ -143,8 +141,7 @@ public void testExecInWorker_happyPath() throws ExecException, InterruptedExcept spawn, key, context, - new SandboxInputs( - ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of()), + new SandboxInputs(ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of()), SandboxOutputs.create(ImmutableSet.of(), ImmutableSet.of()), ImmutableList.of(), inputFileCache, @@ -171,7 +168,6 @@ public void testExecInWorker_virtualInputs_doesntQueryInputFileCache() new WorkerSpawnRunner( new SandboxHelpers(), execRoot, - ImmutableList.of(), createWorkerPool(), reporter, localEnvProvider, @@ -236,8 +232,7 @@ public void testExecInWorker_finishesAsyncOnInterrupt() spawn, key, context, - new SandboxInputs( - ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of()), + new SandboxInputs(ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of()), SandboxOutputs.create(ImmutableSet.of(), ImmutableSet.of()), ImmutableList.of(), inputFileCache, @@ -280,8 +275,7 @@ public void testExecInWorker_sendsCancelMessageOnInterrupt() spawn, key, context, - new SandboxInputs( - ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of()), + new SandboxInputs(ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of()), SandboxOutputs.create(ImmutableSet.of(), ImmutableSet.of()), ImmutableList.of(), inputFileCache, @@ -322,8 +316,7 @@ public void testExecInWorker_unsandboxedDiesOnInterrupt() spawn, key, context, - new SandboxInputs( - ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of()), + new SandboxInputs(ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of()), SandboxOutputs.create(ImmutableSet.of(), ImmutableSet.of()), ImmutableList.of(), inputFileCache, @@ -356,8 +349,7 @@ public void testExecInWorker_noMultiplexWithDynamic() spawn, key, context, - new SandboxInputs( - ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of()), + new SandboxInputs(ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of()), SandboxOutputs.create(ImmutableSet.of(), ImmutableSet.of()), ImmutableList.of(), inputFileCache, @@ -394,8 +386,7 @@ private void assertRecordedResponsethrowsException(String recordedResponse, Stri spawn, key, context, - new SandboxInputs( - ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of()), + new SandboxInputs(ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of()), SandboxOutputs.create(ImmutableSet.of(), ImmutableSet.of()), ImmutableList.of(), inputFileCache, @@ -439,12 +430,12 @@ public void testExpandArgument_expandsArgumentsRecursively() SandboxInputs inputs = new SandboxInputs( ImmutableMap.of( - PathFragment.create("file"), - asRootedPath("/file"), - PathFragment.create("file2"), - asRootedPath("/file2")), - ImmutableMap.of(), - ImmutableMap.of(), ImmutableMap.of()); + PathFragment.create("file"), + fs.getPath("/file"), + PathFragment.create("file2"), + fs.getPath("/file2")), + ImmutableMap.of(), + ImmutableMap.of()); WorkerSpawnRunner.expandArgument(inputs, "@file", requestBuilder); assertThat(requestBuilder.getArgumentsList()) .containsExactly("arg1", "arg2", "arg3", "multi arg", ""); @@ -457,8 +448,9 @@ public void testExpandArgument_expandsOnlyProperArguments() FileSystemUtils.writeIsoLatin1(fs.getPath("/file"), "arg1\n@@nonfile\n@foo//bar\narg2"); SandboxInputs inputs = new SandboxInputs( - ImmutableMap.of(PathFragment.create("file"), asRootedPath("/file")), ImmutableMap.of(), - ImmutableMap.of(), ImmutableMap.of()); + ImmutableMap.of(PathFragment.create("file"), fs.getPath("/file")), + ImmutableMap.of(), + ImmutableMap.of()); WorkerSpawnRunner.expandArgument(inputs, "@file", requestBuilder); assertThat(requestBuilder.getArgumentsList()) .containsExactly("arg1", "@@nonfile", "@foo//bar", "arg2"); @@ -469,8 +461,7 @@ public void testExpandArgument_failsOnMissingFile() { WorkRequest.Builder requestBuilder = WorkRequest.newBuilder(); SandboxInputs inputs = new SandboxInputs( - ImmutableMap.of(PathFragment.create("file"), asRootedPath("/dir/file")), - ImmutableMap.of(), + ImmutableMap.of(PathFragment.create("file"), fs.getPath("/dir/file")), ImmutableMap.of(), ImmutableMap.of()); IOException e = @@ -571,7 +562,6 @@ private WorkerSpawnRunner createWorkerSpawnRunner(WorkerOptions workerOptions) { return new WorkerSpawnRunner( new SandboxHelpers(), fs.getPath("/execRoot"), - ImmutableList.of(), createWorkerPool(), reporter, localEnvProvider, @@ -587,8 +577,7 @@ private WorkerSpawnRunner createWorkerSpawnRunner(WorkerOptions workerOptions) { public void testExpandArgument_failsOnUndeclaredInput() { WorkRequest.Builder requestBuilder = WorkRequest.newBuilder(); SandboxInputs inputs = - new SandboxInputs( - ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of()); + new SandboxInputs(ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of()); IOException e = assertThrows( IOException.class, @@ -597,10 +586,6 @@ public void testExpandArgument_failsOnUndeclaredInput() { assertThat(e).hasMessageThat().contains("declared input"); } - private RootedPath asRootedPath(String path) { - return RootedPath.toRootedPath(Root.absoluteRoot(fs), fs.getPath(path)); - } - private static String logMarker(String text) { return "---8<---8<--- " + text + " ---8<---8<---\n"; } diff --git a/src/test/java/com/google/devtools/build/lib/worker/WorkerSpawnStrategyTest.java b/src/test/java/com/google/devtools/build/lib/worker/WorkerSpawnStrategyTest.java index d20f8626626c84..6c8ff50f2e2729 100644 --- a/src/test/java/com/google/devtools/build/lib/worker/WorkerSpawnStrategyTest.java +++ b/src/test/java/com/google/devtools/build/lib/worker/WorkerSpawnStrategyTest.java @@ -20,9 +20,8 @@ import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.sandbox.SandboxHelpers.SandboxInputs; import com.google.devtools.build.lib.vfs.FileSystem; +import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; -import com.google.devtools.build.lib.vfs.Root; -import com.google.devtools.build.lib.vfs.RootedPath; import com.google.devtools.build.lib.vfs.util.FileSystems; import com.google.devtools.build.lib.worker.WorkerProtocol.WorkRequest; import java.io.File; @@ -51,13 +50,13 @@ public void expandArgumentsPreservesEmptyLines() throws Exception { flags.forEach(pw::println); } - RootedPath path = - RootedPath.toRootedPath(Root.absoluteRoot(fs), fs.getPath(flagfile.getAbsolutePath())); + Path path = fs.getPath(flagfile.getAbsolutePath()); WorkRequest.Builder requestBuilder = WorkRequest.newBuilder(); SandboxInputs inputs = new SandboxInputs( - ImmutableMap.of(PathFragment.create("flagfile.txt"), path), ImmutableMap.of(), - ImmutableMap.of(), ImmutableMap.of()); + ImmutableMap.of(PathFragment.create("flagfile.txt"), path), + ImmutableMap.of(), + ImmutableMap.of()); WorkerSpawnRunner.expandArgument(inputs, "@flagfile.txt", requestBuilder); assertThat(requestBuilder.getArgumentsList()).containsExactlyElementsIn(flags); diff --git a/src/test/shell/bazel/bazel_sandboxing_test.sh b/src/test/shell/bazel/bazel_sandboxing_test.sh index cbe0c7fa3cec60..f09202ef8f3a4b 100755 --- a/src/test/shell/bazel/bazel_sandboxing_test.sh +++ b/src/test/shell/bazel/bazel_sandboxing_test.sh @@ -32,6 +32,14 @@ function set_up { sed -i.bak '/sandbox_tmpfs_path/d' "$bazelrc" } +function assert_not_exists() { + path="$1" + [ ! -f "$path" ] && return 0 + + fail "Expected file '$path' to not exist, but it did" + return 1 +} + function test_sandboxed_tooldir() { mkdir -p examples/genrule @@ -309,6 +317,68 @@ EOF bazel build //pkg:a &>$TEST_log || fail "expected build to succeed" } +# Sets up targets under //test that, when building //test:all, verify that the +# sandbox setup ensures that /tmp contents written by one action are not visible +# to another action. +# +# Arguments: +# - The path to a unique temporary directory under /tmp that a +# file named "bazel_was_here" is written to in actions. +function setup_tmp_hermeticity_check() { + local -r tmpdir=$1 + + mkdir -p test + cat > test/BUILD <<'EOF' +cc_binary( + name = "create_file", + srcs = ["create_file.cc"], +) + +[ + genrule( + name = "gen" + str(i), + outs = ["gen{}.txt".format(i)], + tools = [":create_file"], + cmd = """ + path=$$($(location :create_file)) + cp "$$path" $@ + """, + ) + for i in range(1, 3) +] +EOF + cat > test/create_file.cc < +#include +#include +#include +#include +#include +#include + +int main() { + if (mkdir("$tmpdir", 0755) < 0) { + perror("mkdir"); + return 1; + } + int fd = open("$tmpdir/bazel_was_here", O_CREAT | O_EXCL | O_WRONLY, 0600); + if (fd < 0) { + perror("open"); + return 1; + } + if (write(fd, "HERMETIC\n", 9) != 9) { + perror("write"); + return 1; + } + close(fd); + printf("$tmpdir/bazel_was_here\n"); + return 0; +} +EOF +} + function test_add_mount_pair_tmp_source() { if [[ "$PLATFORM" == "darwin" ]]; then # Tests Linux-specific functionality @@ -321,19 +391,26 @@ function test_add_mount_pair_tmp_source() { trap "rm -fr $mounted" EXIT echo GOOD > "$mounted/data.txt" + local tmp_dir=$(mktemp -d "/tmp/bazel_mounted.XXXXXXXX") + trap "rm -fr $tmp_dir" EXIT + setup_tmp_hermeticity_check "$tmp_dir" + mkdir -p pkg - cat > pkg/BUILD < pkg/BUILD <<'EOF' genrule( name = "gen", outs = ["gen.txt"], - # Verify that /tmp is still hermetic. - cmd = """[ ! -e "${mounted}/data.txt" ] && cp /etc/data.txt \$@""", + cmd = "cp /etc/data.txt $@", ) EOF # This assumes the existence of /etc on the host system - bazel build --sandbox_add_mount_pair="$mounted:/etc" //pkg:gen || fail "build failed" - assert_contains GOOD bazel-bin/pkg/gen.txt + bazel build --sandbox_add_mount_pair="$mounted:/etc" \ + //pkg:gen //test:all || fail "build failed" + assert_equals GOOD "$(cat bazel-bin/pkg/gen.txt)" + assert_equals HERMETIC "$(cat bazel-bin/test/gen1.txt)" + assert_equals HERMETIC "$(cat bazel-bin/test/gen2.txt)" + assert_not_exists "$tmp_dir/bazel_was_here" } function test_add_mount_pair_tmp_target() { @@ -348,20 +425,28 @@ function test_add_mount_pair_tmp_target() { trap "rm -fr $source_dir" EXIT echo BAD > "$source_dir/data.txt" + local tmp_dir=$(mktemp -d "/tmp/bazel_mounted.XXXXXXXX") + trap "rm -fr $tmp_dir" EXIT + setup_tmp_hermeticity_check "$tmp_dir" + mkdir -p pkg cat > pkg/BUILD < \$@""", + cmd = """ls "$source_dir" > \$@""", ) EOF # This assumes the existence of /etc on the host system - bazel build --sandbox_add_mount_pair="/etc:$source_dir" //pkg:gen || fail "build failed" + bazel build --sandbox_add_mount_pair="/etc:$source_dir" \ + //pkg:gen //test:all || fail "build failed" assert_contains passwd bazel-bin/pkg/gen.txt + assert_not_contains data.txt bazel-bin/pkg/gen.txt + assert_equals HERMETIC "$(cat bazel-bin/test/gen1.txt)" + assert_equals HERMETIC "$(cat bazel-bin/test/gen2.txt)" + assert_not_exists "$tmp_dir/bazel_was_here" } function test_add_mount_pair_tmp_target_and_source() { @@ -376,22 +461,25 @@ function test_add_mount_pair_tmp_target_and_source() { trap "rm -fr $mounted" EXIT echo GOOD > "$mounted/data.txt" - local tmp_file=$(mktemp "/tmp/bazel_tmp.XXXXXXXX") - trap "rm $tmp_file" EXIT - echo BAD > "$tmp_file" + local tmp_dir=$(mktemp -d "/tmp/bazel_mounted.XXXXXXXX") + trap "rm -fr $tmp_dir" EXIT + setup_tmp_hermeticity_check "$tmp_dir" mkdir -p pkg cat > pkg/BUILD < $repo/pkg/es1 <<'EOF' +EXTERNAL_SOURCE_CONTENT +EOF + cat > $repo/pkg/BUILD <<'EOF' +exports_files(["es1"]) +genrule( + name="er1", + srcs=[], + outs=[":er1"], + cmd="echo EXTERNAL_GEN_CONTENT > $@", + visibility=["//visibility:public"], +) +EOF + + mkdir -p $repo/examples + cd $repo/examples || fail "cd $repo/examples failed" + + cat > WORKSPACE < pkg/s1 <<'EOF' +SOURCE_CONTENT +EOF cat > pkg/BUILD <<'EOF' load(":r.bzl", "symlink_rule") -genrule(name="r1", srcs=[], outs=[":r1"], cmd="echo CONTENT > $@") +genrule(name="r1", srcs=[], outs=[":r1"], cmd="echo GEN_CONTENT > $@") symlink_rule(name="r2", input=":r1") genrule(name="r3", srcs=[":r2"], outs=[":r3"], cmd="cp $< $@") +symlink_rule(name="s2", input=":s1") +genrule(name="s3", srcs=[":s2"], outs=[":s3"], cmd="cp $< $@") +symlink_rule(name="er2", input="@repo//pkg:er1") +genrule(name="er3", srcs=[":er2"], outs=[":er3"], cmd="cp $< $@") +symlink_rule(name="es2", input="@repo//pkg:es1") +genrule(name="es3", srcs=[":es2"], outs=[":es3"], cmd="cp $< $@") EOF cat > pkg/r.bzl <<'EOF' @@ -425,8 +551,11 @@ 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" build //pkg:{er,es,r,s}3 --sandbox_debug + assert_contains EXTERNAL_GEN_CONTENT bazel-bin/pkg/er3 + assert_contains EXTERNAL_SOURCE_CONTENT bazel-bin/pkg/es3 + assert_contains GEN_CONTENT bazel-bin/pkg/r3 + assert_contains SOURCE_CONTENT bazel-bin/pkg/s3 bazel --output_base="$tmp_output_base" shutdown } @@ -487,24 +616,23 @@ function test_tmpfs_path_under_tmp() { create_workspace_with_default_repos WORKSPACE - local tmp_file=$(mktemp "/tmp/bazel_tmp.XXXXXXXX") - trap "rm $tmp_file" EXIT - echo BAD > "$tmp_file" - local tmpfs=$(mktemp -d "/tmp/bazel_tmpfs.XXXXXXXX") trap "rm -fr $tmpfs" EXIT echo BAD > "$tmpfs/data.txt" + local tmp_dir=$(mktemp -d "/tmp/bazel_mounted.XXXXXXXX") + trap "rm -fr $tmp_dir" EXIT + setup_tmp_hermeticity_check "$tmp_dir" + mkdir -p pkg cat > pkg/BUILD <&2 + return 0 + fi + + temp_dir=$(mktemp -d /tmp/test.XXXXXX) + trap 'rm -rf ${temp_dir}' EXIT + + mkdir -p "${temp_dir}/workspace/a" + mkdir -p "${temp_dir}/package-path/b" + mkdir -p "${temp_dir}/repo/c" + mkdir -p "${temp_dir}/output-base" + + cd "${temp_dir}/workspace" + cat > WORKSPACE < a/BUILD <<'EOF' +genrule( + name = "g", + outs = ["go"], + srcs = [], + cmd = "echo GO > $@", +) +sh_binary( + name = "bin", + srcs = ["bin.sh"], + data = [":s", ":go", "//b:s", "//b:go", "@repo//c:s", "@repo//c:go"], +) +genrule( + name = "t", + tools = [":bin"], + srcs = [":s", ":go", "//b:s", "//b:go", "@repo//c:s", "@repo//c:go"], + outs = ["to"], + cmd = "\n".join([ + "RUNFILES=$(location :bin).runfiles/_main", + "S=$(location :s); GO=$(location :go)", + "BS=$(location //b:s); BGO=$(location //b:go)", + "RS=$(location @repo//c:s); RGO=$(location @repo//c:go)", + "for i in $$S $$GO $$BS $$BGO $$RS $$RGO; do", + " echo reading $$i", + " cat $$i >> $@", + "done", + "for i in a/s a/go b/s b/go ../repo/c/s ../repo/c/go; do", + " echo reading $$RUNFILES/$$i", + " cat $$RUNFILES/$$i >> $@", + "done", + ]), +) +EOF + + touch a/bin.sh + chmod +x a/bin.sh + + touch ../repo/WORKSPACE + cat > ../repo/c/BUILD <<'EOF' +exports_files(["s"]) +genrule( + name = "g", + outs = ["go"], + srcs = [], + cmd = "echo GO > $@", + visibility = ["//visibility:public"], +) +EOF + + cat > ../package-path/b/BUILD <<'EOF' +exports_files(["s"]) +genrule( + name = "g", + outs = ["go"], + srcs = [], + cmd = "echo GO > $@", + visibility = ["//visibility:public"], +) +EOF + + touch "a/s" "../package-path/b/s" "../repo/c/s" + + cat WORKSPACE + bazel \ + --output_base="${temp_dir}/output-base" \ + build \ + --incompatible_sandbox_hermetic_tmp \ + --package_path="%workspace%:${temp_dir}/package-path" \ + //a:t || fail "build failed" +} + +# Regression test for https://github.com/bazelbuild/bazel/issues/21215 +function test_copy_input_symlinks() { + create_workspace_with_default_repos WORKSPACE + + cat > MODULE.bazel <<'EOF' +repo = use_repo_rule("//pkg:repo.bzl", "repo") +repo(name = "some_repo") +EOF + + mkdir -p pkg + cat > pkg/BUILD <<'EOF' +genrule( + name = "copy_files", + srcs = [ + "@some_repo//:files", + ], + outs = [ + "some_file1.json", + "some_file2.json", + ], + cmd = "cp -r $(locations @some_repo//:files) $(RULEDIR)", +) +EOF + cat > pkg/repo.bzl <<'EOF' +def _impl(rctx): + rctx.file("some_file1.json", "hello") + rctx.file("some_file2.json", "world") + rctx.file("BUILD", """filegroup( + name = "files", + srcs = ["some_file1.json", "some_file2.json"], + visibility = ["//visibility:public"], +)""") + +repo = repository_rule(_impl) +EOF + + bazel build //pkg:copy_files || fail "build failed" + assert_equals hello "$(cat bazel-bin/pkg/some_file1.json)" + assert_equals world "$(cat bazel-bin/pkg/some_file2.json)" } # The test shouldn't fail if the environment doesn't support running it.