From 088d8e9426983adf37831f8f9f02e1327dc10e89 Mon Sep 17 00:00:00 2001 From: Googler Date: Tue, 11 Apr 2023 08:25:49 -0700 Subject: [PATCH] Make MetadataHandler not implement MetadataProvider anymore. Before this change, ActionExecutionContext got two references to a MetadataProvider implementation: one directly, one behind MetadataHandler. These could and did get out sync after e.g. calls to withOutputsAsInputs(). This change fixes that. The game plan is to rename these two so that it's obvious that one deals with the inputs of the action, the other, with the outputs, but let's do that very mechanical change after this slightly less mechanical one. RELNOTES: None. PiperOrigin-RevId: 523408132 Change-Id: I557cc5ec939b7a49e33bb857c3b0916d23538048 --- .../build/lib/actions/ActionCacheChecker.java | 39 ++++++++++---- .../lib/actions/cache/MetadataHandler.java | 6 +-- .../lib/skyframe/ActionExecutionFunction.java | 9 +++- .../lib/skyframe/ActionMetadataHandler.java | 3 +- .../lib/skyframe/SkyframeActionExecutor.java | 12 ++++- .../lib/actions/ActionCacheCheckerTest.java | 52 ++++++++++++------- .../lib/actions/util/ActionsTestUtil.java | 3 +- 7 files changed, 86 insertions(+), 38 deletions(-) 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 f29c96f7bbb7a7..ff418778017722 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 @@ -190,6 +190,7 @@ private static boolean validateArtifacts( ActionCache.Entry entry, Action action, NestedSet actionInputs, + MetadataProvider metadataProvider, MetadataHandler metadataHandler, boolean checkOutput, @Nullable CachedOutputMetadata cachedOutputMetadata) @@ -206,7 +207,7 @@ private static boolean validateArtifacts( } } for (Artifact artifact : actionInputs.toList()) { - mdMap.put(artifact.getExecPathString(), getInputMetadataMaybe(metadataHandler, artifact)); + mdMap.put(artifact.getExecPathString(), getInputMetadataMaybe(metadataProvider, artifact)); } return !Arrays.equals(MetadataDigestUtils.fromMetadata(mdMap), entry.getFileDigest()); } @@ -438,6 +439,7 @@ public Token getTokenIfNeedToExecute( Map clientEnv, OutputPermissions outputPermissions, EventHandler handler, + MetadataProvider metadataProvider, MetadataHandler metadataHandler, ArtifactExpander artifactExpander, Map remoteDefaultPlatformProperties, @@ -454,7 +456,7 @@ public Token getTokenIfNeedToExecute( // Some types of middlemen are not checked because they should not // propagate invalidation of their inputs. if (middlemanType != MiddlemanType.SCHEDULING_DEPENDENCY_MIDDLEMAN) { - checkMiddlemanAction(action, handler, metadataHandler); + checkMiddlemanAction(action, handler, metadataProvider, metadataHandler); } return null; } @@ -495,6 +497,7 @@ public Token getTokenIfNeedToExecute( action, entry, handler, + metadataProvider, metadataHandler, artifactExpander, actionInputs, @@ -525,6 +528,7 @@ private boolean mustExecute( Action action, @Nullable ActionCache.Entry entry, EventHandler handler, + MetadataProvider metadataProvider, MetadataHandler metadataHandler, ArtifactExpander artifactExpander, NestedSet actionInputs, @@ -551,7 +555,13 @@ private boolean mustExecute( actionCache.accountMiss(MissReason.CORRUPTED_CACHE_ENTRY); return true; } else if (validateArtifacts( - entry, action, actionInputs, metadataHandler, true, cachedOutputMetadata)) { + entry, + action, + actionInputs, + metadataProvider, + metadataHandler, + true, + cachedOutputMetadata)) { reportChanged(handler, action); actionCache.accountMiss(MissReason.DIFFERENT_FILES); return true; @@ -574,8 +584,8 @@ private boolean mustExecute( } private static FileArtifactValue getInputMetadataOrConstant( - MetadataHandler metadataHandler, Artifact artifact) throws IOException { - FileArtifactValue metadata = metadataHandler.getInputMetadata(artifact); + MetadataProvider metadataProvider, Artifact artifact) throws IOException { + FileArtifactValue metadata = metadataProvider.getInputMetadata(artifact); return (metadata != null && artifact.isConstantMetadata()) ? ConstantMetadataValue.INSTANCE : metadata; @@ -594,9 +604,9 @@ private static FileArtifactValue getOutputMetadataOrConstant( // should propagate the exception, because it is unexpected (e.g., bad file system state). @Nullable private static FileArtifactValue getInputMetadataMaybe( - MetadataHandler metadataHandler, Artifact artifact) { + MetadataProvider metadataProvider, Artifact artifact) { try { - return getInputMetadataOrConstant(metadataHandler, artifact); + return getInputMetadataOrConstant(metadataProvider, artifact); } catch (IOException e) { return null; } @@ -618,6 +628,7 @@ private static FileArtifactValue getOutputMetadataMaybe( public void updateActionCache( Action action, Token token, + MetadataProvider metadataProvider, MetadataHandler metadataHandler, ArtifactExpander artifactExpander, Map clientEnv, @@ -673,7 +684,7 @@ public void updateActionCache( for (Artifact input : action.getInputs().toList()) { entry.addInputFile( input.getExecPath(), - getInputMetadataMaybe(metadataHandler, input), + getInputMetadataMaybe(metadataProvider, input), /* saveExecPath= */ !excludePathsFromActionCache.contains(input)); } entry.getFileDigest(); @@ -755,7 +766,10 @@ public List getCachedInputs(Action action, PackageRootResolver resolve * actions, it consults with the aggregated middleman digest computed here. */ private void checkMiddlemanAction( - Action action, EventHandler handler, MetadataHandler metadataHandler) + Action action, + EventHandler handler, + MetadataProvider metadataProvider, + MetadataHandler metadataHandler) throws InterruptedException { if (!cacheConfig.enabled()) { // Action cache is disabled, don't generate digests. @@ -774,9 +788,10 @@ private void checkMiddlemanAction( entry, action, action.getInputs(), + metadataProvider, metadataHandler, false, - /*cachedOutputMetadata=*/ null)) { + /* cachedOutputMetadata= */ null)) { reportChanged(handler, action); actionCache.accountMiss(MissReason.DIFFERENT_FILES); changed = true; @@ -794,7 +809,7 @@ private void checkMiddlemanAction( for (Artifact input : action.getInputs().toList()) { entry.addInputFile( input.getExecPath(), - getInputMetadataMaybe(metadataHandler, input), + getInputMetadataMaybe(metadataProvider, input), /* saveExecPath= */ true); } } @@ -816,6 +831,7 @@ public Token getTokenUnconditionallyAfterFailureToRecordActionCacheHit( Map clientEnv, OutputPermissions outputPermissions, EventHandler handler, + MetadataProvider metadataProvider, MetadataHandler metadataHandler, ArtifactExpander artifactExpander, Map remoteDefaultPlatformProperties, @@ -830,6 +846,7 @@ public Token getTokenUnconditionallyAfterFailureToRecordActionCacheHit( clientEnv, outputPermissions, handler, + metadataProvider, metadataHandler, artifactExpander, remoteDefaultPlatformProperties, 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 87dc68f0592276..c29304d4cb326f 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 @@ -20,14 +20,12 @@ 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; -/** Handles metadata of inputs and outputs during the execution phase. */ -public interface MetadataHandler extends MetadataProvider, MetadataInjector { - +/** Handles the metadata of the outputs of the action during its execution. */ +public interface MetadataHandler extends MetadataInjector { /** * Returns a {@link FileArtifactValue} for the given {@link ActionInput}. * 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 9eb4d220986534..7513b0f8c6513f 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 @@ -758,6 +758,7 @@ private ActionExecutionValue checkCacheAndExecuteIfNeeded( env.getListener(), action, metadataHandler, + metadataHandler, artifactExpander, actionStartTime, state.allInputs.actionCacheInputs, @@ -787,7 +788,12 @@ private ActionExecutionValue checkCacheAndExecuteIfNeeded( try (SilentCloseable c = Profiler.instance().profile(ProfilerTask.INFO, "discoverInputs")) { state.discoveredInputs = skyframeActionExecutor.discoverInputs( - action, actionLookupData, metadataHandler, env, state.actionFileSystem); + action, + actionLookupData, + metadataHandler, + metadataHandler, + env, + state.actionFileSystem); } discoveredInputsDuration = Duration.ofNanos(BlazeClock.nanoTime() - actionStartTime); if (env.valuesMissing()) { @@ -877,6 +883,7 @@ public void run( skyframeActionExecutor.updateActionCache( action, metadataHandler, + metadataHandler, new Artifact.ArtifactExpanderImpl( // Skipping the filesets in runfiles since those cannot participate in command line // creation. 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 d3f3b7c78b7fd5..4443c16dcbccab 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 @@ -39,6 +39,7 @@ import com.google.devtools.build.lib.actions.FilesetManifest; import com.google.devtools.build.lib.actions.FilesetManifest.RelativeSymlinkBehaviorWithoutError; import com.google.devtools.build.lib.actions.FilesetOutputSymlink; +import com.google.devtools.build.lib.actions.MetadataProvider; import com.google.devtools.build.lib.actions.RemoteFileStatus; import com.google.devtools.build.lib.actions.cache.MetadataHandler; import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor; @@ -79,7 +80,7 @@ * (except those that were {@linkplain #artifactOmitted omitted}) to ensure that declared outputs * were in fact created and are valid. */ -final class ActionMetadataHandler implements MetadataHandler { +final class ActionMetadataHandler implements MetadataProvider, MetadataHandler { private static final GoogleLogger logger = GoogleLogger.forEnclosingClass(); /** 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 4420756abac3fb..1560bf0f7a06b4 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 @@ -479,6 +479,7 @@ ActionExecutionValue executeAction( env, action, metadataHandler, + metadataHandler, artifactExpander, topLevelFilesets, actionFileSystem, @@ -541,6 +542,7 @@ private ExtendedEventHandler selectEventHandler(boolean emitProgressEvents) { private ActionExecutionContext getContext( Environment env, Action action, + MetadataProvider metadataProvider, MetadataHandler metadataHandler, ArtifactExpander artifactExpander, ImmutableMap> topLevelFilesets, @@ -553,7 +555,7 @@ private ActionExecutionContext getContext( FileOutErr fileOutErr = actionLogBufferPathGenerator.generate(artifactPathResolver); return new ActionExecutionContext( executorEngine, - createFileCache(metadataHandler, actionFileSystem), + createFileCache(metadataProvider, actionFileSystem), actionInputPrefetcher, actionKeyContext, metadataHandler, @@ -596,6 +598,7 @@ private static void closeContext( Token checkActionCache( ExtendedEventHandler eventHandler, Action action, + MetadataProvider metadataProvider, MetadataHandler metadataHandler, ArtifactExpander artifactExpander, long actionStartTime, @@ -631,6 +634,7 @@ Token checkActionCache( clientEnv, getOutputPermissions(), handler, + metadataProvider, metadataHandler, artifactExpander, remoteDefaultProperties, @@ -673,6 +677,7 @@ public T getContext(Class type) { clientEnv, getOutputPermissions(), handler, + metadataProvider, metadataHandler, artifactExpander, remoteDefaultProperties, @@ -703,6 +708,7 @@ public T getContext(Class type) { void updateActionCache( Action action, + MetadataProvider metadataProvider, MetadataHandler metadataHandler, ArtifactExpander artifactExpander, Token token, @@ -726,6 +732,7 @@ void updateActionCache( actionCacheChecker.updateActionCache( action, token, + metadataProvider, metadataHandler, artifactExpander, clientEnv, @@ -771,6 +778,7 @@ List getActionCachedInputs(Action action, PackageRootResolver resolver NestedSet discoverInputs( Action action, ActionLookupData actionLookupData, + MetadataProvider metadataProvider, MetadataHandler metadataHandler, Environment env, @Nullable FileSystem actionFileSystem) @@ -783,7 +791,7 @@ NestedSet discoverInputs( ActionExecutionContext actionExecutionContext = ActionExecutionContext.forInputDiscovery( executorEngine, - createFileCache(metadataHandler, actionFileSystem), + createFileCache(metadataProvider, actionFileSystem), actionInputPrefetcher, actionKeyContext, metadataHandler, 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 9567081f12532b..1c58bb7213cbe5 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 @@ -140,8 +140,10 @@ private void runAction(Action action) throws Exception { runAction(action, new HashMap<>()); } - private void runAction(Action action, MetadataHandler metadataHandler) throws Exception { - runAction(action, new HashMap<>(), ImmutableMap.of(), metadataHandler); + private void runAction( + Action action, MetadataProvider metadataProvider, MetadataHandler metadataHandler) + throws Exception { + runAction(action, new HashMap<>(), ImmutableMap.of(), metadataProvider, metadataHandler); } /** @@ -154,13 +156,15 @@ private void runAction(Action action, Map clientEnv) throws Exce private void runAction(Action action, Map clientEnv, Map platform) throws Exception { - runAction(action, clientEnv, platform, new FakeMetadataHandler()); + FakeMetadataHandler metadataHandler = new FakeMetadataHandler(); + runAction(action, clientEnv, platform, metadataHandler, metadataHandler); } private void runAction( Action action, Map clientEnv, Map platform, + MetadataProvider metadataProvider, MetadataHandler metadataHandler) throws Exception { for (Artifact artifact : action.getOutputs()) { @@ -185,6 +189,7 @@ private void runAction( clientEnv, OutputPermissions.READONLY, /* handler= */ null, + metadataProvider, metadataHandler, /* artifactExpander= */ null, platform, @@ -198,6 +203,7 @@ private void runAction( cacheChecker.updateActionCache( action, token, + metadataProvider, metadataHandler, /* artifactExpander= */ null, clientEnv, @@ -443,6 +449,7 @@ public void testDeletedConstantMetadataOutputCausesReexecution() throws Exceptio Action action = new WriteEmptyOutputAction(output); runAction(action); output.getPath().delete(); + FakeMetadataHandler fakeMetadataHandler = new FakeMetadataHandler(); assertThat( cacheChecker.getTokenIfNeedToExecute( action, @@ -450,7 +457,8 @@ public void testDeletedConstantMetadataOutputCausesReexecution() throws Exceptio /* clientEnv= */ ImmutableMap.of(), OutputPermissions.READONLY, /* handler= */ null, - new FakeMetadataHandler(), + fakeMetadataHandler, + fakeMetadataHandler, /* artifactExpander= */ null, /* remoteDefaultPlatformProperties= */ ImmutableMap.of(), /* loadCachedOutputMetadata= */ true)) @@ -574,7 +582,7 @@ public void saveOutputMetadata_remoteFileMetadataLoaded() throws Exception { Artifact output = createArtifact(artifactRoot, "bin/dummy"); String content = "content"; Action action = new InjectOutputFileMetadataAction(output, createRemoteFileMetadata(content)); - MetadataHandler metadataHandler = new FakeMetadataHandler(); + FakeMetadataHandler metadataHandler = new FakeMetadataHandler(); runAction(action); Token token = @@ -585,6 +593,7 @@ public void saveOutputMetadata_remoteFileMetadataLoaded() throws Exception { OutputPermissions.READONLY, /* handler= */ null, metadataHandler, + metadataHandler, /* artifactExpander= */ null, /* remoteDefaultPlatformProperties= */ ImmutableMap.of(), /* loadCachedOutputMetadata= */ true); @@ -608,7 +617,7 @@ public void saveOutputMetadata_remoteFileExpired_remoteFileMetadataNotLoaded() t output, createRemoteFileMetadata( content, /* expireAtEpochMilli= */ 0, /* materializationExecPath= */ null)); - MetadataHandler metadataHandler = new FakeMetadataHandler(); + FakeMetadataHandler metadataHandler = new FakeMetadataHandler(); runAction(action); Token token = @@ -619,6 +628,7 @@ public void saveOutputMetadata_remoteFileExpired_remoteFileMetadataNotLoaded() t OutputPermissions.READONLY, /* handler= */ null, metadataHandler, + metadataHandler, /* artifactExpander= */ null, /* remoteDefaultPlatformProperties= */ ImmutableMap.of(), /* loadCachedOutputMetadata= */ true); @@ -636,7 +646,7 @@ public void saveOutputMetadata_remoteOutputUnavailable_remoteFileMetadataNotLoad Artifact output = createArtifact(artifactRoot, "bin/dummy"); String content = "content"; Action action = new InjectOutputFileMetadataAction(output, createRemoteFileMetadata(content)); - MetadataHandler metadataHandler = new FakeMetadataHandler(); + FakeMetadataHandler metadataHandler = new FakeMetadataHandler(); runAction(action); Token token = @@ -647,6 +657,7 @@ public void saveOutputMetadata_remoteOutputUnavailable_remoteFileMetadataNotLoad OutputPermissions.READONLY, /* handler= */ null, metadataHandler, + metadataHandler, /* artifactExpander= */ null, /* remoteDefaultPlatformProperties= */ ImmutableMap.of(), /* loadCachedOutputMetadata= */ false); @@ -811,7 +822,7 @@ public void saveOutputMetadata_emptyTreeMetadata_notSaved() throws Exception { ImmutableMap.of(), /* archivedArtifactValue= */ Optional.empty(), /* materializationExecPath= */ Optional.empty())); - MetadataHandler metadataHandler = new FakeMetadataHandler(); + FakeMetadataHandler metadataHandler = new FakeMetadataHandler(); runAction(action); Token token = @@ -822,6 +833,7 @@ public void saveOutputMetadata_emptyTreeMetadata_notSaved() throws Exception { OutputPermissions.READONLY, /* handler= */ null, metadataHandler, + metadataHandler, /* artifactExpander= */ null, /* remoteDefaultPlatformProperties= */ ImmutableMap.of(), /* loadCachedOutputMetadata= */ true); @@ -910,7 +922,7 @@ public void saveOutputMetadata_treeMetadata_remoteFileMetadataLoaded() throws Ex children, /* archivedArtifactValue= */ Optional.empty(), /* materializationExecPath= */ Optional.empty())); - MetadataHandler metadataHandler = new FakeMetadataHandler(); + FakeMetadataHandler metadataHandler = new FakeMetadataHandler(); runAction(action); Token token = @@ -921,6 +933,7 @@ public void saveOutputMetadata_treeMetadata_remoteFileMetadataLoaded() throws Ex OutputPermissions.READONLY, /* handler= */ null, metadataHandler, + metadataHandler, /* artifactExpander= */ null, /* remoteDefaultPlatformProperties= */ ImmutableMap.of(), /* loadCachedOutputMetadata= */ true); @@ -966,12 +979,12 @@ public void saveOutputMetadata_treeMetadata_localFileMetadataLoaded() throws Exc children2, /* archivedArtifactValue= */ Optional.empty(), /* materializationExecPath= */ Optional.empty())); - MetadataHandler metadataHandler = new FakeMetadataHandler(); + FakeMetadataHandler metadataHandler = new FakeMetadataHandler(); runAction(action); writeIsoLatin1(output.getPath().getRelative("file2"), "modified_local"); // Not cached since local file changed - runAction(action, metadataHandler); + runAction(action, metadataHandler, metadataHandler); assertStatistics( 0, @@ -1013,12 +1026,12 @@ public void saveOutputMetadata_treeMetadata_localArchivedArtifactLoaded() throws ImmutableMap.of(), Optional.of(createRemoteFileMetadata("modified")), /* materializationExecPath= */ Optional.empty())); - MetadataHandler metadataHandler = new FakeMetadataHandler(); + FakeMetadataHandler metadataHandler = new FakeMetadataHandler(); runAction(action); writeIsoLatin1(ArchivedTreeArtifact.createForTree(output).getPath(), "modified"); // Not cached since local file changed - runAction(action, metadataHandler); + runAction(action, metadataHandler, metadataHandler); assertStatistics( 0, @@ -1059,7 +1072,7 @@ public void saveOutputMetadata_treeFileExpired_treeMetadataNotLoaded() throws Ex children, /* archivedArtifactValue= */ Optional.empty(), /* materializationExecPath= */ Optional.empty())); - MetadataHandler metadataHandler = new FakeMetadataHandler(); + FakeMetadataHandler metadataHandler = new FakeMetadataHandler(); runAction(action); Token token = @@ -1070,6 +1083,7 @@ public void saveOutputMetadata_treeFileExpired_treeMetadataNotLoaded() throws Ex OutputPermissions.READONLY, /* handler= */ null, metadataHandler, + metadataHandler, /* artifactExpander= */ null, /* remoteDefaultPlatformProperties= */ ImmutableMap.of(), /* loadCachedOutputMetadata= */ true); @@ -1102,7 +1116,7 @@ public void saveOutputMetadata_archivedRepresentationExpired_treeMetadataNotLoad /* expireAtEpochMilli= */ 0, /* materializationExecPath= */ null)), /* materializationExecPath= */ Optional.empty())); - MetadataHandler metadataHandler = new FakeMetadataHandler(); + FakeMetadataHandler metadataHandler = new FakeMetadataHandler(); runAction(action); Token token = @@ -1113,6 +1127,7 @@ public void saveOutputMetadata_archivedRepresentationExpired_treeMetadataNotLoad OutputPermissions.READONLY, /* handler= */ null, metadataHandler, + metadataHandler, /* artifactExpander= */ null, /* remoteDefaultPlatformProperties= */ ImmutableMap.of(), /* loadCachedOutputMetadata= */ true); @@ -1153,7 +1168,7 @@ public void saveOutputMetadata_treeMetadataWithSameLocalFileMetadata_cached( children, /* archivedArtifactValue= */ Optional.empty(), materializationExecPath)); - MetadataHandler metadataHandler = new FakeMetadataHandler(); + FakeMetadataHandler metadataHandler = new FakeMetadataHandler(); runAction(action); writeContentAsLatin1(output.getPath().getRelative("file1"), "content1"); @@ -1166,6 +1181,7 @@ public void saveOutputMetadata_treeMetadataWithSameLocalFileMetadata_cached( OutputPermissions.READONLY, /* handler= */ null, metadataHandler, + metadataHandler, /* artifactExpander= */ null, /* remoteDefaultPlatformProperties= */ ImmutableMap.of(), /* loadCachedOutputMetadata= */ true); @@ -1206,12 +1222,12 @@ public void saveOutputMetadata_treeMetadataWithSameLocalArchivedArtifact_cached( ImmutableMap.of(), Optional.of(createRemoteFileMetadata("content")), /* materializationExecPath= */ Optional.empty())); - MetadataHandler metadataHandler = new FakeMetadataHandler(); + FakeMetadataHandler metadataHandler = new FakeMetadataHandler(); runAction(action); writeContentAsLatin1(ArchivedTreeArtifact.createForTree(output).getPath(), "content"); // Cache hit - runAction(action, metadataHandler); + runAction(action, metadataHandler, metadataHandler); assertStatistics(1, new MissDetailsBuilder().set(MissReason.NOT_CACHED, 1).build()); assertThat(output.getPath().exists()).isFalse(); 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 1e141b9b87919a..787540929d3620 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 @@ -57,6 +57,7 @@ import com.google.devtools.build.lib.actions.DiscoveredModulesPruner; import com.google.devtools.build.lib.actions.Executor; import com.google.devtools.build.lib.actions.FileArtifactValue; +import com.google.devtools.build.lib.actions.MetadataProvider; import com.google.devtools.build.lib.actions.MiddlemanType; import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException; import com.google.devtools.build.lib.actions.PackageRootResolver; @@ -1055,7 +1056,7 @@ public String toString() { * of the hooks required by the scenario under test. Tests that need an instance but do not need * any functionality can use {@link #THROWING_METADATA_HANDLER}. */ - public static class FakeMetadataHandlerBase implements MetadataHandler { + public static class FakeMetadataHandlerBase implements MetadataProvider, MetadataHandler { @Override public FileArtifactValue getInputMetadata(ActionInput input) throws IOException { throw new UnsupportedOperationException();