[8.0.0] Use non-snapshotted paths when creating a runfiles symlink tree.

src/main/java/com/google/devtools/build/lib/exec/RunfilesTreeUpdater.java

@@ -153,13 +153,17 @@ private void updateRunfilesTree(
 
     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);
       }
     }

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 @@ public final class SymlinkTreeHelper {
   @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 @@ public final class SymlinkTreeHelper {
    * @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 @@ public void createSymlinksDirectly(Path symlinkTreeRoot, Map<PathFragment, Artif
     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 @@ private void createWorkspaceSubdirectory() throws IOException {
    * 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 @@ public void createSymlinksUsingCommand(
   }
 
   @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());

src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategy.java

@@ -110,10 +110,8 @@ public void createSymlinks(
         } 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 @@ public void createSymlinks(
           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 static void createOutput(
   private SymlinkTreeHelper createSymlinkTreeHelper(
       SymlinkTreeAction action, ActionExecutionContext actionExecutionContext) {
     return new SymlinkTreeHelper(
+        actionExecutionContext.getExecRoot(),
         actionExecutionContext.getInputPath(action.getInputManifest()),
         actionExecutionContext.getInputPath(action.getOutputManifest()).getParentDirectory(),
         action.isFilesetTree(),

src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java

@@ -84,21 +84,36 @@ public Iterable<Class<? extends OptionsBase>> getStartupOptions() {
   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}).
    *
    * @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;

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();
+}

src/test/java/com/google/devtools/build/lib/exec/SymlinkTreeHelperTest.java

@@ -43,8 +43,12 @@ public void checkCreatedSpawn() {
         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();

src/test/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategyTest.java

@@ -129,6 +129,7 @@ public void inprocessSymlinkCreation() throws Exception {
     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());