Rewrite the inprocess symlink strategy
This is currently hidden behind an experimental flag, so this code is
not used yet. This new implementation more closely matches
build-runfiles.cc. Also, it now correctly creates the output manifest,
which was previously missing.
This passes almost all of our tests, but there are still some open issues:
- on Windows, it copies instead of symlinking (this is caught by tests)
- it does not fix the file mode bits if there is an existing file or
directory with incorrect mode bits (not caught by tests)
We are also not sure whether this is acceptable from a performance
standpoint.
PiperOrigin-RevId: 290944709
diff --git a/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java b/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java
index 5aefaa4..877fc47 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java
@@ -27,17 +27,19 @@
import com.google.devtools.build.lib.util.CommandUtils;
import com.google.devtools.build.lib.util.OsUtils;
import com.google.devtools.build.lib.util.io.OutErr;
+import com.google.devtools.build.lib.vfs.Dirent;
+import com.google.devtools.build.lib.vfs.FileStatus;
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.Symlinks;
import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
import java.util.HashMap;
-import java.util.HashSet;
import java.util.List;
import java.util.Map;
-import java.util.Set;
+import javax.annotation.Nullable;
/**
* Helper class responsible for the symlink tree creation. Used to generate runfiles and fileset
@@ -65,27 +67,51 @@
this.filesetTree = filesetTree;
}
- public Path getOutputManifest() {
- return symlinkTreeRoot;
+ private Path getOutputManifest() {
+ return symlinkTreeRoot.getChild("MANIFEST");
}
- /** Creates a symlink tree using direct system calls. */
+ /** Creates a symlink tree by making VFS calls. */
public void createSymlinksDirectly(Path symlinkTreeRoot, Map<PathFragment, Artifact> symlinks)
throws IOException {
Preconditions.checkState(!filesetTree);
- Set<Path> dirs = new HashSet<>();
- for (Map.Entry<PathFragment, Artifact> e : symlinks.entrySet()) {
- Path symlinkPath = symlinkTreeRoot.getRelative(e.getKey());
- Path parentDirectory = symlinkPath.getParentDirectory();
- if (dirs.add(parentDirectory)) {
- parentDirectory.createDirectoryAndParents();
- }
- if (e.getValue() == null) {
- FileSystemUtils.createEmptyFile(symlinkPath);
- } else {
- symlinkPath.createSymbolicLink(e.getValue().getPath().asFragment());
- }
+ // Our strategy is to minimize mutating file system operations as much as possible. Ideally, if
+ // there is an existing symlink tree with the expected contents, we don't make any changes. Our
+ // algorithm goes as follows:
+ //
+ // 1. Create a tree structure that represents the entire set of paths that we want to exist. The
+ // tree structure contains a node for every intermediate directory. For example, this is the
+ // tree structure corresponding to the symlinks {"a/b/c": "foobar", "a/d/e": null}:
+ //
+ // / b - c (symlink to "foobar")
+ // a
+ // \ d - e (empty file)
+ //
+ // Note that we need to distinguish directories, symlinks, and empty files. In the Directory
+ // class below, we use two maps for that purpose: one for directories, and one for symlinks
+ // and empty files. This avoids having to create additional classes / objects to distinguish
+ // them.
+ //
+ // 2. Perform a depth-first traversal over the on-disk file system tree and make each directory
+ // match our expected directory layout. To that end, call readdir, and compare the result
+ // with the contents of the corresponding node in the in-memory tree.
+ //
+ // For each Dirent entry in the readdir result:
+ // - If the entry is not in the current node, if the entry has an incompatible type, or if it
+ // is a symlink that points to the wrong location, delete the entry on disk (recursively).
+ // - Otherwise:
+ // - If the entry is a directory, recurse into that directory
+ // - In all cases, delete the entry in the current in-memory node.
+ //
+ // 3. For every remaining entry in the node, create the corresponding file, symlink, or
+ // directory on disk. If it is a directory, recurse into that directory.
+ Directory root = new Directory();
+ for (Map.Entry<PathFragment, Artifact> entry : symlinks.entrySet()) {
+ // This creates intermediate directory nodes as a side effect.
+ Directory parentDir = root.walk(entry.getKey().getParentDirectory());
+ parentDir.addSymlink(entry.getKey().getBaseName(), entry.getValue());
}
+ root.syncTreeRecursively(symlinkTreeRoot);
}
/**
@@ -112,7 +138,7 @@
// Pretend we created the runfiles tree by copying the manifest
try {
symlinkTreeRoot.createDirectoryAndParents();
- FileSystemUtils.copyFile(inputManifest, symlinkTreeRoot.getChild("MANIFEST"));
+ FileSystemUtils.copyFile(inputManifest, getOutputManifest());
} catch (IOException e) {
throw new EnvironmentalExecException(e);
}
@@ -185,4 +211,82 @@
}
return result;
}
+
+ private static final class Directory {
+ private final Map<String, Artifact> symlinks = new HashMap<>();
+ private final Map<String, Directory> directories = new HashMap<>();
+
+ void addSymlink(String basename, @Nullable Artifact artifact) {
+ symlinks.put(basename, artifact);
+ }
+
+ Directory walk(PathFragment dir) {
+ Directory result = this;
+ for (String segment : dir.getSegments()) {
+ Directory next = result.directories.get(segment);
+ if (next == null) {
+ next = new Directory();
+ result.directories.put(segment, next);
+ }
+ result = next;
+ }
+ return result;
+ }
+
+ void syncTreeRecursively(Path at) throws IOException {
+ // This is a reimplementation of the C++ code in build-runfiles.cc. This avoids having to ship
+ // a separate native tool to create a few runfiles.
+ // TODO(ulfjack): Measure performance.
+ FileStatus stat = at.statNullable(Symlinks.FOLLOW);
+ if (stat == null) {
+ at.createDirectoryAndParents();
+ } else if (!stat.isDirectory()) {
+ at.deleteTree();
+ at.createDirectoryAndParents();
+ }
+ // TODO(ulfjack): provide the mode bits from FileStatus and use that to construct the correct
+ // chmod call here. Note that we do not have any tests for this right now. Something like
+ // this:
+ // if (!stat.isExecutable() || !stat.isReadable()) {
+ // at.chmod(stat.getMods() | 0700);
+ // }
+ for (Dirent dirent : at.readdir(Symlinks.FOLLOW)) {
+ String basename = dirent.getName();
+ Path next = at.getChild(basename);
+ if (symlinks.containsKey(basename)) {
+ Artifact value = symlinks.remove(basename);
+ if (value == null) {
+ if (dirent.getType() != Dirent.Type.FILE) {
+ next.deleteTree();
+ FileSystemUtils.createEmptyFile(next);
+ }
+ // For consistency with build-runfiles.cc, we don't truncate the file if one exists.
+ } else {
+ // TODO(ulfjack): On Windows, this call makes a copy rather than creating a symlink.
+ FileSystemUtils.ensureSymbolicLink(next, value.getPath().asFragment());
+ }
+ } else if (directories.containsKey(basename)) {
+ Directory nextDir = directories.remove(basename);
+ if (dirent.getType() != Dirent.Type.DIRECTORY) {
+ next.deleteTree();
+ }
+ nextDir.syncTreeRecursively(at.getChild(basename));
+ } else {
+ at.getChild(basename).deleteTree();
+ }
+ }
+
+ for (Map.Entry<String, Artifact> entry : symlinks.entrySet()) {
+ Path next = at.getChild(entry.getKey());
+ if (entry.getValue() == null) {
+ FileSystemUtils.createEmptyFile(next);
+ } else {
+ FileSystemUtils.ensureSymbolicLink(next, entry.getValue().getPath().asFragment());
+ }
+ }
+ for (Map.Entry<String, Directory> entry : directories.entrySet()) {
+ entry.getValue().syncTreeRecursively(at.getChild(entry.getKey()));
+ }
+ }
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategy.java
index 2cdcc24..442d2d4 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategy.java
@@ -73,14 +73,7 @@
if (action.getRunfiles() != null) {
try {
symlinks =
- Maps.transformValues(
- action
- .getRunfiles()
- .getRunfilesInputs(
- actionExecutionContext.getEventHandler(),
- action.getOwner().getLocation(),
- actionExecutionContext.getPathResolver()),
- TO_PATH);
+ Maps.transformValues(runfilesToMap(action, actionExecutionContext), TO_PATH);
} catch (IOException e) {
throw new EnvironmentalExecException(e);
}
@@ -99,47 +92,25 @@
symlinks,
action.getOutputManifest().getExecPath().getParentDirectory());
- Path outputManifest = actionExecutionContext.getInputPath(action.getOutputManifest());
- if (inputManifest == null) {
- // If we don't have an input manifest, then create a file containing a fingerprint of
- // the runfiles object.
- Fingerprint fp = new Fingerprint();
- action.getRunfiles().fingerprint(fp);
- String hexDigest = fp.hexDigestAndReset();
- try {
- FileSystemUtils.writeContentAsLatin1(outputManifest, hexDigest);
- } catch (IOException e) {
- throw new EnvironmentalExecException(
- "Failed to link output manifest '" + outputManifest.getPathString() + "'", e);
- }
- } else {
- // Link output manifest on success. We avoid a file copy as these manifests may be
- // large. Note that this step has to come last because the OutputService may delete any
- // pre-existing symlink tree before creating a new one.
- try {
- outputManifest.createSymbolicLink(inputManifest);
- } catch (IOException e) {
- throw new EnvironmentalExecException(
- "Failed to link output manifest '" + outputManifest.getPathString() + "'", e);
- }
- }
+ createOutput(action, actionExecutionContext, inputManifest);
} else if (!action.isRunfilesEnabled()) {
createSymlinkTreeHelper(action, actionExecutionContext).copyManifest();
} else if (action.getInputManifest() == null
|| (action.inprocessSymlinkCreation() && !action.isFilesetTree())) {
try {
+ Map<PathFragment, Artifact> runfiles = runfilesToMap(action, actionExecutionContext);
createSymlinkTreeHelper(action, actionExecutionContext)
.createSymlinksDirectly(
- action.getOutputManifest().getPath().getParentDirectory(),
- action
- .getRunfiles()
- .getRunfilesInputs(
- actionExecutionContext.getEventHandler(),
- action.getOwner().getLocation(),
- actionExecutionContext.getPathResolver()));
+ action.getOutputManifest().getPath().getParentDirectory(), runfiles);
} catch (IOException e) {
throw new EnvironmentalExecException(e).toActionExecutionException(action);
}
+
+ Path inputManifest =
+ action.getInputManifest() == null
+ ? null
+ : actionExecutionContext.getInputPath(action.getInputManifest());
+ createOutput(action, actionExecutionContext, inputManifest);
} else {
Map<String, String> resolvedEnv = new LinkedHashMap<>();
action.getEnvironment().resolve(resolvedEnv, actionExecutionContext.getClientEnv());
@@ -157,6 +128,49 @@
}
}
+ private static Map<PathFragment, Artifact> runfilesToMap(
+ SymlinkTreeAction action, ActionExecutionContext actionExecutionContext) throws IOException {
+ // This call outputs warnings about overlapping symlinks. However, this is already called by the
+ // SourceManifestAction, so it can happen that we generate the warning twice. If the input
+ // manifest is null, then we print the warning. Otherwise we assume that the
+ // SourceManifestAction already printed it.
+ return action
+ .getRunfiles()
+ .getRunfilesInputs(
+ action.getInputManifest() == null ? actionExecutionContext.getEventHandler() : null,
+ action.getOwner().getLocation(),
+ actionExecutionContext.getPathResolver());
+ }
+
+ private static void createOutput(
+ SymlinkTreeAction action, ActionExecutionContext actionExecutionContext, Path inputManifest)
+ throws EnvironmentalExecException {
+ Path outputManifest = actionExecutionContext.getInputPath(action.getOutputManifest());
+ if (inputManifest == null) {
+ // If we don't have an input manifest, then create a file containing a fingerprint of
+ // the runfiles object.
+ Fingerprint fp = new Fingerprint();
+ action.getRunfiles().fingerprint(fp);
+ String hexDigest = fp.hexDigestAndReset();
+ try {
+ FileSystemUtils.writeContentAsLatin1(outputManifest, hexDigest);
+ } catch (IOException e) {
+ throw new EnvironmentalExecException(
+ "Failed to link output manifest '" + outputManifest.getPathString() + "'", e);
+ }
+ } else {
+ // Link output manifest on success. We avoid a file copy as these manifests may be
+ // large. Note that this step has to come last because the OutputService may delete any
+ // pre-existing symlink tree before creating a new one.
+ try {
+ outputManifest.createSymbolicLink(inputManifest);
+ } catch (IOException e) {
+ throw new EnvironmentalExecException(
+ "Failed to link output manifest '" + outputManifest.getPathString() + "'", e);
+ }
+ }
+ }
+
private static SymlinkTreeHelper createSymlinkTreeHelper(
SymlinkTreeAction action, ActionExecutionContext actionExecutionContext) {
return new SymlinkTreeHelper(
