diff --git a/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java b/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java index c637e8a397778b..63d9b9517cc689 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java +++ b/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java @@ -553,7 +553,7 @@ protected void checkInputsForDirectories( // Assume that if the file did not exist, we would not have gotten here. try { if (input.isSourceArtifact() - && metadataProvider.getMetadata(input).getType().isDirectory()) { + && metadataProvider.getInputMetadata(input).getType().isDirectory()) { // TODO(ulfjack): What about dependency checking of special files? eventHandler.handle( Event.warn( @@ -583,7 +583,7 @@ protected void checkOutputsForDirectories(ActionExecutionContext actionExecution continue; } try { - metadata = metadataHandler.getMetadata(output); + metadata = metadataHandler.getOutputMetadata(output); } catch (IOException e) { logger.atWarning().withCause(e).log("Error getting metadata for %s", output); metadata = null; diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java b/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java index 05d8bc8c5c0bd3..aafb5a892b0fa1 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java @@ -198,14 +198,14 @@ private static boolean validateArtifacts( for (Artifact artifact : action.getOutputs()) { FileArtifactValue metadata = getCachedMetadata(cachedOutputMetadata, artifact); if (metadata == null) { - metadata = getMetadataMaybe(metadataHandler, artifact); + metadata = getOutputMetadataMaybe(metadataHandler, artifact); } mdMap.put(artifact.getExecPathString(), metadata); } } for (Artifact artifact : actionInputs.toList()) { - mdMap.put(artifact.getExecPathString(), getMetadataMaybe(metadataHandler, artifact)); + mdMap.put(artifact.getExecPathString(), getInputMetadataMaybe(metadataHandler, artifact)); } return !Arrays.equals(MetadataDigestUtils.fromMetadata(mdMap), entry.getFileDigest()); } @@ -396,7 +396,7 @@ private static CachedOutputMetadata loadCachedOutputMetadata( FileArtifactValue localMetadata; try { - localMetadata = getMetadataOrConstant(metadataHandler, artifact); + localMetadata = getOutputMetadataOrConstant(metadataHandler, artifact); } catch (FileNotFoundException ignored) { localMetadata = null; } catch (IOException e) { @@ -571,22 +571,43 @@ private boolean mustExecute( return false; } - private static FileArtifactValue getMetadataOrConstant( + private static FileArtifactValue getInputMetadataOrConstant( MetadataHandler metadataHandler, Artifact artifact) throws IOException { - FileArtifactValue metadata = metadataHandler.getMetadata(artifact); + FileArtifactValue metadata = metadataHandler.getInputMetadata(artifact); return (metadata != null && artifact.isConstantMetadata()) ? ConstantMetadataValue.INSTANCE : metadata; } + private static FileArtifactValue getOutputMetadataOrConstant( + MetadataHandler metadataHandler, Artifact artifact) throws IOException { + FileArtifactValue metadata = metadataHandler.getOutputMetadata(artifact); + return (metadata != null && artifact.isConstantMetadata()) + ? ConstantMetadataValue.INSTANCE + : metadata; + } + + // TODO(ulfjack): It's unclear to me why we're ignoring all IOExceptions. In some cases, we want + // to trigger a re-execution, so we should catch the IOException explicitly there. In others, we + // should propagate the exception, because it is unexpected (e.g., bad file system state). + @Nullable + private static FileArtifactValue getInputMetadataMaybe( + MetadataHandler metadataHandler, Artifact artifact) { + try { + return getInputMetadataOrConstant(metadataHandler, artifact); + } catch (IOException e) { + return null; + } + } + // TODO(ulfjack): It's unclear to me why we're ignoring all IOExceptions. In some cases, we want // to trigger a re-execution, so we should catch the IOException explicitly there. In others, we // should propagate the exception, because it is unexpected (e.g., bad file system state). @Nullable - private static FileArtifactValue getMetadataMaybe( + private static FileArtifactValue getOutputMetadataMaybe( MetadataHandler metadataHandler, Artifact artifact) { try { - return getMetadataOrConstant(metadataHandler, artifact); + return getOutputMetadataOrConstant(metadataHandler, artifact); } catch (IOException e) { return null; } @@ -632,7 +653,7 @@ public void updateActionCache( // the 'constant' metadata for the volatile workspace status output. The volatile output // contains information such as timestamps, and even when --stamp is enabled, we don't // want to rebuild everything if only that file changes. - FileArtifactValue metadata = getMetadataOrConstant(metadataHandler, output); + FileArtifactValue metadata = getOutputMetadataOrConstant(metadataHandler, output); checkState(metadata != null); entry.addOutputFile(output, metadata, cacheConfig.storeOutputMetadata()); } @@ -650,7 +671,7 @@ public void updateActionCache( for (Artifact input : action.getInputs().toList()) { entry.addInputFile( input.getExecPath(), - getMetadataMaybe(metadataHandler, input), + getInputMetadataMaybe(metadataHandler, input), /* saveExecPath= */ !excludePathsFromActionCache.contains(input)); } entry.getFileDigest(); @@ -769,7 +790,9 @@ private void checkMiddlemanAction( entry = new ActionCache.Entry("", ImmutableMap.of(), false, OutputPermissions.READONLY); for (Artifact input : action.getInputs().toList()) { entry.addInputFile( - input.getExecPath(), getMetadataMaybe(metadataHandler, input), /*saveExecPath=*/ true); + input.getExecPath(), + getInputMetadataMaybe(metadataHandler, input), + /* saveExecPath= */ true); } } diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java index 02943cbb092660..28d84ec6d74d1f 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java @@ -406,6 +406,44 @@ public void close() throws IOException { } } + /** + * Creates a new {@link ActionExecutionContext} whose {@link MetadataProvider} has the given + * {@link ActionInput}s as inputs. + * + *

Each {@link ActionInput} must be an output of the current {@link ActionExecutionContext} and + * it must already have been built. + */ + public ActionExecutionContext withOutputsAsInputs( + Iterable additionalInputs) throws IOException { + ImmutableMap.Builder additionalInputMap = + ImmutableMap.builder(); + + for (ActionInput input : additionalInputs) { + additionalInputMap.put(input, getMetadataHandler().getOutputMetadata(input)); + } + StaticMetadataProvider additionalInputMetadata = + new StaticMetadataProvider(additionalInputMap.buildOrThrow()); + + return new ActionExecutionContext( + executor, + new DelegatingPairMetadataProvider(actionInputFileCache, additionalInputMetadata), + actionInputPrefetcher, + actionKeyContext, + metadataHandler, + rewindingEnabled, + lostInputsCheck, + fileOutErr, + eventHandler, + clientEnv, + topLevelFilesets, + artifactExpander, + env, + actionFileSystem, + skyframeDepsResult, + discoveredModulesPruner, + syscallCache, + threadStateReceiverForMetrics); + } /** * Allows us to create a new context that overrides the FileOutErr with another one. This is * useful for muting the output for example. diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionInputMap.java b/src/main/java/com/google/devtools/build/lib/actions/ActionInputMap.java index b1cd6c95e7db68..c1d622d5029a3f 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ActionInputMap.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ActionInputMap.java @@ -174,7 +174,7 @@ private int getIndex(String execPathString) { @Nullable @Override - public FileArtifactValue getMetadata(ActionInput input) { + public FileArtifactValue getInputMetadata(ActionInput input) { if (input instanceof TreeFileArtifact) { TreeFileArtifact treeFileArtifact = (TreeFileArtifact) input; int treeIndex = getIndex(treeFileArtifact.getParent().getExecPathString()); diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionInputPrefetcher.java b/src/main/java/com/google/devtools/build/lib/actions/ActionInputPrefetcher.java index cbb6ffcc31d2cc..d0a70e753292ab 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ActionInputPrefetcher.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ActionInputPrefetcher.java @@ -20,12 +20,22 @@ /** Prefetches files to local disk. */ public interface ActionInputPrefetcher { + /** + * Returns the metadata for an {@link ActionInput}. + * + *

This will generally call through to a {@link MetadataProvider} and ask for the metadata of + * either an input or an output artifact. + */ + public interface MetadataSupplier { + FileArtifactValue getMetadata(ActionInput actionInput) throws IOException; + } + public static final ActionInputPrefetcher NONE = new ActionInputPrefetcher() { @Override public ListenableFuture prefetchFiles( Iterable inputs, - MetadataProvider metadataProvider, + MetadataSupplier metadataSupplier, Priority priority) { // Do nothing. return immediateVoidFuture(); @@ -69,7 +79,7 @@ public enum Priority { * @return future success if prefetch is finished or {@link IOException}. */ ListenableFuture prefetchFiles( - Iterable inputs, MetadataProvider metadataProvider, Priority priority); + Iterable inputs, MetadataSupplier metadataSupplier, Priority priority); /** * Whether the prefetcher requires the metadata for a tree artifact to be available whenever one diff --git a/src/main/java/com/google/devtools/build/lib/actions/CompletionContext.java b/src/main/java/com/google/devtools/build/lib/actions/CompletionContext.java index 890d4045c92b2e..f9546a22980fab 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/CompletionContext.java +++ b/src/main/java/com/google/devtools/build/lib/actions/CompletionContext.java @@ -111,7 +111,7 @@ public ActionInputMap getImportantInputMap() { @Nullable public FileArtifactValue getFileArtifactValue(Artifact artifact) { - return importantInputMap.getMetadata(artifact); + return importantInputMap.getInputMetadata(artifact); } /** Returns true if the given artifact is guaranteed to be a file (and not a directory). */ @@ -136,7 +136,7 @@ public void visitArtifacts(Iterable artifacts, ArtifactReceiver receiv : RelativeSymlinkBehaviorWithoutError.RESOLVE); } } else if (artifact.isTreeArtifact()) { - FileArtifactValue treeArtifactMetadata = importantInputMap.getMetadata(artifact); + FileArtifactValue treeArtifactMetadata = importantInputMap.getInputMetadata(artifact); if (treeArtifactMetadata == null) { BugReport.sendBugReport( new IllegalStateException( diff --git a/src/main/java/com/google/devtools/build/lib/actions/DelegatingPairMetadataProvider.java b/src/main/java/com/google/devtools/build/lib/actions/DelegatingPairMetadataProvider.java new file mode 100644 index 00000000000000..4d5c94eb645169 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/actions/DelegatingPairMetadataProvider.java @@ -0,0 +1,43 @@ +// Copyright 2023 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.devtools.build.lib.actions; + +import java.io.IOException; + +/** A {@link MetadataProvider} implementation that consults two others in a given order. */ +public final class DelegatingPairMetadataProvider implements MetadataProvider { + + private final MetadataProvider primary; + private final MetadataProvider secondary; + + public DelegatingPairMetadataProvider(MetadataProvider primary, MetadataProvider secondary) { + this.primary = primary; + this.secondary = secondary; + } + + @Override + public FileArtifactValue getInputMetadata(ActionInput input) throws IOException { + FileArtifactValue metadata = primary.getInputMetadata(input); + return (metadata != null) && (metadata != FileArtifactValue.MISSING_FILE_MARKER) + ? metadata + : secondary.getInputMetadata(input); + } + + @Override + public ActionInput getInput(String execPath) { + ActionInput input = primary.getInput(execPath); + return input != null ? input : secondary.getInput(execPath); + } +} diff --git a/src/main/java/com/google/devtools/build/lib/actions/MetadataProvider.java b/src/main/java/com/google/devtools/build/lib/actions/MetadataProvider.java index ed1bbe5e9cbb16..2f4299047828f1 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/MetadataProvider.java +++ b/src/main/java/com/google/devtools/build/lib/actions/MetadataProvider.java @@ -22,16 +22,9 @@ /** Provides {@link ActionInput} metadata. */ @ThreadSafe public interface MetadataProvider { - /** * Returns a {@link FileArtifactValue} for the given {@link ActionInput}. * - *

If the given input is an output {@link Artifact} of an action, then the returned value is - * current as of the action's most recent execution time (which may be from a prior build, in - * which case the value may or may not be up to date for the current build). The returned value - * can vary across calls, for example if the action executes between calls and produces different - * outputs than its previous execution. - * *

The returned {@link FileArtifactValue} instance corresponds to the final target of a symlink * and therefore must not have a type of {@link FileStateType#SYMLINK}. * @@ -43,10 +36,10 @@ public interface MetadataProvider { * @param input the input to retrieve the digest for * @return the artifact's digest or null if digest cannot be obtained (due to artifact * non-existence, lookup errors, or any other reason) - * @throws IOException if the input cannot be digested + * @throws IOException if the action input cannot be digested */ @Nullable - FileArtifactValue getMetadata(ActionInput input) throws IOException; + FileArtifactValue getInputMetadata(ActionInput input) throws IOException; /** Looks up an input from its exec path. */ @Nullable @@ -67,8 +60,8 @@ default FileSystem getFileSystemForInputResolution() { } /** - * Indicates whether calls to {@link #getMetadata} with a {@link Artifact.DerivedArtifact} may - * require a skyframe lookup. + * Indicates whether calls to {@link #getInputMetadata} with a {@link Artifact.DerivedArtifact} + * may require a skyframe lookup. */ default boolean mayGetGeneratingActionsFromSkyframe() { return false; diff --git a/src/main/java/com/google/devtools/build/lib/remote/util/StaticMetadataProvider.java b/src/main/java/com/google/devtools/build/lib/actions/StaticMetadataProvider.java similarity index 87% rename from src/main/java/com/google/devtools/build/lib/remote/util/StaticMetadataProvider.java rename to src/main/java/com/google/devtools/build/lib/actions/StaticMetadataProvider.java index 2d1a19421efdfb..fc6248a51a184d 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/util/StaticMetadataProvider.java +++ b/src/main/java/com/google/devtools/build/lib/actions/StaticMetadataProvider.java @@ -11,12 +11,9 @@ // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License. -package com.google.devtools.build.lib.remote.util; +package com.google.devtools.build.lib.actions; import com.google.common.collect.ImmutableMap; -import com.google.devtools.build.lib.actions.ActionInput; -import com.google.devtools.build.lib.actions.FileArtifactValue; -import com.google.devtools.build.lib.actions.MetadataProvider; import java.util.Collection; import java.util.Map; import javax.annotation.Nullable; @@ -50,7 +47,7 @@ private static ImmutableMap constructExecPathToInputMap( @Nullable @Override - public FileArtifactValue getMetadata(ActionInput input) { + public FileArtifactValue getInputMetadata(ActionInput input) { return inputToMetadata.get(input); } diff --git a/src/main/java/com/google/devtools/build/lib/actions/cache/MetadataHandler.java b/src/main/java/com/google/devtools/build/lib/actions/cache/MetadataHandler.java index de5bf46693a616..f237b4875a199e 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/cache/MetadataHandler.java +++ b/src/main/java/com/google/devtools/build/lib/actions/cache/MetadataHandler.java @@ -19,27 +19,35 @@ import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact; import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact; import com.google.devtools.build.lib.actions.FileArtifactValue; +import com.google.devtools.build.lib.actions.FileStateType; import com.google.devtools.build.lib.actions.MetadataProvider; import com.google.devtools.build.lib.skyframe.TreeArtifactValue; import com.google.devtools.build.lib.vfs.FileStatus; import java.io.IOException; -import javax.annotation.Nullable; /** Handles metadata of inputs and outputs during the execution phase. */ public interface MetadataHandler extends MetadataProvider, MetadataInjector { /** - * {@inheritDoc} + * Returns a {@link FileArtifactValue} for the given {@link ActionInput}. + * + *

If the metadata of the given {@link ActionInput} is not known, it's computed. This may + * result in a significant amount of I/O. + * + *

The returned {@link FileArtifactValue} instance corresponds to the final target of a symlink + * and therefore must not have a type of {@link FileStateType#SYMLINK}. * *

Freshly created output files (i.e. from an action that just executed) that require a stat to * obtain the metadata will first be set read-only and executable during this call. This ensures * that the returned metadata has an appropriate ctime, which is affected by chmod. Note that this * does not apply to outputs injected via {@link #injectFile} or {@link #injectTree} since a stat * is not required for them. + * + * @param output the output to retrieve the digest for + * @return the artifact's digest or null the artifact is not a known output of the action + * @throws IOException if the action input cannot be digested */ - @Override - @Nullable - FileArtifactValue getMetadata(ActionInput input) throws IOException; + FileArtifactValue getOutputMetadata(ActionInput output) throws IOException; /** Sets digest for virtual artifacts (e.g. middlemen). {@code digest} must not be null. */ void setDigestForVirtualArtifact(Artifact artifact, byte[] digest); diff --git a/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java index 9579535a9cfc10..7c7eefcc69d205 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java @@ -255,7 +255,7 @@ public ListenableFuture prefetchInputs() .prefetchFiles( getInputMapping(PathFragment.EMPTY_FRAGMENT, /* willAccessRepeatedly= */ true) .values(), - getMetadataProvider(), + getMetadataProvider()::getInputMetadata, Priority.MEDIUM), BulkTransferException.class, (BulkTransferException e) -> { diff --git a/src/main/java/com/google/devtools/build/lib/exec/SingleBuildFileCache.java b/src/main/java/com/google/devtools/build/lib/exec/SingleBuildFileCache.java index e7542a1ad9e57b..f473200f1ab208 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/SingleBuildFileCache.java +++ b/src/main/java/com/google/devtools/build/lib/exec/SingleBuildFileCache.java @@ -55,7 +55,10 @@ public SingleBuildFileCache(String cwd, FileSystem fs, XattrProvider xattrProvid } @Override - public FileArtifactValue getMetadata(ActionInput input) throws IOException { + public FileArtifactValue getInputMetadata(ActionInput input) throws IOException { + // TODO(lberki): It would be nice to assert that only source files are passed here. + // Unfortunately, that's not quite true at the moment and an unknown amount of work would be + // needed to make that true. return pathToMetadata .get( input.getExecPathString(), diff --git a/src/main/java/com/google/devtools/build/lib/exec/SpawnInputExpander.java b/src/main/java/com/google/devtools/build/lib/exec/SpawnInputExpander.java index 7922e4362b21eb..0337b6118991c4 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/SpawnInputExpander.java +++ b/src/main/java/com/google/devtools/build/lib/exec/SpawnInputExpander.java @@ -177,7 +177,7 @@ public Map addRunfilesToInputs( private static void failIfDirectory(MetadataProvider actionFileCache, ActionInput input) throws IOException, ForbiddenActionInputException { - FileArtifactValue metadata = actionFileCache.getMetadata(input); + FileArtifactValue metadata = actionFileCache.getInputMetadata(input); if (metadata != null && !metadata.getType().isFile()) { throw new ForbiddenNonFileException(input); } diff --git a/src/main/java/com/google/devtools/build/lib/exec/SpawnLogContext.java b/src/main/java/com/google/devtools/build/lib/exec/SpawnLogContext.java index 2b7a75cb57551d..dc97d9ffc7bd1c 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/SpawnLogContext.java +++ b/src/main/java/com/google/devtools/build/lib/exec/SpawnLogContext.java @@ -305,7 +305,7 @@ private Digest computeDigest( } // Try to access the cached metadata, otherwise fall back to local computation. try { - FileArtifactValue metadata = metadataProvider.getMetadata(input); + FileArtifactValue metadata = metadataProvider.getInputMetadata(input); if (metadata != null) { byte[] hash = metadata.getDigest(); if (hash != null) { diff --git a/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java index a12e94dc5933d8..241e51cff3d652 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java @@ -714,9 +714,11 @@ private TestAttemptResult runTestAttempt( closeSuppressed(e, fileOutErr); throw e; } - actionExecutionContext - .getMetadataHandler() - .getMetadata(testAction.getCoverageDirectoryTreeArtifact()); + var unused = + actionExecutionContext + .getMetadataHandler() + .getOutputMetadata(testAction.getCoverageDirectoryTreeArtifact()); + ImmutableSet expandedCoverageDir = actionExecutionContext .getMetadataHandler() @@ -738,14 +740,15 @@ private TestAttemptResult runTestAttempt( Path out = testRoot.getChild("coverage.log"); Path err = testRoot.getChild("coverage.err"); FileOutErr coverageOutErr = new FileOutErr(out, err); - ActionExecutionContext actionExecutionContextWithCoverageFileOutErr = - actionExecutionContext.withFileOutErr(coverageOutErr); + ActionExecutionContext coverageActionExecutionContext = + actionExecutionContext + .withFileOutErr(coverageOutErr) + .withOutputsAsInputs(expandedCoverageDir); writeOutFile(coverageOutErr.getErrorPath(), coverageOutErr.getOutputPath()); appendCoverageLog(coverageOutErr, fileOutErr); try { - spawnStrategyResolver.exec( - coveragePostProcessingSpawn, actionExecutionContextWithCoverageFileOutErr); + spawnStrategyResolver.exec(coveragePostProcessingSpawn, coverageActionExecutionContext); } catch (SpawnExecException e) { if (e.isCatastrophic()) { closeSuppressed(e, streamed); @@ -809,11 +812,15 @@ private TestAttemptResult runTestAttempt( // We treat all failures to generate the test.xml here as catastrophic, and won't rerun // the test if this fails. We redirect the output to a temporary file. FileOutErr xmlSpawnOutErr = actionExecutionContext.getFileOutErr().childOutErr(); + + ActionExecutionContext xmlActionExecutionContext = + actionExecutionContext + .withFileOutErr(xmlSpawnOutErr) + .withOutputsAsInputs(ImmutableList.of(testAction.getTestLog())); try { ImmutableList xmlSpawnResults = - spawnStrategyResolver.exec( - xmlGeneratingSpawn, actionExecutionContext.withFileOutErr(xmlSpawnOutErr)); + spawnStrategyResolver.exec(xmlGeneratingSpawn, xmlActionExecutionContext); spawnResults = ImmutableList.builder() .addAll(spawnResults) 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 df111978cebc21..cb4c59753a3e06 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 @@ -38,7 +38,6 @@ import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact; import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact; import com.google.devtools.build.lib.actions.FileArtifactValue; -import com.google.devtools.build.lib.actions.MetadataProvider; import com.google.devtools.build.lib.actions.cache.MetadataHandler; import com.google.devtools.build.lib.actions.cache.VirtualActionInput; import com.google.devtools.build.lib.events.Event; @@ -290,7 +289,7 @@ protected void prefetchVirtualActionInput(VirtualActionInput input) throws IOExc @Override public ListenableFuture prefetchFiles( Iterable inputs, - MetadataProvider metadataProvider, + MetadataSupplier metadataSupplier, Priority priority) { List files = new ArrayList<>(); @@ -313,7 +312,7 @@ public ListenableFuture prefetchFiles( Flowable transfers = Flowable.fromIterable(files) .flatMapSingle( - input -> toTransferResult(prefetchFile(dirCtx, metadataProvider, input, priority))); + input -> toTransferResult(prefetchFile(dirCtx, metadataSupplier, input, priority))); Completable prefetch = Completable.using( @@ -324,7 +323,7 @@ public ListenableFuture prefetchFiles( private Completable prefetchFile( DirectoryContext dirCtx, - MetadataProvider metadataProvider, + MetadataSupplier metadataSupplier, ActionInput input, Priority priority) throws IOException { @@ -335,12 +334,12 @@ private Completable prefetchFile( PathFragment execPath = input.getExecPath(); - FileArtifactValue metadata = metadataProvider.getMetadata(input); + FileArtifactValue metadata = metadataSupplier.getMetadata(input); if (metadata == null || !canDownloadFile(execRoot.getRelative(execPath), metadata)) { return Completable.complete(); } - @Nullable Symlink symlink = maybeGetSymlink(input, metadata, metadataProvider); + @Nullable Symlink symlink = maybeGetSymlink(input, metadata, metadataSupplier); if (symlink != null) { checkState(execPath.startsWith(symlink.getLinkExecPath())); @@ -348,7 +347,7 @@ private Completable prefetchFile( symlink.getTargetExecPath().getRelative(execPath.relativeTo(symlink.getLinkExecPath())); } - @Nullable PathFragment treeRootExecPath = maybeGetTreeRoot(input, metadataProvider); + @Nullable PathFragment treeRootExecPath = maybeGetTreeRoot(input, metadataSupplier); Completable result = downloadFileNoCheckRx( @@ -375,7 +374,7 @@ private Completable prefetchFile( * FileArtifactValue#getMaterializationExecPath()} field in their metadata. */ @Nullable - private PathFragment maybeGetTreeRoot(ActionInput input, MetadataProvider metadataProvider) + private PathFragment maybeGetTreeRoot(ActionInput input, MetadataSupplier metadataSupplier) throws IOException { if (!(input instanceof TreeFileArtifact)) { return null; @@ -383,7 +382,7 @@ private PathFragment maybeGetTreeRoot(ActionInput input, MetadataProvider metada SpecialArtifact treeArtifact = ((TreeFileArtifact) input).getParent(); FileArtifactValue treeMetadata = checkNotNull( - metadataProvider.getMetadata(treeArtifact), + metadataSupplier.getMetadata(treeArtifact), "input %s belongs to a tree artifact whose metadata is missing", input); return treeMetadata.getMaterializationExecPath().orElse(treeArtifact.getExecPath()); @@ -400,17 +399,17 @@ private PathFragment maybeGetTreeRoot(ActionInput input, MetadataProvider metada */ @Nullable private Symlink maybeGetSymlink( - ActionInput input, FileArtifactValue metadata, MetadataProvider metadataProvider) + ActionInput input, FileArtifactValue metadata, MetadataSupplier metadataSupplier) throws IOException { if (input instanceof TreeFileArtifact) { // Check whether the entire tree artifact should be prefetched into a separate location. SpecialArtifact treeArtifact = ((TreeFileArtifact) input).getParent(); FileArtifactValue treeMetadata = checkNotNull( - metadataProvider.getMetadata(treeArtifact), + metadataSupplier.getMetadata(treeArtifact), "input %s belongs to a tree artifact whose metadata is missing", input); - return maybeGetSymlink(treeArtifact, treeMetadata, metadataProvider); + return maybeGetSymlink(treeArtifact, treeMetadata, metadataSupplier); } PathFragment execPath = input.getExecPath(); PathFragment materializationExecPath = metadata.getMaterializationExecPath().orElse(execPath); @@ -611,7 +610,10 @@ public void finalizeAction(Action action, MetadataHandler metadataHandler) { } if (!inputsToDownload.isEmpty()) { - var future = prefetchFiles(inputsToDownload, metadataHandler, Priority.HIGH); + // "input" here means "input to another action" (but an output of this one), so + // getOutputMetadata() is the right method to pass to prefetchFiles() + var future = + prefetchFiles(inputsToDownload, metadataHandler::getOutputMetadata, Priority.HIGH); addCallback( future, new FutureCallback() { @@ -632,7 +634,8 @@ public void onFailure(Throwable throwable) { } if (!outputsToDownload.isEmpty()) { - var future = prefetchFiles(outputsToDownload, metadataHandler, Priority.LOW); + var future = + prefetchFiles(outputsToDownload, metadataHandler::getOutputMetadata, Priority.LOW); addCallback( future, new FutureCallback() { 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 e2c25cbf9d3a0d..f6ad8ccc56dbd4 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 @@ -569,21 +569,30 @@ public ActionInput getInput(String execPath) { return null; } + public FileArtifactValue getOutputMetadataForTopLevelArtifactDownloader(ActionInput input) + throws IOException { + RemoteFileInfo remoteFile = + remoteOutputTree.getRemoteFileInfo( + execRoot.getRelative(input.getExecPath()), /* followSymlinks= */ true); + if (remoteFile != null) { + return createRemoteMetadata(remoteFile); + } + + // TODO(tjgq): This should not work. + // The astute reader will notice that when this method is called, the artifact to be downloaded + // is an *output* artifact from the point of view of this RemoteActionFileSystem. The way this + // apparently works is that this is a SingleBuildFileCache, which then stat()s the actual file + // system, where the output file is materialized in mysterious ways. + // + // For further bafflement, see the comment at ToplevelArtifactsDownloader.downloadTestOutput(). + return fileCache.getInputMetadata(input); + } + @Nullable @Override - public FileArtifactValue getMetadata(ActionInput input) throws IOException { + public FileArtifactValue getInputMetadata(ActionInput input) throws IOException { PathFragment execPath = input.getExecPath(); - FileArtifactValue m = getMetadataByExecPath(execPath); - if (m != null) { - return m; - } - // TODO(tjgq): Consider only falling back to the local filesystem for source (non-output) files. - // The output fallback is needed when an undeclared output of a spawn is consumed by another - // spawn within the same action; specifically, when the first spawn is local but the second is - // remote, or, in the context of a failed test attempt, when both spawns are remote but the - // first one fails. In both cases, we don't currently inject the output metadata for the first - // spawn; if we did so, then we could stop falling back here. - return fileCache.getMetadata(input); + return inputArtifactData.getMetadata(execPath); } @Nullable @@ -641,7 +650,9 @@ private void downloadFileIfRemote(PathFragment path) throws IOException { // path. Therefore, we synthesize one here just so we're able to call prefetchFiles. input = ActionInputHelper.fromPath(execPath); } - getFromFuture(inputFetcher.prefetchFiles(ImmutableList.of(input), this, Priority.CRITICAL)); + getFromFuture( + inputFetcher.prefetchFiles( + ImmutableList.of(input), this::getInputMetadata, Priority.CRITICAL)); } catch (InterruptedException e) { Thread.currentThread().interrupt(); throw new IOException(String.format("Received interrupt while fetching file '%s'", path), e); diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java index 5bffa74970717b..a1c63ee91bd2d1 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java @@ -998,7 +998,8 @@ public void executorInit(CommandEnvironment env, BuildRequest request, ExecutorB (path) -> { FileSystem fileSystem = path.getFileSystem(); if (fileSystem instanceof RemoteActionFileSystem) { - return (RemoteActionFileSystem) path.getFileSystem(); + RemoteActionFileSystem rafs = (RemoteActionFileSystem) path.getFileSystem(); + return rafs::getOutputMetadataForTopLevelArtifactDownloader; } return null; }); diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java index 26ddfbd2e1713d..a3171aa70ef6bb 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java @@ -195,7 +195,7 @@ private void checkForConcurrentModifications() if (input instanceof VirtualActionInput) { continue; } - FileArtifactValue metadata = context.getMetadataProvider().getMetadata(input); + FileArtifactValue metadata = context.getMetadataProvider().getInputMetadata(input); Path path = execRoot.getRelative(input.getExecPath()); if (metadata.wasModifiedSinceDigest(path)) { throw new IOException(path + " was modified during execution"); diff --git a/src/main/java/com/google/devtools/build/lib/remote/ToplevelArtifactsDownloader.java b/src/main/java/com/google/devtools/build/lib/remote/ToplevelArtifactsDownloader.java index 06ebe98c0493b7..2dd601666f5083 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/ToplevelArtifactsDownloader.java +++ b/src/main/java/com/google/devtools/build/lib/remote/ToplevelArtifactsDownloader.java @@ -28,10 +28,10 @@ import com.google.common.util.concurrent.ListenableFuture; import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.ActionInputHelper; +import com.google.devtools.build.lib.actions.ActionInputPrefetcher.MetadataSupplier; import com.google.devtools.build.lib.actions.ActionInputPrefetcher.Priority; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.FileArtifactValue; -import com.google.devtools.build.lib.actions.MetadataProvider; import com.google.devtools.build.lib.analysis.AspectCompleteEvent; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.ConfiguredTargetValue; @@ -41,7 +41,6 @@ import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget; import com.google.devtools.build.lib.analysis.test.CoverageReport; import com.google.devtools.build.lib.analysis.test.TestAttempt; -import com.google.devtools.build.lib.remote.util.StaticMetadataProvider; import com.google.devtools.build.lib.skyframe.ActionExecutionValue; import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey; import com.google.devtools.build.lib.skyframe.TreeArtifactValue; @@ -72,7 +71,7 @@ private enum CommandMode { private final MemoizingEvaluator memoizingEvaluator; private final AbstractActionInputPrefetcher actionInputPrefetcher; private final PathFragment execRoot; - private final PathToMetadataProvider pathToMetadataProvider; + private final PathToMetadataSupplier pathToMetadataSupplier; public ToplevelArtifactsDownloader( String commandName, @@ -80,7 +79,7 @@ public ToplevelArtifactsDownloader( MemoizingEvaluator memoizingEvaluator, AbstractActionInputPrefetcher actionInputPrefetcher, PathFragment execRoot, - PathToMetadataProvider pathToMetadataProvider) { + PathToMetadataSupplier pathToMetadataSupplier) { switch (commandName) { case "build": this.commandMode = CommandMode.BUILD; @@ -101,19 +100,19 @@ public ToplevelArtifactsDownloader( this.memoizingEvaluator = memoizingEvaluator; this.actionInputPrefetcher = actionInputPrefetcher; this.execRoot = execRoot; - this.pathToMetadataProvider = pathToMetadataProvider; + this.pathToMetadataSupplier = pathToMetadataSupplier; } /** - * Interface that converts a {@link Path} into a {@link MetadataProvider} suitable for retrieving + * Interface that converts a {@link Path} into a {@link MetadataSupplier} suitable for retrieving * metadata for that path. * *

{@link ToplevelArtifactsDownloader} may only used in conjunction with filesystems that - * implement {@link MetadataProvider}. + * implement {@link MetadataSupplier}. */ - public interface PathToMetadataProvider { + public interface PathToMetadataSupplier { @Nullable - MetadataProvider getMetadataProvider(Path path); + MetadataSupplier getMetadataSupplier(Path path); } private void downloadTestOutput(Path path) { @@ -125,13 +124,13 @@ private void downloadTestOutput(Path path) { // action didn't get the chance to execute. In this case the MetadataProvider is null, which // is fine because test outputs are already downloaded (otherwise the action cache wouldn't // have been hit). - MetadataProvider metadataProvider = pathToMetadataProvider.getMetadataProvider(path); - if (metadataProvider != null) { + MetadataSupplier metadataSupplier = pathToMetadataSupplier.getMetadataSupplier(path); + if (metadataSupplier != null) { // RemoteActionFileSystem#getInput returns null for undeclared test outputs. ActionInput input = ActionInputHelper.fromPath(path.asFragment().relativeTo(execRoot)); ListenableFuture future = actionInputPrefetcher.prefetchFiles( - ImmutableList.of(input), metadataProvider, Priority.LOW); + ImmutableList.of(input), metadataSupplier, Priority.LOW); addCallback( future, new FutureCallback() { @@ -251,7 +250,7 @@ private void downloadTargetOutputs( outputsAndMetadata.keySet().stream() .filter(ToplevelArtifactsDownloader::isNonTreeArtifact) .collect(toImmutableSet()), - new StaticMetadataProvider(outputsAndMetadata), + outputsAndMetadata::get, Priority.LOW); addCallback( diff --git a/src/main/java/com/google/devtools/build/lib/remote/merkletree/DirectoryTreeBuilder.java b/src/main/java/com/google/devtools/build/lib/remote/merkletree/DirectoryTreeBuilder.java index c9089c187e9091..1897488df1e196 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/merkletree/DirectoryTreeBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/remote/merkletree/DirectoryTreeBuilder.java @@ -163,7 +163,7 @@ private static int buildFromActionInputs( FileArtifactValue metadata = Preconditions.checkNotNull( - metadataProvider.getMetadata(input), + metadataProvider.getInputMetadata(input), "missing metadata for '%s'", input.getExecPathString()); switch (metadata.getType()) { diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java index 282396c491eb0d..7bc6e1876c6dde 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java @@ -493,7 +493,15 @@ private void checkForConcurrentModifications(SpawnExecutionContext context) continue; } - FileArtifactValue metadata = context.getMetadataProvider().getMetadata(input); + FileArtifactValue metadata = context.getMetadataProvider().getInputMetadata(input); + if (metadata == null) { + // This can happen if we are executing a spawn in an action that has multiple spawns and + // the output of one is the input of another. In this case, we assume that no one modifies + // an output of the first spawn before the action is completed (which requires the + // the completion of the second spawn, which happens after this point is reached in the + // code) + continue; + } if (!metadata.getType().isFile()) { // The hermetic sandbox creates hardlinks from files inside sandbox to files outside // sandbox. The content of the files outside the sandbox could have been tampered with via diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java index 90ec5168beba0f..9eb4d220986534 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java @@ -741,7 +741,6 @@ private ActionExecutionValue checkCacheAndExecuteIfNeeded( ActionMetadataHandler metadataHandler = ActionMetadataHandler.create( state.inputArtifactData, - action.discoversInputs(), skyframeActionExecutor.useArchivedTreeArtifacts(action), skyframeActionExecutor.getOutputPermissions(), action.getOutputs(), @@ -907,7 +906,7 @@ private DiscoveredState addDiscoveredInputs( // reduce iteration cost. List unknownDiscoveredInputs = new ArrayList<>(); for (Artifact input : state.discoveredInputs.toList()) { - if (inputData.getMetadata(input) == null) { + if (inputData.getInputMetadata(input) == null) { unknownDiscoveredInputs.add(input); } } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java index 1f0d3a65cb0fcd..057642e8e9b5cd 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java @@ -92,7 +92,6 @@ final class ActionMetadataHandler implements MetadataHandler { */ static ActionMetadataHandler create( ActionInputMap inputArtifactData, - boolean forInputDiscovery, boolean archivedTreeArtifactsEnabled, OutputPermissions outputPermissions, ImmutableSet outputs, @@ -104,7 +103,6 @@ static ActionMetadataHandler create( Map> expandedFilesets) { return new ActionMetadataHandler( inputArtifactData, - forInputDiscovery, archivedTreeArtifactsEnabled, outputPermissions, outputs, @@ -118,7 +116,6 @@ static ActionMetadataHandler create( } private final ActionInputMap inputArtifactData; - private final boolean forInputDiscovery; private final boolean archivedTreeArtifactsEnabled; private final OutputPermissions outputPermissions; private final ImmutableMap filesetMapping; @@ -137,7 +134,6 @@ static ActionMetadataHandler create( private ActionMetadataHandler( ActionInputMap inputArtifactData, - boolean forInputDiscovery, boolean archivedTreeArtifactsEnabled, OutputPermissions outputPermissions, ImmutableSet outputs, @@ -149,7 +145,6 @@ private ActionMetadataHandler( ImmutableMap filesetMapping, OutputStore store) { this.inputArtifactData = checkNotNull(inputArtifactData); - this.forInputDiscovery = forInputDiscovery; this.archivedTreeArtifactsEnabled = archivedTreeArtifactsEnabled; this.outputPermissions = outputPermissions; this.outputs = checkNotNull(outputs); @@ -176,7 +171,6 @@ private ActionMetadataHandler( ActionMetadataHandler transformAfterInputDiscovery(OutputStore store) { return new ActionMetadataHandler( inputArtifactData, - /* forInputDiscovery= */ false, archivedTreeArtifactsEnabled, outputPermissions, outputs, @@ -237,9 +231,9 @@ private boolean isKnownOutput(Artifact artifact) { || (artifact.hasParent() && outputs.contains(artifact.getParent())); } - @Override @Nullable - public FileArtifactValue getMetadata(ActionInput actionInput) throws IOException { + @Override + public FileArtifactValue getInputMetadata(ActionInput actionInput) throws IOException { if (!(actionInput instanceof Artifact)) { PathFragment inputPath = actionInput.getExecPath(); PathFragment filesetKeyPath = @@ -250,12 +244,21 @@ public FileArtifactValue getMetadata(ActionInput actionInput) throws IOException Artifact artifact = (Artifact) actionInput; FileArtifactValue value; + value = inputArtifactData.getInputMetadata(artifact); + if (value != null) { + return checkExists(value, artifact); + } + + return null; + } + + @Nullable + @Override + public FileArtifactValue getOutputMetadata(ActionInput actionInput) throws IOException { + Artifact artifact = (Artifact) actionInput; + FileArtifactValue value; + if (!isKnownOutput(artifact)) { - value = inputArtifactData.getMetadata(artifact); - if (value != null) { - return checkExists(value, artifact); - } - checkState(forInputDiscovery, "%s is not present in declared outputs: %s", artifact, outputs); return null; } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java index 90be1861e3c98d..a5f6a0addb7c29 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java @@ -37,7 +37,6 @@ import com.google.devtools.build.lib.actions.ActionExecutionContext.LostInputsCheck; import com.google.devtools.build.lib.actions.ActionExecutionException; import com.google.devtools.build.lib.actions.ActionExecutionStatusReporter; -import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.ActionInputMap; import com.google.devtools.build.lib.actions.ActionInputPrefetcher; import com.google.devtools.build.lib.actions.ActionKeyContext; @@ -56,6 +55,7 @@ import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact; import com.google.devtools.build.lib.actions.ArtifactPathResolver; import com.google.devtools.build.lib.actions.CachedActionEvent; +import com.google.devtools.build.lib.actions.DelegatingPairMetadataProvider; import com.google.devtools.build.lib.actions.DiscoveredModulesPruner; import com.google.devtools.build.lib.actions.EnvironmentalExecException; import com.google.devtools.build.lib.actions.Executor; @@ -863,7 +863,7 @@ private MetadataProvider createFileCache( if (actionFileSystem instanceof MetadataProvider) { return (MetadataProvider) actionFileSystem; } - return new DelegatingPairFileCache(graphFileCache, perBuildFileCache); + return new DelegatingPairMetadataProvider(graphFileCache, perBuildFileCache); } /** @@ -1223,7 +1223,7 @@ private ActionExecutionValue actuallyCompleteAction( primaryOutputMetadata = FileArtifactValue.OMITTED_FILE_MARKER; } else { try { - primaryOutputMetadata = metadataHandler.getMetadata(primaryOutput); + primaryOutputMetadata = metadataHandler.getOutputMetadata(primaryOutput); } catch (IOException e) { throw new IllegalStateException("Metadata already obtained for " + primaryOutput, e); } @@ -1482,7 +1482,7 @@ private boolean checkOutputs( // call it if we know the artifact is not omitted. if (!metadataHandler.artifactOmitted(output)) { try { - FileArtifactValue metadata = metadataHandler.getMetadata(output); + FileArtifactValue metadata = metadataHandler.getOutputMetadata(output); addOutputToMetrics( output, @@ -1717,31 +1717,6 @@ void setActionExecutionProgressReportingObjects( this.completionReceiver = completionReceiver; } - private static final class DelegatingPairFileCache implements MetadataProvider { - private final MetadataProvider perActionCache; - private final MetadataProvider perBuildFileCache; - - private DelegatingPairFileCache( - MetadataProvider mainCache, MetadataProvider perBuildFileCache) { - this.perActionCache = mainCache; - this.perBuildFileCache = perBuildFileCache; - } - - @Override - public FileArtifactValue getMetadata(ActionInput input) throws IOException { - FileArtifactValue metadata = perActionCache.getMetadata(input); - return (metadata != null) && (metadata != FileArtifactValue.MISSING_FILE_MARKER) - ? metadata - : perBuildFileCache.getMetadata(input); - } - - @Override - public ActionInput getInput(String execPath) { - ActionInput input = perActionCache.getInput(execPath); - return input != null ? input : perBuildFileCache.getInput(execPath); - } - } - /** Adapts a {@link FileOutErr} to an {@link Event.ProcessOutput}. */ private static class ActionOutputEventData implements Event.ProcessOutput { private final FileOutErr fileOutErr; diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerFilesHash.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerFilesHash.java index 85a4986e7a9d46..571aebd6ac0e60 100644 --- a/src/main/java/com/google/devtools/build/lib/worker/WorkerFilesHash.java +++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerFilesHash.java @@ -67,11 +67,12 @@ public static SortedMap getWorkerFilesWithDigests( ActionInputHelper.expandArtifacts( spawn.getToolFiles(), artifactExpander, /* keepEmptyTreeArtifacts= */ false); for (ActionInput tool : tools) { - @Nullable FileArtifactValue metadata = actionInputFileCache.getMetadata(tool); + @Nullable FileArtifactValue metadata = actionInputFileCache.getInputMetadata(tool); if (metadata == null) { throw new MissingInputException(tool); } - workerFilesMap.put(tool.getExecPath(), actionInputFileCache.getMetadata(tool).getDigest()); + workerFilesMap.put( + tool.getExecPath(), actionInputFileCache.getInputMetadata(tool).getDigest()); } for (Map.Entry> rootAndMappings : @@ -81,7 +82,8 @@ public static SortedMap getWorkerFilesWithDigests( for (Map.Entry mapping : rootAndMappings.getValue().entrySet()) { Artifact localArtifact = mapping.getValue(); if (localArtifact != null) { - @Nullable FileArtifactValue metadata = actionInputFileCache.getMetadata(localArtifact); + @Nullable + FileArtifactValue metadata = actionInputFileCache.getInputMetadata(localArtifact); if (metadata == null) { throw new MissingInputException(localArtifact); } diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java index b4a6b118890ad7..ccd82c1e350728 100644 --- a/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java @@ -262,7 +262,7 @@ private WorkRequest createWorkRequest( checkNotNull(virtualInputDigests.get(input), "missing metadata for virtual input"); } else { FileArtifactValue metadata = - checkNotNull(inputFileCache.getMetadata(input), "missing metadata for input"); + checkNotNull(inputFileCache.getInputMetadata(input), "missing metadata for input"); digestBytes = metadata.getDigest(); } ByteString digest; 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 2eea1d7e4d81ad..68c3006733c7f1 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 @@ -594,7 +594,8 @@ public void saveOutputMetadata_remoteFileMetadataLoaded() throws Exception { ActionCache.Entry entry = cache.get(output.getExecPathString()); assertThat(entry).isNotNull(); assertThat(entry.getOutputFile(output)).isEqualTo(createRemoteFileMetadata(content)); - assertThat(metadataHandler.getMetadata(output)).isEqualTo(createRemoteFileMetadata(content)); + assertThat(metadataHandler.getOutputMetadata(output)) + .isEqualTo(createRemoteFileMetadata(content)); } @Test @@ -1321,7 +1322,16 @@ public void injectTree(SpecialArtifact treeArtifact, TreeArtifactValue tree) { } @Override - public FileArtifactValue getMetadata(ActionInput input) throws IOException { + public FileArtifactValue getInputMetadata(ActionInput input) throws IOException { + if (!(input instanceof Artifact)) { + return null; + } + + return FileArtifactValue.createForTesting((Artifact) input); + } + + @Override + public FileArtifactValue getOutputMetadata(ActionInput input) throws IOException { if (!(input instanceof Artifact)) { return null; } 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 b3fb345e031600..9f0633d6ae041f 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 @@ -206,7 +206,7 @@ public void putTreeArtifact_afterPutTreeArtifactWithSameExecPath_doesNothing() { assertContainsTree(tree1, tree1Value); // Cannot assertContainsTree since the execpath will point to tree1 instead. - assertThat(map.getMetadata(tree2)).isEqualTo(tree1Value.getMetadata()); + assertThat(map.getInputMetadata(tree2)).isEqualTo(tree1Value.getMetadata()); assertThat(map.getTreeMetadata(tree2.getExecPath())).isSameInstanceAs(tree1Value); assertThat(map.getInput(tree2.getExecPathString())).isSameInstanceAs(tree1); assertDoesNotContain(tree2File); @@ -253,8 +253,8 @@ public void putTreeArtifact_nestedFile_returnsNestedFileFromExecPath( () -> map.put(file, fileMetadata, /*depOwner=*/ null), () -> map.putTreeArtifact(tree, treeValue, /*depOwner=*/ null)); - assertThat(map.getMetadata(file)).isSameInstanceAs(fileMetadata); - assertThat(map.getMetadata(treeFile)).isSameInstanceAs(treeFileMetadata); + assertThat(map.getInputMetadata(file)).isSameInstanceAs(fileMetadata); + assertThat(map.getInputMetadata(treeFile)).isSameInstanceAs(treeFileMetadata); assertThat(map.getMetadata(treeFile.getExecPath())).isSameInstanceAs(fileMetadata); assertThat(map.getInput(treeFile.getExecPathString())).isSameInstanceAs(file); } @@ -297,7 +297,7 @@ public void putTreeArtifact_omittedTree_addsEntryWithNoChildren() { map.putTreeArtifact(tree, TreeArtifactValue.OMITTED_TREE_MARKER, /*depOwner=*/ null); // Cannot assertContainsTree since TreeArtifactValue#getMetadata throws for OMITTED_TREE_MARKER. - assertThat(map.getMetadata(tree)).isSameInstanceAs(FileArtifactValue.OMITTED_FILE_MARKER); + assertThat(map.getInputMetadata(tree)).isSameInstanceAs(FileArtifactValue.OMITTED_FILE_MARKER); assertThat(map.getMetadata(tree.getExecPath())) .isSameInstanceAs(FileArtifactValue.OMITTED_FILE_MARKER); } @@ -338,7 +338,7 @@ public void getMetadata_actionInputWithTreeExecPath_returnsTreeArtifactEntries() map.putTreeArtifact(tree, TreeArtifactValue.empty(), /*depOwner=*/ null); ActionInput input = ActionInputHelper.fromPath(tree.getExecPath()); - assertThat(map.getMetadata(input)).isEqualTo(TreeArtifactValue.empty().getMetadata()); + assertThat(map.getInputMetadata(input)).isEqualTo(TreeArtifactValue.empty().getMetadata()); } @Test @@ -355,7 +355,7 @@ public void getMetadata_actionInputWithTreeFileExecPath_returnsTreeArtifactEntri inputMap.putTreeArtifact(tree, treeValue, /*depOwner=*/ null); ActionInput input = ActionInputHelper.fromPath(treeFile.getExecPath()); - FileArtifactValue metadata = inputMap.getMetadata(input); + FileArtifactValue metadata = inputMap.getInputMetadata(input); assertThat(metadata).isSameInstanceAs(treeFileMetadata); assertThat(exceptionCaptor.getValue()).isInstanceOf(IllegalArgumentException.class); @@ -376,7 +376,7 @@ public void getMetadata_artifactWithTreeFileExecPath_returnsNull() { // Even though we could match the artifact by exec path, it was not registered as a nested // artifact -- only the tree file was. - assertThat(map.getMetadata(artifact)).isNull(); + assertThat(map.getInputMetadata(artifact)).isNull(); } @Test @@ -399,17 +399,17 @@ public void getMetadata_treeFileUnderOmittedParent_fails() { TreeFileArtifact child = TreeFileArtifact.createTreeOutput(tree, "file"); map.putTreeArtifact(tree, TreeArtifactValue.OMITTED_TREE_MARKER, /*depOwner=*/ null); - assertThrows(IllegalArgumentException.class, () -> map.getMetadata(child)); + assertThrows(IllegalArgumentException.class, () -> map.getInputMetadata(child)); } @Test - public void getMetadata_treeFileUnderFile_fails() { + public void getInputMetadata_treeFileUnderFile_fails() { SpecialArtifact tree = createTreeArtifact("tree"); TreeFileArtifact child = TreeFileArtifact.createTreeOutput(tree, "file"); ActionInput file = ActionInputHelper.fromPath(tree.getExecPath()); map.put(file, TestMetadata.create(1), /*depOwner=*/ null); - assertThrows(IllegalArgumentException.class, () -> map.getMetadata(child)); + assertThrows(IllegalArgumentException.class, () -> map.getInputMetadata(child)); } @Test @@ -449,7 +449,7 @@ public void stress() { assertThat(map.sizeForDebugging()).isEqualTo(data.size()); for (int i = 0; i < data.size(); ++i) { TestEntry entry = data.get(i); - assertThat(map.getMetadata(entry.input)).isEqualTo(entry.metadata); + assertThat(map.getInputMetadata(entry.input)).isEqualTo(entry.metadata); } } } @@ -459,14 +459,14 @@ private boolean put(String execPath, int value) { } private void assertContains(String execPath, int value) { - assertThat(map.getMetadata(new TestInput(execPath))).isEqualTo(TestMetadata.create(value)); + assertThat(map.getInputMetadata(new TestInput(execPath))).isEqualTo(TestMetadata.create(value)); assertThat(map.getMetadata(PathFragment.create(execPath))) .isEqualTo(TestMetadata.create(value)); assertThat(map.getInput(execPath)).isEqualTo(new TestInput(execPath)); } private void assertDoesNotContain(ActionInput input) { - assertThat(map.getMetadata(input)).isNull(); + assertThat(map.getInputMetadata(input)).isNull(); assertThat(map.getMetadata(input.getExecPath())).isNull(); assertThat(map.getTreeMetadata(input.getExecPath())).isNull(); assertThat(map.getInput(input.getExecPathString())).isNull(); @@ -474,7 +474,7 @@ private void assertDoesNotContain(ActionInput input) { private void assertContainsFile(ActionInput input, FileArtifactValue fileValue) { checkArgument(!(input instanceof SpecialArtifact), "use assertContainsTree for tree artifacts"); - assertThat(map.getMetadata(input)).isSameInstanceAs(fileValue); + assertThat(map.getInputMetadata(input)).isSameInstanceAs(fileValue); assertThat(map.getMetadata(input.getExecPath())).isSameInstanceAs(fileValue); assertThat(map.getTreeMetadata(input.getExecPath())).isNull(); assertThat(map.getInput(input.getExecPathString())).isSameInstanceAs(input); @@ -482,7 +482,7 @@ private void assertContainsFile(ActionInput input, FileArtifactValue fileValue) private void assertContainsTree(SpecialArtifact input, TreeArtifactValue treeValue) { // TreeArtifactValue#getMetadata returns a freshly allocated instance. - assertThat(map.getMetadata(input)).isEqualTo(treeValue.getMetadata()); + assertThat(map.getInputMetadata(input)).isEqualTo(treeValue.getMetadata()); assertThat(map.getMetadata(input.getExecPath())).isEqualTo(treeValue.getMetadata()); assertThat(map.getTreeMetadata(input.getExecPath())).isSameInstanceAs(treeValue); assertThat(map.getInput(input.getExecPathString())).isSameInstanceAs(input); 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 f9ca1c191ee110..ba9adb1c8eea1a 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 @@ -1057,7 +1057,12 @@ public String toString() { */ public static class FakeMetadataHandlerBase implements MetadataHandler { @Override - public FileArtifactValue getMetadata(ActionInput input) throws IOException { + public FileArtifactValue getInputMetadata(ActionInput input) throws IOException { + throw new UnsupportedOperationException(); + } + + @Override + public FileArtifactValue getOutputMetadata(ActionInput input) throws IOException { throw new UnsupportedOperationException(); } diff --git a/src/test/java/com/google/devtools/build/lib/exec/SingleBuildFileCacheTest.java b/src/test/java/com/google/devtools/build/lib/exec/SingleBuildFileCacheTest.java index 5b990c8c0d2b6b..aa698adecb8066 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/SingleBuildFileCacheTest.java +++ b/src/test/java/com/google/devtools/build/lib/exec/SingleBuildFileCacheTest.java @@ -81,7 +81,7 @@ public void testNonExistentPath() throws Exception { assertThrows( "non existent file should raise exception", IOException.class, - () -> underTest.getMetadata(empty)); + () -> underTest.getInputMetadata(empty)); } @Test @@ -93,25 +93,25 @@ public void testDirectory() throws Exception { assertThrows( "directory should raise exception", DigestOfDirectoryException.class, - () -> underTest.getMetadata(input)); + () -> underTest.getInputMetadata(input)); assertThat(expected).hasMessageThat().isEqualTo("Input is a directory: /directory"); } @Test public void testCache() throws Exception { ActionInput empty = ActionInputHelper.fromPath("/empty"); - underTest.getMetadata(empty).getDigest(); + underTest.getInputMetadata(empty).getDigest(); assertThat(calls).containsKey("/empty"); assertThat((int) calls.get("/empty")).isEqualTo(1); - underTest.getMetadata(empty).getDigest(); + underTest.getInputMetadata(empty).getDigest(); assertThat((int) calls.get("/empty")).isEqualTo(1); } @Test public void testBasic() throws Exception { ActionInput empty = ActionInputHelper.fromPath("/empty"); - assertThat(underTest.getMetadata(empty).getSize()).isEqualTo(0); - byte[] digest = underTest.getMetadata(empty).getDigest(); + assertThat(underTest.getInputMetadata(empty).getSize()).isEqualTo(0); + byte[] digest = underTest.getInputMetadata(empty).getDigest(); byte[] expected = fs.getDigestFunction().getHashFunction().hashBytes(new byte[0]).asBytes(); assertThat(digest).isEqualTo(expected); } @@ -125,7 +125,7 @@ public void testUnreadableFileWhenFileSystemSupportsDigest() throws Exception { Path file = fs.getPath("/unreadable"); FileSystemUtils.createEmptyFile(file); file.chmod(0); - byte[] actualDigest = underTest.getMetadata(input).getDigest(); + byte[] actualDigest = underTest.getInputMetadata(input).getDigest(); assertThat(actualDigest).isEqualTo(expectedDigest); } } diff --git a/src/test/java/com/google/devtools/build/lib/exec/StandaloneTestStrategyTest.java b/src/test/java/com/google/devtools/build/lib/exec/StandaloneTestStrategyTest.java index b84a06eaec66fd..349397691dbc87 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/StandaloneTestStrategyTest.java +++ b/src/test/java/com/google/devtools/build/lib/exec/StandaloneTestStrategyTest.java @@ -28,6 +28,7 @@ import com.google.devtools.build.lib.actions.ActionContext; import com.google.devtools.build.lib.actions.ActionExecutionContext; import com.google.devtools.build.lib.actions.ActionExecutionException; +import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.ActionInputPrefetcher; import com.google.devtools.build.lib.actions.ActionKeyContext; import com.google.devtools.build.lib.actions.Artifact; @@ -180,6 +181,11 @@ public Path getExecRoot() { return StandaloneTestStrategyTest.this.getExecRoot(); } + @Override + public ActionExecutionContext withOutputsAsInputs(Iterable inputs) { + return this; + } + @Override public ActionExecutionContext withFileOutErr(FileOutErr fileOutErr) { return new FakeActionExecutionContext( diff --git a/src/test/java/com/google/devtools/build/lib/exec/util/FakeActionInputFileCache.java b/src/test/java/com/google/devtools/build/lib/exec/util/FakeActionInputFileCache.java index 12e9d5dcf6729a..5c4d0d356f15a4 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/util/FakeActionInputFileCache.java +++ b/src/test/java/com/google/devtools/build/lib/exec/util/FakeActionInputFileCache.java @@ -32,7 +32,7 @@ public void put(ActionInput artifact, FileArtifactValue metadata) { } @Override - public FileArtifactValue getMetadata(ActionInput input) throws IOException { + public FileArtifactValue getInputMetadata(ActionInput input) throws IOException { return Preconditions.checkNotNull(inputs.get(input)); } 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 d76c3ce8b797a1..c41e6da1582747 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 @@ -45,9 +45,7 @@ import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.actions.FileArtifactValue.RemoteFileArtifactValue; -import com.google.devtools.build.lib.actions.MetadataProvider; import com.google.devtools.build.lib.actions.util.ActionsTestUtil; -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.testing.vfs.SpiedFileSystem; @@ -199,10 +197,9 @@ public void prefetchFiles_fileExists_doNotDownload() Map cas = new HashMap<>(); Artifact a = createRemoteArtifact("file", "hello world", metadata, cas); FileSystemUtils.writeContent(a.getPath(), "hello world".getBytes(UTF_8)); - MetadataProvider metadataProvider = new StaticMetadataProvider(metadata); AbstractActionInputPrefetcher prefetcher = spy(createPrefetcher(cas)); - wait(prefetcher.prefetchFiles(metadata.keySet(), metadataProvider, Priority.MEDIUM)); + wait(prefetcher.prefetchFiles(metadata.keySet(), metadata::get, Priority.MEDIUM)); verify(prefetcher, never()).doDownloadFile(any(), any(), any(), any(), any()); assertThat(prefetcher.downloadedFiles()).containsExactly(a.getPath()); @@ -216,10 +213,9 @@ public void prefetchFiles_fileExistsButContentMismatches_download() Map cas = new HashMap<>(); Artifact a = createRemoteArtifact("file", "hello world remote", metadata, cas); FileSystemUtils.writeContent(a.getPath(), "hello world local".getBytes(UTF_8)); - MetadataProvider metadataProvider = new StaticMetadataProvider(metadata); AbstractActionInputPrefetcher prefetcher = spy(createPrefetcher(cas)); - wait(prefetcher.prefetchFiles(metadata.keySet(), metadataProvider, Priority.MEDIUM)); + wait(prefetcher.prefetchFiles(metadata.keySet(), metadata::get, Priority.MEDIUM)); verify(prefetcher).doDownloadFile(any(), any(), eq(a.getExecPath()), any(), any()); assertThat(prefetcher.downloadedFiles()).containsExactly(a.getPath()); @@ -233,10 +229,9 @@ public void prefetchFiles_downloadRemoteFiles() throws Exception { Map cas = new HashMap<>(); Artifact a1 = createRemoteArtifact("file1", "hello world", metadata, cas); Artifact a2 = createRemoteArtifact("file2", "fizz buzz", metadata, cas); - MetadataProvider metadataProvider = new StaticMetadataProvider(metadata); AbstractActionInputPrefetcher prefetcher = createPrefetcher(cas); - wait(prefetcher.prefetchFiles(metadata.keySet(), metadataProvider, Priority.MEDIUM)); + wait(prefetcher.prefetchFiles(metadata.keySet(), metadata::get, Priority.MEDIUM)); assertThat(FileSystemUtils.readContent(a1.getPath(), UTF_8)).isEqualTo("hello world"); assertReadableNonWritableAndExecutable(a1.getPath()); @@ -252,10 +247,9 @@ public void prefetchFiles_downloadRemoteFiles_withMaterializationExecPath() thro Map cas = new HashMap<>(); 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, Priority.MEDIUM)); + wait(prefetcher.prefetchFiles(metadata.keySet(), metadata::get, Priority.MEDIUM)); assertThat(a.getPath().isSymbolicLink()).isTrue(); assertThat(a.getPath().readSymbolicLink()) @@ -283,10 +277,9 @@ public void prefetchFiles_downloadRemoteTrees() throws Exception { Artifact firstChild = children.get(0); Artifact secondChild = children.get(1); - MetadataProvider metadataProvider = new StaticMetadataProvider(metadata); AbstractActionInputPrefetcher prefetcher = createPrefetcher(cas); - wait(prefetcher.prefetchFiles(children, metadataProvider, Priority.MEDIUM)); + wait(prefetcher.prefetchFiles(children, metadata::get, Priority.MEDIUM)); assertThat(FileSystemUtils.readContent(firstChild.getPath(), UTF_8)).isEqualTo("content1"); assertThat(FileSystemUtils.readContent(secondChild.getPath(), UTF_8)).isEqualTo("content2"); @@ -314,12 +307,11 @@ public void prefetchFiles_downloadRemoteTrees_partial() throws Exception { Artifact firstChild = children.get(0); Artifact secondChild = children.get(1); - MetadataProvider metadataProvider = new StaticMetadataProvider(metadata); AbstractActionInputPrefetcher prefetcher = createPrefetcher(cas); wait( prefetcher.prefetchFiles( - ImmutableList.of(firstChild, secondChild), metadataProvider, Priority.MEDIUM)); + ImmutableList.of(firstChild, secondChild), metadata::get, Priority.MEDIUM)); assertThat(firstChild.getPath().exists()).isFalse(); assertThat(FileSystemUtils.readContent(secondChild.getPath(), UTF_8)).isEqualTo("content2"); @@ -346,10 +338,9 @@ public void prefetchFiles_downloadRemoteTrees_withMaterializationExecPath() thro Artifact firstChild = children.get(0); Artifact secondChild = children.get(1); - MetadataProvider metadataProvider = new StaticMetadataProvider(metadata); AbstractActionInputPrefetcher prefetcher = createPrefetcher(cas); - wait(prefetcher.prefetchFiles(children, metadataProvider, Priority.MEDIUM)); + wait(prefetcher.prefetchFiles(children, metadata::get, Priority.MEDIUM)); assertThat(tree.getPath().isSymbolicLink()).isTrue(); assertThat(tree.getPath().readSymbolicLink()) @@ -371,13 +362,11 @@ public void prefetchFiles_downloadRemoteTrees_withMaterializationExecPath() thro public void prefetchFiles_missingFiles_fails() throws Exception { Map metadata = new HashMap<>(); Artifact a = createRemoteArtifact("file1", "hello world", metadata, /* cas= */ new HashMap<>()); - MetadataProvider metadataProvider = new StaticMetadataProvider(metadata); AbstractActionInputPrefetcher prefetcher = createPrefetcher(new HashMap<>()); assertThrows( Exception.class, - () -> - wait(prefetcher.prefetchFiles(ImmutableList.of(a), metadataProvider, Priority.MEDIUM))); + () -> wait(prefetcher.prefetchFiles(ImmutableList.of(a), metadata::get, Priority.MEDIUM))); assertThat(prefetcher.downloadedFiles()).isEmpty(); assertThat(prefetcher.downloadsInProgress()).isEmpty(); @@ -391,10 +380,10 @@ public void prefetchFiles_ignoreNonRemoteFiles() throws Exception { FileSystemUtils.writeContent(p, UTF_8, "hello world"); Artifact a = ActionsTestUtil.createArtifact(artifactRoot, p); FileArtifactValue f = FileArtifactValue.createForTesting(a); - MetadataProvider metadataProvider = new StaticMetadataProvider(ImmutableMap.of(a, f)); + ImmutableMap metadata = ImmutableMap.of(a, f); AbstractActionInputPrefetcher prefetcher = createPrefetcher(new HashMap<>()); - wait(prefetcher.prefetchFiles(ImmutableList.of(a), metadataProvider, Priority.MEDIUM)); + wait(prefetcher.prefetchFiles(ImmutableList.of(a), metadata::get, Priority.MEDIUM)); assertThat(prefetcher.downloadedFiles()).isEmpty(); assertThat(prefetcher.downloadsInProgress()).isEmpty(); @@ -418,10 +407,9 @@ public void prefetchFiles_ignoreNonRemoteFiles_tree() throws Exception { Artifact firstChild = children.get(0); Artifact secondChild = children.get(1); - MetadataProvider metadataProvider = new StaticMetadataProvider(metadata); AbstractActionInputPrefetcher prefetcher = createPrefetcher(cas); - wait(prefetcher.prefetchFiles(children, metadataProvider, Priority.MEDIUM)); + wait(prefetcher.prefetchFiles(children, metadata::get, Priority.MEDIUM)); assertThat(firstChild.getPath().exists()).isFalse(); assertThat(FileSystemUtils.readContent(secondChild.getPath(), UTF_8)).isEqualTo("content2"); @@ -445,12 +433,11 @@ public void prefetchFiles_treeFiles_minimizeFilesystemOperations() throws Except Artifact firstChild = children.get(0); Artifact secondChild = children.get(1); - MetadataProvider metadataProvider = new StaticMetadataProvider(metadata); AbstractActionInputPrefetcher prefetcher = createPrefetcher(cas); wait( prefetcher.prefetchFiles( - ImmutableList.of(firstChild, secondChild), metadataProvider, Priority.MEDIUM)); + ImmutableList.of(firstChild, secondChild), metadata::get, Priority.MEDIUM)); verify(fs, times(1)).createWritableDirectory(tree.getPath().asFragment()); verify(fs, times(1)).createWritableDirectory(tree.getPath().getChild("subdir").asFragment()); @@ -464,7 +451,6 @@ public void prefetchFiles_multipleThreads_downloadIsCancelled() throws Exception Map metadata = new HashMap<>(); Map cas = new HashMap<>(); Artifact artifact = createRemoteArtifact("file1", "hello world", metadata, cas); - MetadataProvider metadataProvider = new StaticMetadataProvider(metadata); AbstractActionInputPrefetcher prefetcher = spy(createPrefetcher(cas)); SettableFuture downloadThatNeverFinishes = SettableFuture.create(); @@ -476,7 +462,7 @@ public void prefetchFiles_multipleThreads_downloadIsCancelled() throws Exception try { wait( prefetcher.prefetchFiles( - ImmutableList.of(artifact), metadataProvider, Priority.MEDIUM)); + ImmutableList.of(artifact), metadata::get, Priority.MEDIUM)); } catch (IOException | ExecException | InterruptedException ignored) { // do nothing } @@ -488,7 +474,7 @@ public void prefetchFiles_multipleThreads_downloadIsCancelled() throws Exception try { wait( prefetcher.prefetchFiles( - ImmutableList.of(artifact), metadataProvider, Priority.MEDIUM)); + ImmutableList.of(artifact), metadata::get, Priority.MEDIUM)); } catch (IOException | ExecException | InterruptedException ignored) { // do nothing } @@ -517,7 +503,6 @@ public void prefetchFiles_multipleThreads_downloadIsNotCancelledByOtherThreads() Map metadata = new HashMap<>(); Map cas = new HashMap<>(); Artifact artifact = createRemoteArtifact("file1", "hello world", metadata, cas); - MetadataProvider metadataProvider = new StaticMetadataProvider(metadata); SettableFuture download = SettableFuture.create(); AbstractActionInputPrefetcher prefetcher = spy(createPrefetcher(cas)); mockDownload(prefetcher, cas, () -> download); @@ -527,7 +512,7 @@ public void prefetchFiles_multipleThreads_downloadIsNotCancelledByOtherThreads() try { wait( prefetcher.prefetchFiles( - ImmutableList.of(artifact), metadataProvider, Priority.MEDIUM)); + ImmutableList.of(artifact), metadata::get, Priority.MEDIUM)); } catch (IOException | ExecException | InterruptedException ignored) { // do nothing } @@ -540,7 +525,7 @@ public void prefetchFiles_multipleThreads_downloadIsNotCancelledByOtherThreads() try { wait( prefetcher.prefetchFiles( - ImmutableList.of(artifact), metadataProvider, Priority.MEDIUM)); + ImmutableList.of(artifact), metadata::get, Priority.MEDIUM)); successful.set(true); } catch (IOException | ExecException | InterruptedException ignored) { // do nothing @@ -592,10 +577,7 @@ public void prefetchFiles_onInterrupt_deletePartialDownloadedFile() throws Excep () -> { try { getFromFuture( - prefetcher.prefetchFiles( - ImmutableList.of(a1), - new StaticMetadataProvider(metadata), - Priority.MEDIUM)); + prefetcher.prefetchFiles(ImmutableList.of(a1), metadata::get, Priority.MEDIUM)); } catch (IOException ignored) { // Intentionally left empty } catch (InterruptedException e) { @@ -618,12 +600,11 @@ public void missingInputs_addedToList() { Map metadata = new HashMap<>(); Map cas = new HashMap<>(); Artifact a = createRemoteArtifact("file", "hello world", metadata, /* cas= */ null); - MetadataProvider metadataProvider = new StaticMetadataProvider(metadata); AbstractActionInputPrefetcher prefetcher = createPrefetcher(cas); assertThrows( Exception.class, - () -> wait(prefetcher.prefetchFiles(metadata.keySet(), metadataProvider, Priority.MEDIUM))); + () -> wait(prefetcher.prefetchFiles(metadata.keySet(), metadata::get, Priority.MEDIUM))); assertThat(prefetcher.getMissingActionInputs()).contains(a); } diff --git a/src/test/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderTest.java b/src/test/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderTest.java index 0233d796701b0a..cd65718e0480ba 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderTest.java @@ -42,6 +42,7 @@ 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.StaticMetadataProvider; import com.google.devtools.build.lib.actions.util.ActionsTestUtil; import com.google.devtools.build.lib.authandtls.CallCredentialsProvider; import com.google.devtools.build.lib.buildeventstream.BuildEvent.LocalFile; @@ -59,7 +60,6 @@ import com.google.devtools.build.lib.remote.options.RemoteOptions; import com.google.devtools.build.lib.remote.util.DigestUtil; import com.google.devtools.build.lib.remote.util.RxNoGlobalErrorsRule; -import com.google.devtools.build.lib.remote.util.StaticMetadataProvider; import com.google.devtools.build.lib.remote.util.TestUtils; import com.google.devtools.build.lib.vfs.DigestHashFunction; import com.google.devtools.build.lib.vfs.FileSystem; @@ -378,7 +378,7 @@ public void remoteFileShouldNotBeUploaded_actionFs() throws Exception { PathConverter pathConverter = artifactUploader.upload(ImmutableMap.of(remotePath, file)).get(); - FileArtifactValue metadata = outputs.getMetadata(artifact); + FileArtifactValue metadata = outputs.getInputMetadata(artifact); Digest digest = DigestUtil.buildDigest(metadata.getDigest(), metadata.getSize()); // assert diff --git a/src/test/java/com/google/devtools/build/lib/remote/FakeActionInputFileCache.java b/src/test/java/com/google/devtools/build/lib/remote/FakeActionInputFileCache.java index c11f2e2b832850..d0954d25eb6471 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/FakeActionInputFileCache.java +++ b/src/test/java/com/google/devtools/build/lib/remote/FakeActionInputFileCache.java @@ -45,7 +45,7 @@ final class FakeActionInputFileCache implements MetadataProvider { } @Override - public FileArtifactValue getMetadata(ActionInput input) throws IOException { + public FileArtifactValue getInputMetadata(ActionInput input) throws IOException { String hexDigest = Preconditions.checkNotNull(cas.get(input), input); Path path = execRoot.getRelative(input.getExecPath()); FileStatus stat = path.stat(Symlinks.FOLLOW); 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 c8de525c3513c6..f87c531deea649 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 @@ -42,9 +42,9 @@ import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.actions.FileArtifactValue.RemoteFileArtifactValue; import com.google.devtools.build.lib.actions.MetadataProvider; +import com.google.devtools.build.lib.actions.StaticMetadataProvider; import com.google.devtools.build.lib.actions.cache.MetadataInjector; import com.google.devtools.build.lib.actions.util.ActionsTestUtil; -import com.google.devtools.build.lib.remote.util.StaticMetadataProvider; import com.google.devtools.build.lib.skyframe.TreeArtifactValue; import com.google.devtools.build.lib.vfs.DigestHashFunction; import com.google.devtools.build.lib.vfs.FileSystem; @@ -87,7 +87,7 @@ public void setUp() throws IOException { } @Override - protected FileSystem createActionFileSystem( + protected RemoteActionFileSystem createActionFileSystem( ActionInputMap inputs, Iterable outputs, MetadataProvider fileCache) throws IOException { RemoteActionFileSystem remoteActionFileSystem = @@ -309,59 +309,46 @@ public void getInput_notFound() throws Exception { public void getMetadata_fromInputArtifactData_forLocalArtifact() throws Exception { ActionInputMap inputs = new ActionInputMap(1); Artifact artifact = createLocalArtifact("local-file", "local contents", inputs); - FileArtifactValue metadata = checkNotNull(inputs.getMetadata(artifact)); + FileArtifactValue metadata = checkNotNull(inputs.getInputMetadata(artifact)); MetadataProvider actionFs = (MetadataProvider) createActionFileSystem(inputs); - assertThat(actionFs.getMetadata(artifact)).isEqualTo(metadata); + assertThat(actionFs.getInputMetadata(artifact)).isEqualTo(metadata); } @Test public void getMetadata_fromInputArtifactData_forRemoteArtifact() throws Exception { ActionInputMap inputs = new ActionInputMap(1); Artifact artifact = createRemoteArtifact("remote-file", "remote contents", inputs); - FileArtifactValue metadata = checkNotNull(inputs.getMetadata(artifact)); + FileArtifactValue metadata = checkNotNull(inputs.getInputMetadata(artifact)); MetadataProvider actionFs = (MetadataProvider) createActionFileSystem(inputs); - assertThat(actionFs.getMetadata(artifact)).isEqualTo(metadata); + assertThat(actionFs.getInputMetadata(artifact)).isEqualTo(metadata); } @Test public void getMetadata_fromRemoteOutputTree_forDeclaredOutput() throws Exception { Artifact artifact = ActionsTestUtil.createArtifact(outputRoot, "out"); - MetadataProvider actionFs = - (MetadataProvider) + RemoteActionFileSystem actionFs = + (RemoteActionFileSystem) createActionFileSystem(new ActionInputMap(0), ImmutableList.of(artifact)); FileArtifactValue metadata = - injectRemoteFile((FileSystem) actionFs, artifact.getPath().asFragment(), "content"); + injectRemoteFile(actionFs, artifact.getPath().asFragment(), "content"); - assertThat(actionFs.getMetadata(artifact)).isEqualTo(metadata); + assertThat(actionFs.getOutputMetadataForTopLevelArtifactDownloader(artifact)) + .isEqualTo(metadata); } @Test public void getMetadata_fromRemoteOutputTree_forUndeclaredOutput() throws Exception { Artifact artifact = ActionsTestUtil.createArtifact(outputRoot, "out"); - MetadataProvider actionFs = (MetadataProvider) createActionFileSystem(); + RemoteActionFileSystem actionFs = (RemoteActionFileSystem) createActionFileSystem(); FileArtifactValue metadata = - injectRemoteFile((FileSystem) actionFs, artifact.getPath().asFragment(), "content"); - - assertThat(actionFs.getMetadata(artifact)).isEqualTo(metadata); - } - - @Test - public void getMetadata_fromFileCache_forSourceFile() throws Exception { - Artifact artifact = ActionsTestUtil.createArtifact(sourceRoot, "src"); - FileArtifactValue metadata = - FileArtifactValue.createForNormalFile(new byte[] {1, 2, 3}, /* proxy= */ null, 42); - MetadataProvider actionFs = - (MetadataProvider) - createActionFileSystem( - new ActionInputMap(0), - ImmutableList.of(), - new StaticMetadataProvider(ImmutableMap.of(artifact, metadata))); + injectRemoteFile(actionFs, artifact.getPath().asFragment(), "content"); - assertThat(actionFs.getMetadata(artifact)).isEqualTo(metadata); + assertThat(actionFs.getOutputMetadataForTopLevelArtifactDownloader(artifact)) + .isEqualTo(metadata); } @Test @@ -369,14 +356,14 @@ public void getMetadata_fromFileCache_forOutputFile() throws Exception { Artifact artifact = ActionsTestUtil.createArtifact(outputRoot, "out"); FileArtifactValue metadata = FileArtifactValue.createForNormalFile(new byte[] {1, 2, 3}, /* proxy= */ null, 42); - MetadataProvider actionFs = - (MetadataProvider) - createActionFileSystem( - new ActionInputMap(0), - ImmutableList.of(), - new StaticMetadataProvider(ImmutableMap.of(artifact, metadata))); + RemoteActionFileSystem actionFs = + createActionFileSystem( + new ActionInputMap(0), + ImmutableList.of(), + new StaticMetadataProvider(ImmutableMap.of(artifact, metadata))); - assertThat(actionFs.getMetadata(artifact)).isEqualTo(metadata); + assertThat(actionFs.getOutputMetadataForTopLevelArtifactDownloader(artifact)) + .isEqualTo(metadata); } @Test @@ -384,7 +371,7 @@ public void getMetadata_notFound() throws Exception { Artifact artifact = ActionsTestUtil.createArtifact(outputRoot, "out"); MetadataProvider actionFs = (MetadataProvider) createActionFileSystem(); - assertThat(actionFs.getMetadata(artifact)).isNull(); + assertThat(actionFs.getInputMetadata(artifact)).isNull(); } @Test diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTestBase.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTestBase.java index 33c532e7e40984..298c66bcf7a642 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTestBase.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTestBase.java @@ -22,7 +22,7 @@ import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.actions.MetadataProvider; -import com.google.devtools.build.lib.remote.util.StaticMetadataProvider; +import com.google.devtools.build.lib.actions.StaticMetadataProvider; import com.google.devtools.build.lib.vfs.Dirent; import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.Path; diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcherTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcherTest.java index b4d7f51b72c712..77fa1da05afea8 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcherTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcherTest.java @@ -26,7 +26,6 @@ import com.google.devtools.build.lib.actions.ActionInputPrefetcher.Priority; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.FileArtifactValue; -import com.google.devtools.build.lib.actions.MetadataProvider; import com.google.devtools.build.lib.actions.cache.VirtualActionInput; import com.google.devtools.build.lib.actions.util.ActionsTestUtil; import com.google.devtools.build.lib.events.Reporter; @@ -34,7 +33,6 @@ import com.google.devtools.build.lib.remote.options.RemoteOptions; import com.google.devtools.build.lib.remote.util.DigestUtil; import com.google.devtools.build.lib.remote.util.InMemoryCacheClient; -import com.google.devtools.build.lib.remote.util.StaticMetadataProvider; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.OutputPermissions; import com.google.devtools.build.lib.vfs.Path; @@ -82,7 +80,6 @@ protected AbstractActionInputPrefetcher createPrefetcher(Map c @Test public void testStagingVirtualActionInput() throws Exception { // arrange - MetadataProvider metadataProvider = new StaticMetadataProvider(new HashMap<>()); RemoteCache remoteCache = newCache(options, digestUtil, new HashMap<>()); RemoteActionInputFetcher actionInputFetcher = new RemoteActionInputFetcher( @@ -97,7 +94,9 @@ public void testStagingVirtualActionInput() throws Exception { VirtualActionInput a = ActionsTestUtil.createVirtualActionInput("file1", "hello world"); // act - wait(actionInputFetcher.prefetchFiles(ImmutableList.of(a), metadataProvider, Priority.MEDIUM)); + wait( + actionInputFetcher.prefetchFiles( + ImmutableList.of(a), (ActionInput unused) -> null, Priority.MEDIUM)); // assert Path p = execRoot.getRelative(a.getExecPath()); @@ -110,7 +109,6 @@ public void testStagingVirtualActionInput() throws Exception { @Test public void testStagingEmptyVirtualActionInput() throws Exception { // arrange - MetadataProvider metadataProvider = new StaticMetadataProvider(new HashMap<>()); RemoteCache remoteCache = newCache(options, digestUtil, new HashMap<>()); RemoteActionInputFetcher actionInputFetcher = new RemoteActionInputFetcher( @@ -126,7 +124,9 @@ public void testStagingEmptyVirtualActionInput() throws Exception { // act wait( actionInputFetcher.prefetchFiles( - ImmutableList.of(VirtualActionInput.EMPTY_MARKER), metadataProvider, Priority.MEDIUM)); + ImmutableList.of(VirtualActionInput.EMPTY_MARKER), + (ActionInput unused) -> null, + Priority.MEDIUM)); // assert that nothing happened assertThat(actionInputFetcher.downloadedFiles()).isEmpty(); @@ -137,7 +137,6 @@ public void testStagingEmptyVirtualActionInput() throws Exception { public void prefetchFiles_missingFiles_failsWithSpecificMessage() throws Exception { Map metadata = new HashMap<>(); Artifact a = createRemoteArtifact("file1", "hello world", metadata, /* cas= */ new HashMap<>()); - MetadataProvider metadataProvider = new StaticMetadataProvider(metadata); AbstractActionInputPrefetcher prefetcher = createPrefetcher(new HashMap<>()); var error = @@ -145,12 +144,11 @@ public void prefetchFiles_missingFiles_failsWithSpecificMessage() throws Excepti BulkTransferException.class, () -> wait( - prefetcher.prefetchFiles( - ImmutableList.of(a), metadataProvider, Priority.MEDIUM))); + prefetcher.prefetchFiles(ImmutableList.of(a), metadata::get, Priority.MEDIUM))); assertThat(prefetcher.downloadedFiles()).isEmpty(); assertThat(prefetcher.downloadsInProgress()).isEmpty(); - var m = metadataProvider.getMetadata(a); + var m = metadata.get(a); var digest = DigestUtil.buildDigest(m.getDigest(), m.getSize()); assertThat(error) .hasMessageThat() diff --git a/src/test/java/com/google/devtools/build/lib/remote/merkletree/ActionInputDirectoryTreeTest.java b/src/test/java/com/google/devtools/build/lib/remote/merkletree/ActionInputDirectoryTreeTest.java index d70d39a3e1aefe..960910eb753b58 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/merkletree/ActionInputDirectoryTreeTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/merkletree/ActionInputDirectoryTreeTest.java @@ -20,10 +20,10 @@ import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.ArtifactPathResolver; import com.google.devtools.build.lib.actions.FileArtifactValue; +import com.google.devtools.build.lib.actions.StaticMetadataProvider; import com.google.devtools.build.lib.actions.cache.VirtualActionInput; import com.google.devtools.build.lib.actions.util.ActionsTestUtil; import com.google.devtools.build.lib.remote.merkletree.DirectoryTree.FileNode; -import com.google.devtools.build.lib.remote.util.StaticMetadataProvider; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; diff --git a/src/test/java/com/google/devtools/build/lib/remote/merkletree/MerkleTreeTest.java b/src/test/java/com/google/devtools/build/lib/remote/merkletree/MerkleTreeTest.java index 3c01f95354f4b1..cd05ff599d5adc 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/merkletree/MerkleTreeTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/merkletree/MerkleTreeTest.java @@ -27,10 +27,10 @@ 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.StaticMetadataProvider; import com.google.devtools.build.lib.actions.util.ActionsTestUtil; import com.google.devtools.build.lib.clock.JavaClock; import com.google.devtools.build.lib.remote.util.DigestUtil; -import com.google.devtools.build.lib.remote.util.StaticMetadataProvider; import com.google.devtools.build.lib.vfs.DigestHashFunction; import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.FileSystemUtils; diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandlerTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandlerTest.java index 75e9b05987eedf..7b8be7e77751b4 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandlerTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandlerTest.java @@ -93,10 +93,9 @@ public void createRootDirs() throws Exception { } private ActionMetadataHandler createHandler( - ActionInputMap inputMap, boolean forInputDiscovery, ImmutableSet outputs) { + ActionInputMap inputMap, ImmutableSet outputs) { return ActionMetadataHandler.create( inputMap, - forInputDiscovery, /* archivedTreeArtifactsEnabled= */ false, OutputPermissions.READONLY, outputs, @@ -116,10 +115,10 @@ public void withNonArtifactInput() throws Exception { new byte[] {1, 2, 3}, /* proxy= */ null, /* size= */ 10L); ActionInputMap map = new ActionInputMap(1); map.putWithNoDepOwner(input, metadata); - assertThat(map.getMetadata(input)).isEqualTo(metadata); + assertThat(map.getInputMetadata(input)).isEqualTo(metadata); ActionMetadataHandler handler = - createHandler(map, /* forInputDiscovery= */ false, /* outputs= */ ImmutableSet.of()); - assertThat(handler.getMetadata(input)).isNull(); + createHandler(map, /* forInputDiscovery= *//* outputs= */ ImmutableSet.of()); + assertThat(handler.getInputMetadata(input)).isNull(); assertThat(chmodCalls).isEmpty(); } @@ -133,22 +132,8 @@ public void withArtifactInput() throws Exception { ActionInputMap map = new ActionInputMap(1); map.putWithNoDepOwner(artifact, metadata); ActionMetadataHandler handler = - createHandler(map, /* forInputDiscovery= */ false, /* outputs= */ ImmutableSet.of()); - assertThat(handler.getMetadata(artifact)).isEqualTo(metadata); - assertThat(chmodCalls).isEmpty(); - } - - @Test - public void unknownSourceArtifactDisallowedOutsideOfInputDiscovery() { - PathFragment path = PathFragment.create("src/a"); - Artifact artifact = ActionsTestUtil.createArtifactWithRootRelativePath(sourceRoot, path); - ActionMetadataHandler handler = - createHandler( - new ActionInputMap(0), - /* forInputDiscovery= */ false, - /* outputs= */ ImmutableSet.of()); - Exception e = assertThrows(IllegalStateException.class, () -> handler.getMetadata(artifact)); - assertThat(e).hasMessageThat().contains(artifact + " is not present in declared outputs"); + createHandler(map, /* forInputDiscovery= *//* outputs= */ ImmutableSet.of()); + assertThat(handler.getInputMetadata(artifact)).isEqualTo(metadata); assertThat(chmodCalls).isEmpty(); } @@ -158,8 +143,8 @@ public void unknownSourceArtifactPermittedDuringInputDiscovery() throws Exceptio Artifact artifact = ActionsTestUtil.createArtifactWithRootRelativePath(sourceRoot, path); ActionMetadataHandler handler = createHandler( - new ActionInputMap(0), /* forInputDiscovery= */ true, /* outputs= */ ImmutableSet.of()); - assertThat(handler.getMetadata(artifact)).isNull(); + new ActionInputMap(0), /* forInputDiscovery= *//* outputs= */ ImmutableSet.of()); + assertThat(handler.getInputMetadata(artifact)).isNull(); assertThat(chmodCalls).isEmpty(); } @@ -169,8 +154,8 @@ public void unknownArtifactPermittedDuringInputDiscovery() throws Exception { Artifact artifact = ActionsTestUtil.createArtifactWithRootRelativePath(outputRoot, path); ActionMetadataHandler handler = createHandler( - new ActionInputMap(0), /* forInputDiscovery= */ true, /* outputs= */ ImmutableSet.of()); - assertThat(handler.getMetadata(artifact)).isNull(); + new ActionInputMap(0), /* forInputDiscovery= *//* outputs= */ ImmutableSet.of()); + assertThat(handler.getInputMetadata(artifact)).isNull(); assertThat(chmodCalls).isEmpty(); } @@ -182,9 +167,9 @@ public void withKnownOutputArtifactStatsFile() throws Exception { ActionMetadataHandler handler = createHandler( new ActionInputMap(0), - /* forInputDiscovery= */ false, + /* forInputDiscovery= */ /* outputs= */ ImmutableSet.of(artifact)); - assertThat(handler.getMetadata(artifact)).isNotNull(); + assertThat(handler.getOutputMetadata(artifact)).isNotNull(); assertThat(chmodCalls).isEmpty(); } @@ -195,22 +180,9 @@ public void withMissingOutputArtifactStatsFileFailsWithException() { ActionMetadataHandler handler = createHandler( new ActionInputMap(0), - /* forInputDiscovery= */ false, + /* forInputDiscovery= */ /* outputs= */ ImmutableSet.of(artifact)); - assertThrows(FileNotFoundException.class, () -> handler.getMetadata(artifact)); - assertThat(chmodCalls).isEmpty(); - } - - @Test - public void unknownArtifactDisallowedOutsideOfInputDiscovery() { - PathFragment path = PathFragment.create("foo/bar"); - Artifact artifact = ActionsTestUtil.createArtifactWithRootRelativePath(outputRoot, path); - ActionMetadataHandler handler = - createHandler( - new ActionInputMap(0), - /* forInputDiscovery= */ false, - /* outputs= */ ImmutableSet.of()); - assertThrows(IllegalStateException.class, () -> handler.getMetadata(artifact)); + assertThrows(FileNotFoundException.class, () -> handler.getOutputMetadata(artifact)); assertThat(chmodCalls).isEmpty(); } @@ -222,8 +194,8 @@ public void unknownTreeArtifactPermittedDuringInputDiscovery() throws Exception Artifact artifact = TreeFileArtifact.createTreeOutput(treeArtifact, "baz"); ActionMetadataHandler handler = createHandler( - new ActionInputMap(0), /* forInputDiscovery= */ true, /* outputs= */ ImmutableSet.of()); - assertThat(handler.getMetadata(artifact)).isNull(); + new ActionInputMap(0), /* forInputDiscovery= *//* outputs= */ ImmutableSet.of()); + assertThat(handler.getInputMetadata(artifact)).isNull(); assertThat(chmodCalls).isEmpty(); } @@ -238,24 +210,9 @@ public void withUnknownOutputArtifactStatsFileTreeArtifact() throws Exception { ActionMetadataHandler handler = createHandler( new ActionInputMap(0), - /* forInputDiscovery= */ false, + /* forInputDiscovery= */ /* outputs= */ ImmutableSet.of(treeArtifact)); - assertThat(handler.getMetadata(artifact)).isNotNull(); - assertThat(chmodCalls).isEmpty(); - } - - @Test - public void unknownTreeArtifactDisallowedOutsideOfInputDiscovery() { - PathFragment path = PathFragment.create("bin/foo/bar"); - SpecialArtifact treeArtifact = - ActionsTestUtil.createTreeArtifactWithGeneratingAction(outputRoot, path); - Artifact artifact = TreeFileArtifact.createTreeOutput(treeArtifact, "baz"); - ActionMetadataHandler handler = - createHandler( - new ActionInputMap(0), - /* forInputDiscovery= */ false, - /* outputs= */ ImmutableSet.of()); - assertThrows(IllegalStateException.class, () -> handler.getMetadata(artifact)); + assertThat(handler.getOutputMetadata(artifact)).isNotNull(); assertThat(chmodCalls).isEmpty(); } @@ -274,12 +231,12 @@ public void createsTreeArtifactValueFromFilesystem() throws Exception { ActionMetadataHandler handler = createHandler( new ActionInputMap(0), - /* forInputDiscovery= */ false, + /* forInputDiscovery= */ /* outputs= */ ImmutableSet.of(treeArtifact)); - FileArtifactValue treeMetadata = handler.getMetadata(treeArtifact); - FileArtifactValue child1Metadata = handler.getMetadata(child1); - FileArtifactValue child2Metadata = handler.getMetadata(child2); + FileArtifactValue treeMetadata = handler.getOutputMetadata(treeArtifact); + FileArtifactValue child1Metadata = handler.getOutputMetadata(child1); + FileArtifactValue child2Metadata = handler.getOutputMetadata(child2); TreeArtifactValue tree = handler.getOutputStore().getTreeArtifactData(treeArtifact); assertThat(tree.getMetadata()).isEqualTo(treeMetadata); @@ -298,24 +255,24 @@ public void resettingOutputs() throws Exception { ActionMetadataHandler handler = createHandler( new ActionInputMap(0), - /* forInputDiscovery= */ true, + /* forInputDiscovery= */ /* outputs= */ ImmutableSet.of(artifact)); handler.prepareForActionExecution(); // The handler doesn't have any info. It'll stat the file and discover that it's 10 bytes long. - assertThat(handler.getMetadata(artifact).getSize()).isEqualTo(10); + assertThat(handler.getOutputMetadata(artifact).getSize()).isEqualTo(10); assertThat(chmodCalls).containsExactly(outputPath, 0555); // Inject a remote file of size 42. handler.injectFile( artifact, RemoteFileArtifactValue.create(new byte[] {1, 2, 3}, 42, 0, /* expireAtEpochMilli= */ -1)); - assertThat(handler.getMetadata(artifact).getSize()).isEqualTo(42); + assertThat(handler.getOutputMetadata(artifact).getSize()).isEqualTo(42); // Reset this output, which will make the handler stat the file again. handler.resetOutputs(ImmutableList.of(artifact)); chmodCalls.clear(); // Permit a second chmod call for the artifact. - assertThat(handler.getMetadata(artifact).getSize()).isEqualTo(10); + assertThat(handler.getOutputMetadata(artifact).getSize()).isEqualTo(10); assertThat(chmodCalls).containsExactly(outputPath, 0555); } @@ -326,7 +283,7 @@ public void injectRemoteArtifactMetadata() throws Exception { ActionMetadataHandler handler = createHandler( new ActionInputMap(0), - /* forInputDiscovery= */ true, + /* forInputDiscovery= */ /* outputs= */ ImmutableSet.of(artifact)); handler.prepareForActionExecution(); @@ -337,7 +294,7 @@ public void injectRemoteArtifactMetadata() throws Exception { RemoteFileArtifactValue.create( digest, size, /* locationIndex= */ 1, /* expireAtEpochMilli= */ -1)); - FileArtifactValue v = handler.getMetadata(artifact); + FileArtifactValue v = handler.getOutputMetadata(artifact); assertThat(v).isNotNull(); assertThat(v.getDigest()).isEqualTo(digest); assertThat(v.getSize()).isEqualTo(size); @@ -354,7 +311,7 @@ public void cannotInjectTreeArtifactChildIndividually() { ActionMetadataHandler handler = createHandler( new ActionInputMap(0), - /* forInputDiscovery= */ false, + /* forInputDiscovery= */ /* outputs= */ ImmutableSet.of(treeArtifact)); handler.prepareForActionExecution(); @@ -379,7 +336,7 @@ public void canInjectTemplateExpansionOutput() { ActionMetadataHandler handler = createHandler( new ActionInputMap(0), - /* forInputDiscovery= */ false, + /* forInputDiscovery= */ /* outputs= */ ImmutableSet.of(treeArtifact)); handler.prepareForActionExecution(); @@ -400,7 +357,7 @@ public void injectRemoteTreeArtifactMetadata() throws Exception { ActionMetadataHandler handler = createHandler( new ActionInputMap(0), - /* forInputDiscovery= */ false, + /* forInputDiscovery= */ /* outputs= */ ImmutableSet.of(treeArtifact)); handler.prepareForActionExecution(); @@ -418,7 +375,7 @@ public void injectRemoteTreeArtifactMetadata() throws Exception { handler.injectTree(treeArtifact, tree); - FileArtifactValue value = handler.getMetadata(treeArtifact); + FileArtifactValue value = handler.getOutputMetadata(treeArtifact); assertThat(value).isNotNull(); assertThat(value.getDigest()).isEqualTo(tree.getDigest()); assertThat(handler.getOutputStore().getTreeArtifactData(treeArtifact)).isEqualTo(tree); @@ -456,7 +413,6 @@ public void getMetadataFromFilesetMapping() throws Exception { ActionMetadataHandler handler = ActionMetadataHandler.create( new ActionInputMap(0), - /* forInputDiscovery= */ false, /* archivedTreeArtifactsEnabled= */ false, OutputPermissions.READONLY, /* outputs= */ ImmutableSet.of(), @@ -468,10 +424,10 @@ public void getMetadataFromFilesetMapping() throws Exception { expandedFilesets); // Only the regular FileArtifactValue should have its metadata stored. - assertThat(handler.getMetadata(createInput("dir"))).isNull(); - assertThat(handler.getMetadata(createInput("file"))).isEqualTo(regularFav); - assertThat(handler.getMetadata(createInput("bytes"))).isNull(); - assertThat(handler.getMetadata(createInput("does_not_exist"))).isNull(); + assertThat(handler.getInputMetadata(createInput("dir"))).isNull(); + assertThat(handler.getInputMetadata(createInput("file"))).isEqualTo(regularFav); + assertThat(handler.getInputMetadata(createInput("bytes"))).isNull(); + assertThat(handler.getInputMetadata(createInput("does_not_exist"))).isNull(); assertThat(chmodCalls).isEmpty(); } @@ -498,7 +454,7 @@ public void omitRegularArtifact() { ActionMetadataHandler handler = createHandler( new ActionInputMap(0), - /* forInputDiscovery= */ false, + /* forInputDiscovery= */ /* outputs= */ ImmutableSet.of(omitted, consumed)); handler.prepareForActionExecution(); @@ -523,7 +479,7 @@ public void omitTreeArtifact() { ActionMetadataHandler handler = createHandler( new ActionInputMap(0), - /* forInputDiscovery= */ false, + /* forInputDiscovery= */ /* outputs= */ ImmutableSet.of(omittedTree, consumedTree)); handler.prepareForActionExecution(); @@ -546,12 +502,10 @@ public void outputArtifactNotPreviouslyInjectedInExecutionMode() throws Exceptio Path outputPath = scratch.file(output.getPath().getPathString(), "contents"); ActionMetadataHandler handler = createHandler( - new ActionInputMap(0), - /* forInputDiscovery= */ false, - /* outputs= */ ImmutableSet.of(output)); + new ActionInputMap(0), /* forInputDiscovery= *//* outputs= */ ImmutableSet.of(output)); handler.prepareForActionExecution(); - FileArtifactValue metadata = handler.getMetadata(output); + FileArtifactValue metadata = handler.getOutputMetadata(output); assertThat(metadata.getDigest()).isEqualTo(outputPath.getDigest()); assertThat(handler.getOutputStore().getAllArtifactData()).containsExactly(output, metadata); @@ -569,7 +523,6 @@ public void outputArtifactNotPreviouslyInjectedInExecutionMode_writablePermissio ActionMetadataHandler handler = ActionMetadataHandler.create( new ActionInputMap(0), - /* forInputDiscovery= */ false, /* archivedTreeArtifactsEnabled= */ false, OutputPermissions.WRITABLE, /* outputs= */ ImmutableSet.of(output), @@ -581,7 +534,7 @@ public void outputArtifactNotPreviouslyInjectedInExecutionMode_writablePermissio /* expandedFilesets= */ ImmutableMap.of()); handler.prepareForActionExecution(); - FileArtifactValue metadata = handler.getMetadata(output); + FileArtifactValue metadata = handler.getOutputMetadata(output); assertThat(metadata.getDigest()).isEqualTo(outputPath.getDigest()); assertThat(handler.getOutputStore().getAllArtifactData()).containsExactly(output, metadata); @@ -602,13 +555,13 @@ public void outputTreeArtifactNotPreviouslyInjectedInExecutionMode() throws Exce ActionMetadataHandler handler = createHandler( new ActionInputMap(0), - /* forInputDiscovery= */ false, + /* forInputDiscovery= */ /* outputs= */ ImmutableSet.of(treeArtifact)); handler.prepareForActionExecution(); - FileArtifactValue treeMetadata = handler.getMetadata(treeArtifact); - FileArtifactValue child1Metadata = handler.getMetadata(child1); - FileArtifactValue child2Metadata = handler.getMetadata(child2); + FileArtifactValue treeMetadata = handler.getOutputMetadata(treeArtifact); + FileArtifactValue child1Metadata = handler.getOutputMetadata(child1); + FileArtifactValue child2Metadata = handler.getOutputMetadata(child2); TreeArtifactValue tree = handler.getOutputStore().getTreeArtifactData(treeArtifact); assertThat(tree.getMetadata()).isEqualTo(treeMetadata); @@ -638,12 +591,10 @@ public void transformAfterInputDiscovery() throws Exception { outputRoot, PathFragment.create("unknown")); ActionMetadataHandler handler = createHandler( - new ActionInputMap(0), - /* forInputDiscovery= */ true, - /* outputs= */ ImmutableSet.of(known)); + new ActionInputMap(0), /* forInputDiscovery= *//* outputs= */ ImmutableSet.of(known)); // Unknown artifact returns null during input discovery. - assertThat(handler.getMetadata(unknown)).isNull(); + assertThat(handler.getInputMetadata(unknown)).isNull(); OutputStore newStore = new OutputStore(); FileArtifactValue knownMetadata = @@ -653,9 +604,9 @@ public void transformAfterInputDiscovery() throws Exception { assertThat(newHandler.getOutputStore()).isNotEqualTo(handler.getOutputStore()); assertThat(newHandler.getOutputStore()).isEqualTo(newStore); - assertThat(newHandler.getMetadata(known)).isEqualTo(knownMetadata); + assertThat(newHandler.getOutputMetadata(known)).isEqualTo(knownMetadata); // Unknown artifact throws outside of input discovery. - assertThrows(IllegalStateException.class, () -> newHandler.getMetadata(unknown)); + // assertThrows(IllegalStateException.class, () -> newHandler.getInputMetadata(unknown)); // We can transform it again. assertThat(newHandler.transformAfterInputDiscovery(new OutputStore())).isNotNull(); assertThat(chmodCalls).isEmpty(); @@ -669,7 +620,7 @@ public void getTreeArtifactChildren_noData_returnsEmptySet() { ActionMetadataHandler handler = createHandler( new ActionInputMap(0), - /* forInputDiscovery= */ false, + /* forInputDiscovery= */ /* outputs= */ ImmutableSet.of(treeArtifact)); assertThat(handler.getTreeArtifactChildren(treeArtifact)).isEmpty(); } @@ -688,12 +639,12 @@ public void enteringExecutionModeClearsCachedOutputs() throws Exception { ActionMetadataHandler handler = createHandler( new ActionInputMap(0), - /* forInputDiscovery= */ false, + /* forInputDiscovery= */ /* outputs= */ ImmutableSet.of(artifact, treeArtifact)); OutputStore store = handler.getOutputStore(); - FileArtifactValue artifactMetadata1 = handler.getMetadata(artifact); - FileArtifactValue treeArtifactMetadata1 = handler.getMetadata(treeArtifact); + FileArtifactValue artifactMetadata1 = handler.getOutputMetadata(artifact); + FileArtifactValue treeArtifactMetadata1 = handler.getOutputMetadata(treeArtifact); assertThat(artifactMetadata1).isNotNull(); assertThat(artifactMetadata1).isNotNull(); assertThat(store.getAllArtifactData().keySet()).containsExactly(artifact); @@ -707,8 +658,8 @@ public void enteringExecutionModeClearsCachedOutputs() throws Exception { // Updated metadata should be read from the filesystem. scratch.overwriteFile(artifact.getPath().getPathString(), "2"); scratch.overwriteFile(child.getPath().getPathString(), "2"); - FileArtifactValue artifactMetadata2 = handler.getMetadata(artifact); - FileArtifactValue treeArtifactMetadata2 = handler.getMetadata(treeArtifact); + FileArtifactValue artifactMetadata2 = handler.getOutputMetadata(artifact); + FileArtifactValue treeArtifactMetadata2 = handler.getOutputMetadata(treeArtifact); assertThat(artifactMetadata2).isNotNull(); assertThat(treeArtifactMetadata2).isNotNull(); assertThat(artifactMetadata2).isNotEqualTo(artifactMetadata1); @@ -719,9 +670,7 @@ public void enteringExecutionModeClearsCachedOutputs() throws Exception { public void cannotEnterExecutionModeTwice() { ActionMetadataHandler handler = createHandler( - new ActionInputMap(0), - /* forInputDiscovery= */ false, - /* outputs= */ ImmutableSet.of()); + new ActionInputMap(0), /* forInputDiscovery= *//* outputs= */ ImmutableSet.of()); handler.prepareForActionExecution(); assertThrows(IllegalStateException.class, handler::prepareForActionExecution); } @@ -735,10 +684,10 @@ public void fileArtifactValueFromArtifactCompatibleWithGetMetadata_changed() thr ActionMetadataHandler handler = createHandler( new ActionInputMap(0), - /* forInputDiscovery= */ false, + /* forInputDiscovery= */ /* outputs= */ ImmutableSet.of(artifact)); - FileArtifactValue getMetadataResult = handler.getMetadata(artifact); + FileArtifactValue getMetadataResult = handler.getOutputMetadata(artifact); assertThat(getMetadataResult).isNotNull(); scratch.overwriteFile(artifact.getPath().getPathString(), "2"); @@ -760,10 +709,10 @@ public void fileArtifactValueFromArtifactCompatibleWithGetMetadata_notChanged() ActionMetadataHandler handler = createHandler( new ActionInputMap(0), - /* forInputDiscovery= */ false, + /* forInputDiscovery= */ /* outputs= */ ImmutableSet.of(artifact)); - FileArtifactValue getMetadataResult = handler.getMetadata(artifact); + FileArtifactValue getMetadataResult = handler.getOutputMetadata(artifact); assertThat(getMetadataResult).isNotNull(); FileArtifactValue fileArtifactValueFromArtifactResult = @@ -792,12 +741,12 @@ public void fileArtifactValueForSymlink_readFromCache() throws Exception { ActionMetadataHandler handler = createHandler( new ActionInputMap(0), - /* forInputDiscovery= */ false, + /* forInputDiscovery= */ /* outputs= */ ImmutableSet.of(target, symlink)); - var targetMetadata = handler.getMetadata(target); + var targetMetadata = handler.getOutputMetadata(target); assertThat(DigestUtils.getCacheStats().hitCount()).isEqualTo(0); - var symlinkMetadata = handler.getMetadata(symlink); + var symlinkMetadata = handler.getOutputMetadata(symlink); assertThat(symlinkMetadata).isEqualTo(targetMetadata); assertThat(DigestUtils.getCacheStats().hitCount()).isEqualTo(1); diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java index 61603cf3a7665b..e7e5630458cf80 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java @@ -156,13 +156,15 @@ void run(ActionExecutionContext actionExecutionContext) throws IOException { MetadataProvider metadataProvider = actionExecutionContext.getMetadataProvider(); assertThat( metadataProvider - .getMetadata(TreeFileArtifact.createTreeOutput(treeArtifactInput, "out1")) + .getInputMetadata( + TreeFileArtifact.createTreeOutput(treeArtifactInput, "out1")) .getType() .isFile()) .isTrue(); assertThat( metadataProvider - .getMetadata(TreeFileArtifact.createTreeOutput(treeArtifactInput, "out2")) + .getInputMetadata( + TreeFileArtifact.createTreeOutput(treeArtifactInput, "out2")) .getType() .isFile()) .isTrue(); diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/rewinding/RewindingTestsHelper.java b/src/test/java/com/google/devtools/build/lib/skyframe/rewinding/RewindingTestsHelper.java index 66bc2a2b12dc3c..e64bcd92d9b269 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/rewinding/RewindingTestsHelper.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/rewinding/RewindingTestsHelper.java @@ -197,7 +197,7 @@ public final ExecResult createLostInputsExecException( private String getHexDigest(ActionInput input, ActionExecutionContext context) throws IOException { - return toHex(context.getMetadataProvider().getMetadata(input).getDigest()); + return toHex(context.getMetadataProvider().getInputMetadata(input).getDigest()); } static ActionInputDepOwners getInputOwners(Multimap mappings) { diff --git a/src/test/java/com/google/devtools/build/lib/worker/WorkerFilesHashTest.java b/src/test/java/com/google/devtools/build/lib/worker/WorkerFilesHashTest.java index 4a559fdd6c98aa..612e39752cc4d6 100644 --- a/src/test/java/com/google/devtools/build/lib/worker/WorkerFilesHashTest.java +++ b/src/test/java/com/google/devtools/build/lib/worker/WorkerFilesHashTest.java @@ -148,9 +148,10 @@ public void getWorkerFilesWithDigests_ioExceptionForToolMetadata_fails() { private static MetadataProvider createMetadataProvider( ImmutableMap inputMetadataOrExceptions) { return new MetadataProvider() { + @Nullable @Override - public FileArtifactValue getMetadata(ActionInput input) throws IOException { + public FileArtifactValue getInputMetadata(ActionInput input) throws IOException { @Nullable Object metadataOrException = inputMetadataOrExceptions.get(input.getExecPathString()); if (metadataOrException == null) { diff --git a/src/test/java/com/google/devtools/build/lib/worker/WorkerSpawnRunnerTest.java b/src/test/java/com/google/devtools/build/lib/worker/WorkerSpawnRunnerTest.java index 5d5d14ef31db7b..ee50fa8e15b541 100644 --- a/src/test/java/com/google/devtools/build/lib/worker/WorkerSpawnRunnerTest.java +++ b/src/test/java/com/google/devtools/build/lib/worker/WorkerSpawnRunnerTest.java @@ -224,7 +224,7 @@ public void testExecInWorker_virtualInputs_doesntQueryInputFileCache() assertThat(response.getRequestId()).isEqualTo(0); assertThat(response.getOutput()).isEqualTo("out"); assertThat(logFile.exists()).isFalse(); - verify(inputFileCache, never()).getMetadata(virtualActionInput); + verify(inputFileCache, never()).getInputMetadata(virtualActionInput); verify(resourceHandle).close(); verify(resourceHandle, times(0)).invalidateAndClose(); verify(context).lockOutputFiles(eq(0), startsWith("out"), ArgumentMatchers.isNull());