Skip to content

Commit d811376

Browse files
zhengwei143copybara-github
authored andcommitted
Remove ActionTemplate.getOutputTreeArtifact() and bubble it up to the subclasses that implement ActionTemplate where needed. #getOutputTreeArtifact() is mainly a convenience method to access one of the underlying output tree artifacts in tests, and has no contractual significance in the expansion of the ActionTemplate.
OTOH, `#getOutputs` is the main method that drives the logic (validation of the `ActionTemplate`) within `ActionTemplateExpansionFunction` and holds contractual significance whereby outputs of expanded actions need to be children `TreeFileArtifacts` of `#getOutputs()`. RELNOTES: None. PiperOrigin-RevId: 808640815 Change-Id: Ied5e577a0c8a674cd6910d091b284830ff6e500c
1 parent 370232e commit d811376

File tree

9 files changed

+23
-46
lines changed

9 files changed

+23
-46
lines changed

src/main/java/com/google/devtools/build/lib/actions/ActionTemplate.java

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
import com.google.common.collect.ImmutableList;
1919
import com.google.common.collect.ImmutableListMultimap;
2020
import com.google.common.collect.ImmutableMap;
21-
import com.google.common.collect.ImmutableSet;
2221
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
2322
import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
2423
import com.google.devtools.build.lib.analysis.platform.PlatformInfo;
@@ -92,26 +91,14 @@ ImmutableList<T> generateActionsForInputArtifacts(
9291
.collect(toImmutableListMultimap(TreeFileArtifact::getParent, x -> x));
9392
}
9493

95-
/** Returns the output TreeArtifact. */
96-
SpecialArtifact getOutputTreeArtifact();
97-
9894
@Override
9995
default SpecialArtifact getPrimaryInput() {
10096
return getInputTreeArtifacts().get(0);
10197
}
10298

