From 3519d1eb6d81037080df71ec627b4fdbd56e5836 Mon Sep 17 00:00:00 2001 From: Tiago Quelhas Date: Thu, 15 Sep 2022 11:34:05 +0200 Subject: [PATCH] Inject metadata when creating a filesystem symlink for a non-symlink artifact. A ctx.actions.symlink whose output is a declare_file or declare_directory (as opposed to a declare_symlink) has "copy" semantics, i.e., the output artifact is indistinguishable from the referent except for its name; the symlink is just a filesystem-level optimization to avoid an actual copy, and is transparently resolved when collecting the action output metadata. When the symlink points to an artifact that was built remotely and without the bytes, we currently must download it before executing the symlink action, so that the output metadata can be constructed from the local filesystem. This change short-circuits the filesystem traversal by injecting output metadata, which is identical to the input plus a pointer to the original path. This is used by the prefetcher to avoid downloading the same files multiple times when they're symlinked more than once. (An alternative would have been to teach all of the RemoteActionFileSystem methods to resolve symlinks by patching together the local and remote metadata, but that would have resulted in an awful lot of complexity.) Fixes #15678. --- .../build/lib/actions/FileArtifactValue.java | 100 ++++++- .../build/lib/actions/cache/ActionCache.java | 17 +- .../cache/CompactPersistentActionCache.java | 56 +++- .../remote/AbstractActionInputPrefetcher.java | 112 +++++-- .../google/devtools/build/lib/remote/BUILD | 1 + .../lib/remote/RemoteActionFileSystem.java | 153 ++++++++-- .../build/lib/skyframe/TreeArtifactValue.java | 63 +++- .../vfs/inmemoryfs/InMemoryFileSystem.java | 2 +- .../lib/actions/ActionCacheCheckerTest.java | 175 +++++++++-- .../build/lib/actions/ActionInputMapTest.java | 1 + .../CompactPersistentActionCacheTest.java | 77 ++++- .../lib/actions/util/ActionsTestUtil.java | 12 + .../remote/ActionInputPrefetcherTestBase.java | 152 ++++++++-- .../google/devtools/build/lib/remote/BUILD | 1 + .../BuildWithoutTheBytesIntegrationTest.java | 271 +++++++++++++++++ .../remote/RemoteActionFileSystemTest.java | 273 +++++++++++++++--- .../devtools/build/lib/remote/util/BUILD | 1 + .../skyframe/FilesystemValueCheckerTest.java | 2 +- .../lib/skyframe/TreeArtifactValueTest.java | 12 + 19 files changed, 1313 insertions(+), 168 deletions(-) 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..ebcc2900994b0b 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,19 @@ protected boolean couldBeModifiedByMetadata(FileArtifactValue lastKnown) { return true; } + /** + * Optional materialization path. + * + *

