Change signatures to DerivedArtifact, and get Labels from non-Artifact sources when possible.
In tests, try to get artifacts directly from the actual configured target, rather than creating fresh ones.
This change should be a prod functional no-op, just changing signatures. Split out from the follow-up to reduce the diff.
PiperOrigin-RevId: 250527865
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java b/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java
index 2b80208..cf4b1a3 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java
@@ -536,7 +536,7 @@
update();
ConfiguredTarget a = getConfiguredTarget("//a:a");
- NestedSet<Artifact> extraActionArtifacts =
+ NestedSet<Artifact.DerivedArtifact> extraActionArtifacts =
a.getProvider(ExtraActionArtifactsProvider.class).getTransitiveExtraActionArtifacts();
for (Artifact artifact : extraActionArtifacts) {
assertThat(artifact.getOwnerLabel()).isEqualTo(Label.create("@//a", "b"));
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/actions/SymlinkActionTest.java b/src/test/java/com/google/devtools/build/lib/analysis/actions/SymlinkActionTest.java
index 37ac7d0..444eb68 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/actions/SymlinkActionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/actions/SymlinkActionTest.java
@@ -45,7 +45,7 @@
private Path input;
private Artifact inputArtifact;
private Path output;
- private Artifact outputArtifact;
+ private Artifact.DerivedArtifact outputArtifact;
private SymlinkAction action;
@Before
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
index 5ef6c9f..374f98e 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
@@ -29,6 +29,7 @@
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
import com.google.devtools.build.lib.actions.ActionGraph;
import com.google.devtools.build.lib.actions.ActionKeyContext;
+import com.google.devtools.build.lib.actions.ActionLookupValue;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.analysis.AnalysisOptions;
import com.google.devtools.build.lib.analysis.AnalysisResult;
@@ -528,6 +529,25 @@
protected Artifact getBinArtifact(String packageRelativePath, ConfiguredTarget owner)
throws InterruptedException {
Label label = owner.getLabel();
+ ActionLookupValue.ActionLookupKey actionLookupKey =
+ ConfiguredTargetKey.of(label, owner.getConfigurationKey(), /*isHostConfiguration=*/ false);
+ ActionLookupValue actionLookupValue;
+ try {
+ actionLookupValue =
+ (ActionLookupValue)
+ skyframeExecutor.getEvaluatorForTesting().getExistingValue(actionLookupKey);
+ } catch (InterruptedException e) {
+ throw new IllegalStateException(e);
+ }
+ PathFragment rootRelativePath = label.getPackageFragment().getRelative(packageRelativePath);
+ for (ActionAnalysisMetadata action : actionLookupValue.getActions()) {
+ for (Artifact output : action.getOutputs()) {
+ if (output.getRootRelativePath().equals(rootRelativePath)) {
+ return output;
+ }
+ }
+ }
+ // Fall back: some tests don't actually need the right owner.
return buildView
.getArtifactFactory()
.getDerivedArtifact(
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java
index 71d9c08..09caee3 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java
@@ -23,11 +23,11 @@
import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.ActionExecutionException;
import com.google.devtools.build.lib.actions.ActionKeyContext;
+import com.google.devtools.build.lib.actions.ActionLookupValue;
import com.google.devtools.build.lib.actions.ActionOwner;
import com.google.devtools.build.lib.actions.ActionResult;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
-import com.google.devtools.build.lib.actions.ArtifactOwner;
import com.google.devtools.build.lib.actions.ArtifactRoot;
import com.google.devtools.build.lib.actions.ExecutionStrategy;
import com.google.devtools.build.lib.actions.MiddlemanFactory;
@@ -53,6 +53,7 @@
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.skyframe.SkyFunction;
+import com.google.devtools.build.skyframe.SkyFunctionName;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
@@ -119,7 +120,8 @@
}
@Override
- public Artifact getDerivedArtifact(PathFragment rootRelativePath, ArtifactRoot root) {
+ public Artifact.DerivedArtifact getDerivedArtifact(
+ PathFragment rootRelativePath, ArtifactRoot root) {
return original.getDerivedArtifact(rootRelativePath, root);
}
@@ -180,7 +182,7 @@
}
@Override
- public ArtifactOwner getOwner() {
+ public ActionLookupValue.ActionLookupKey getOwner() {
return original.getOwner();
}
@@ -300,6 +302,14 @@
/** An AnalysisEnvironment with stubbed-out methods. */
public static class StubAnalysisEnvironment implements AnalysisEnvironment {
+ private static final ActionLookupValue.ActionLookupKey DUMMY_KEY =
+ new ActionLookupValue.ActionLookupKey() {
+ @Override
+ public SkyFunctionName functionName() {
+ return null;
+ }
+ };
+
@Override
public void registerAction(ActionAnalysisMetadata... action) {
}
@@ -355,7 +365,8 @@
}
@Override
- public Artifact getDerivedArtifact(PathFragment rootRelativePath, ArtifactRoot root) {
+ public Artifact.DerivedArtifact getDerivedArtifact(
+ PathFragment rootRelativePath, ArtifactRoot root) {
return null;
}
@@ -376,8 +387,8 @@
}
@Override
- public ArtifactOwner getOwner() {
- return ArtifactOwner.NullArtifactOwner.INSTANCE;
+ public ActionLookupValue.ActionLookupKey getOwner() {
+ return DUMMY_KEY;
}
@Override
@@ -403,14 +414,13 @@
Pattern.compile("(?<=" + TestConstants.PRODUCT_NAME + "-out/)gcc[^/]*-grte-\\w+-");
/**
- * Given a collection of Artifacts, returns a corresponding set of strings of
- * the form "{root} {relpath}", such as "bin x/libx.a". Such strings make
- * assertions easier to write.
+ * Given a collection of Artifacts, returns a corresponding set of strings of the form "{root}
+ * {relpath}", such as "bin x/libx.a". Such strings make assertions easier to write.
*
* <p>The returned set preserves the order of the input.
*/
- public static Set<String> artifactsToStrings(BuildConfigurationCollection configurations,
- Iterable<Artifact> artifacts) {
+ public static Set<String> artifactsToStrings(
+ BuildConfigurationCollection configurations, Iterable<? extends Artifact> artifacts) {
Map<String, String> rootMap = new HashMap<>();
BuildConfiguration targetConfiguration =
Iterables.getOnlyElement(configurations.getTargetConfigurations());
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
index f7f44d9..0a85f26 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
@@ -38,6 +38,7 @@
import com.google.devtools.build.lib.actions.ActionInput;
import com.google.devtools.build.lib.actions.ActionKeyContext;
import com.google.devtools.build.lib.actions.ActionLogBufferPathGenerator;
+import com.google.devtools.build.lib.actions.ActionLookupValue;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander;
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
@@ -158,6 +159,8 @@
import com.google.devtools.build.skyframe.ErrorInfo;
import com.google.devtools.build.skyframe.MemoizingEvaluator;
import com.google.devtools.build.skyframe.SkyFunction;
+import com.google.devtools.build.skyframe.SkyFunctionName;
+import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.common.options.Options;
import com.google.devtools.common.options.OptionsParser;
import com.google.devtools.common.options.OptionsParsingException;
@@ -566,7 +569,12 @@
return new CachingAnalysisEnvironment(
view.getArtifactFactory(),
actionKeyContext,
- ArtifactOwner.NullArtifactOwner.INSTANCE,
+ new ActionLookupValue.ActionLookupKey() {
+ @Override
+ public SkyFunctionName functionName() {
+ return null;
+ }
+ },
/*isSystemEnv=*/ true,
/*extendedSanityChecks=*/ false,
/*allowAnalysisFailures=*/ false,
@@ -1130,13 +1138,12 @@
}
/**
- * Given a collection of Artifacts, returns a corresponding set of strings of
- * the form "[root] [relpath]", such as "bin x/libx.a". Such strings make
- * assertions easier to write.
+ * Given a collection of Artifacts, returns a corresponding set of strings of the form "[root]
+ * [relpath]", such as "bin x/libx.a". Such strings make assertions easier to write.
*
* <p>The returned set preserves the order of the input.
*/
- protected Set<String> artifactsToStrings(Iterable<Artifact> artifacts) {
+ protected Set<String> artifactsToStrings(Iterable<? extends Artifact> artifacts) {
return AnalysisTestUtil.artifactsToStrings(masterConfig, artifacts);
}
@@ -1196,8 +1203,27 @@
* that does not exercise the analysis phase, the convenience methods {@link
* #getBinArtifactWithNoOwner} or {@link #getGenfilesArtifactWithNoOwner} should be used instead.
*/
- protected Artifact getDerivedArtifact(
+ protected final Artifact.DerivedArtifact getDerivedArtifact(
PathFragment rootRelativePath, ArtifactRoot root, ArtifactOwner owner) {
+ if ((owner instanceof ActionLookupValue.ActionLookupKey)) {
+ ActionLookupValue actionLookupValue;
+ try {
+ actionLookupValue =
+ (ActionLookupValue)
+ skyframeExecutor.getEvaluatorForTesting().getExistingValue((SkyKey) owner);
+ } catch (InterruptedException e) {
+ throw new IllegalStateException(e);
+ }
+ for (ActionAnalysisMetadata action : actionLookupValue.getActions()) {
+ for (Artifact output : action.getOutputs()) {
+ if (output.getRootRelativePath().equals(rootRelativePath)
+ && output.getRoot().equals(root)) {
+ return (Artifact.DerivedArtifact) output;
+ }
+ }
+ }
+ }
+ // Fall back: some tests don't actually need an artifact with an owner.
return view.getArtifactFactory().getDerivedArtifact(rootRelativePath, root, owner);
}
@@ -1274,7 +1300,7 @@
* to own this artifact. If the test runs the analysis phase, {@link #getBinArtifact(String,
* ConfiguredTarget)} or its convenience methods should be used instead.
*/
- protected Artifact getBinArtifactWithNoOwner(String rootRelativePath) {
+ protected Artifact.DerivedArtifact getBinArtifactWithNoOwner(String rootRelativePath) {
return getDerivedArtifact(PathFragment.create(rootRelativePath),
targetConfig.getBinDirectory(RepositoryName.MAIN),
ActionsTestUtil.NULL_ARTIFACT_OWNER);
@@ -1674,7 +1700,7 @@
: provider.getOutputGroup(outputGroup);
}
- protected NestedSet<Artifact> getExtraActionArtifacts(ConfiguredTarget target) {
+ protected NestedSet<Artifact.DerivedArtifact> getExtraActionArtifacts(ConfiguredTarget target) {
return target.getProvider(ExtraActionArtifactsProvider.class).getExtraActionArtifacts();
}
@@ -1997,7 +2023,8 @@
}
@Override
- public Artifact getDerivedArtifact(PathFragment rootRelativePath, ArtifactRoot root) {
+ public Artifact.DerivedArtifact getDerivedArtifact(
+ PathFragment rootRelativePath, ArtifactRoot root) {
throw new UnsupportedOperationException();
}
@@ -2018,7 +2045,7 @@
}
@Override
- public ArtifactOwner getOwner() {
+ public ActionLookupValue.ActionLookupKey getOwner() {
throw new UnsupportedOperationException();
}