103-
/**
104-
* By default, returns just {@link #getOutputTreeArtifact}, but may be overridden with additional
105-
* tree artifacts.
106-
*/
107-
@Override
108-
default ImmutableSet<Artifact> getOutputs() {
109-
return ImmutableSet.of(getOutputTreeArtifact());
110-
}
111-
11299
@Override
113-
default SpecialArtifact getPrimaryOutput() {
114-
return getOutputTreeArtifact();
100+
default Artifact getPrimaryOutput() {
101+
return getOutputs().iterator().next();
115102
}
116103

117104
@Override

src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTemplate.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -176,8 +176,8 @@ public ImmutableList<SpecialArtifact> getInputTreeArtifacts() {
176176
}
177177

178178
@Override
179-
public SpecialArtifact getOutputTreeArtifact() {
180-
return outputTreeArtifact;
179+
public ImmutableSet<Artifact> getOutputs() {
180+
return ImmutableSet.of(outputTreeArtifact);
181181
}
182182

183183
@Override

src/main/java/com/google/devtools/build/lib/analysis/actions/StarlarkActionTemplate.java

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -165,11 +165,6 @@ public ImmutableList<SpecialArtifact> getInputTreeArtifacts() {
165165
return ImmutableList.of(inputTreeArtifact);
166166
}
167167

168-
@Override
169-
public SpecialArtifact getOutputTreeArtifact() {
170-
return outputTreeArtifact;
171-
}
172-
173168
@Override
174169
public ImmutableSet<Artifact> getOutputs() {
175170
return ImmutableSet.copyOf(allOutputs);

src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionTemplate.java

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -302,11 +302,6 @@ public ImmutableList<SpecialArtifact> getInputTreeArtifacts() {
302302
return ImmutableList.of(sourceTreeArtifact);
303303
}
304304

305-
@Override
306-
public SpecialArtifact getOutputTreeArtifact() {
307-
return outputTreeArtifact;
308-
}
309-
310305
@Override
311306
public ActionOwner getOwner() {
312307
return actionOwner;

src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendActionTemplate.java

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -363,11 +363,6 @@ public ImmutableList<SpecialArtifact> getInputTreeArtifacts() {
363363
return ImmutableList.of(firstNonNull(indexAndImportsTreeArtifact, fullBitcodeTreeArtifact));
364364
}
365365

366-
@Override
367-
public SpecialArtifact getOutputTreeArtifact() {
368-
return objectFileTreeArtifact;
369-
}
370-
371366
@Override
372367
public ActionOwner getOwner() {
373368
return actionOwner;

src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunction.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,10 @@
3939
import com.google.devtools.build.skyframe.SkyKey;
4040
import com.google.devtools.build.skyframe.SkyValue;
4141
import com.google.devtools.build.skyframe.SkyframeLookupResult;
42+
import java.util.Collection;
4243
import java.util.HashMap;
4344
import java.util.List;
4445
import java.util.Map;
45-
import java.util.Set;
4646
import javax.annotation.Nullable;
4747

4848
/**
@@ -141,7 +141,7 @@ private static ImmutableList<ActionAnalysisMetadata> generateAndValidateActionsF
141141
ImmutableList<TreeFileArtifact> inputTreeFileArtifacts,
142142
ActionTemplateExpansionKey key)
143143
throws ActionExecutionException, InterruptedException {
144-
Set<Artifact> outputs = actionTemplate.getOutputs();
144+
Collection<Artifact> outputs = actionTemplate.getOutputs();
145145
for (Artifact output : outputs) {
146146
Preconditions.checkState(
147147
output.isTreeArtifact(),

src/test/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunctionTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -469,8 +469,8 @@ public ImmutableList<SpecialArtifact> getInputTreeArtifacts() {
469469
}
470470

471471
@Override
472-
public SpecialArtifact getOutputTreeArtifact() {
473-
return outputTreeArtifact;
472+
public ImmutableSet<Artifact> getOutputs() {
473+
return ImmutableSet.of(outputTreeArtifact);
474474
}
475475

476476
@Override

src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTest.java

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
import com.google.devtools.build.skyframe.SkyFunction;
5757
import com.google.devtools.build.skyframe.SkyKey;
5858
import com.google.devtools.build.skyframe.SkyValue;
59+
import com.google.errorprone.annotations.CanIgnoreReturnValue;
5960
import java.io.IOException;
6061
import java.util.Arrays;
6162
import java.util.HashMap;
@@ -177,9 +178,9 @@ public void testSpawnActionTemplate() throws Throwable {
177178
ActionsTestUtil.createDummySpawnActionTemplate(artifact1, artifact2);
178179
actions.add(actionTemplate);
179180
TreeFileArtifact treeFileArtifact1 =
180-
createFakeExpansionTreeFileArtifact(actionTemplate, "child1", "hello1");
181+
createFakeExpansionTreeFileArtifact(actionTemplate, artifact2, "child1", "hello1");
181182
TreeFileArtifact treeFileArtifact2 =
182-
createFakeExpansionTreeFileArtifact(actionTemplate, "child2", "hello2");
183+
createFakeExpansionTreeFileArtifact(actionTemplate, artifact2, "child2", "hello2");
183184

184185
TreeArtifactValue value = (TreeArtifactValue) evaluateArtifactValue(artifact2);
185186
assertThat(value.getChildValues()).containsKey(treeFileArtifact1);
@@ -200,18 +201,18 @@ public void testConsecutiveSpawnActionTemplates() throws Throwable {
200201
SpawnActionTemplate template2 =
201202
ActionsTestUtil.createDummySpawnActionTemplate(artifact1, artifact2);
202203
actions.add(template2);
203-
createFakeExpansionTreeFileArtifact(template2, "child1", "hello1");
204-
createFakeExpansionTreeFileArtifact(template2, "child2", "hello2");
204+
createFakeExpansionTreeFileArtifact(template2, artifact2, "child1", "hello1");
205+
createFakeExpansionTreeFileArtifact(template2, artifact2, "child2", "hello2");
205206

206207
// artifact3 is a tree artifact generated by action template.
207208
SpecialArtifact artifact3 = createDerivedTreeArtifactOnly("treeArtifact3");
208209
SpawnActionTemplate template3 =
209210
ActionsTestUtil.createDummySpawnActionTemplate(artifact2, artifact3);
210211
actions.add(template3);
211212
TreeFileArtifact treeFileArtifact1 =
212-
createFakeExpansionTreeFileArtifact(template3, "child1", "hello1");
213+
createFakeExpansionTreeFileArtifact(template3, artifact3, "child1", "hello1");
213214
TreeFileArtifact treeFileArtifact2 =
214-
createFakeExpansionTreeFileArtifact(template3, "child2", "hello2");
215+
createFakeExpansionTreeFileArtifact(template3, artifact3, "child2", "hello2");
215216

216217
TreeArtifactValue value = (TreeArtifactValue) evaluateArtifactValue(artifact3);
217218
assertThat(value.getChildValues()).containsKey(treeFileArtifact1);
@@ -266,14 +267,18 @@ private static TreeFileArtifact createFakeTreeFileArtifact(
266267
return treeFileArtifact;
267268
}
268269

270+
@CanIgnoreReturnValue
269271
private TreeFileArtifact createFakeExpansionTreeFileArtifact(
270-
ActionTemplate<?> actionTemplate, String parentRelativePath, String content)
272+
ActionTemplate<?> actionTemplate,
273+
SpecialArtifact outputTreeArtifact,
274+
String parentRelativePath,
275+
String content)
271276
throws Exception {
272277
int actionIndex = Iterables.indexOf(actions, actionTemplate::equals);
273278
Preconditions.checkState(actionIndex >= 0, "%s not registered", actionTemplate);
274279
TreeFileArtifact treeFileArtifact =
275280
TreeFileArtifact.createTemplateExpansionOutput(
276-
actionTemplate.getOutputTreeArtifact(),
281+
outputTreeArtifact,
277282
parentRelativePath,
278283
ActionTemplateExpansionValue.key(ALL_OWNER, actionIndex));
279284
Path path = treeFileArtifact.getPath();

src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1413,8 +1413,8 @@ public ImmutableList<SpecialArtifact> getInputTreeArtifacts() {
14131413
}
14141414

14151415
@Override
1416-
public SpecialArtifact getOutputTreeArtifact() {
1417-
return outputArtifact;
1416+
public ImmutableSet<Artifact> getOutputs() {
1417+
return ImmutableSet.of(outputArtifact);
14181418
}
14191419

14201420
@Override

0 commit comments

Comments
 (0)