Skip to content

Commit

Permalink
Inject unresolved symlinks with BwoB
Browse files Browse the repository at this point in the history
Creating unresolved symlinks locally when building with `--remote_download_outputs=minimal` results in unnecessary I/O and may not even be supported by the host (e.g. on Windows). Instead, only create it in the remote action file system.
  • Loading branch information
fmeum committed Jan 7, 2025
1 parent fbf6884 commit 837ccff
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
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.FileArtifactValue;
import com.google.devtools.build.lib.actions.FileArtifactValue.UnresolvedSymlinkArtifactValue;
import com.google.devtools.build.lib.actions.InputMetadataProvider;
import com.google.devtools.build.lib.actions.cache.OutputMetadataStore;
import com.google.devtools.build.lib.actions.cache.VirtualActionInput;
Expand Down Expand Up @@ -374,7 +375,10 @@ private ListenableFuture<Void> prefetchFile(
PathFragment execPath = input.getExecPath();

FileArtifactValue metadata = metadataSupplier.getMetadata(input);
if (metadata == null || !canDownloadFile(execRoot.getRelative(execPath), metadata)) {
if (metadata instanceof UnresolvedSymlinkArtifactValue unresolvedSymlink) {
return toListenableFuture(
plantRelativeSymlink(execPath, unresolvedSymlink.getSymlinkTarget()));
} else if (metadata == null || !canDownloadFile(execRoot.getRelative(execPath), metadata)) {
return immediateVoidFuture();
}

Expand All @@ -399,7 +403,7 @@ private ListenableFuture<Void> prefetchFile(
priority);

if (symlink != null) {
result = result.andThen(plantSymlink(symlink));
result = result.andThen(plantAbsoluteSymlink(symlink));
}

return toListenableFuture(result);
Expand Down Expand Up @@ -651,7 +655,21 @@ private void deletePartialDownload(Path path) {
}
}

private Completable plantSymlink(Symlink symlink) {
private Completable plantRelativeSymlink(PathFragment linkPath, String target) {
return downloadCache.executeIfNot(
execRoot.getRelative(linkPath),
Completable.defer(
() -> {
Path link = execRoot.getRelative(linkPath);
// Delete the link path if it already exists. This is the case for tree artifacts,
// whose root directory is created before the action runs.
link.delete();
link.createSymbolicLink(PathFragment.create(target));
return Completable.complete();
}));
}

private Completable plantAbsoluteSymlink(Symlink symlink) {
return downloadCache.executeIfNot(
execRoot.getRelative(symlink.linkExecPath()),
Completable.defer(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,10 @@ void injectRemoteFile(PathFragment path, byte[] digest, long size, long expireAt
remoteOutputTree.injectFile(path, metadata);
}

void injectUnresolvedSymlink(PathFragment symlink, PathFragment target) throws IOException {
remoteOutputTree.injectSymlink(symlink, target);
}

@Override
public String getFileSystemType(PathFragment path) {
return "remoteActionFS";
Expand Down Expand Up @@ -937,6 +941,11 @@ protected void injectFile(PathFragment path, FileArtifactValue metadata) throws
remoteInMemoryFileInfo.set(metadata);
}

protected void injectSymlink(PathFragment path, PathFragment target) throws IOException {
createDirectoryAndParents(path.getParentDirectory());
createSymbolicLink(path, target);
}

// Override for access within this class
@Nullable
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1006,20 +1006,29 @@ private void moveOutputsToFinalLocation(
}
}

private void createSymlinks(Iterable<SymlinkMetadata> symlinks) throws IOException {
private void createSymlinks(
RemoteActionResult result,
@Nullable RemoteActionFileSystem remoteActionFileSystem,
Iterable<SymlinkMetadata> symlinks)
throws IOException {
for (SymlinkMetadata symlink : symlinks) {
Preconditions.checkNotNull(
symlink.path().getParentDirectory(),
"Failed creating directory and parents for %s",
symlink.path())
.createDirectoryAndParents();
// If a directory output is being materialized as a symlink, we must first delete the
// preexisting empty directory.
if (symlink.path().exists(Symlinks.NOFOLLOW)
&& symlink.path().isDirectory(Symlinks.NOFOLLOW)) {
symlink.path().delete();
if (shouldDownload(result, symlink.path().relativeTo(execRoot))) {
Preconditions.checkNotNull(
symlink.path().getParentDirectory(),
"Failed creating directory and parents for %s",
symlink.path())
.createDirectoryAndParents();
// If a directory output is being materialized as a symlink, we must first delete the
// preexisting empty directory.
if (symlink.path().exists(Symlinks.NOFOLLOW)
&& symlink.path().isDirectory(Symlinks.NOFOLLOW)) {
symlink.path().delete();
}
symlink.path().createSymbolicLink(symlink.target());
} else if (!(outputService instanceof BazelOutputService)) {
checkNotNull(remoteActionFileSystem)
.injectUnresolvedSymlink(symlink.path().asFragment(), symlink.target());
}
symlink.path().createSymbolicLink(symlink.target());
}
}

Expand Down Expand Up @@ -1439,7 +1448,7 @@ public InMemoryOutput downloadOutputs(RemoteAction action, RemoteActionResult re

// Create the symbolic links after all downloads are finished, because dangling symlinks
// might not be supported on all platforms.
createSymlinks(symlinks);
createSymlinks(result, remoteActionFileSystem, symlinks);

if (result.success()) {
// Check that all mandatory outputs are created.
Expand Down

0 comments on commit 837ccff

Please sign in to comment.