From 5107b62a7dfe1e4f358940f6435190034bf6f472 Mon Sep 17 00:00:00 2001 From: Googler Date: Mon, 28 Oct 2024 09:45:58 -0700 Subject: [PATCH] Remove OutputMetadataStore.getTreeArtifactChildren(). It was only a thin wrapper around .getTreeArtifactValue(). RELNOTES: None. PiperOrigin-RevId: 690648081 Change-Id: I4b84ebc1a381ca8298dff2652ec60e00911294e0 --- .../build/lib/exec/StandaloneTestStrategy.java | 4 ++-- .../lib/remote/AbstractActionInputPrefetcher.java | 3 ++- .../build/lib/remote/BazelOutputService.java | 3 ++- .../skyframe/ActionOutputMetadataStoreTest.java | 15 +++------------ 4 files changed, 9 insertions(+), 16 deletions(-) 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 455f422476a8f9..ff11db64aff345 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 @@ -759,8 +759,8 @@ private TestAttemptResult runTestAttempt( ImmutableSet expandedCoverageDir = actionExecutionContext .getOutputMetadataStore() - .getTreeArtifactChildren( - (SpecialArtifact) testAction.getCoverageDirectoryTreeArtifact()); + .getTreeArtifactValue((SpecialArtifact) testAction.getCoverageDirectoryTreeArtifact()) + .getChildren(); ImmutableSet coverageSpawnMetadata = ImmutableSet.builder() .addAll(expandedCoverageDir) 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 fbaed815e6d01a..180308b250dbc3 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 @@ -670,7 +670,8 @@ public void finalizeAction(Action action, OutputMetadataStore outputMetadataStor } if (output.isTreeArtifact()) { - var children = outputMetadataStore.getTreeArtifactChildren((SpecialArtifact) output); + var children = + outputMetadataStore.getTreeArtifactValue((SpecialArtifact) output).getChildren(); for (var file : children) { if (remoteOutputChecker.shouldDownloadOutput(file)) { outputsToDownload.add(file); diff --git a/src/main/java/com/google/devtools/build/lib/remote/BazelOutputService.java b/src/main/java/com/google/devtools/build/lib/remote/BazelOutputService.java index a626b0d7b71866..fbc7a8d1f1d359 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/BazelOutputService.java +++ b/src/main/java/com/google/devtools/build/lib/remote/BazelOutputService.java @@ -373,7 +373,8 @@ public void finalizeAction(Action action, OutputMetadataStore outputMetadataStor if (output.isTreeArtifact()) { // TODO(chiwang): Use TreeArtifactLocator - var children = outputMetadataStore.getTreeArtifactChildren((SpecialArtifact) output); + var children = + outputMetadataStore.getTreeArtifactValue((SpecialArtifact) output).getChildren(); for (var child : children) { addArtifact(outputMetadataStore, execRoot, outputPath, request, child); } diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStoreTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStoreTest.java index e846decde824c2..1471e1216c3f52 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStoreTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStoreTest.java @@ -259,7 +259,7 @@ public void createsTreeArtifactValueFromFilesystem() throws Exception { assertThat(tree.getMetadata()).isEqualTo(treeMetadata); assertThat(tree.getChildValues()) .containsExactly(child1, child1Metadata, child2, child2Metadata); - assertThat(store.getTreeArtifactChildren(treeArtifact)).isEqualTo(tree.getChildren()); + assertThat(store.getTreeArtifactValue(treeArtifact)).isEqualTo(tree); assertThat(store.getAllArtifactData()).isEmpty(); assertThat(chmodCalls).isEmpty(); } @@ -390,7 +390,7 @@ public void injectRemoteTreeArtifactMetadata() throws Exception { assertThat(store.getAllTreeArtifactData().get(treeArtifact)).isEqualTo(tree); assertThat(chmodCalls).isEmpty(); - assertThat(store.getTreeArtifactChildren(treeArtifact)).isEqualTo(tree.getChildren()); + assertThat(store.getTreeArtifactValue(treeArtifact)).isEqualTo(tree); // Make sure that all children are transferred properly into the ActionExecutionValue. If any // child is missing, getExistingFileArtifactValue will throw. @@ -699,7 +699,7 @@ public void outputTreeArtifactNotPreviouslyInjectedInExecutionMode() throws Exce assertThat(tree.getMetadata()).isEqualTo(treeMetadata); assertThat(tree.getChildValues()) .containsExactly(child1, child1Metadata, child2, child2Metadata); - assertThat(store.getTreeArtifactChildren(treeArtifact)).isEqualTo(tree.getChildren()); + assertThat(store.getTreeArtifactValue(treeArtifact)).isEqualTo(tree); assertThat(store.getAllArtifactData()).isEmpty(); assertThat(chmodCalls) .containsExactly( @@ -713,15 +713,6 @@ public void outputTreeArtifactNotPreviouslyInjectedInExecutionMode() throws Exce 0555); } - @Test - public void getTreeArtifactChildren_noData_returnsEmptySet() { - SpecialArtifact treeArtifact = - ActionsTestUtil.createTreeArtifactWithGeneratingAction( - outputRoot, PathFragment.create("tree")); - ActionOutputMetadataStore store = createStore(/* outputs= */ ImmutableSet.of(treeArtifact)); - assertThat(store.getTreeArtifactChildren(treeArtifact)).isEmpty(); - } - @Test public void enteringExecutionModeClearsCachedOutputs() throws Exception { Artifact artifact =