From 27a3e4d3583b756c4658c6cc6bc1309b4bf4bfba Mon Sep 17 00:00:00 2001 From: Googler Date: Mon, 28 Oct 2024 03:26:31 -0700 Subject: [PATCH] [8.0.0] 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 --- .../build/lib/exec/RunfilesTreeUpdater.java | 10 ++++-- .../build/lib/exec/SymlinkTreeHelper.java | 34 ++++++++++++++----- .../build/lib/exec/SymlinkTreeStrategy.java | 10 ++---- .../build/lib/runtime/BlazeModule.java | 19 +++++++++-- .../build/lib/vfs/SnapshottingFileSystem.java | 26 ++++++++++++++ .../build/lib/exec/SymlinkTreeHelperTest.java | 8 +++-- .../lib/exec/SymlinkTreeStrategyTest.java | 1 + 7 files changed, 85 insertions(+), 23 deletions(-) create mode 100644 src/main/java/com/google/devtools/build/lib/vfs/SnapshottingFileSystem.java 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 738358e3d3920e..9bd947a12b16b8 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 @@ 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); } } 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 032a4d8a8fca87..487bd6b8a1942d 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 @@ 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 symlinks) - throws IOException { + public void createSymlinksDirectly(Map 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 entry : symlinks.entrySet()) { + for (Map.Entry 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 shellEnvironment, OutErr outErr) + BinTools binTools, Map 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 shellEnvironment) { + Command createCommand(BinTools binTools, Map shellEnvironment) { Preconditions.checkNotNull(shellEnvironment); List 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 39b45494e0e3fc..3c9a174ff44807 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 @@ public void createSymlinks( } else if (action.getRunfileSymlinksMode() == RunfileSymlinksMode.INTERNAL && !action.isFilesetTree()) { try { - Map 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(), 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 b38fbdb0366df9..1a79dd442b38de 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 Iterable> 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. + * + *

Exactly one module must return a non-null value from this method, or an error will occur. * *

This method will be called at the beginning of Bazel startup (in-between {@link #globalInit} * and {@link #blazeStartup}). @@ -93,12 +94,26 @@ public void globalInit(OptionsParsingResult startupOptions) throws AbruptExitExc * @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. + * + *

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

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 00000000000000..57590169ec8070 --- /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. + * + *

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 925125988922c2..3e28af0b2f5bc2 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 @@ 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 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 63ea2e62292387..210f0ac3452d9c 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 @@ 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());