diff --git a/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java b/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java index f08bdb949f834a..f4c9842bfec7de 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java +++ b/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java @@ -36,6 +36,7 @@ import java.io.IOException; import java.util.Arrays; import java.util.Objects; +import java.util.Optional; import javax.annotation.Nullable; /** @@ -172,6 +173,17 @@ protected boolean couldBeModifiedByMetadata(FileArtifactValue lastKnown) { return true; } + /** + * Optional symlink target path. + * + *

If present, this artifact is a copy of another artifact. It is materialized in the local + * filesystem as a symlink to the original artifact, whose contents live at this location. This is + * used to implement zero-cost copies of remotely stored artifacts. + */ + public Optional getSymlinkTargetExecPath() { + return Optional.empty(); + } + /** * Marker interface for singleton implementations of this class. * @@ -514,9 +526,7 @@ public long getModifiedTime() { @Override public String toString() { return MoreObjects.toStringHelper(this) - .add( - "digest", - digest == null ? "(null)" : BaseEncoding.base16().lowerCase().encode(digest)) + .add("digest", BaseEncoding.base16().lowerCase().encode(digest)) .add("size", size) .add("proxy", proxy) .toString(); @@ -535,10 +545,10 @@ protected boolean couldBeModifiedByMetadata(FileArtifactValue o) { /** Metadata for remotely stored files. */ public static class RemoteFileArtifactValue extends FileArtifactValue { - private final byte[] digest; - private final long size; - private final int locationIndex; - private final String actionId; + protected final byte[] digest; + protected final long size; + protected final int locationIndex; + protected final String actionId; private RemoteFileArtifactValue(byte[] digest, long size, int locationIndex, String actionId) { this.digest = Preconditions.checkNotNull(digest, actionId); @@ -556,6 +566,19 @@ public static RemoteFileArtifactValue create(byte[] digest, long size, int locat return new RemoteFileArtifactValue(digest, size, locationIndex, /* actionId= */ ""); } + public static RemoteFileArtifactValue create( + byte[] digest, + long size, + int locationIndex, + String actionId, + @Nullable PathFragment symlinkTargetExecPath) { + if (symlinkTargetExecPath != null) { + return new SymlinkToRemoteFileArtifactValue( + digest, size, locationIndex, actionId, symlinkTargetExecPath); + } + return new RemoteFileArtifactValue(digest, size, locationIndex, actionId); + } + @Override public boolean equals(Object o) { if (!(o instanceof RemoteFileArtifactValue)) { @@ -602,7 +625,7 @@ public String getActionId() { @Override public long getModifiedTime() { throw new UnsupportedOperationException( - "RemoteFileArifactValue doesn't support getModifiedTime"); + "RemoteFileArtifactValue doesn't support getModifiedTime"); } @Override @@ -626,6 +649,58 @@ public String toString() { .add("digest", bytesToString(digest)) .add("size", size) .add("locationIndex", locationIndex) + .add("actionId", actionId) + .toString(); + } + } + + /** An artifact that should be materialized as a symlink to another remote artifact. */ + public static final class SymlinkToRemoteFileArtifactValue extends RemoteFileArtifactValue { + private PathFragment symlinkTargetExecPath; + + private SymlinkToRemoteFileArtifactValue( + byte[] digest, + long size, + int locationIndex, + String actionId, + PathFragment symlinkTargetExecPath) { + super(digest, size, locationIndex, actionId); + this.symlinkTargetExecPath = Preconditions.checkNotNull(symlinkTargetExecPath); + } + + @Override + public Optional getSymlinkTargetExecPath() { + return Optional.ofNullable(symlinkTargetExecPath); + } + + @Override + public boolean equals(Object o) { + if (!(o instanceof SymlinkToRemoteFileArtifactValue)) { + return false; + } + + SymlinkToRemoteFileArtifactValue that = (SymlinkToRemoteFileArtifactValue) o; + return Arrays.equals(digest, that.digest) + && size == that.size + && locationIndex == that.locationIndex + && Objects.equals(actionId, that.actionId) + && Objects.equals(symlinkTargetExecPath, that.symlinkTargetExecPath); + } + + @Override + public int hashCode() { + return Objects.hash( + Arrays.hashCode(digest), size, locationIndex, actionId, symlinkTargetExecPath); + } + + @Override + public String toString() { + return MoreObjects.toStringHelper(this) + .add("digest", bytesToString(digest)) + .add("size", size) + .add("locationIndex", locationIndex) + .add("actionId", actionId) + .add("symlinkTargetExecPath", symlinkTargetExecPath) .toString(); } } diff --git a/src/main/java/com/google/devtools/build/lib/actions/cache/ActionCache.java b/src/main/java/com/google/devtools/build/lib/actions/cache/ActionCache.java index ff1b13c85be8e7..65866cacb65082 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/cache/ActionCache.java +++ b/src/main/java/com/google/devtools/build/lib/actions/cache/ActionCache.java @@ -109,9 +109,10 @@ final class Entry { public abstract static class SerializableTreeArtifactValue { public static SerializableTreeArtifactValue create( ImmutableMap childValues, - Optional archivedFileValue) { + Optional archivedFileValue, + Optional symlinkTargetExecPath) { return new AutoValue_ActionCache_Entry_SerializableTreeArtifactValue( - childValues, archivedFileValue); + childValues, archivedFileValue, symlinkTargetExecPath); } /** @@ -138,17 +139,25 @@ public static Optional createSerializable( .filter(ar -> ar.archivedFileValue().isRemote()) .map(ar -> (RemoteFileArtifactValue) ar.archivedFileValue()); - if (childValues.isEmpty() && archivedFileValue.isEmpty()) { + Optional symlinkTargetExecPath = treeMetadata.getSymlinkTargetExecPath(); + + if (childValues.isEmpty() + && archivedFileValue.isEmpty() + && symlinkTargetExecPath.isEmpty()) { return Optional.empty(); } - return Optional.of(SerializableTreeArtifactValue.create(childValues, archivedFileValue)); + return Optional.of( + SerializableTreeArtifactValue.create( + childValues, archivedFileValue, symlinkTargetExecPath)); } // A map from parentRelativePath to the file metadata public abstract ImmutableMap childValues(); public abstract Optional archivedFileValue(); + + public abstract Optional symlinkTargetExecPath(); } public Entry(String key, Map usedClientEnv, boolean discoversInputs) { diff --git a/src/main/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCache.java b/src/main/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCache.java index 6fc3d4956d65ca..0cad6071114bc4 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCache.java +++ b/src/main/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCache.java @@ -36,6 +36,7 @@ import com.google.devtools.build.lib.util.VarInt; import com.google.devtools.build.lib.vfs.DigestUtils; import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.SyscallCache; import com.google.devtools.build.lib.vfs.UnixGlob; import java.io.ByteArrayOutputStream; @@ -75,7 +76,7 @@ public class CompactPersistentActionCache implements ActionCache { private static final int NO_INPUT_DISCOVERY_COUNT = -1; - private static final int VERSION = 13; + private static final int VERSION = 14; private static final class ActionMap extends PersistentMap { private final Clock clock; @@ -466,6 +467,14 @@ private static void encodeRemoteMetadata( VarInt.putVarInt(value.getLocationIndex(), sink); VarInt.putVarInt(indexer.getOrCreateIndex(value.getActionId()), sink); + + Optional symlinkTargetExecPath = value.getSymlinkTargetExecPath(); + if (symlinkTargetExecPath.isPresent()) { + VarInt.putVarInt(1, sink); + VarInt.putVarInt(indexer.getOrCreateIndex(symlinkTargetExecPath.get().toString()), sink); + } else { + VarInt.putVarInt(0, sink); + } } private static final int MAX_REMOTE_METADATA_SIZE = @@ -484,7 +493,18 @@ private static RemoteFileArtifactValue decodeRemoteMetadata( String actionId = getStringForIndex(indexer, VarInt.getVarInt(source)); - return RemoteFileArtifactValue.create(digest, size, locationIndex, actionId); + PathFragment symlinkTargetExecPath = null; + int numSymlinkTargetExecPath = VarInt.getVarInt(source); + if (numSymlinkTargetExecPath > 0) { + if (numSymlinkTargetExecPath != 1) { + throw new IOException("Invalid number of symlink target paths"); + } + symlinkTargetExecPath = + PathFragment.create(getStringForIndex(indexer, VarInt.getVarInt(source))); + } + + return RemoteFileArtifactValue.create( + digest, size, locationIndex, actionId, symlinkTargetExecPath); } /** @return action data encoded as a byte[] array. */ @@ -497,6 +517,7 @@ private static byte[] encode(StringIndexer indexer, ActionCache.Entry entry) thr int maxOutputFilesSize = VarInt.MAX_VARINT_SIZE // entry.getOutputFiles().size() + (VarInt.MAX_VARINT_SIZE // execPath + + (1 + VarInt.MAX_VARINT_SIZE) // symlinkTargetExecPath optional + MAX_REMOTE_METADATA_SIZE) * entry.getOutputFiles().size(); @@ -513,9 +534,12 @@ private static byte[] encode(StringIndexer indexer, ActionCache.Entry entry) thr + MAX_REMOTE_METADATA_SIZE) * value.childValues().size(); - maxOutputTreesSize += VarInt.MAX_VARINT_SIZE; // value.archivedFileValue() optional maxOutputTreesSize += - value.archivedFileValue().map(ignored -> MAX_REMOTE_METADATA_SIZE).orElse(0); + VarInt.MAX_VARINT_SIZE // value.archivedFileValue() optional + + value.archivedFileValue().map(ignored -> MAX_REMOTE_METADATA_SIZE).orElse(0); + maxOutputTreesSize += + VarInt.MAX_VARINT_SIZE // value.symlinkTargetExecPath() optional + + value.symlinkTargetExecPath().map(ignored -> VarInt.MAX_VARINT_SIZE).orElse(0); } // Estimate the size of the buffer: @@ -578,6 +602,15 @@ private static byte[] encode(StringIndexer indexer, ActionCache.Entry entry) thr } else { VarInt.putVarInt(0, sink); } + + Optional symlinkTargetExecPath = + serializableTreeArtifactValue.symlinkTargetExecPath(); + if (symlinkTargetExecPath.isPresent()) { + VarInt.putVarInt(1, sink); + VarInt.putVarInt(indexer.getOrCreateIndex(symlinkTargetExecPath.get().toString()), sink); + } else { + VarInt.putVarInt(0, sink); + } } return sink.toByteArray(); @@ -654,8 +687,20 @@ private static ActionCache.Entry decode(StringIndexer indexer, byte[] data) thro archivedFileValue = Optional.of(decodeRemoteMetadata(indexer, source)); } + Optional symlinkTargetPath = Optional.empty(); + int numSymlinkTargetExecPath = VarInt.getVarInt(source); + if (numSymlinkTargetExecPath > 0) { + if (numSymlinkTargetExecPath != 1) { + throw new IOException("Invalid number of symlink target paths"); + } + symlinkTargetPath = + Optional.of( + PathFragment.create(getStringForIndex(indexer, VarInt.getVarInt(source)))); + } + SerializableTreeArtifactValue value = - SerializableTreeArtifactValue.create(childValues.buildOrThrow(), archivedFileValue); + SerializableTreeArtifactValue.create( + childValues.buildOrThrow(), archivedFileValue, symlinkTargetPath); outputTrees.put(treeKey, value); } diff --git a/src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java b/src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java index 8db09901ac7ab6..c19e9570ffffa9 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java +++ b/src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java @@ -45,7 +45,6 @@ import com.google.devtools.build.lib.vfs.PathFragment; import io.reactivex.rxjava3.core.Completable; import io.reactivex.rxjava3.core.Flowable; -import io.reactivex.rxjava3.core.Single; import java.io.IOException; import java.util.ArrayList; import java.util.HashMap; @@ -142,6 +141,7 @@ protected ListenableFuture prefetchFiles( Priority priority) { Map> trees = new HashMap<>(); List files = new ArrayList<>(); + for (ActionInput input : inputs) { if (input instanceof Artifact && ((Artifact) input).isSourceArtifact()) { continue; @@ -162,34 +162,75 @@ protected ListenableFuture prefetchFiles( .flatMapSingle( entry -> toTransferResult( - prefetchInputTree( + prefetchInputTreeOrSymlink( metadataProvider, entry.getKey(), entry.getValue(), priority))); + Flowable fileDownloads = Flowable.fromIterable(files) .flatMapSingle( - input -> toTransferResult(prefetchInputFile(metadataProvider, input, priority))); + input -> + toTransferResult( + prefetchInputFileOrSymlink(metadataProvider, input, priority))); + Flowable transfers = Flowable.merge(treeDownloads, fileDownloads); Completable prefetch = mergeBulkTransfer(transfers).onErrorResumeNext(this::onErrorResumeNext); return toListenableFuture(prefetch); } - private Completable prefetchInputTree( + private Completable prefetchInputTreeOrSymlink( MetadataProvider provider, SpecialArtifact tree, List treeFiles, + Priority priority) + throws IOException { + + PathFragment execPath = tree.getExecPath(); + + FileArtifactValue treeMetadata = provider.getMetadata(tree); + // TODO(tjgq): Only download individual files that were requested within the tree. + // This isn't straightforward because multiple tree artifacts may share the same output tree + // when a ctx.actions.symlink is involved. + if (treeMetadata == null || !shouldDownloadAnyTreeFiles(treeFiles, treeMetadata)) { + return Completable.complete(); + } + + PathFragment prefetchExecPath = treeMetadata.getSymlinkTargetExecPath().orElse(execPath); + + Completable prefetch = prefetchInputTree(provider, prefetchExecPath, treeFiles, priority); + + // If prefetching to a different path, plant a symlink into it. + if (!prefetchExecPath.equals(execPath)) { + Completable prefetchAndSymlink = + prefetch.doOnComplete(() -> createSymlink(execPath, prefetchExecPath)); + return downloadCache.executeIfNot(execRoot.getRelative(execPath), prefetchAndSymlink); + } + + return prefetch; + } + + private boolean shouldDownloadAnyTreeFiles( + Iterable treeFiles, FileArtifactValue metadata) { + for (TreeFileArtifact treeFile : treeFiles) { + if (shouldDownloadFile(treeFile.getPath(), metadata)) { + return true; + } + } + return false; + } + + private Completable prefetchInputTree( + MetadataProvider provider, + PathFragment execPath, + List treeFiles, Priority priority) { - Path treeRoot = execRoot.getRelative(tree.getExecPath()); + Path treeRoot = execRoot.getRelative(execPath); HashMap treeFileTmpPathMap = new HashMap<>(); Flowable transfers = Flowable.fromIterable(treeFiles) .flatMapSingle( treeFile -> { - Path path = treeRoot.getRelative(treeFile.getParentRelativePath()); FileArtifactValue metadata = provider.getMetadata(treeFile); - if (!shouldDownloadFile(path, metadata)) { - return Single.just(TransferResult.ok()); - } Path tempPath = tempPathGenerator.generateTempPath(); treeFileTmpPathMap.put(treeFile, tempPath); @@ -198,7 +239,7 @@ private Completable prefetchInputTree( toCompletable( () -> doDownloadFile( - tempPath, path.relativeTo(execRoot), metadata, priority), + tempPath, treeFile.getExecPath(), metadata, priority), directExecutor())); }); @@ -209,10 +250,11 @@ private Completable prefetchInputTree( () -> { HashSet dirs = new HashSet<>(); - // Tree root is created by Bazel before action execution, but the permission is - // changed to 0555 afterwards. We need to set it as writable in order to move - // files into it. - treeRoot.setWritable(true); + // Even though the root directory for a tree artifact is created prior to action + // execution, we might be prefetching to a different directory that doesn't yet + // exist (when FileArtifactValue#getSymlinkTargetExecPath() is present). + // In any case, we need to make it writable to move files into it. + createWritableDirectory(treeRoot); dirs.add(treeRoot); for (Map.Entry entry : treeFileTmpPathMap.entrySet()) { @@ -227,8 +269,7 @@ private Completable prefetchInputTree( break; } if (dirs.add(dir)) { - dir.createDirectory(); - dir.setWritable(true); + createWritableDirectory(dir); } } checkState(dir.equals(path)); @@ -257,20 +298,33 @@ private Completable prefetchInputTree( return downloadCache.executeIfNot(treeRoot, download); } - private Completable prefetchInputFile( + private Completable prefetchInputFileOrSymlink( MetadataProvider metadataProvider, ActionInput input, Priority priority) throws IOException { if (input instanceof VirtualActionInput) { prefetchVirtualActionInput((VirtualActionInput) input); return Completable.complete(); } + PathFragment execPath = input.getExecPath(); + FileArtifactValue metadata = metadataProvider.getMetadata(input); - if (metadata == null) { + if (metadata == null || !shouldDownloadFile(execRoot.getRelative(execPath), metadata)) { return Completable.complete(); } - Path path = execRoot.getRelative(input.getExecPath()); - return downloadFileRx(path, metadata, priority); + PathFragment prefetchExecPath = metadata.getSymlinkTargetExecPath().orElse(execPath); + + Completable prefetch = + downloadFileNoCheckRx(execRoot.getRelative(prefetchExecPath), metadata, priority); + + // If prefetching to a different path, plant a symlink into it. + if (!prefetchExecPath.equals(execPath)) { + Completable prefetchAndSymlink = + prefetch.doOnComplete(() -> createSymlink(execPath, prefetchExecPath)); + return downloadCache.executeIfNot(execRoot.getRelative(execPath), prefetchAndSymlink); + } + + return prefetch; } /** @@ -283,7 +337,11 @@ private Completable downloadFileRx(Path path, FileArtifactValue metadata, Priori if (!shouldDownloadFile(path, metadata)) { return Completable.complete(); } + return downloadFileNoCheckRx(path, metadata, priority); + } + private Completable downloadFileNoCheckRx( + Path path, FileArtifactValue metadata, Priority priority) { if (path.isSymbolicLink()) { try { path = path.getRelative(path.readSymbolicLink()); @@ -348,6 +406,20 @@ private void deletePartialDownload(Path path) { } } + private void createWritableDirectory(Path dir) throws IOException { + dir.createDirectory(); + dir.setWritable(true); + } + + private void createSymlink(PathFragment linkPath, PathFragment targetPath) throws IOException { + Path link = execRoot.getRelative(linkPath); + Path target = execRoot.getRelative(targetPath); + // Delete the link path if it already exists. + // This will happen for output directories, which get created before the action runs. + link.delete(); + link.createSymbolicLink(target); + } + public ImmutableSet downloadedFiles() { return downloadCache.getFinishedTasks(); } diff --git a/src/main/java/com/google/devtools/build/lib/remote/BUILD b/src/main/java/com/google/devtools/build/lib/remote/BUILD index bf9aebcb2d0af1..8f20d75994dab0 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/BUILD +++ b/src/main/java/com/google/devtools/build/lib/remote/BUILD @@ -178,6 +178,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/actions:artifacts", "//src/main/java/com/google/devtools/build/lib/actions:file_metadata", "//src/main/java/com/google/devtools/build/lib/remote/util", + "//src/main/java/com/google/devtools/build/lib/skyframe:tree_artifact_value", "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", "//third_party:flogger", diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java index 99221c0ec6083f..aa61d8c790622a 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java @@ -17,6 +17,7 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.base.Preconditions.checkState; import static com.google.common.collect.ImmutableMap.toImmutableMap; import static com.google.common.collect.Streams.stream; @@ -48,12 +49,13 @@ import java.io.OutputStream; import java.nio.channels.ReadableByteChannel; import java.util.Collection; +import java.util.HashMap; import java.util.Map; import javax.annotation.Nullable; /** * This is a basic implementation and incomplete implementation of an action file system that's been - * tuned to what native (non-spawn) actions in Bazel currently use. + * tuned to what internal (non-spawn) actions in Bazel currently use. * *

The implementation mostly delegates to the local file system except for the case where an * action input is a remotely stored action output. Most notably {@link @@ -62,13 +64,13 @@ *

This implementation only supports creating local action outputs. */ public class RemoteActionFileSystem extends DelegateFileSystem { - private final PathFragment execRoot; private final PathFragment outputBase; private final ActionInputMap inputArtifactData; private final ImmutableMap outputMapping; private final RemoteActionInputFetcher inputFetcher; private final RemoteInMemoryFileSystem remoteOutputTree; + private final HashMap symlinkToTargetExecPath = new HashMap<>(); @Nullable private MetadataInjector metadataInjector = null; @@ -87,6 +89,12 @@ public class RemoteActionFileSystem extends DelegateFileSystem { stream(outputArtifacts).collect(toImmutableMap(Artifact::getExecPath, a -> a)); this.inputFetcher = checkNotNull(inputFetcher, "inputFetcher"); this.remoteOutputTree = new RemoteInMemoryFileSystem(getDigestFunction()); + + try { + remoteOutputTree.getPath(outputBase).createDirectoryAndParents(); + } catch (IOException e) { + throw new IllegalStateException("Unable to create RemoteActionFileSystem output path", e); + } } @VisibleForTesting @@ -123,11 +131,18 @@ void flush() throws IOException { PathFragment execPath = entry.getKey(); PathFragment path = execRoot.getRelative(execPath); Artifact output = entry.getValue(); + + PathFragment symlinkTargetExecPath = symlinkToTargetExecPath.get(path); + if (output.isTreeArtifact()) { if (remoteOutputTree.exists(path)) { SpecialArtifact parent = (SpecialArtifact) output; TreeArtifactValue.Builder tree = TreeArtifactValue.newBuilder(parent); + if (symlinkTargetExecPath != null) { + tree.setSymlinkTargetExecPath(symlinkTargetExecPath); + } + // TODO: Check directory content on the local fs to support mixed tree. TreeArtifactValue.visitTree( remoteOutputTree.getPath(path), @@ -141,7 +156,7 @@ void flush() throws IOException { if (remoteFile != null) { TreeFileArtifact child = TreeFileArtifact.createTreeOutput(parent, parentRelativePath); - tree.putChild(child, createRemoteMetadata(remoteFile)); + tree.putChild(child, createRemoteMetadata(remoteFile, /* symlinkTargetExecPath= */ null)); } }); @@ -151,18 +166,19 @@ void flush() throws IOException { RemoteFileInfo remoteFile = remoteOutputTree.getRemoteFileInfo(path, /* followSymlinks= */ true); if (remoteFile != null) { - metadataInjector.injectFile(output, createRemoteMetadata(remoteFile)); + metadataInjector.injectFile(output, createRemoteMetadata(remoteFile, symlinkTargetExecPath)); } } } } - private RemoteFileArtifactValue createRemoteMetadata(RemoteFileInfo remoteFile) { + private RemoteFileArtifactValue createRemoteMetadata(RemoteFileInfo remoteFile, PathFragment symlinkTargetExecPath) { return RemoteFileArtifactValue.create( remoteFile.getFastDigest(), remoteFile.getSize(), /* locationIndex= */ 1, - remoteFile.getActionId()); + remoteFile.getActionId(), + symlinkTargetExecPath); } @Override @@ -285,11 +301,47 @@ protected PathFragment readSymbolicLink(PathFragment path) throws IOException { @Override protected void createSymbolicLink(PathFragment linkPath, PathFragment targetFragment) throws IOException { - /* - * TODO(buchgr): Optimize the case where we are creating a symlink to a remote output. This does - * add a non-trivial amount of complications though (as symlinks tend to do). - */ - downloadFileIfRemote(execRoot.getRelative(targetFragment)); + if (!linkPath.startsWith(outputBase)) { + throw new IOException("RemoteActionFileSystem cannot createSymbolicLink outside the output base"); + } + + Artifact output = outputMapping.get(linkPath.relativeTo(execRoot)); + checkNotNull(output, "RemoteActionFileSystem can only create a symlink as an action output"); + + // If the output is a regular file or tree artifact, it has "copy" semantics, i.e., its contents + // are identical to the target artifact. + // + // When the target artifact is stored remotely, instead of creating a symlink in the local + // filesystem (which would necessarily dangle), we inject a special type of metadata that is + // later used by the input prefetcher to download the original artifact and put the symlink in + // place. + // + // Note that this implies createSymbolicLink() followed by readSymbolicLink() won't round trip, + // but that's ok as internal actions aren't expected to read the symlinks they create. + if (output.isTreeArtifact()) { + TreeArtifactValue metadata = getRemoteInputTreeMetadata(targetFragment); + if (metadata != null) { + checkState(targetFragment.isAbsolute()); + for (Map.Entry entry : metadata.getChildValues().entrySet()) { + PathFragment childExecPath = linkPath.getRelative(entry.getKey().getParentRelativePath()); + RemoteFileArtifactValue childMetadata = (RemoteFileArtifactValue) entry.getValue(); + injectRemoteFile(childExecPath, childMetadata.getDigest(), childMetadata.getSize(), childMetadata.getActionId()); + } + symlinkToTargetExecPath.put(linkPath, metadata.getSymlinkTargetExecPath().orElse(targetFragment.relativeTo(execRoot))); + return; + } + } else if (!output.isSymlink()) { + RemoteFileArtifactValue metadata = getRemoteInputMetadata(targetFragment); + if (metadata != null) { + checkState(targetFragment.isAbsolute()); + injectRemoteFile(linkPath, metadata.getDigest(), metadata.getSize(), metadata.getActionId()); + symlinkToTargetExecPath.put(linkPath, metadata.getSymlinkTargetExecPath().orElse(targetFragment.relativeTo(execRoot))); + return; + } + } + + // If the output is a symlink, or if the target artifact isn't stored remotely, fall back to + // creating an actual symlink. super.createSymbolicLink(linkPath, targetFragment); } @@ -432,12 +484,26 @@ private RemoteFileArtifactValue getRemoteInputMetadata(PathFragment path) { RemoteFileInfo remoteFile = remoteOutputTree.getRemoteFileInfo(path, /* followSymlinks= */ true); if (remoteFile != null) { - return createRemoteMetadata(remoteFile); + return createRemoteMetadata(remoteFile, /* symlinkToTargetExecPath= */ null); } return null; } + @Nullable + private TreeArtifactValue getRemoteInputTreeMetadata(PathFragment path) { + if (!path.startsWith(outputBase)) { + return null; + } + PathFragment execPath = path.relativeTo(execRoot); + TreeArtifactValue m = inputArtifactData.getTreeMetadata(execPath); + // TODO: Handle partially remote tree artifacts. + if (m != null && m.isEntirelyRemote()) { + return m; + } + return null; + } + private void downloadFileIfRemote(PathFragment path) throws IOException { FileArtifactValue m = getRemoteInputMetadata(path); if (m != null) { @@ -504,7 +570,7 @@ public boolean createDirectory(PathFragment path) throws IOException { * -------------------- TODO(buchgr): Not yet implemented -------------------- * * The below methods have not (yet) been properly implemented due to time constraints mostly and - * with little risk as they currently don't seem to be used by native actions in Bazel. However, + * with little risk as they currently don't seem to be used by internal actions in Bazel. However, * before making the --experimental_remote_download_outputs flag non-experimental we should make * sure to fully implement this file system. */ diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java index 6f429b43073876..258d6300517da4 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java @@ -49,6 +49,7 @@ import java.util.Comparator; import java.util.HashMap; import java.util.Map; +import java.util.Objects; import java.util.Optional; import javax.annotation.Nullable; @@ -179,6 +180,7 @@ public static ArchivedRepresentation create( EMPTY_MAP, 0L, /*archivedRepresentation=*/ null, + /*symlinkTargetExecPath=*/ null, /*entirelyRemote=*/ false); private final byte[] digest; @@ -191,16 +193,28 @@ public static ArchivedRepresentation create( */ @Nullable private final ArchivedRepresentation archivedRepresentation; + /** + * Optional symlink target path. + * + *

If set, this artifact is a copy of another artifact. It is materialized in the local + * filesystem as a symlink to the original artifact, whose contents live at this location. This is + * used to implement zero-cost copies of remotely stored artifacts. + */ + @Nullable private final PathFragment symlinkTargetExecPath; + private final boolean entirelyRemote; /** A FileArtifactValue used to stand in for a TreeArtifactValue. */ private static final class TreeArtifactCompositeFileArtifactValue extends FileArtifactValue { private final byte[] digest; private final boolean isRemote; + @Nullable private final PathFragment symlinkTargetExecPath; - TreeArtifactCompositeFileArtifactValue(byte[] digest, boolean isRemote) { + TreeArtifactCompositeFileArtifactValue( + byte[] digest, boolean isRemote, @Nullable PathFragment symlinkTargetExecPath) { this.digest = digest; this.isRemote = isRemote; + this.symlinkTargetExecPath = symlinkTargetExecPath; } @Override @@ -212,12 +226,13 @@ public boolean equals(Object o) { return false; } TreeArtifactCompositeFileArtifactValue that = (TreeArtifactCompositeFileArtifactValue) o; - return Arrays.equals(digest, that.digest); + return Arrays.equals(digest, that.digest) + && Objects.equals(symlinkTargetExecPath, that.symlinkTargetExecPath); } @Override public int hashCode() { - return Arrays.hashCode(digest); + return Objects.hash(Arrays.hashCode(digest), symlinkTargetExecPath); } @Override @@ -256,6 +271,7 @@ public long getModifiedTime() { public String toString() { return MoreObjects.toStringHelper(this) .add("digest", BaseEncoding.base16().lowerCase().encode(digest)) + .add("symlinkTargetExecPath", symlinkTargetExecPath) .toString(); } @@ -268,6 +284,11 @@ protected boolean couldBeModifiedByMetadata(FileArtifactValue o) { public boolean isRemote() { return isRemote; } + + @Override + public Optional getSymlinkTargetExecPath() { + return Optional.ofNullable(symlinkTargetExecPath); + } } private TreeArtifactValue( @@ -275,16 +296,18 @@ private TreeArtifactValue( ImmutableSortedMap childData, long totalChildSize, @Nullable ArchivedRepresentation archivedRepresentation, + @Nullable PathFragment symlinkTargetExecPath, boolean entirelyRemote) { this.digest = digest; this.childData = childData; this.totalChildSize = totalChildSize; this.archivedRepresentation = archivedRepresentation; + this.symlinkTargetExecPath = symlinkTargetExecPath; this.entirelyRemote = entirelyRemote; } public FileArtifactValue getMetadata() { - return new TreeArtifactCompositeFileArtifactValue(digest, entirelyRemote); + return new TreeArtifactCompositeFileArtifactValue(digest, entirelyRemote, symlinkTargetExecPath); } ImmutableSet getChildPaths() { @@ -311,6 +334,11 @@ public Optional getArchivedRepresentation() { return Optional.ofNullable(archivedRepresentation); } + /** Return symlinkTarget path (if present). */ + public Optional getSymlinkTargetExecPath() { + return Optional.ofNullable(symlinkTargetExecPath); + } + @VisibleForTesting @Nullable public ArchivedTreeArtifact getArchivedArtifactForTesting() { @@ -373,6 +401,7 @@ public String toString() { .add("digest", digest) .add("childData", childData) .add("archivedRepresentation", archivedRepresentation) + .add("symlinkTargetExecPath", symlinkTargetExecPath) .toString(); } @@ -392,7 +421,12 @@ public String toString() { private static TreeArtifactValue createMarker(String toStringRepresentation) { return new TreeArtifactValue( - null, EMPTY_MAP, 0L, /*archivedRepresentation=*/ null, /*entirelyRemote=*/ false) { + null, + EMPTY_MAP, + 0L, + /*archivedRepresentation=*/ null, + /*symlinkTargetExecPath=*/ null, + /*entirelyRemote=*/ false) { @Override public ImmutableSet getChildren() { throw new UnsupportedOperationException(toString()); @@ -527,6 +561,7 @@ public static final class Builder { private final ImmutableSortedMap.Builder childData = childDataBuilder(); private ArchivedRepresentation archivedRepresentation; + private PathFragment symlinkTargetExecPath; private final SpecialArtifact parent; Builder(SpecialArtifact parent) { @@ -593,11 +628,19 @@ public Builder setArchivedRepresentation(ArchivedRepresentation archivedRepresen return this; } + @CanIgnoreReturnValue + public Builder setSymlinkTargetExecPath(PathFragment symlinkTargetExecPath) { + this.symlinkTargetExecPath = symlinkTargetExecPath; + return this; + } + /** Builds the final {@link TreeArtifactValue}. */ public TreeArtifactValue build() { ImmutableSortedMap finalChildData = childData.buildOrThrow(); - if (finalChildData.isEmpty() && archivedRepresentation == null) { + if (finalChildData.isEmpty() + && archivedRepresentation == null + && symlinkTargetExecPath == null) { return EMPTY; } @@ -629,6 +672,7 @@ public TreeArtifactValue build() { finalChildData, totalChildSize, archivedRepresentation, + symlinkTargetExecPath, entirelyRemote); } } diff --git a/src/test/java/com/google/devtools/build/lib/actions/ActionCacheCheckerTest.java b/src/test/java/com/google/devtools/build/lib/actions/ActionCacheCheckerTest.java index 2244795ea915aa..a270a7b84b0f43 100644 --- a/src/test/java/com/google/devtools/build/lib/actions/ActionCacheCheckerTest.java +++ b/src/test/java/com/google/devtools/build/lib/actions/ActionCacheCheckerTest.java @@ -447,14 +447,21 @@ public void testDeletedConstantMetadataOutputCausesReexecution() throws Exceptio } private RemoteFileArtifactValue createRemoteFileMetadata(String content) { + return createRemoteFileMetadata(content, /* symlinkTargetExecPath= */ null); + } + + private RemoteFileArtifactValue createRemoteFileMetadata( + String content, @Nullable PathFragment symlinkTargetExecPath) { byte[] bytes = content.getBytes(UTF_8); - return RemoteFileArtifactValue.create(digest(bytes), bytes.length, 1, "action-id"); + return RemoteFileArtifactValue.create( + digest(bytes), bytes.length, 1, "action-id", symlinkTargetExecPath); } private static TreeArtifactValue createTreeMetadata( SpecialArtifact parent, ImmutableMap children, - Optional archivedArtifactValue) { + Optional archivedArtifactValue, + Optional symlinkTargetExecPath) { TreeArtifactValue.Builder builder = TreeArtifactValue.newBuilder(parent); for (Map.Entry entry : children.entrySet()) { builder.putChild( @@ -466,6 +473,7 @@ private static TreeArtifactValue createTreeMetadata( builder.setArchivedRepresentation( TreeArtifactValue.ArchivedRepresentation.create(artifact, metadata)); }); + symlinkTargetExecPath.ifPresent(execPath -> builder.setSymlinkTargetExecPath(execPath)); return builder.build(); } @@ -616,6 +624,30 @@ public void saveOutputMetadata_localMetadataIsSameAsRemoteMetadata_cached() thro assertThat(entry.getOutputFile(output)).isEqualTo(createRemoteFileMetadata(content)); } + @Test + public void saveOutputMetadata_localMetadataIsSameAsRemoteMetadata_withTargetExecPath_cached() + throws Exception { + cacheChecker = createActionCacheChecker(/*storeOutputMetadata=*/ true); + Artifact output = createArtifact(artifactRoot, "bin/dummy"); + String content = "content"; + PathFragment symlinkTargetExecPath = PathFragment.create("/target/path"); + Action action = + new InjectOutputFileMetadataAction( + output, createRemoteFileMetadata(content, symlinkTargetExecPath)); + runAction(action); + assertStatistics(0, new MissDetailsBuilder().set(MissReason.NOT_CACHED, 1).build()); + + writeContentAsLatin1(output.getPath(), content); + // Cached since local metadata is same as remote metadata + runAction(action); + + assertStatistics(1, new MissDetailsBuilder().set(MissReason.NOT_CACHED, 1).build()); + ActionCache.Entry entry = cache.get(output.getExecPathString()); + assertThat(entry).isNotNull(); + assertThat(entry.getOutputFile(output)) + .isEqualTo(createRemoteFileMetadata(content, symlinkTargetExecPath)); + } + @Test public void saveOutputMetadata_localMetadataIsDifferentFromRemoteMetadata_notCached() throws Exception { @@ -655,7 +687,7 @@ public void saveOutputMetadata_treeMetadata_remoteFileMetadataSaved() throws Exc "file2", createRemoteFileMetadata("content2")); Action action = new InjectOutputTreeMetadataAction( - output, createTreeMetadata(output, children, Optional.empty())); + output, createTreeMetadata(output, children, Optional.empty(), Optional.empty())); runAction(action); @@ -663,7 +695,8 @@ public void saveOutputMetadata_treeMetadata_remoteFileMetadataSaved() throws Exc ActionCache.Entry entry = cache.get(output.getExecPathString()); assertThat(entry).isNotNull(); assertThat(entry.getOutputTree(output)) - .isEqualTo(SerializableTreeArtifactValue.create(children, Optional.empty())); + .isEqualTo( + SerializableTreeArtifactValue.create(children, Optional.empty(), Optional.empty())); assertStatistics(0, new MissDetailsBuilder().set(MissReason.NOT_CACHED, 1).build()); } @@ -676,7 +709,38 @@ public void saveOutputMetadata_treeMetadata_remoteArchivedArtifactSaved() throws new InjectOutputTreeMetadataAction( output, createTreeMetadata( - output, ImmutableMap.of(), Optional.of(createRemoteFileMetadata("content")))); + output, + ImmutableMap.of(), + Optional.of(createRemoteFileMetadata("content")), + Optional.empty())); + + runAction(action); + + assertThat(output.getPath().exists()).isFalse(); + ActionCache.Entry entry = cache.get(output.getExecPathString()); + assertThat(entry).isNotNull(); + assertThat(entry.getOutputTree(output)) + .isEqualTo( + SerializableTreeArtifactValue.create( + ImmutableMap.of(), + Optional.of(createRemoteFileMetadata("content")), + Optional.empty())); + assertStatistics(0, new MissDetailsBuilder().set(MissReason.NOT_CACHED, 1).build()); + } + + @Test + public void saveOutputMetadata_treeMetadata_symlinkTargetExecPathSaved() throws Exception { + cacheChecker = createActionCacheChecker(/*storeOutputMetadata=*/ true); + SpecialArtifact output = + createTreeArtifactWithGeneratingAction(artifactRoot, PathFragment.create("bin/dummy")); + Action action = + new InjectOutputTreeMetadataAction( + output, + createTreeMetadata( + output, + ImmutableMap.of(), + Optional.empty(), + Optional.of(PathFragment.create("/target/path")))); runAction(action); @@ -686,7 +750,9 @@ public void saveOutputMetadata_treeMetadata_remoteArchivedArtifactSaved() throws assertThat(entry.getOutputTree(output)) .isEqualTo( SerializableTreeArtifactValue.create( - ImmutableMap.of(), Optional.of(createRemoteFileMetadata("content")))); + ImmutableMap.of(), + Optional.empty(), + Optional.of(PathFragment.create("/target/path")))); assertStatistics(0, new MissDetailsBuilder().set(MissReason.NOT_CACHED, 1).build()); } @@ -697,7 +763,8 @@ public void saveOutputMetadata_emptyTreeMetadata_notSaved() throws Exception { createTreeArtifactWithGeneratingAction(artifactRoot, PathFragment.create("bin/dummy")); Action action = new InjectOutputTreeMetadataAction( - output, createTreeMetadata(output, ImmutableMap.of(), Optional.empty())); + output, + createTreeMetadata(output, ImmutableMap.of(), Optional.empty(), Optional.empty())); MetadataHandler metadataHandler = new FakeMetadataHandler(); runAction(action); @@ -733,7 +800,7 @@ public void saveOutputMetadata_treeMetadata_localFileMetadataNotSaved() throws E fileSystem.getPath("/file2").delete(); Action action = new InjectOutputTreeMetadataAction( - output, createTreeMetadata(output, children, Optional.empty())); + output, createTreeMetadata(output, children, Optional.empty(), Optional.empty())); runAction(action); @@ -743,7 +810,9 @@ public void saveOutputMetadata_treeMetadata_localFileMetadataNotSaved() throws E assertThat(entry.getOutputTree(output)) .isEqualTo( SerializableTreeArtifactValue.create( - ImmutableMap.of("file1", createRemoteFileMetadata("content1")), Optional.empty())); + ImmutableMap.of("file1", createRemoteFileMetadata("content1")), + Optional.empty(), + Optional.empty())); assertStatistics(0, new MissDetailsBuilder().set(MissReason.NOT_CACHED, 1).build()); } @@ -759,7 +828,8 @@ public void saveOutputMetadata_treeMetadata_localArchivedArtifactNotSaved() thro createTreeMetadata( output, ImmutableMap.of(), - Optional.of(FileArtifactValue.createForTesting(fileSystem.getPath("/archive"))))); + Optional.of(FileArtifactValue.createForTesting(fileSystem.getPath("/archive"))), + Optional.empty())); fileSystem.getPath("/archive").delete(); runAction(action); @@ -782,7 +852,7 @@ public void saveOutputMetadata_treeMetadata_remoteFileMetadataLoaded() throws Ex "file2", createRemoteFileMetadata("content2")); Action action = new InjectOutputTreeMetadataAction( - output, createTreeMetadata(output, children, Optional.empty())); + output, createTreeMetadata(output, children, Optional.empty(), Optional.empty())); MetadataHandler metadataHandler = new FakeMetadataHandler(); runAction(action); @@ -797,7 +867,8 @@ public void saveOutputMetadata_treeMetadata_remoteFileMetadataLoaded() throws Ex /*remoteDefaultPlatformProperties=*/ ImmutableMap.of(), /*isRemoteCacheEnabled=*/ true); - TreeArtifactValue expectedMetadata = createTreeMetadata(output, children, Optional.empty()); + TreeArtifactValue expectedMetadata = + createTreeMetadata(output, children, Optional.empty(), Optional.empty()); assertThat(token).isNull(); assertThat(output.getPath().exists()).isFalse(); ActionCache.Entry entry = cache.get(output.getExecPathString()); @@ -823,8 +894,8 @@ public void saveOutputMetadata_treeMetadata_localFileMetadataLoaded() throws Exc Action action = new InjectOutputTreeMetadataAction( output, - createTreeMetadata(output, children1, Optional.empty()), - createTreeMetadata(output, children2, Optional.empty())); + createTreeMetadata(output, children1, Optional.empty(), Optional.empty()), + createTreeMetadata(output, children2, Optional.empty(), Optional.empty())); MetadataHandler metadataHandler = new FakeMetadataHandler(); runAction(action); @@ -845,6 +916,7 @@ public void saveOutputMetadata_treeMetadata_localFileMetadataLoaded() throws Exc ImmutableMap.of( "file1", createRemoteFileMetadata("content1"), "file2", createRemoteFileMetadata("modified_remote")), + Optional.empty(), Optional.empty()); ActionCache.Entry entry = cache.get(output.getExecPathString()); assertThat(entry).isNotNull(); @@ -862,9 +934,15 @@ public void saveOutputMetadata_treeMetadata_localArchivedArtifactLoaded() throws new InjectOutputTreeMetadataAction( output, createTreeMetadata( - output, ImmutableMap.of(), Optional.of(createRemoteFileMetadata("content"))), + output, + ImmutableMap.of(), + Optional.of(createRemoteFileMetadata("content")), + Optional.empty()), createTreeMetadata( - output, ImmutableMap.of(), Optional.of(createRemoteFileMetadata("modified")))); + output, + ImmutableMap.of(), + Optional.of(createRemoteFileMetadata("modified")), + Optional.empty())); MetadataHandler metadataHandler = new FakeMetadataHandler(); runAction(action); @@ -881,7 +959,10 @@ public void saveOutputMetadata_treeMetadata_localArchivedArtifactLoaded() throws assertThat(output.getPath().exists()).isFalse(); TreeArtifactValue expectedMetadata = createTreeMetadata( - output, ImmutableMap.of(), Optional.of(createRemoteFileMetadata("modified"))); + output, + ImmutableMap.of(), + Optional.of(createRemoteFileMetadata("modified")), + Optional.empty()); ActionCache.Entry entry = cache.get(output.getExecPathString()); assertThat(entry).isNotNull(); assertThat(entry.getOutputTree(output)) @@ -908,7 +989,63 @@ public void saveOutputMetadata_treeMetadataWithSameLocalFileMetadata_cached() th "file2", createRemoteFileMetadata("content2")); Action action = new InjectOutputTreeMetadataAction( - output, createTreeMetadata(output, children, Optional.empty())); + output, createTreeMetadata(output, children, Optional.empty(), Optional.empty())); + MetadataHandler metadataHandler = new FakeMetadataHandler(); + + runAction(action); + writeContentAsLatin1(output.getPath().getRelative("file1"), "content1"); + // Cache hit + Token token = + cacheChecker.getTokenIfNeedToExecute( + action, + /*resolvedCacheArtifacts=*/ null, + /*clientEnv=*/ ImmutableMap.of(), + /*handler=*/ null, + metadataHandler, + /*artifactExpander=*/ null, + /*remoteDefaultPlatformProperties=*/ ImmutableMap.of(), + /*isRemoteCacheEnabled=*/ true); + + assertThat(token).isNull(); + assertStatistics(1, new MissDetailsBuilder().set(MissReason.NOT_CACHED, 1).build()); + assertThat(output.getPath().exists()).isTrue(); + ActionCache.Entry entry = cache.get(output.getExecPathString()); + assertThat(entry).isNotNull(); + assertThat(entry.getOutputTree(output)) + .isEqualTo( + SerializableTreeArtifactValue.create(children, Optional.empty(), Optional.empty())); + assertThat(metadataHandler.getTreeArtifactValue(output)) + .isEqualTo( + createTreeMetadata( + output, + ImmutableMap.of( + "file1", + FileArtifactValue.createForTesting(output.getPath().getRelative("file1")), + "file2", + createRemoteFileMetadata("content2")), + Optional.empty(), + Optional.empty())); + } + + @Test + public void + saveOutputMetadata_treeMetadataWithSameLocalFileMetadata_withSymlinkTargetExecPath_cached() + throws Exception { + cacheChecker = createActionCacheChecker(/*storeOutputMetadata=*/ true); + SpecialArtifact output = + createTreeArtifactWithGeneratingAction(artifactRoot, PathFragment.create("bin/dummy")); + ImmutableMap children = + ImmutableMap.of( + "file1", createRemoteFileMetadata("content1"), + "file2", createRemoteFileMetadata("content2")); + Action action = + new InjectOutputTreeMetadataAction( + output, + createTreeMetadata( + output, + children, + Optional.empty(), + Optional.of(PathFragment.create("/target/path")))); MetadataHandler metadataHandler = new FakeMetadataHandler(); runAction(action); @@ -931,7 +1068,9 @@ public void saveOutputMetadata_treeMetadataWithSameLocalFileMetadata_cached() th ActionCache.Entry entry = cache.get(output.getExecPathString()); assertThat(entry).isNotNull(); assertThat(entry.getOutputTree(output)) - .isEqualTo(SerializableTreeArtifactValue.create(children, Optional.empty())); + .isEqualTo( + SerializableTreeArtifactValue.create( + children, Optional.empty(), Optional.of(PathFragment.create("/target/path")))); assertThat(metadataHandler.getTreeArtifactValue(output)) .isEqualTo( createTreeMetadata( @@ -941,6 +1080,7 @@ public void saveOutputMetadata_treeMetadataWithSameLocalFileMetadata_cached() th FileArtifactValue.createForTesting(output.getPath().getRelative("file1")), "file2", createRemoteFileMetadata("content2")), + Optional.empty(), Optional.empty())); } @@ -954,7 +1094,10 @@ public void saveOutputMetadata_treeMetadataWithSameLocalArchivedArtifact_cached( new InjectOutputTreeMetadataAction( output, createTreeMetadata( - output, ImmutableMap.of(), Optional.of(createRemoteFileMetadata("content")))); + output, + ImmutableMap.of(), + Optional.of(createRemoteFileMetadata("content")), + Optional.empty())); MetadataHandler metadataHandler = new FakeMetadataHandler(); runAction(action); @@ -966,7 +1109,10 @@ public void saveOutputMetadata_treeMetadataWithSameLocalArchivedArtifact_cached( assertThat(output.getPath().exists()).isFalse(); TreeArtifactValue expectedMetadata = createTreeMetadata( - output, ImmutableMap.of(), Optional.of(createRemoteFileMetadata("content"))); + output, + ImmutableMap.of(), + Optional.of(createRemoteFileMetadata("content")), + Optional.empty()); ActionCache.Entry entry = cache.get(output.getExecPathString()); assertThat(entry).isNotNull(); assertThat(entry.getOutputTree(output)) diff --git a/src/test/java/com/google/devtools/build/lib/actions/ActionInputMapTest.java b/src/test/java/com/google/devtools/build/lib/actions/ActionInputMapTest.java index 62b1e2df951bf4..d9a76aa2edefb4 100644 --- a/src/test/java/com/google/devtools/build/lib/actions/ActionInputMapTest.java +++ b/src/test/java/com/google/devtools/build/lib/actions/ActionInputMapTest.java @@ -109,6 +109,7 @@ public void clear_removesAllElements() { @Test public void putTreeArtifact_addsEmptyTreeArtifact() { SpecialArtifact tree = createTreeArtifact("tree"); + TreeArtifactValue treeValue = TreeArtifactValue.empty(); map.putTreeArtifact(tree, TreeArtifactValue.empty(), /*depOwner=*/ null); diff --git a/src/test/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCacheTest.java b/src/test/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCacheTest.java index 6bb87ee6331d28..725f4bcdc8ab3c 100644 --- a/src/test/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCacheTest.java +++ b/src/test/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCacheTest.java @@ -36,6 +36,7 @@ import java.nio.charset.StandardCharsets; import java.util.Map; import java.util.Optional; +import javax.annotation.Nullable; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -194,7 +195,8 @@ private FileArtifactValue createLocalMetadata(Artifact artifact, String content) return FileArtifactValue.createForTesting(artifact.getPath()); } - private RemoteFileArtifactValue createRemoteMetadata(Artifact artifact, String content) { + private RemoteFileArtifactValue createRemoteMetadata( + Artifact artifact, String content, @Nullable PathFragment symlinkTargetExecPath) { byte[] bytes = content.getBytes(StandardCharsets.UTF_8); byte[] digest = artifact @@ -204,13 +206,19 @@ private RemoteFileArtifactValue createRemoteMetadata(Artifact artifact, String c .getHashFunction() .hashBytes(bytes) .asBytes(); - return RemoteFileArtifactValue.create(digest, bytes.length, 1, "action-id"); + return RemoteFileArtifactValue.create( + digest, bytes.length, 1, "action-id", symlinkTargetExecPath); + } + + private RemoteFileArtifactValue createRemoteMetadata(Artifact artifact, String content) { + return createRemoteMetadata(artifact, content, /* symlinkTargetExecPath= */ null); } private TreeArtifactValue createTreeMetadata( SpecialArtifact parent, ImmutableMap children, - Optional archivedArtifactValue) { + Optional archivedArtifactValue, + Optional symlinkTargetExecPath) { TreeArtifactValue.Builder builder = TreeArtifactValue.newBuilder(parent); for (Map.Entry entry : children.entrySet()) { builder.putChild( @@ -222,6 +230,9 @@ private TreeArtifactValue createTreeMetadata( builder.setArchivedRepresentation( TreeArtifactValue.ArchivedRepresentation.create(artifact, metadata)); }); + if (symlinkTargetExecPath.isPresent()) { + builder.setSymlinkTargetExecPath(symlinkTargetExecPath.get()); + } return builder.build(); } @@ -239,6 +250,21 @@ public void putAndGet_savesRemoteFileMetadata() { assertThat(entry.getOutputFile(artifact)).isEqualTo(metadata); } + @Test + public void putAndGet_savesRemoteFileMetadata_withSymlinkTargetExecPath() { + String key = "key"; + ActionCache.Entry entry = new ActionCache.Entry(key, ImmutableMap.of(), false); + Artifact artifact = ActionsTestUtil.DUMMY_ARTIFACT; + RemoteFileArtifactValue metadata = + createRemoteMetadata(artifact, "content", PathFragment.create("/execroot/some/path")); + entry.addOutputFile(artifact, metadata, /*saveFileMetadata=*/ true); + + cache.put(key, entry); + entry = cache.get(key); + + assertThat(entry.getOutputFile(artifact)).isEqualTo(metadata); + } + @Test public void putAndGet_ignoresLocalFileMetadata() throws IOException { String key = "key"; @@ -274,6 +300,7 @@ public void putAndGet_treeMetadata_onlySavesRemoteFileMetadata() throws IOExcept Artifact.TreeFileArtifact.createTreeOutput( artifact, PathFragment.create("file2")), "content2")), + Optional.empty(), Optional.empty()); entry.addOutputTree(artifact, metadata, /* saveTreeMetadata= */ true); @@ -289,6 +316,7 @@ public void putAndGet_treeMetadata_onlySavesRemoteFileMetadata() throws IOExcept Artifact.TreeFileArtifact.createTreeOutput( artifact, PathFragment.create("file1")), "content1")), + Optional.empty(), Optional.empty())); } @@ -301,7 +329,10 @@ public void putAndGet_treeMetadata_savesRemoteArchivedArtifact() { artifactRoot, PathFragment.create("bin/dummy")); TreeArtifactValue metadata = createTreeMetadata( - artifact, ImmutableMap.of(), Optional.of(createRemoteMetadata(artifact, "content"))); + artifact, + ImmutableMap.of(), + Optional.of(createRemoteMetadata(artifact, "content")), + Optional.empty()); entry.addOutputTree(artifact, metadata, /* saveTreeMetadata= */ true); cache.put(key, entry); @@ -310,7 +341,9 @@ public void putAndGet_treeMetadata_savesRemoteArchivedArtifact() { assertThat(entry.getOutputTree(artifact)) .isEqualTo( SerializableTreeArtifactValue.create( - ImmutableMap.of(), Optional.of(createRemoteMetadata(artifact, "content")))); + ImmutableMap.of(), + Optional.of(createRemoteMetadata(artifact, "content")), + Optional.empty())); } @Test @@ -326,7 +359,8 @@ public void putAndGet_treeMetadata_ignoresLocalArchivedArtifact() throws IOExcep ImmutableMap.of(), Optional.of( createLocalMetadata( - ActionsTestUtil.createArtifact(artifactRoot, "bin/archive"), "content"))); + ActionsTestUtil.createArtifact(artifactRoot, "bin/archive"), "content")), + Optional.empty()); entry.addOutputTree(artifact, metadata, /* saveTreeMetadata= */ true); cache.put(key, entry); @@ -335,6 +369,28 @@ public void putAndGet_treeMetadata_ignoresLocalArchivedArtifact() throws IOExcep assertThat(entry.getOutputTree(artifact)).isNull(); } + @Test + public void putAndGet_treeMetadata_savesSymlinkTargetExecPath() { + String key = "key"; + PathFragment symlinkTargetExecPath = PathFragment.create("/execroot/some/path"); + ActionCache.Entry entry = new ActionCache.Entry(key, ImmutableMap.of(), false); + SpecialArtifact artifact = + ActionsTestUtil.createTreeArtifactWithGeneratingAction( + artifactRoot, PathFragment.create("bin/dummy")); + TreeArtifactValue metadata = + createTreeMetadata( + artifact, ImmutableMap.of(), Optional.empty(), Optional.of(symlinkTargetExecPath)); + entry.addOutputTree(artifact, metadata, /* saveTreeMetadata= */ true); + + cache.put(key, entry); + entry = cache.get(key); + + assertThat(entry.getOutputTree(artifact)) + .isEqualTo( + SerializableTreeArtifactValue.create( + ImmutableMap.of(), Optional.empty(), Optional.of(symlinkTargetExecPath))); + } + private static void assertKeyEquals(ActionCache cache1, ActionCache cache2, String key) { Object entry = cache1.get(key); assertThat(entry).isNotNull(); diff --git a/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java b/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java index 46326b223b03b5..ff64c7ee4923a1 100644 --- a/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java +++ b/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java @@ -267,6 +267,18 @@ public static SpecialArtifact createTreeArtifactWithGeneratingAction( return treeArtifact; } + public static SpecialArtifact createTreeArtifactWithGeneratingAction( + ArtifactRoot root, String path) { + return createTreeArtifactWithGeneratingAction( + root, root.getExecPath().getRelative(PathFragment.create(path))); + } + + public static SpecialArtifact createUnresolvedSymlinkArtifact( + ArtifactRoot root, String execPath) { + return createUnresolvedSymlinkArtifactWithExecPath( + root, root.getExecPath().getRelative(execPath)); + } + public static SpecialArtifact createUnresolvedSymlinkArtifactWithExecPath( ArtifactRoot root, PathFragment execPath) { return SpecialArtifact.create( diff --git a/src/test/java/com/google/devtools/build/lib/remote/ActionInputPrefetcherTestBase.java b/src/test/java/com/google/devtools/build/lib/remote/ActionInputPrefetcherTestBase.java index 8865a9bc7a1949..9ffed0bdecfdfc 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/ActionInputPrefetcherTestBase.java +++ b/src/test/java/com/google/devtools/build/lib/remote/ActionInputPrefetcherTestBase.java @@ -42,6 +42,8 @@ import com.google.devtools.build.lib.remote.common.BulkTransferException; import com.google.devtools.build.lib.remote.util.StaticMetadataProvider; import com.google.devtools.build.lib.remote.util.TempPathGenerator; +import com.google.devtools.build.lib.skyframe.TreeArtifactValue; +import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.vfs.DigestHashFunction; import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.FileSystemUtils; @@ -55,6 +57,7 @@ import java.util.concurrent.Semaphore; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Supplier; +import javax.annotation.Nullable; import org.junit.Before; import org.junit.Test; @@ -80,15 +83,22 @@ public void setUp() throws IOException { } protected Artifact createRemoteArtifact( - Artifact a, + String pathFragment, String contents, + @Nullable PathFragment symlinkTargetExecPath, Map metadata, Map cas) { + Path p = artifactRoot.getRoot().getRelative(pathFragment); + Artifact a = ActionsTestUtil.createArtifact(artifactRoot, p); byte[] contentsBytes = contents.getBytes(UTF_8); HashCode hashCode = HASH_FUNCTION.getHashFunction().hashBytes(contentsBytes); - FileArtifactValue f = + RemoteFileArtifactValue f = RemoteFileArtifactValue.create( - hashCode.asBytes(), contentsBytes.length, /* locationIndex= */ 1, "action-id"); + hashCode.asBytes(), + contentsBytes.length, + /* locationIndex= */ 1, + "action-id", + symlinkTargetExecPath); metadata.put(a, f); cas.put(hashCode, contentsBytes); return a; @@ -99,9 +109,49 @@ protected Artifact createRemoteArtifact( String contents, Map metadata, Map cas) { - Path p = artifactRoot.getRoot().getRelative(pathFragment); - Artifact a = ActionsTestUtil.createArtifact(artifactRoot, p); - return createRemoteArtifact(a, contents, metadata, cas); + return createRemoteArtifact( + pathFragment, contents, /* symlinkTargetExecPath= */ null, metadata, cas); + } + + protected Pair> createRemoteTreeArtifact( + String pathFragment, + Map contentMap, + @Nullable PathFragment symlinkTargetExecPath, + Map metadata, + Map cas) + throws IOException { + SpecialArtifact parent = createTreeArtifactWithGeneratingAction(artifactRoot, pathFragment); + parent.getPath().createDirectoryAndParents(); + parent.getPath().chmod(0555); + TreeArtifactValue.Builder treeBuilder = TreeArtifactValue.newBuilder(parent); + for (Map.Entry entry : contentMap.entrySet()) { + byte[] contentsBytes = entry.getValue().getBytes(UTF_8); + HashCode hashCode = HASH_FUNCTION.getHashFunction().hashBytes(contentsBytes); + TreeFileArtifact child = + TreeFileArtifact.createTreeOutput(parent, PathFragment.create(entry.getKey())); + RemoteFileArtifactValue childValue = + RemoteFileArtifactValue.create( + hashCode.asBytes(), contentsBytes.length, /* locationIndex= */ 1, "action-id"); + treeBuilder.putChild(child, childValue); + metadata.put(child, childValue); + cas.put(hashCode, contentsBytes); + } + if (symlinkTargetExecPath != null) { + treeBuilder.setSymlinkTargetExecPath(symlinkTargetExecPath); + } + TreeArtifactValue treeValue = treeBuilder.build(); + metadata.put(parent, treeValue.getMetadata()); + return Pair.of(parent, treeValue.getChildren().asList()); + } + + protected Pair> createRemoteTreeArtifact( + String pathFragment, + Map contentMap, + Map metadata, + Map cas) + throws IOException { + return createRemoteTreeArtifact( + pathFragment, contentMap, /* symlinkTargetExecPath= */ null, metadata, cas); } protected abstract AbstractActionInputPrefetcher createPrefetcher(Map cas); @@ -121,44 +171,94 @@ public void prefetchFiles_downloadRemoteFiles() throws Exception { assertReadableNonWritableAndExecutable(a1.getPath()); assertThat(FileSystemUtils.readContent(a2.getPath(), UTF_8)).isEqualTo("fizz buzz"); assertReadableNonWritableAndExecutable(a2.getPath()); - assertThat(prefetcher.downloadedFiles()).hasSize(2); - assertThat(prefetcher.downloadedFiles()).containsAtLeast(a1.getPath(), a2.getPath()); + assertThat(prefetcher.downloadedFiles()).containsExactly(a1.getPath(), a2.getPath()); assertThat(prefetcher.downloadsInProgress()).isEmpty(); } @Test - public void prefetchFiles_downloadTrees() throws Exception { + public void prefetchFiles_downloadRemoteFiles_withSymlinkTargetExecPath() throws Exception { Map metadata = new HashMap<>(); Map cas = new HashMap<>(); - SpecialArtifact parent = - createTreeArtifactWithGeneratingAction(artifactRoot, PathFragment.create("root/dir")); - parent.getPath().createDirectoryAndParents(); - parent.getPath().chmod(0555); - Artifact a1 = - createRemoteArtifact( - TreeFileArtifact.createTreeOutput(parent, "file1"), "content1", metadata, cas); - Artifact a2 = - createRemoteArtifact( - TreeFileArtifact.createTreeOutput(parent, "nested_dir/file2"), - "content2", + PathFragment targetExecPath = artifactRoot.getExecPath().getChild("target"); + Artifact a = createRemoteArtifact("file", "hello world", targetExecPath, metadata, cas); + MetadataProvider metadataProvider = new StaticMetadataProvider(metadata); + AbstractActionInputPrefetcher prefetcher = createPrefetcher(cas); + + wait(prefetcher.prefetchFiles(metadata.keySet(), metadataProvider)); + + assertThat(a.getPath().isSymbolicLink()).isTrue(); + assertThat(a.getPath().readSymbolicLink()) + .isEqualTo(execRoot.getRelative(targetExecPath).asFragment()); + assertThat(FileSystemUtils.readContent(a.getPath(), UTF_8)).isEqualTo("hello world"); + assertThat(prefetcher.downloadedFiles()) + .containsExactly(a.getPath(), execRoot.getRelative(targetExecPath)); + assertThat(prefetcher.downloadsInProgress()).isEmpty(); + } + + @Test + public void prefetchFiles_downloadRemoteTrees() throws Exception { + Map metadata = new HashMap<>(); + Map cas = new HashMap<>(); + Pair> tree = + createRemoteTreeArtifact( + "dir", + ImmutableMap.of("file1", "content1", "nested_dir/file2", "content2"), metadata, cas); + SpecialArtifact parent = tree.getFirst(); + Artifact firstChild = tree.getSecond().get(0); + Artifact secondChild = tree.getSecond().get(1); + MetadataProvider metadataProvider = new StaticMetadataProvider(metadata); AbstractActionInputPrefetcher prefetcher = createPrefetcher(cas); - wait(prefetcher.prefetchFiles(metadata.keySet(), metadataProvider)); + wait(prefetcher.prefetchFiles(tree.getSecond(), metadataProvider)); assertReadableNonWritableAndExecutable(parent.getPath()); - assertThat(FileSystemUtils.readContent(a1.getPath(), UTF_8)).isEqualTo("content1"); - assertReadableNonWritableAndExecutable(a1.getPath()); - assertThat(FileSystemUtils.readContent(a2.getPath(), UTF_8)).isEqualTo("content2"); - assertReadableNonWritableAndExecutable(a2.getPath()); - assertThat(prefetcher.downloadedFiles()).hasSize(1); + assertThat(FileSystemUtils.readContent(firstChild.getPath(), UTF_8)).isEqualTo("content1"); + assertReadableNonWritableAndExecutable(firstChild.getPath()); + assertThat(FileSystemUtils.readContent(secondChild.getPath(), UTF_8)).isEqualTo("content2"); + assertReadableNonWritableAndExecutable(secondChild.getPath()); assertThat(prefetcher.downloadedFiles()).containsExactly(parent.getPath()); assertThat(prefetcher.downloadsInProgress()).isEmpty(); assertReadableNonWritableAndExecutable(parent.getPath().getRelative("nested_dir")); } + @Test + public void prefetchFiles_downloadRemoteTrees_withSymlinkTargetExecPath() throws Exception { + Map metadata = new HashMap<>(); + Map cas = new HashMap<>(); + PathFragment targetExecPath = artifactRoot.getExecPath().getChild("target"); + Pair> tree = + createRemoteTreeArtifact( + "dir", + ImmutableMap.of("file1", "content1", "nested_dir/file2", "content2"), + targetExecPath, + metadata, + cas); + SpecialArtifact parent = tree.getFirst(); + Artifact firstChild = tree.getSecond().get(0); + Artifact secondChild = tree.getSecond().get(1); + + MetadataProvider metadataProvider = new StaticMetadataProvider(metadata); + AbstractActionInputPrefetcher prefetcher = createPrefetcher(cas); + + wait(prefetcher.prefetchFiles(tree.getSecond(), metadataProvider)); + + assertThat(parent.getPath().isSymbolicLink()).isTrue(); + assertThat(parent.getPath().readSymbolicLink()) + .isEqualTo(execRoot.getRelative(targetExecPath).asFragment()); + assertReadableNonWritableAndExecutable(parent.getPath()); + assertThat(FileSystemUtils.readContent(firstChild.getPath(), UTF_8)).isEqualTo("content1"); + assertReadableNonWritableAndExecutable(firstChild.getPath()); + assertThat(FileSystemUtils.readContent(secondChild.getPath(), UTF_8)).isEqualTo("content2"); + assertReadableNonWritableAndExecutable(secondChild.getPath()); + assertThat(prefetcher.downloadedFiles()) + .containsExactly(parent.getPath(), execRoot.getRelative(targetExecPath)); + assertThat(prefetcher.downloadsInProgress()).isEmpty(); + assertReadableNonWritableAndExecutable(parent.getPath().getRelative("nested_dir")); + } + @Test public void prefetchFiles_missingFiles_fails() throws Exception { Map metadata = new HashMap<>(); diff --git a/src/test/java/com/google/devtools/build/lib/remote/BUILD b/src/test/java/com/google/devtools/build/lib/remote/BUILD index ea01ccbce96776..9d0411ebcc121b 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/BUILD +++ b/src/test/java/com/google/devtools/build/lib/remote/BUILD @@ -78,6 +78,7 @@ java_test( "//src/main/java/com/google/devtools/build/lib/remote/util", "//src/main/java/com/google/devtools/build/lib/runtime/commands", "//src/main/java/com/google/devtools/build/lib/skyframe:tree_artifact_value", + "//src/main/java/com/google/devtools/build/lib/util", "//src/main/java/com/google/devtools/build/lib/util:abrupt_exit_exception", "//src/main/java/com/google/devtools/build/lib/util:exit_code", "//src/main/java/com/google/devtools/build/lib/util/io", diff --git a/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTest.java index 0344d476db39f4..e06b866f7b549d 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTest.java @@ -361,6 +361,277 @@ public void changeOutputMode_invalidateActions() throws Exception { } } + @Test + public void symlinkToSourceFile() throws Exception { + addRemoteModeOptions(); + addOutputModeOptions(); + + write( + "a/defs.bzl", + "def _impl(ctx):", + " if ctx.attr.chain_length < 1:", + " fail('chain_length must be > 0')", + "", + " file = ctx.file.target", + "", + " for i in range(ctx.attr.chain_length):", + " sym = ctx.actions.declare_file(ctx.label.name + '.sym' + str(i))", + " ctx.actions.symlink(output = sym, target_file = file)", + " file = sym", + "", + " out = ctx.actions.declare_file(ctx.label.name + '.out')", + " ctx.actions.run_shell(", + " inputs = [sym],", + " outputs = [out],", + " command = '[[ hello == $(cat $1) ]] && touch $2',", + " arguments = [sym.path, out.path],", + " execution_requirements = {'no-remote': ''} if ctx.attr.local else {},", + " )", + "", + " return DefaultInfo(files = depset([out]))", + "", + "my_rule = rule(", + " implementation = _impl,", + " attrs = {", + " 'target': attr.label(allow_single_file = True),", + " 'chain_length': attr.int(),", + " 'local': attr.bool(),", + " },", + ")"); + + write( + "a/BUILD", + "load(':defs.bzl', 'my_rule')", + "", + "my_rule(name = 'one_local', target = 'src.txt', local = True, chain_length = 1)", + "my_rule(name = 'two_local', target = 'src.txt', local = True, chain_length = 2)", + "my_rule(name = 'one_remote', target = 'src.txt', local = False, chain_length = 1)", + "my_rule(name = 'two_remote', target = 'src.txt', local = False, chain_length = 2)"); + + write("a/src.txt", "hello"); + + buildTarget("//a:one_local", "//a:two_local", "//a:one_remote", "//a:two_remote"); + } + + @Test + public void symlinkToGeneratedFile() throws Exception { + addRemoteModeOptions(); + addOutputModeOptions(); + + write( + "a/defs.bzl", + "def _impl(ctx):", + " if ctx.attr.chain_length < 1:", + " fail('chain_length must be > 0')", + "", + " file = ctx.actions.declare_file(ctx.label.name + '.file')", + // Use ctx.actions.run_shell instead of ctx.actions.write, so that it runs remotely. + " ctx.actions.run_shell(", + " outputs = [file],", + " command = 'echo hello > $1',", + " arguments = [file.path],", + " )", + "", + " for i in range(ctx.attr.chain_length):", + " sym = ctx.actions.declare_file(ctx.label.name + '.sym' + str(i))", + " ctx.actions.symlink(output = sym, target_file = file)", + " file = sym", + "", + " out = ctx.actions.declare_file(ctx.label.name + '.out')", + " ctx.actions.run_shell(", + " inputs = [sym],", + " outputs = [out],", + " command = '[[ hello == $(cat $1) ]] && touch $2',", + " arguments = [sym.path, out.path],", + " execution_requirements = {'no-remote': ''} if ctx.attr.local else {},", + " )", + "", + " return DefaultInfo(files = depset([out]))", + "", + "my_rule = rule(", + " implementation = _impl,", + " attrs = {", + " 'chain_length': attr.int(),", + " 'local': attr.bool(),", + " },", + ")"); + + write( + "a/BUILD", + "load(':defs.bzl', 'my_rule')", + "", + "my_rule(name = 'one_local', local = True, chain_length = 1)", + "my_rule(name = 'two_local', local = True, chain_length = 2)", + "my_rule(name = 'one_remote', local = False, chain_length = 1)", + "my_rule(name = 'two_remote', local = False, chain_length = 2)"); + + buildTarget("//a:one_local", "//a:two_local", "//a:one_remote", "//a:two_remote"); + } + + @Test + public void symlinkToDirectory() throws Exception { + addRemoteModeOptions(); + addOutputModeOptions(); + + write( + "a/defs.bzl", + "def _impl(ctx):", + " if ctx.attr.chain_length < 1:", + " fail('chain_length must be > 0')", + "", + " dir = ctx.actions.declare_directory(ctx.label.name + '.dir')", + " ctx.actions.run_shell(", + " outputs = [dir],", + " command = 'mkdir -p $1/some/path && echo hello > $1/some/path/inside.txt',", + " arguments = [dir.path],", + " )", + "", + " for i in range(ctx.attr.chain_length):", + " sym = ctx.actions.declare_directory(ctx.label.name + '.sym' + str(i))", + " ctx.actions.symlink(output = sym, target_file = dir)", + " dir = sym", + "", + " out = ctx.actions.declare_file(ctx.label.name + '.out')", + " ctx.actions.run_shell(", + " inputs = [sym],", + " outputs = [out],", + " command = '[[ hello == $(cat $1/some/path/inside.txt) ]] && touch $2',", + " arguments = [sym.path, out.path],", + " execution_requirements = {'no-remote': ''} if ctx.attr.local else {},", + " )", + "", + " return DefaultInfo(files = depset([out]))", + "", + "my_rule = rule(", + " implementation = _impl,", + " attrs = {", + " 'chain_length': attr.int(),", + " 'local': attr.bool()", + " },", + ")"); + + write( + "a/BUILD", + "load(':defs.bzl', 'my_rule')", + "", + "my_rule(name = 'one_local', local = True, chain_length = 1)", + "my_rule(name = 'two_local', local = True, chain_length = 2)", + "my_rule(name = 'one_remote', local = False, chain_length = 1)", + "my_rule(name = 'two_remote', local = False, chain_length = 2)"); + + buildTarget("//a:one_local", "//a:two_local", "//a:one_remote", "//a:two_remote"); + } + + @Test + public void symlinkToNestedFile() throws Exception { + addRemoteModeOptions(); + addOutputModeOptions(); + + write( + "a/defs.bzl", + "def _impl(ctx):", + " if ctx.attr.chain_length < 1:", + " fail('chain_length must be > 0')", + "", + " dir = ctx.actions.declare_directory(ctx.label.name + '.dir')", + " file = ctx.actions.declare_file(ctx.label.name + '.dir/some/path/inside.txt')", + " ctx.actions.run_shell(", + " outputs = [dir, file],", + " command = 'mkdir -p $1/some/path && echo hello > $1/some/path/inside.txt',", + " arguments = [dir.path],", + " )", + "", + " for i in range(ctx.attr.chain_length):", + " sym = ctx.actions.declare_file(ctx.label.name + '.sym' + str(i))", + " ctx.actions.symlink(output = sym, target_file = file)", + " file = sym", + "", + " out = ctx.actions.declare_file(ctx.label.name + '.out')", + " ctx.actions.run_shell(", + " inputs = [sym],", + " outputs = [out],", + " command = '[[ hello == $(cat $1) ]] && touch $2',", + " arguments = [sym.path, out.path],", + " execution_requirements = {'no-remote': ''} if ctx.attr.local else {},", + " )", + "", + " return DefaultInfo(files = depset([out]))", + "", + "my_rule = rule(", + " implementation = _impl,", + " attrs = {", + " 'chain_length': attr.int(),", + " 'local': attr.bool(),", + " },", + ")"); + + write( + "a/BUILD", + "load(':defs.bzl', 'my_rule')", + "", + "my_rule(name = 'one_local', local = True, chain_length = 1)", + "my_rule(name = 'two_local', local = True, chain_length = 2)", + "my_rule(name = 'one_remote', local = False, chain_length = 1)", + "my_rule(name = 'two_remote', local = False, chain_length = 2)"); + + buildTarget("//a:one_local", "//a:two_local", "//a:one_remote", "//a:two_remote"); + } + + @Test + public void symlinkToNestedDirectory() throws Exception { + addRemoteModeOptions(); + addOutputModeOptions(); + + write( + "a/defs.bzl", + "def _impl(ctx):", + " if ctx.attr.chain_length < 1:", + " fail('chain_length must be > 0')", + "", + " dir = ctx.actions.declare_directory(ctx.label.name + '.dir')", + " subdir = ctx.actions.declare_directory(ctx.label.name + '.dir/some/path')", + " ctx.actions.run_shell(", + " outputs = [dir, subdir],", + " command = 'mkdir -p $1/some/path && echo hello > $1/some/path/inside.txt',", + " arguments = [dir.path],", + " )", + "", + " for i in range(ctx.attr.chain_length):", + " sym = ctx.actions.declare_directory(ctx.label.name + '.sym' + str(i))", + " ctx.actions.symlink(output = sym, target_file = subdir)", + " subdir = sym", + "", + " out = ctx.actions.declare_file(ctx.label.name + '.out')", + " ctx.actions.run_shell(", + " inputs = [sym],", + " outputs = [out],", + " command = '[[ hello == $(cat $1/inside.txt) ]] && touch $2',", + " arguments = [sym.path, out.path],", + " execution_requirements = {'no-remote': ''} if ctx.attr.local else {},", + " )", + "", + " return DefaultInfo(files = depset([out]))", + "", + "my_rule = rule(", + " implementation = _impl,", + " attrs = {", + " 'chain_length': attr.int(),", + " 'local': attr.bool(),", + " },", + ")"); + + write( + "a/BUILD", + "load(':defs.bzl', 'my_rule')", + "", + "my_rule(name = 'one_local', local = True, chain_length = 1)", + "my_rule(name = 'two_local', local = True, chain_length = 2)", + "my_rule(name = 'one_remote', local = False, chain_length = 1)", + "my_rule(name = 'two_remote', local = False, chain_length = 2)"); + + buildTarget("//a:one_local", "//a:two_local", "//a:one_remote", "//a:two_remote"); + } + private static class ActionEventCollector { private final List actionExecutedEvents = new ArrayList<>(); private final List cachedActionEvents = new ArrayList<>(); diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTest.java index ef94269a9a6454..900905ef62b63b 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTest.java @@ -14,23 +14,30 @@ package com.google.devtools.build.lib.remote; import static com.google.common.truth.Truth.assertThat; +import static com.google.common.truth.Truth8.assertThat; import static org.junit.Assert.assertThrows; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.verifyNoMoreInteractions; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.hash.HashCode; import com.google.common.util.concurrent.Futures; import com.google.devtools.build.lib.actions.ActionInputMap; import com.google.devtools.build.lib.actions.Artifact; +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.ArtifactRoot; import com.google.devtools.build.lib.actions.ArtifactRoot.RootType; import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.actions.FileArtifactValue.RemoteFileArtifactValue; +import com.google.devtools.build.lib.actions.cache.MetadataInjector; import com.google.devtools.build.lib.actions.util.ActionsTestUtil; +import com.google.devtools.build.lib.skyframe.TreeArtifactValue; import com.google.devtools.build.lib.vfs.DigestHashFunction; import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.FileSystemUtils; @@ -42,10 +49,12 @@ import java.io.FileNotFoundException; import java.io.IOException; import java.nio.charset.StandardCharsets; +import java.util.Map; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; +import org.mockito.ArgumentCaptor; /** Tests for {@link RemoteActionFileSystem} */ @RunWith(JUnit4.class) @@ -54,6 +63,7 @@ public final class RemoteActionFileSystemTest { private static final DigestHashFunction HASH_FUNCTION = DigestHashFunction.SHA256; private final RemoteActionInputFetcher inputFetcher = mock(RemoteActionInputFetcher.class); + private final MetadataInjector metadataInjector = mock(MetadataInjector.class); private final FileSystem fs = new InMemoryFileSystem(HASH_FUNCTION); private final Path execRoot = fs.getPath("/exec"); private final ArtifactRoot outputRoot = @@ -62,6 +72,7 @@ public final class RemoteActionFileSystemTest { @Before public void createOutputRoot() throws IOException { outputRoot.getRoot().asPath().createDirectoryAndParents(); + } @Test @@ -98,33 +109,147 @@ public void testGetInputStream() throws Exception { } @Test - public void testCreateSymbolicLink() throws InterruptedException, IOException { + public void testCreateSymbolicLink_localFileArtifact() throws IOException { + // arrange + ActionInputMap inputs = new ActionInputMap(1); + Artifact localArtifact = createLocalArtifact("local-file", "local contents", inputs); + Artifact outputArtifact = ActionsTestUtil.createArtifact(outputRoot, "out"); + ImmutableList outputs = ImmutableList.of(outputArtifact); + RemoteActionFileSystem actionFs = newRemoteActionFileSystem(inputs, outputs); + + // act + PathFragment targetPath = localArtifact.getPath().asFragment(); + Path symlinkActionFs = actionFs.getPath(outputArtifact.getPath().asFragment()); + symlinkActionFs.createSymbolicLink(actionFs.getPath(targetPath)); + + // assert + assertThat(symlinkActionFs.getFileSystem()).isSameInstanceAs(actionFs); + assertThat(symlinkActionFs.readSymbolicLink()).isEqualTo(targetPath); + assertThat(outputArtifact.getPath().readSymbolicLink()).isEqualTo(targetPath); + + // act + actionFs.flush(); + + // assert + verifyNoInteractions(metadataInjector); + } + + @Test + public void testCreateSymbolicLink_remoteFileArtifact() throws IOException { // arrange ActionInputMap inputs = new ActionInputMap(1); Artifact remoteArtifact = createRemoteArtifact("remote-file", "remote contents", inputs); - Path symlink = outputRoot.getRoot().getRelative("symlink"); - FileSystem actionFs = newRemoteActionFileSystem(inputs); - doAnswer( - invocationOnMock -> { - FileSystemUtils.writeContent( - remoteArtifact.getPath(), StandardCharsets.UTF_8, "remote contents"); - return Futures.immediateFuture(null); - }) - .when(inputFetcher) - .downloadFile(eq(remoteArtifact.getPath()), eq(inputs.getMetadata(remoteArtifact))); + Artifact outputArtifact = ActionsTestUtil.createArtifact(outputRoot, "out"); + ImmutableList outputs = ImmutableList.of(outputArtifact); + RemoteActionFileSystem actionFs = newRemoteActionFileSystem(inputs, outputs); // act - Path symlinkActionFs = actionFs.getPath(symlink.getPathString()); - symlinkActionFs.createSymbolicLink(actionFs.getPath(remoteArtifact.getPath().asFragment())); + PathFragment targetPath = remoteArtifact.getPath().asFragment(); + Path symlinkActionFs = actionFs.getPath(outputArtifact.getPath().asFragment()); + symlinkActionFs.createSymbolicLink(actionFs.getPath(targetPath)); + + // assert + assertThat(outputArtifact.getPath().exists(Symlinks.NOFOLLOW)).isFalse(); + + // act + actionFs.flush(); + + // assert + ArgumentCaptor metadataCaptor = + ArgumentCaptor.forClass(FileArtifactValue.class); + verify(metadataInjector).injectFile(eq(outputArtifact), metadataCaptor.capture()); + assertThat(metadataCaptor.getValue()).isInstanceOf(RemoteFileArtifactValue.class); + assertThat((metadataCaptor.getValue()).getSymlinkTargetExecPath()) + .hasValue(targetPath.relativeTo(execRoot.asFragment())); + verifyNoMoreInteractions(metadataInjector); + } + + @Test + public void testCreateSymbolicLink_localTreeArtifact() throws IOException { + // arrange + ActionInputMap inputs = new ActionInputMap(1); + ImmutableMap contentMap = + ImmutableMap.of("foo", "foo contents", "bar", "bar contents"); + Artifact localArtifact = createLocalTreeArtifact("remote-dir", contentMap, inputs); + SpecialArtifact outputArtifact = + ActionsTestUtil.createTreeArtifactWithGeneratingAction(outputRoot, "out"); + ImmutableList outputs = ImmutableList.of(outputArtifact); + RemoteActionFileSystem actionFs = newRemoteActionFileSystem(inputs, outputs); + + // act + PathFragment targetPath = localArtifact.getPath().asFragment(); + Path symlinkActionFs = actionFs.getPath(outputArtifact.getPath().asFragment()); + symlinkActionFs.createSymbolicLink(actionFs.getPath(targetPath)); // assert assertThat(symlinkActionFs.getFileSystem()).isSameInstanceAs(actionFs); - verify(inputFetcher) - .downloadFile(eq(remoteArtifact.getPath()), eq(inputs.getMetadata(remoteArtifact))); - String symlinkTargetContents = - FileSystemUtils.readContent(symlinkActionFs, StandardCharsets.UTF_8); - assertThat(symlinkTargetContents).isEqualTo("remote contents"); - verifyNoMoreInteractions(inputFetcher); + assertThat(symlinkActionFs.readSymbolicLink()).isEqualTo(targetPath); + assertThat(outputArtifact.getPath().readSymbolicLink()).isEqualTo(targetPath); + + // act + actionFs.flush(); + + // assert + verifyNoInteractions(metadataInjector); + } + + + @Test + public void testCreateSymbolicLink_remoteTreeArtifact() throws IOException { + // arrange + ActionInputMap inputs = new ActionInputMap(1); + ImmutableMap contentMap = + ImmutableMap.of("foo", "foo contents", "bar", "bar contents"); + Artifact remoteArtifact = createRemoteTreeArtifact("remote-dir", contentMap, inputs); + SpecialArtifact outputArtifact = + ActionsTestUtil.createTreeArtifactWithGeneratingAction(outputRoot, "out"); + ImmutableList outputs = ImmutableList.of(outputArtifact); + RemoteActionFileSystem actionFs = newRemoteActionFileSystem(inputs, outputs); + + // act + PathFragment targetPath = remoteArtifact.getPath().asFragment(); + Path symlinkActionFs = actionFs.getPath(outputArtifact.getPath().asFragment()); + symlinkActionFs.createSymbolicLink(actionFs.getPath(targetPath)); + + // assert + assertThat(outputArtifact.getPath().exists(Symlinks.NOFOLLOW)).isFalse(); + + // act + actionFs.flush(); + + // assert + ArgumentCaptor metadataCaptor = + ArgumentCaptor.forClass(TreeArtifactValue.class); + verify(metadataInjector).injectTree(eq(outputArtifact), metadataCaptor.capture()); + assertThat(metadataCaptor.getValue().getSymlinkTargetExecPath()) + .hasValue(targetPath.relativeTo(execRoot.asFragment())); + verifyNoMoreInteractions(metadataInjector); + } + + @Test + public void testCreateSymbolicLink_unresolvedSymlink() throws IOException { + // arrange + ActionInputMap inputs = new ActionInputMap(1); + SpecialArtifact outputArtifact = + ActionsTestUtil.createUnresolvedSymlinkArtifact(outputRoot, "out"); + ImmutableList outputs = ImmutableList.of(outputArtifact); + RemoteActionFileSystem actionFs = newRemoteActionFileSystem(inputs, outputs); + PathFragment targetPath = PathFragment.create("some/path"); + + // act + Path symlinkActionFs = actionFs.getPath(outputArtifact.getPath().asFragment()); + symlinkActionFs.createSymbolicLink(targetPath); + + // assert + assertThat(symlinkActionFs.getFileSystem()).isSameInstanceAs(actionFs); + assertThat(symlinkActionFs.readSymbolicLink()).isEqualTo(targetPath); + assertThat(outputArtifact.getPath().readSymbolicLink()).isEqualTo(targetPath); + + // act + actionFs.flush(); + + // assert + verifyNoInteractions(metadataInjector); } @Test @@ -163,7 +288,7 @@ public void testDeleteLocalFile() throws Exception { } @Test - public void renameTo_fileDoesNotExist_throwError() { + public void renameTo_fileDoesNotExist_throwError() throws Exception { RemoteActionFileSystem actionFs = newRemoteActionFileSystem(); PathFragment path = outputRoot.getRoot().asPath().getRelative("file").asFragment(); PathFragment newPath = outputRoot.getRoot().asPath().getRelative("file-new").asFragment(); @@ -254,39 +379,65 @@ private void writeLocalFile(RemoteActionFileSystem actionFs, PathFragment path, FileSystemUtils.writeContent(actionFs.getPath(path), StandardCharsets.UTF_8, content); } - private RemoteActionFileSystem newRemoteActionFileSystem() { + private RemoteActionFileSystem newRemoteActionFileSystem() throws IOException { ActionInputMap inputs = new ActionInputMap(0); - return newRemoteActionFileSystem(inputs); + return newRemoteActionFileSystem(inputs, ImmutableList.of()); } - private RemoteActionFileSystem newRemoteActionFileSystem(ActionInputMap inputs) { + private RemoteActionFileSystem newRemoteActionFileSystem(ActionInputMap inputs) throws IOException { return newRemoteActionFileSystem(inputs, ImmutableList.of()); } private RemoteActionFileSystem newRemoteActionFileSystem( - ActionInputMap inputs, Iterable outputs) { - return new RemoteActionFileSystem( - fs, - execRoot.asFragment(), - outputRoot.getRoot().asPath().relativeTo(execRoot).getPathString(), - inputs, - outputs, - inputFetcher); + ActionInputMap inputs, Iterable outputs) throws IOException { + RemoteActionFileSystem remoteActionFileSystem = + new RemoteActionFileSystem( + fs, + execRoot.asFragment(), + outputRoot.getRoot().asPath().relativeTo(execRoot).getPathString(), + inputs, + outputs, + inputFetcher); + remoteActionFileSystem.updateContext(metadataInjector); + return remoteActionFileSystem; } /** Returns a remote artifact and puts its metadata into the action input map. */ private Artifact createRemoteArtifact( String pathFragment, String contents, ActionInputMap inputs) { - Path p = outputRoot.getRoot().asPath().getRelative(pathFragment); - Artifact a = ActionsTestUtil.createArtifact(outputRoot, p); + Artifact a = ActionsTestUtil.createArtifact(outputRoot, pathFragment); byte[] b = contents.getBytes(StandardCharsets.UTF_8); HashCode h = HASH_FUNCTION.getHashFunction().hashBytes(b); - FileArtifactValue f = + RemoteFileArtifactValue f = RemoteFileArtifactValue.create(h.asBytes(), b.length, /* locationIndex= */ 1, "action-id"); inputs.putWithNoDepOwner(a, f); return a; } + /** Returns a remote tree artifact and puts its metadata into the action input map. */ + private SpecialArtifact createRemoteTreeArtifact( + String pathFragment, Map contentMap, ActionInputMap inputs) { + SpecialArtifact a = + ActionsTestUtil.createTreeArtifactWithGeneratingAction(outputRoot, pathFragment); + inputs.putTreeArtifact(a, createRemoteTreeArtifactValue(a, contentMap), /* depOwner= */ null); + return a; + } + + private TreeArtifactValue createRemoteTreeArtifactValue( + SpecialArtifact a, Map contentMap) { + TreeArtifactValue.Builder builder = TreeArtifactValue.newBuilder(a); + for (Map.Entry entry : contentMap.entrySet()) { + TreeFileArtifact child = TreeFileArtifact.createTreeOutput(a, entry.getKey()); + byte[] b = entry.getValue().getBytes(StandardCharsets.UTF_8); + HashCode h = HASH_FUNCTION.getHashFunction().hashBytes(b); + RemoteFileArtifactValue childMeta = + RemoteFileArtifactValue.create( + h.asBytes(), b.length, /* locationIndex= */ 0, "action-id"); + builder.putChild(child, childMeta); + } + return builder.build(); + } + /** Returns a local artifact and puts its metadata into the action input map. */ private Artifact createLocalArtifact(String pathFragment, String contents, ActionInputMap inputs) throws IOException { @@ -302,4 +453,33 @@ private Artifact createLocalArtifact(String pathFragment, String contents, Actio FileArtifactValue.createFromStat(path, path.stat(Symlinks.FOLLOW), SyscallCache.NO_CACHE)); return a; } + + /** Returns a local tree artifact and puts its metadata into the action input map. */ + private SpecialArtifact createLocalTreeArtifact( + String pathFragment, Map contentMap, ActionInputMap inputs) throws IOException { + Path dir = outputRoot.getRoot().asPath().getRelative(pathFragment); + dir.createDirectoryAndParents(); + for (Map.Entry entry : contentMap.entrySet()) { + Path child = dir.getRelative(entry.getKey()); + child.getParentDirectory().createDirectoryAndParents(); + FileSystemUtils.writeContent(child, entry.getValue().getBytes(StandardCharsets.UTF_8)); + } + SpecialArtifact a = + ActionsTestUtil.createTreeArtifactWithGeneratingAction(outputRoot, pathFragment); + inputs.putTreeArtifact(a, createLocalTreeArtifactValue(a, contentMap), /* depOwner= */ null); + return a; + } + + private TreeArtifactValue createLocalTreeArtifactValue( + SpecialArtifact a, Map contentMap) throws IOException { + TreeArtifactValue.Builder builder = TreeArtifactValue.newBuilder(a); + for (String name : contentMap.keySet()) { + Path path = a.getPath().getRelative(name); + TreeFileArtifact child = TreeFileArtifact.createTreeOutput(a, name); + FileArtifactValue childMeta = + FileArtifactValue.createFromStat(path, path.stat(Symlinks.FOLLOW), SyscallCache.NO_CACHE); + builder.putChild(child, childMeta); + } + return builder.build(); + } } diff --git a/src/test/java/com/google/devtools/build/lib/remote/util/BUILD b/src/test/java/com/google/devtools/build/lib/remote/util/BUILD index ef1abf5595e292..8da2e520c0e4be 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/util/BUILD +++ b/src/test/java/com/google/devtools/build/lib/remote/util/BUILD @@ -25,6 +25,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/remote", "//src/main/java/com/google/devtools/build/lib/remote/common", "//src/main/java/com/google/devtools/build/lib/remote/util", + "//src/main/java/com/google/devtools/build/lib/skyframe:tree_artifact_value", "//src/main/java/com/google/devtools/build/lib/util/io", "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactValueTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactValueTest.java index d3bd5e25628f50..0fd24c855f1ddb 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactValueTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactValueTest.java @@ -109,6 +109,18 @@ public void createsCorrectValueWithArchivedRepresentation() { .hasValue(ArchivedRepresentation.create(archivedTreeArtifact, archivedArtifactMetadata)); } + @Test + public void createsCorrectValueWithSymlinkTargetExecPath() { + PathFragment targetPath = PathFragment.create("some/target/path"); + SpecialArtifact parent = createTreeArtifact("bin/tree"); + + TreeArtifactValue tree = + TreeArtifactValue.newBuilder(parent).setSymlinkTargetExecPath(targetPath).build(); + + assertThat(tree.getSymlinkTargetExecPath()).hasValue(targetPath); + assertThat(tree.getMetadata().getSymlinkTargetExecPath()).hasValue(targetPath); + } + @Test public void empty() { TreeArtifactValue emptyTree = TreeArtifactValue.empty();