Pass only the top-level artifacts to `ExecutorLifecycleListener` and make the computation more efficient.
Delete the now unused `ArtifactsToOwnerLabels`.
PiperOrigin-RevId: 372152937
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/ArtifactsToOwnerLabelsTest.java b/src/test/java/com/google/devtools/build/lib/analysis/ArtifactsToOwnerLabelsTest.java
deleted file mode 100644
index 6f3bebc..0000000
--- a/src/test/java/com/google/devtools/build/lib/analysis/ArtifactsToOwnerLabelsTest.java
+++ /dev/null
@@ -1,65 +0,0 @@
-// Copyright 2018 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.analysis;
-
-import static com.google.common.truth.Truth.assertThat;
-
-import com.google.devtools.build.lib.actions.Artifact;
-import com.google.devtools.build.lib.cmdline.Label;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.JUnit4;
-import org.mockito.Mockito;
-
-/** Tests for {@link ArtifactsToOwnerLabels}. */
-@RunWith(JUnit4.class)
-public class ArtifactsToOwnerLabelsTest {
- @Test
- public void smoke() {
- Artifact label1Artifact = Mockito.mock(Artifact.class);
- Label label1 = Label.parseAbsoluteUnchecked("//label:one");
- Mockito.when(label1Artifact.getOwnerLabel()).thenReturn(label1);
- ArtifactsToOwnerLabels underTest =
- new ArtifactsToOwnerLabels.Builder().addArtifact(label1Artifact).build();
- assertThat(underTest.getOwners(label1Artifact)).containsExactly(label1);
- underTest = new ArtifactsToOwnerLabels.Builder().addArtifact(label1Artifact, label1).build();
- assertThat(underTest.getOwners(label1Artifact)).containsExactly(label1);
- underTest =
- new ArtifactsToOwnerLabels.Builder()
- .addArtifact(label1Artifact)
- .addArtifact(label1Artifact, label1)
- .build();
- assertThat(underTest.getOwners(label1Artifact)).containsExactly(label1);
- underTest =
- new ArtifactsToOwnerLabels.Builder()
- .addArtifact(label1Artifact, label1)
- .addArtifact(label1Artifact)
- .build();
- assertThat(underTest.getOwners(label1Artifact)).containsExactly(label1);
- Label label2 = Label.parseAbsoluteUnchecked("//label:two");
- underTest =
- new ArtifactsToOwnerLabels.Builder()
- .addArtifact(label1Artifact, label2)
- .addArtifact(label1Artifact)
- .build();
- assertThat(underTest.getOwners(label1Artifact)).containsExactly(label1, label2);
- underTest =
- new ArtifactsToOwnerLabels.Builder()
- .addArtifact(label1Artifact)
- .addArtifact(label1Artifact, label2)
- .build();
- assertThat(underTest.getOwners(label1Artifact)).containsExactly(label1, label2);
- }
-}
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 deaf363..a1f9d2a 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
@@ -64,7 +64,7 @@
public class AspectTest extends AnalysisTestCase {
private void pkg(String name, String... contents) throws Exception {
- scratch.file("" + name + "/BUILD", contents);
+ scratch.file(name + "/BUILD", contents);
}
@Test
@@ -628,7 +628,7 @@
// Get owners of all extra-action artifacts.
List<Label> extraArtifactOwners = new ArrayList<>();
- for (Artifact artifact : analysisResult.getTopLevelArtifactsToOwnerLabels().getArtifacts()) {
+ for (Artifact artifact : analysisResult.getArtifactsToBuild()) {
if (artifact.getRootRelativePathString().endsWith(".xa")) {
extraArtifactOwners.add(artifact.getOwnerLabel());
}
@@ -642,7 +642,7 @@
// Get owners of all extra-action artifacts.
List<Label> extraArtifactOwners = new ArrayList<>();
- for (Artifact artifact : analysisResult.getTopLevelArtifactsToOwnerLabels().getArtifacts()) {
+ for (Artifact artifact : analysisResult.getArtifactsToBuild()) {
if (artifact.getRootRelativePathString().endsWith(".xa")) {
extraArtifactOwners.add(artifact.getOwnerLabel());
}
@@ -841,9 +841,7 @@
@Test
public void aspectApplyingToFiles() throws Exception {
AspectApplyingToFiles aspectApplyingToFiles = new AspectApplyingToFiles();
- setRulesAndAspectsAvailableInTests(
- ImmutableList.<NativeAspectClass>of(aspectApplyingToFiles),
- ImmutableList.<RuleDefinition>of());
+ setRulesAndAspectsAvailableInTests(ImmutableList.of(aspectApplyingToFiles), ImmutableList.of());
pkg(
"a",
"java_binary(name = 'x', main_class = 'x.FooBar', srcs = ['x.java'])"
@@ -861,9 +859,7 @@
@Test
public void aspectApplyingToSourceFilesIgnored() throws Exception {
AspectApplyingToFiles aspectApplyingToFiles = new AspectApplyingToFiles();
- setRulesAndAspectsAvailableInTests(
- ImmutableList.<NativeAspectClass>of(aspectApplyingToFiles),
- ImmutableList.<RuleDefinition>of());
+ setRulesAndAspectsAvailableInTests(ImmutableList.of(aspectApplyingToFiles), ImmutableList.of());
pkg(
"a",
"java_binary(name = 'x', main_class = 'x.FooBar', srcs = ['x.java'])"
@@ -879,9 +875,7 @@
@Test
public void duplicateAspectsDeduped() throws Exception {
AspectApplyingToFiles aspectApplyingToFiles = new AspectApplyingToFiles();
- setRulesAndAspectsAvailableInTests(
- ImmutableList.<NativeAspectClass>of(aspectApplyingToFiles),
- ImmutableList.<RuleDefinition>of());
+ setRulesAndAspectsAvailableInTests(ImmutableList.of(aspectApplyingToFiles), ImmutableList.of());
pkg("a", "java_binary(name = 'x', main_class = 'x.FooBar', srcs = ['x.java'])");
AnalysisResult analysisResult =
update(
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/BUILD b/src/test/java/com/google/devtools/build/lib/analysis/BUILD
index b287c02..1bfe049 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/analysis/BUILD
@@ -55,7 +55,6 @@
"//src/main/java/com/google/devtools/build/lib/analysis:actions/template",
"//src/main/java/com/google/devtools/build/lib/analysis:actions/template_expansion_action",
"//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",
- "//src/main/java/com/google/devtools/build/lib/analysis:artifacts_to_owner_labels",
"//src/main/java/com/google/devtools/build/lib/analysis:aspect_aware_attribute_mapper",
"//src/main/java/com/google/devtools/build/lib/analysis:aspect_collection",
"//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories",
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java b/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java
index 77bd10bd..3692817 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java
@@ -1300,7 +1300,7 @@
AnalysisResult analysisResult = update("//x:a");
List<String> owners = new ArrayList<>();
- for (Artifact artifact : analysisResult.getTopLevelArtifactsToOwnerLabels().getArtifacts()) {
+ for (Artifact artifact : analysisResult.getArtifactsToBuild()) {
if ("xa".equals(artifact.getExtension())) {
owners.add(artifact.getOwnerLabel().toString());
}
diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/BUILD b/src/test/java/com/google/devtools/build/lib/buildtool/BUILD
index 7dd46cf..1c14e9b 100644
--- a/src/test/java/com/google/devtools/build/lib/buildtool/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/buildtool/BUILD
@@ -145,7 +145,7 @@
deps = [
"//src/main/java/com/google/devtools/build/lib:runtime",
"//src/main/java/com/google/devtools/build/lib/actions",
- "//src/main/java/com/google/devtools/build/lib/analysis:artifacts_to_owner_labels",
+ "//src/main/java/com/google/devtools/build/lib/actions:artifacts",
"//src/main/java/com/google/devtools/build/lib/exec:executor_builder",
"//src/main/java/com/google/devtools/build/lib/exec:executor_lifecycle_listener",
"//src/main/java/com/google/devtools/build/lib/util:abrupt_exit_exception",
diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/ContextProviderInitializationTest.java b/src/test/java/com/google/devtools/build/lib/buildtool/ContextProviderInitializationTest.java
index 383314e..83c6f8c 100644
--- a/src/test/java/com/google/devtools/build/lib/buildtool/ContextProviderInitializationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/buildtool/ContextProviderInitializationTest.java
@@ -15,10 +15,10 @@
import static org.junit.Assert.assertThrows;
-import com.google.common.base.Supplier;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.ActionGraph;
-import com.google.devtools.build.lib.analysis.ArtifactsToOwnerLabels;
+import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.buildtool.util.BuildIntegrationTestCase;
import com.google.devtools.build.lib.exec.ExecutorBuilder;
import com.google.devtools.build.lib.exec.ExecutorLifecycleListener;
@@ -30,10 +30,12 @@
import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
import com.google.devtools.build.lib.util.AbruptExitException;
import com.google.devtools.build.lib.util.DetailedExitCode;
+import java.util.function.Supplier;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
+
/**
* Test to make sure that context provider initialization failure is handled correctly.
*/
@@ -51,8 +53,7 @@
@Override
public void executionPhaseStarting(
- ActionGraph actionGraph,
- Supplier<ArtifactsToOwnerLabels> topLevelArtifactsToAccountingGroups)
+ ActionGraph actionGraph, Supplier<ImmutableSet<Artifact>> topLevelArtifacts)
throws AbruptExitException {
throw new AbruptExitException(
DetailedExitCode.of(
@@ -75,7 +76,7 @@
}
@Test
- public void testContextProviderInitializationFailure() throws Exception {
+ public void testContextProviderInitializationFailure() {
assertThrows(AbruptExitException.class, () -> runtimeWrapper.executeBuild(ImmutableList.of()));
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkDefinedAspectsTest.java b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkDefinedAspectsTest.java
index 7476aad..92800ae 100644
--- a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkDefinedAspectsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkDefinedAspectsTest.java
@@ -166,7 +166,7 @@
assertThat(configuredAspect.get(barKey).getProvider().getKey()).isEqualTo(barKey);
}
- private Iterable<String> getAspectDescriptions(AnalysisResult analysisResult) {
+ private static Iterable<String> getAspectDescriptions(AnalysisResult analysisResult) {
return Iterables.transform(
analysisResult.getAspectsMap().keySet(),
aspectKey ->
@@ -214,7 +214,7 @@
.containsExactly("@local//:aspect.bzl%MyAspect(//test:xxx)");
}
- private Iterable<String> getLabelsToBuild(AnalysisResult analysisResult) {
+ private static Iterable<String> getLabelsToBuild(AnalysisResult analysisResult) {
return Iterables.transform(
analysisResult.getTargetsToBuild(),
configuredTarget -> configuredTarget.getLabel().toString());
@@ -1219,7 +1219,7 @@
reporter.removeHandler(failFastHandler);
try {
- AnalysisResult result = update(ImmutableList.<String>of(), "//test:xxx");
+ AnalysisResult result = update(ImmutableList.of(), "//test:xxx");
assertThat(keepGoing()).isTrue();
assertThat(result.hasError()).isTrue();
} catch (Exception e) {
@@ -1252,7 +1252,7 @@
reporter.removeHandler(failFastHandler);
try {
- AnalysisResult result = update(ImmutableList.<String>of(), "//test:xxx");
+ AnalysisResult result = update(ImmutableList.of(), "//test:xxx");
assertThat(keepGoing()).isTrue();
assertThat(result.hasError()).isTrue();
} catch (Exception e) {
@@ -1287,7 +1287,7 @@
reporter.removeHandler(failFastHandler);
try {
- AnalysisResult result = update(ImmutableList.<String>of(), "//test:xxx");
+ AnalysisResult result = update(ImmutableList.of(), "//test:xxx");
assertThat(keepGoing()).isTrue();
assertThat(result.hasError()).isTrue();
} catch (Exception e) {
@@ -1319,7 +1319,7 @@
reporter.removeHandler(failFastHandler);
try {
- AnalysisResult result = update(ImmutableList.<String>of(), "//test:xxx");
+ AnalysisResult result = update(ImmutableList.of(), "//test:xxx");
assertThat(keepGoing()).isTrue();
assertThat(result.hasError()).isTrue();
} catch (Exception e) {
@@ -1357,7 +1357,7 @@
reporter.removeHandler(failFastHandler);
try {
- AnalysisResult result = update(ImmutableList.<String>of(), "//test:xxx");
+ AnalysisResult result = update(ImmutableList.of(), "//test:xxx");
assertThat(keepGoing()).isTrue();
assertThat(result.hasError()).isTrue();
} catch (Exception e) {
@@ -1390,7 +1390,7 @@
"load('//test:aspect.bzl', 'my_rule')",
"my_rule(name = 'xxx', my_attr = 'aaa')");
- AnalysisResult result = update(ImmutableList.<String>of(), "//test:xxx");
+ AnalysisResult result = update(ImmutableList.of(), "//test:xxx");
assertThat(result.hasError()).isFalse();
}
@@ -1413,7 +1413,7 @@
")");
scratch.file("test/BUILD", "load('//test:aspect.bzl', 'my_rule')", "my_rule(name = 'xxx')");
- AnalysisResult result = update(ImmutableList.<String>of(), "//test:xxx");
+ AnalysisResult result = update(ImmutableList.of(), "//test:xxx");
assertThat(result.hasError()).isFalse();
}
@@ -1466,7 +1466,7 @@
")");
scratch.file("test/BUILD", "load('//test:aspect.bzl', 'my_rule')", "my_rule(name = 'xxx')");
- AnalysisResult result = update(ImmutableList.<String>of(), "//test:xxx");
+ AnalysisResult result = update(ImmutableList.of(), "//test:xxx");
assertThat(result.hasError()).isFalse();
}
@@ -1494,7 +1494,7 @@
"load('//test:aspect.bzl', 'my_rule')",
"my_rule(name = 'xxx', my_attr = 'b')");
- AnalysisResult result = update(ImmutableList.<String>of(), "//test:xxx");
+ AnalysisResult result = update(ImmutableList.of(), "//test:xxx");
assertThat(result.hasError()).isFalse();
}
@@ -1519,35 +1519,35 @@
"foo/BUILD",
"load(':extension.bzl', 'my_rule')",
"my_rule(name = 'main', exe1 = ':tool.sh', exe2 = ':tool.sh')");
- AnalysisResult analysisResultOfRule = update(ImmutableList.<String>of(), "//foo:main");
+ AnalysisResult analysisResultOfRule = update(ImmutableList.of(), "//foo:main");
assertThat(analysisResultOfRule.hasError()).isFalse();
AnalysisResult analysisResultOfAspect =
- update(ImmutableList.<String>of("/foo/extension.bzl%my_aspect"), "//foo:main");
+ update(ImmutableList.of("/foo/extension.bzl%my_aspect"), "//foo:main");
assertThat(analysisResultOfAspect.hasError()).isFalse();
}
@Test
public void aspectFragmentAccessSuccess() throws Exception {
- getConfiguredTargetForAspectFragment(
+ analyzeConfiguredTargetForAspectFragment(
"ctx.fragments.java.strict_java_deps", "'java'", "", "", "");
assertNoEvents();
}
@Test
public void aspectHostFragmentAccessSuccess() throws Exception {
- getConfiguredTargetForAspectFragment(
+ analyzeConfiguredTargetForAspectFragment(
"ctx.host_fragments.java.strict_java_deps", "", "'java'", "", "");
assertNoEvents();
}
@Test
- public void aspectFragmentAccessError() throws Exception {
+ public void aspectFragmentAccessError() {
reporter.removeHandler(failFastHandler);
assertThrows(
ViewCreationFailedException.class,
() ->
- getConfiguredTargetForAspectFragment(
+ analyzeConfiguredTargetForAspectFragment(
"ctx.fragments.java.strict_java_deps", "'cpp'", "'java'", "'java'", ""));
assertContainsEvent(
"//test:aspect.bzl%MyAspect aspect on my_rule has to declare 'java' as a "
@@ -1557,12 +1557,12 @@
}
@Test
- public void aspectHostFragmentAccessError() throws Exception {
+ public void aspectHostFragmentAccessError() {
reporter.removeHandler(failFastHandler);
assertThrows(
ViewCreationFailedException.class,
() ->
- getConfiguredTargetForAspectFragment(
+ analyzeConfiguredTargetForAspectFragment(
"ctx.host_fragments.java.java_strict_deps", "'java'", "'cpp'", "", "'java'"));
assertContainsEvent(
"//test:aspect.bzl%MyAspect aspect on my_rule has to declare 'java' as a "
@@ -1571,7 +1571,7 @@
+ "(for example: host_fragments = [\"java\"])");
}
- private ConfiguredTarget getConfiguredTargetForAspectFragment(
+ private void analyzeConfiguredTargetForAspectFragment(
String fullFieldName,
String fragments,
String hostFragments,
@@ -1624,7 +1624,7 @@
.build());
}
- return getConfiguredTarget("//test:xxx");
+ assertThat(getConfiguredTarget("//test:xxx")).isNotNull();
}
@Test
@@ -1646,14 +1646,14 @@
}
private void buildTargetAndCheckRuleInfo(String... expectedLabels) throws Exception {
- AnalysisResult result = update(ImmutableList.<String>of(), "//test:r2");
+ AnalysisResult result = update(ImmutableList.of(), "//test:r2");
ConfiguredTarget configuredTarget = result.getTargetsToBuild().iterator().next();
Depset ruleInfoValue = (Depset) configuredTarget.get("rule_info");
assertThat(ruleInfoValue.getSet(String.class).toList())
.containsExactlyElementsIn(expectedLabels);
}
- private String[] aspectBzlFile(String attrAspects) {
+ private static String[] aspectBzlFile(String attrAspects) {
return new String[] {
"def _repro_aspect_impl(target, ctx):",
" s = depset([str(target.label)], transitive =",
@@ -1718,7 +1718,7 @@
"rule_bin_out(name = 'rbin')",
"rule_gen_out(name = 'rgen')",
"main_rule(name = 'main', deps = [':rbin', ':rgen'])");
- AnalysisResult analysisResult = update(ImmutableList.<String>of(), "//foo:main");
+ AnalysisResult analysisResult = update(ImmutableList.of(), "//foo:main");
ConfiguredTarget target = analysisResult.getTargetsToBuild().iterator().next();
NestedSet<Artifact> aspectFiles = ((Depset) target.get("aspect_files")).getSet(Artifact.class);
assertThat(Iterables.transform(aspectFiles.toList(), Artifact::getFilename))
@@ -1823,11 +1823,8 @@
"java_library(name = 'xxx')");
useConfiguration("--experimental_action_listener=//test:al");
AnalysisResult analysisResult =
- update(ImmutableList.<String>of("test/aspect.bzl%my_aspect"), "//test:xxx");
- assertThat(
- Iterables.transform(
- analysisResult.getTopLevelArtifactsToOwnerLabels().getArtifacts(),
- Artifact::getFilename))
+ update(ImmutableList.of("test/aspect.bzl%my_aspect"), "//test:xxx");
+ assertThat(Iterables.transform(analysisResult.getArtifactsToBuild(), Artifact::getFilename))
.contains("file.xa");
}
@@ -1950,7 +1947,7 @@
"r2(name = 'r2', dep = ':r1')");
AnalysisResult analysisResult = update("//test:r2");
ConfiguredTarget target = Iterables.getOnlyElement(analysisResult.getTargetsToBuild());
- Sequence<?> result = (Sequence) target.get("result");
+ Sequence<?> result = (Sequence<?>) target.get("result");
// "yes" means that aspect a2 sees a1's providers.
assertThat(result)
@@ -2014,7 +2011,7 @@
"rcollect(name = 'rcollect', deps = [':r1', ':r2'])");
AnalysisResult analysisResult = update("//test:rcollect");
ConfiguredTarget target = Iterables.getOnlyElement(analysisResult.getTargetsToBuild());
- Sequence<?> result = (Sequence) target.get("result");
+ Sequence<?> result = (Sequence<?>) target.get("result");
assertThat(result)
.containsExactly(
"//test:r0[\"//test:aspect.bzl%a1\", \"//test:aspect.bzl%a3\"]=a1p",
@@ -2064,7 +2061,7 @@
"r2(name = 'r2', dep = ':r1')");
AnalysisResult analysisResult = update("//test:r2");
ConfiguredTarget target = Iterables.getOnlyElement(analysisResult.getTargetsToBuild());
- Sequence<?> result = (Sequence) target.get("result");
+ Sequence<?> result = (Sequence<?>) target.get("result");
// "yes" means that aspect a2 sees a1's providers.
assertThat(result)
.containsExactly(