If present, this artifact is a copy of another artifact. It is still tracked as a + * non-symlink by Bazel, but materialized in the local filesystem as a symlink to the original + * artifact, whose contents live at this location. This is used by {@link + * com.google.devtools.build.lib.remote.AbstractActionInputPrefetcher} to implement zero-cost + * copies of remotely stored artifacts. + */ + public Optional getMaterializationExecPath() { + return Optional.empty(); + } + /** * Marker interface for singleton implementations of this class. * @@ -514,9 +528,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 +547,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 +568,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 materializationExecPath) { + if (materializationExecPath != null) { + return new RemoteFileArtifactValueWithMaterializationPath( + digest, size, locationIndex, actionId, materializationExecPath); + } + return new RemoteFileArtifactValue(digest, size, locationIndex, actionId); + } + @Override public boolean equals(Object o) { if (!(o instanceof RemoteFileArtifactValue)) { @@ -602,7 +627,7 @@ public String getActionId() { @Override public long getModifiedTime() { throw new UnsupportedOperationException( - "RemoteFileArifactValue doesn't support getModifiedTime"); + "RemoteFileArtifactValue doesn't support getModifiedTime"); } @Override @@ -626,6 +651,65 @@ public String toString() { .add("digest", bytesToString(digest)) .add("size", size) .add("locationIndex", locationIndex) + .add("actionId", actionId) + .toString(); + } + } + + /** + * A remote artifact that should be materialized in the local filesystem as a symlink to another + * location. + * + *

See the documentation for {@link FileArtifactValue#getMaterializationExecPath}. + */ + public static final class RemoteFileArtifactValueWithMaterializationPath + extends RemoteFileArtifactValue { + private PathFragment materializationExecPath; + + private RemoteFileArtifactValueWithMaterializationPath( + byte[] digest, + long size, + int locationIndex, + String actionId, + PathFragment materializationExecPath) { + super(digest, size, locationIndex, actionId); + this.materializationExecPath = Preconditions.checkNotNull(materializationExecPath); + } + + @Override + public Optional getMaterializationExecPath() { + return Optional.ofNullable(materializationExecPath); + } + + @Override + public boolean equals(Object o) { + if (!(o instanceof RemoteFileArtifactValueWithMaterializationPath)) { + return false; + } + + RemoteFileArtifactValueWithMaterializationPath that = + (RemoteFileArtifactValueWithMaterializationPath) o; + return Arrays.equals(digest, that.digest) + && size == that.size + && locationIndex == that.locationIndex + && Objects.equals(actionId, that.actionId) + && Objects.equals(materializationExecPath, that.materializationExecPath); + } + + @Override + public int hashCode() { + return Objects.hash( + Arrays.hashCode(digest), size, locationIndex, actionId, materializationExecPath); + } + + @Override + public String toString() { + return MoreObjects.toStringHelper(this) + .add("digest", bytesToString(digest)) + .add("size", size) + .add("locationIndex", locationIndex) + .add("actionId", actionId) + .add("materializationExecPath", materializationExecPath) .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..6204e6f06038f1 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 materializationExecPath) { return new AutoValue_ActionCache_Entry_SerializableTreeArtifactValue( - childValues, archivedFileValue); + childValues, archivedFileValue, materializationExecPath); } /** @@ -138,17 +139,25 @@ public static Optional createSerializable( .filter(ar -> ar.archivedFileValue().isRemote()) .map(ar -> (RemoteFileArtifactValue) ar.archivedFileValue()); - if (childValues.isEmpty() && archivedFileValue.isEmpty()) { + Optional materializationExecPath = treeMetadata.getMaterializationExecPath(); + + if (childValues.isEmpty() + && archivedFileValue.isEmpty() + && materializationExecPath.isEmpty()) { return Optional.empty(); } - return Optional.of(SerializableTreeArtifactValue.create(childValues, archivedFileValue)); + return Optional.of( + SerializableTreeArtifactValue.create( + childValues, archivedFileValue, materializationExecPath)); } // A map from parentRelativePath to the file metadata public abstract ImmutableMap childValues(); public abstract Optional archivedFileValue(); + + public abstract Optional materializationExecPath(); } 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..635e8127922d08 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 materializationExecPath = value.getMaterializationExecPath(); + if (materializationExecPath.isPresent()) { + VarInt.putVarInt(1, sink); + VarInt.putVarInt(indexer.getOrCreateIndex(materializationExecPath.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 materializationExecPath = null; + int nummaterializationExecPath = VarInt.getVarInt(source); + if (nummaterializationExecPath > 0) { + if (nummaterializationExecPath != 1) { + throw new IOException("Invalid number of symlink target paths"); + } + materializationExecPath = + PathFragment.create(getStringForIndex(indexer, VarInt.getVarInt(source))); + } + + return RemoteFileArtifactValue.create( + digest, size, locationIndex, actionId, materializationExecPath); } /** @return action data encoded as a byte[] array. */ @@ -513,9 +533,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); + (1 + VarInt.MAX_VARINT_SIZE) // value.archivedFileValue() optional + + value.archivedFileValue().map(ignored -> MAX_REMOTE_METADATA_SIZE).orElse(0); + maxOutputTreesSize += + (1 + VarInt.MAX_VARINT_SIZE) // value.materializationExecPath() optional + + value.materializationExecPath().map(ignored -> MAX_REMOTE_METADATA_SIZE).orElse(0); } // Estimate the size of the buffer: @@ -578,6 +601,15 @@ private static byte[] encode(StringIndexer indexer, ActionCache.Entry entry) thr } else { VarInt.putVarInt(0, sink); } + + Optional materializationExecPath = + serializableTreeArtifactValue.materializationExecPath(); + if (materializationExecPath.isPresent()) { + VarInt.putVarInt(1, sink); + VarInt.putVarInt(indexer.getOrCreateIndex(materializationExecPath.get().toString()), sink); + } else { + VarInt.putVarInt(0, sink); + } } return sink.toByteArray(); @@ -649,13 +681,25 @@ private static ActionCache.Entry decode(StringIndexer indexer, byte[] data) thro int numArchivedFileValue = VarInt.getVarInt(source); if (numArchivedFileValue > 0) { if (numArchivedFileValue != 1) { - throw new IOException("Invalid number of archived artifacts"); + throw new IOException("Invalid presence marker for archived representation"); } archivedFileValue = Optional.of(decodeRemoteMetadata(indexer, source)); } + Optional symlinkTargetPath = Optional.empty(); + int nummaterializationExecPath = VarInt.getVarInt(source); + if (nummaterializationExecPath > 0) { + if (nummaterializationExecPath != 1) { + throw new IOException("Invalid presence marker for symlink target"); + } + 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..a5ea0e46a785f7 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.getMaterializationExecPath().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#getMaterializationExecPath() 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.getMaterializationExecPath().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 fff15853a558cf..6b5e96b9209100 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/BUILD +++ b/src/main/java/com/google/devtools/build/lib/remote/BUILD @@ -179,6 +179,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 efbc146394b88f..ddf86d35f5b6d7 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; @@ -38,6 +39,7 @@ import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; +import com.google.devtools.build.lib.vfs.Symlinks; import com.google.devtools.build.lib.vfs.inmemoryfs.FileInfo; import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryContentInfo; import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; @@ -53,7 +55,7 @@ /** * 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,7 +64,6 @@ *

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; @@ -101,7 +102,7 @@ protected FileSystem getLocalFileSystem() { /** Returns true if {@code path} is a file that's stored remotely. */ boolean isRemote(Path path) { - return getRemoteInputMetadata(path.asFragment()) != null; + return getRemoteMetadata(path.asFragment()) != null; } public void updateContext(MetadataInjector metadataInjector) { @@ -120,9 +121,13 @@ void flush() throws IOException { checkNotNull(metadataInjector, "metadataInjector is null"); for (Map.Entry entry : outputMapping.entrySet()) { - PathFragment execPath = entry.getKey(); - PathFragment path = execRoot.getRelative(execPath); + PathFragment path = execRoot.getRelative(entry.getKey()); Artifact output = entry.getValue(); + + if (maybeInjectMetadataForSymlink(path, output)) { + continue; + } + if (output.isTreeArtifact()) { if (remoteOutputTree.exists(path)) { SpecialArtifact parent = (SpecialArtifact) output; @@ -157,6 +162,91 @@ void flush() throws IOException { } } + /** + * Inject metadata for non-symlink outputs that were materialized as a symlink to a remote + * artifact. + * + *

If a non-symlink output is materialized as a symlink, the symlink has "copy" semantics, + * i.e., the output metadata is identical to that of the symlink target. For these artifacts, we + * inject their metadata instead of collecting it from the filesystem. This is done for two + * reasons: + * + *