Use non-snapshotted paths when creating a runfiles symlink tree.
It has been a long-standing feature of runfiles symlink trees that they reflect changes made to files referenced in them after the tree has been built. In certain scenarios (currently only at Google), the FileSystem implementation may rewrite symlink target paths to point to read-only snapshots, which breaks this feature when using in-process symlink creation. With this change, the rewriting is bypassed in this case.
Also improve the documentation for the BlazeModule getFileSystem() and getFileSystemForBuildArtifacts() methods.
PiperOrigin-RevId: 690541050
Change-Id: I140cc68ca1b9236e9fb4049830cc3b5629eaf770
diff --git a/src/main/java/com/google/devtools/build/lib/exec/RunfilesTreeUpdater.java b/src/main/java/com/google/devtools/build/lib/exec/RunfilesTreeUpdater.java
index 738358e..9bd947a 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/RunfilesTreeUpdater.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/RunfilesTreeUpdater.java
@@ -153,13 +153,17 @@
SymlinkTreeHelper helper =
new SymlinkTreeHelper(
- inputManifest, runfilesDir, /* filesetTree= */ false, tree.getWorkspaceName());
+ execRoot,
+ inputManifest,
+ runfilesDir,
+ /* filesetTree= */ false,
+ tree.getWorkspaceName());
switch (tree.getSymlinksMode()) {
case SKIP -> helper.clearRunfilesDirectory();
- case EXTERNAL -> helper.createSymlinksUsingCommand(execRoot, binTools, env, outErr);
+ case EXTERNAL -> helper.createSymlinksUsingCommand(binTools, env, outErr);
case INTERNAL -> {
- helper.createSymlinksDirectly(runfilesDir, tree.getMapping());
+ helper.createSymlinksDirectly(tree.getMapping());
outputManifest.createSymbolicLink(inputManifest);
}
}
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 032a4d8..487bd6b 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
@@ -38,6 +38,7 @@
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.SnapshottingFileSystem;
import com.google.devtools.build.lib.vfs.Symlinks;
import java.io.IOException;
import java.util.HashMap;
@@ -53,6 +54,7 @@
@VisibleForTesting
public static final String BUILD_RUNFILES = "build-runfiles" + OsUtils.executableExtension();
+ private final Path execRoot;
private final Path inputManifest;
private final Path symlinkTreeRoot;
private final boolean filesetTree;
@@ -68,20 +70,34 @@
* @param workspaceName the name of the workspace, used to create the workspace subdirectory
*/
public SymlinkTreeHelper(
- Path inputManifest, Path symlinkTreeRoot, boolean filesetTree, String workspaceName) {
- this.inputManifest = inputManifest;
- this.symlinkTreeRoot = symlinkTreeRoot;
+ Path execRoot,
+ Path inputManifest,
+ Path symlinkTreeRoot,
+ boolean filesetTree,
+ String workspaceName) {
+ this.execRoot = ensureNonSnapshotting(execRoot);
+ this.inputManifest = ensureNonSnapshotting(inputManifest);
+ this.symlinkTreeRoot = ensureNonSnapshotting(symlinkTreeRoot);
this.filesetTree = filesetTree;
this.workspaceName = workspaceName;
}
+ private static Path ensureNonSnapshotting(Path path) {
+ // Changes made to a file referenced by a symlink tree should be reflected in the symlink tree
+ // without having to rebuild. Therefore, if a snapshotting file system is used, we must use the
+ // underlying non-snapshotting file system instead to create the symlink tree.
+ if (path.getFileSystem() instanceof SnapshottingFileSystem snapshottingFs) {
+ return snapshottingFs.getUnderlyingNonSnapshottingFileSystem().getPath(path.asFragment());
+ }
+ return path;
+ }
+
private Path getOutputManifest() {
return symlinkTreeRoot.getChild("MANIFEST");
}
/** Creates a symlink tree by making VFS calls. */
- public void createSymlinksDirectly(Path symlinkTreeRoot, Map<PathFragment, Artifact> symlinks)
- throws IOException {
+ public void createSymlinksDirectly(Map<PathFragment, Artifact> symlinkMap) throws IOException {
// 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:
@@ -115,7 +131,7 @@
try (SilentCloseable c = Profiler.instance().profile("Create symlink tree in-process")) {
Preconditions.checkState(!filesetTree);
Directory root = new Directory();
- for (Map.Entry<PathFragment, Artifact> entry : symlinks.entrySet()) {
+ for (Map.Entry<PathFragment, Artifact> entry : symlinkMap.entrySet()) {
// This creates intermediate directory nodes as a side effect.
Directory parentDir = root.walk(entry.getKey().getParentDirectory());
parentDir.addSymlink(entry.getKey().getBaseName(), entry.getValue());
@@ -177,10 +193,10 @@
* kind of synchronization, locking, or anything else.
*/
public void createSymlinksUsingCommand(
- Path execRoot, BinTools binTools, Map<String, String> shellEnvironment, OutErr outErr)
+ BinTools binTools, Map<String, String> shellEnvironment, OutErr outErr)
throws EnvironmentalExecException, InterruptedException {
try (SilentCloseable c = Profiler.instance().profile("Create symlink tree out-of-process")) {
- Command command = createCommand(execRoot, binTools, shellEnvironment);
+ Command command = createCommand(binTools, shellEnvironment);
try {
if (outErr != null) {
command.execute(outErr.getOutputStream(), outErr.getErrorStream());
@@ -205,7 +221,7 @@
}
@VisibleForTesting
- Command createCommand(Path execRoot, BinTools binTools, Map<String, String> shellEnvironment) {
+ Command createCommand(BinTools binTools, Map<String, String> shellEnvironment) {
Preconditions.checkNotNull(shellEnvironment);
List<String> args = Lists.newArrayList();
args.add(binTools.getEmbeddedPath(BUILD_RUNFILES).asFragment().getPathString());
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 39b4549..3c9a174 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
@@ -110,10 +110,8 @@
} else if (action.getRunfileSymlinksMode() == RunfileSymlinksMode.INTERNAL
&& !action.isFilesetTree()) {
try {
- Map<PathFragment, Artifact> runfiles = runfilesToMap(action);
createSymlinkTreeHelper(action, actionExecutionContext)
- .createSymlinksDirectly(
- action.getOutputManifest().getPath().getParentDirectory(), runfiles);
+ .createSymlinksDirectly(runfilesToMap(action));
} catch (IOException e) {
throw ActionExecutionException.fromExecException(
new EnvironmentalExecException(e, Code.SYMLINK_TREE_CREATION_IO_EXCEPTION), action);
@@ -126,10 +124,7 @@
action.getEnvironment().resolve(resolvedEnv, actionExecutionContext.getClientEnv());
createSymlinkTreeHelper(action, actionExecutionContext)
.createSymlinksUsingCommand(
- actionExecutionContext.getExecRoot(),
- binTools,
- resolvedEnv,
- actionExecutionContext.getFileOutErr());
+ binTools, resolvedEnv, actionExecutionContext.getFileOutErr());
}
} catch (ExecException e) {
throw ActionExecutionException.fromExecException(e, action);
@@ -165,6 +160,7 @@
private SymlinkTreeHelper createSymlinkTreeHelper(
SymlinkTreeAction action, ActionExecutionContext actionExecutionContext) {
return new SymlinkTreeHelper(
+ actionExecutionContext.getExecRoot(),
actionExecutionContext.getInputPath(action.getInputManifest()),
actionExecutionContext.getInputPath(action.getOutputManifest()).getParentDirectory(),
action.isFilesetTree(),
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java
index b38fbdb..1a79dd44 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java
@@ -84,8 +84,9 @@
public void globalInit(OptionsParsingResult startupOptions) throws AbruptExitException {}
/**
- * Returns the file system implementation used by Bazel. It is an error if more than one module
- * returns a file system. If all return null, the default unix file system is used.
+ * Returns the file system implementation used by Bazel.
+ *
+ * <p>Exactly one module must return a non-null value from this method, or an error will occur.
*
* <p>This method will be called at the beginning of Bazel startup (in-between {@link #globalInit}
* and {@link #blazeStartup}).
@@ -93,12 +94,26 @@
* @param startupOptions the server's startup options
* @param realExecRootBase absolute path fragment of the actual, underlying execution root
*/
+ @Nullable
public ModuleFileSystem getFileSystem(
OptionsParsingResult startupOptions, PathFragment realExecRootBase)
throws AbruptExitException {
return null;
}
+ /**
+ * Returns the file system implementation used by Bazel to read or write build artifacts.
+ *
+ * <p>At most one module may return a non-null value from this method, or an error will occur. If
+ * no module returns a non-null value, the file system returned by {@link #getFileSystem} from
+ * this or another module will be used.
+ *
+ * <p>This method will be called at the beginning of Bazel startup (in-between {@link #globalInit}
+ * and {@link #blazeStartup}).
+ *
+ * @param fileSystem the file system returned by {@link #getFileSystem} from this or another
+ * module
+ */
@Nullable
public FileSystem getFileSystemForBuildArtifacts(FileSystem fileSystem) {
return null;
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/SnapshottingFileSystem.java b/src/main/java/com/google/devtools/build/lib/vfs/SnapshottingFileSystem.java
new file mode 100644
index 0000000..5759016
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/vfs/SnapshottingFileSystem.java
@@ -0,0 +1,26 @@
+// Copyright 2024 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.vfs;
+
+/**
+ * An interface implemented by file systems that support pinning files to a read-only snapshot.
+ *
+ * <p>This is used by {@link SymlinkTreeHelper} to obtain the underlying non-snapshotting file
+ * system, so that symlink trees are not pinned to a read-only snapshot and reflect changes made
+ * after the symlink tree was built.
+ */
+public interface SnapshottingFileSystem {
+ /** Returns the underlying non-snapshotting file system. */
+ FileSystem getUnderlyingNonSnapshottingFileSystem();
+}
diff --git a/src/test/java/com/google/devtools/build/lib/exec/SymlinkTreeHelperTest.java b/src/test/java/com/google/devtools/build/lib/exec/SymlinkTreeHelperTest.java
index 9251259..3e28af0 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/SymlinkTreeHelperTest.java
+++ b/src/test/java/com/google/devtools/build/lib/exec/SymlinkTreeHelperTest.java
@@ -43,8 +43,12 @@
BinTools.forUnitTesting(execRoot, ImmutableList.of(SymlinkTreeHelper.BUILD_RUNFILES));
Command command =
new SymlinkTreeHelper(
- inputManifestPath, execRoot.getRelative("output/MANIFEST"), false, "__main__")
- .createCommand(execRoot, binTools, ImmutableMap.of());
+ execRoot,
+ inputManifestPath,
+ execRoot.getRelative("output/MANIFEST"),
+ false,
+ "__main__")
+ .createCommand(binTools, ImmutableMap.of());
assertThat(command.getEnvironment()).isEmpty();
assertThat(command.getWorkingDirectory()).isEqualTo(execRoot.getPathFile());
ImmutableList<String> commandLine = command.getArguments();
diff --git a/src/test/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategyTest.java b/src/test/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategyTest.java
index 63ea2e6..210f0ac 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategyTest.java
@@ -129,6 +129,7 @@
OutputService outputService = mock(OutputService.class);
StoredEventHandler eventHandler = new StoredEventHandler();
+ when(context.getExecRoot()).thenReturn(getExecRoot());
when(context.getContext(SymlinkTreeActionContext.class))
.thenReturn(new SymlinkTreeStrategy(outputService, null, "__main__"));
when(context.getInputPath(any())).thenAnswer((i) -> ((Artifact) i.getArgument(0)).getPath());
