Use ImmutableSet instead of ImmutableList for (Skyframe)AnalysisResult.
Also made some private methods in SkyframeBuildView explicitly return Immutable collections instead of a general one.
PiperOrigin-RevId: 442744276
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisAndExecutionResult.java b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisAndExecutionResult.java
index 0d5f569..9f86899 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisAndExecutionResult.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisAndExecutionResult.java
@@ -14,7 +14,6 @@
package com.google.devtools.build.lib.analysis;
-import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSortedSet;
@@ -36,7 +35,7 @@
BuildConfigurationCollection configurations,
ImmutableSet<ConfiguredTarget> targetsToBuild,
ImmutableMap<AspectKey, ConfiguredAspect> aspects,
- @Nullable ImmutableList<ConfiguredTarget> targetsToTest,
+ @Nullable ImmutableSet<ConfiguredTarget> targetsToTest,
ImmutableSet<ConfiguredTarget> targetsToSkip,
@Nullable FailureDetail failureDetail,
ImmutableSet<Artifact> artifactsToBuild,
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisResult.java b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisResult.java
index dae3d10..2c5c153 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisResult.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisResult.java
@@ -14,7 +14,6 @@
package com.google.devtools.build.lib.analysis;
-import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSortedSet;
@@ -32,7 +31,7 @@
public class AnalysisResult {
private final BuildConfigurationCollection configurations;
private final ImmutableSet<ConfiguredTarget> targetsToBuild;
- @Nullable private final ImmutableList<ConfiguredTarget> targetsToTest;
+ @Nullable private final ImmutableSet<ConfiguredTarget> targetsToTest;
private final ImmutableSet<ConfiguredTarget> targetsToSkip;
@Nullable private final FailureDetail failureDetail;
private final ActionGraph actionGraph;
@@ -50,7 +49,7 @@
BuildConfigurationCollection configurations,
ImmutableSet<ConfiguredTarget> targetsToBuild,
ImmutableMap<AspectKey, ConfiguredAspect> aspects,
- @Nullable ImmutableList<ConfiguredTarget> targetsToTest,
+ @Nullable ImmutableSet<ConfiguredTarget> targetsToTest,
ImmutableSet<ConfiguredTarget> targetsToSkip,
@Nullable FailureDetail failureDetail,
ActionGraph actionGraph,
@@ -105,7 +104,7 @@
* (e.g. "build" command rather than "test" command).
*/
@Nullable
- public ImmutableList<ConfiguredTarget> getTargetsToTest() {
+ public ImmutableSet<ConfiguredTarget> getTargetsToTest() {
return targetsToTest;
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
index 917aa98..950f325 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
@@ -597,7 +597,7 @@
configurations,
ImmutableSet.copyOf(configuredTargets),
aspects,
- allTargetsToTest == null ? null : ImmutableList.copyOf(allTargetsToTest),
+ allTargetsToTest == null ? null : ImmutableSet.copyOf(allTargetsToTest),
ImmutableSet.copyOf(targetsToSkip),
failureDetail,
artifactsToBuild.build(),
@@ -640,7 +640,7 @@
configurations,
ImmutableSet.copyOf(configuredTargets),
aspects,
- allTargetsToTest == null ? null : ImmutableList.copyOf(allTargetsToTest),
+ allTargetsToTest == null ? null : ImmutableSet.copyOf(allTargetsToTest),
ImmutableSet.copyOf(targetsToSkip),
failureDetail,
actionGraph,
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/constraints/TopLevelConstraintSemantics.java b/src/main/java/com/google/devtools/build/lib/analysis/constraints/TopLevelConstraintSemantics.java
index ce4d568..6b11133 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/constraints/TopLevelConstraintSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/constraints/TopLevelConstraintSemantics.java
@@ -122,7 +122,7 @@
* command line.
*/
public PlatformRestrictionsResult checkPlatformRestrictions(
- ImmutableList<ConfiguredTarget> topLevelTargets,
+ ImmutableSet<ConfiguredTarget> topLevelTargets,
ImmutableSet<String> explicitTargetPatterns,
boolean keepGoing)
throws ViewCreationFailedException {
@@ -249,7 +249,7 @@
* environment declared through {@link CoreOptions#targetEnvironments}
*/
public Set<ConfiguredTarget> checkTargetEnvironmentRestrictions(
- ImmutableList<ConfiguredTarget> topLevelTargets)
+ ImmutableSet<ConfiguredTarget> topLevelTargets)
throws ViewCreationFailedException, InterruptedException {
ImmutableSet.Builder<ConfiguredTarget> badTargets = ImmutableSet.builder();
// Maps targets that are missing *explicitly* required environments to the set of environments
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeAnalysisAndExecutionResult.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeAnalysisAndExecutionResult.java
index babd0f7..75b9d18 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeAnalysisAndExecutionResult.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeAnalysisAndExecutionResult.java
@@ -13,7 +13,6 @@
// limitations under the License.
package com.google.devtools.build.lib.skyframe;
-import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
@@ -33,7 +32,7 @@
boolean hasLoadingError,
boolean hasAnalysisError,
boolean hasActionConflicts,
- ImmutableList<ConfiguredTarget> configuredTargets,
+ ImmutableSet<ConfiguredTarget> configuredTargets,
WalkableGraph walkableGraph,
ImmutableMap<AspectKey, ConfiguredAspect> aspects,
PackageRoots packageRoots,
@@ -65,9 +64,7 @@
hasLoadingError(),
/*hasAnalysisError=*/ true,
hasActionConflicts(),
- Sets.difference(ImmutableSet.copyOf(getConfiguredTargets()), erroredTargets)
- .immutableCopy()
- .asList(),
+ Sets.difference(getConfiguredTargets(), erroredTargets).immutableCopy(),
getWalkableGraph(),
getAspects(),
getPackageRoots(),
@@ -75,7 +72,7 @@
}
public static SkyframeAnalysisAndExecutionResult success(
- ImmutableList<ConfiguredTarget> configuredTargets,
+ ImmutableSet<ConfiguredTarget> configuredTargets,
WalkableGraph walkableGraph,
ImmutableMap<AspectKey, ConfiguredAspect> aspects,
PackageRoots packageRoots) {
@@ -94,7 +91,7 @@
boolean hasLoadingError,
boolean hasAnalysisError,
boolean hasActionConflicts,
- ImmutableList<ConfiguredTarget> configuredTargets,
+ ImmutableSet<ConfiguredTarget> configuredTargets,
WalkableGraph walkableGraph,
ImmutableMap<AspectKey, ConfiguredAspect> aspects,
PackageRoots packageRoots,
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeAnalysisResult.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeAnalysisResult.java
index 6e17204..efb6e28 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeAnalysisResult.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeAnalysisResult.java
@@ -13,7 +13,6 @@
// limitations under the License.
package com.google.devtools.build.lib.skyframe;
-import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
@@ -30,7 +29,7 @@
private final boolean hasLoadingError;
private final boolean hasAnalysisError;
private final boolean hasActionConflicts;
- private final ImmutableList<ConfiguredTarget> configuredTargets;
+ private final ImmutableSet<ConfiguredTarget> configuredTargets;
private final WalkableGraph walkableGraph;
private final ImmutableMap<AspectKey, ConfiguredAspect> aspects;
private final PackageRoots packageRoots;
@@ -39,7 +38,7 @@
boolean hasLoadingError,
boolean hasAnalysisError,
boolean hasActionConflicts,
- ImmutableList<ConfiguredTarget> configuredTargets,
+ ImmutableSet<ConfiguredTarget> configuredTargets,
WalkableGraph walkableGraph,
ImmutableMap<AspectKey, ConfiguredAspect> aspects,
PackageRoots packageRoots) {
@@ -69,7 +68,7 @@
return hasActionConflicts;
}
- public ImmutableList<ConfiguredTarget> getConfiguredTargets() {
+ public ImmutableSet<ConfiguredTarget> getConfiguredTargets() {
return configuredTargets;
}
@@ -95,9 +94,7 @@
hasLoadingError,
/*hasAnalysisError=*/ true,
hasActionConflicts,
- Sets.difference(ImmutableSet.copyOf(configuredTargets), erroredTargets)
- .immutableCopy()
- .asList(),
+ Sets.difference(configuredTargets, erroredTargets).immutableCopy(),
walkableGraph,
aspects,
packageRoots);
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
index 100d58c..6798bcb 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
@@ -479,7 +479,7 @@
/*hasLoadingError=*/ false,
/*hasAnalysisError=*/ false,
foundActionConflictInLatestCheck,
- ImmutableList.copyOf(cts),
+ ImmutableSet.copyOf(cts),
result.getWalkableGraph(),
ImmutableMap.copyOf(aspects),
packageRoots);
@@ -597,7 +597,7 @@
errorProcessingResult.hasLoadingError(),
result.hasError() || foundActionConflictInLatestCheck,
foundActionConflictInLatestCheck,
- ImmutableList.copyOf(cts),
+ ImmutableSet.copyOf(cts),
result.getWalkableGraph(),
ImmutableMap.copyOf(aspects),
packageRoots);
@@ -713,13 +713,13 @@
}
if (!evaluationResult.hasError() && detailedExitCodes.isEmpty()) {
- Map<AspectKey, ConfiguredAspect> successfulAspects =
+ ImmutableMap<AspectKey, ConfiguredAspect> successfulAspects =
getSuccessfulAspectMap(
topLevelAspectsKeys.size(),
evaluationResult,
buildDriverAspectKeys,
/*topLevelActionConflictReport=*/ null);
- Set<ConfiguredTarget> successfulConfiguredTargets =
+ ImmutableSet<ConfiguredTarget> successfulConfiguredTargets =
getSuccessfulConfiguredTargets(
ctKeys.size(),
evaluationResult,
@@ -727,7 +727,7 @@
/*topLevelActionConflictReport=*/ null);
return SkyframeAnalysisAndExecutionResult.success(
- ImmutableList.copyOf(successfulConfiguredTargets),
+ successfulConfiguredTargets,
evaluationResult.getWalkableGraph(),
ImmutableMap.copyOf(successfulAspects),
/*packageRoots=*/ null);
@@ -759,13 +759,13 @@
errorProcessingResult)
: null;
- Map<AspectKey, ConfiguredAspect> successfulAspects =
+ ImmutableMap<AspectKey, ConfiguredAspect> successfulAspects =
getSuccessfulAspectMap(
topLevelAspectsKeys.size(),
evaluationResult,
buildDriverAspectKeys,
topLevelActionConflictReport);
- Set<ConfiguredTarget> successfulConfiguredTargets =
+ ImmutableSet<ConfiguredTarget> successfulConfiguredTargets =
getSuccessfulConfiguredTargets(
ctKeys.size(), evaluationResult, buildDriverCTKeys, topLevelActionConflictReport);
@@ -773,7 +773,7 @@
/*hasLoadingError=*/ errorProcessingResult.hasLoadingError(),
/*hasAnalysisError=*/ errorProcessingResult.hasAnalysisError(),
/*hasActionConflicts=*/ foundActionConflictInLatestCheck,
- ImmutableList.copyOf(successfulConfiguredTargets),
+ successfulConfiguredTargets,
evaluationResult.getWalkableGraph(),
ImmutableMap.copyOf(successfulAspects),
/*packageRoots=*/ null,
@@ -949,12 +949,12 @@
return aspectKeysBuilder.build();
}
- private static Set<ConfiguredTarget> getSuccessfulConfiguredTargets(
+ private static ImmutableSet<ConfiguredTarget> getSuccessfulConfiguredTargets(
int expectedSize,
EvaluationResult<BuildDriverValue> evaluationResult,
List<BuildDriverKey> buildDriverCTKeys,
@Nullable TopLevelActionConflictReport topLevelActionConflictReport) {
- Set<ConfiguredTarget> cts = Sets.newHashSetWithExpectedSize(expectedSize);
+ ImmutableSet.Builder<ConfiguredTarget> cts = ImmutableSet.builderWithExpectedSize(expectedSize);
for (BuildDriverKey bdCTKey : buildDriverCTKeys) {
if (topLevelActionConflictReport != null
&& !topLevelActionConflictReport.isErrorFree(bdCTKey.getActionLookupKey())) {
@@ -968,15 +968,17 @@
cts.add(ctValue.getConfiguredTarget());
}
- return cts;
+ return cts.build();
}
- private Map<AspectKey, ConfiguredAspect> getSuccessfulAspectMap(
+ private ImmutableMap<AspectKey, ConfiguredAspect> getSuccessfulAspectMap(
int expectedSize,
EvaluationResult<BuildDriverValue> evaluationResult,
List<BuildDriverKey> buildDriverAspectKeys,
@Nullable TopLevelActionConflictReport topLevelActionConflictReport) {
- Map<AspectKey, ConfiguredAspect> aspects = Maps.newHashMapWithExpectedSize(expectedSize);
+ // There can't be duplicate Aspects after resolving --aspects, so this is safe.
+ ImmutableMap.Builder<AspectKey, ConfiguredAspect> aspects =
+ ImmutableMap.builderWithExpectedSize(expectedSize);
for (BuildDriverKey bdAspectKey : buildDriverAspectKeys) {
if (topLevelActionConflictReport != null
&& !topLevelActionConflictReport.isErrorFree(bdAspectKey.getActionLookupKey())) {
@@ -993,7 +995,7 @@
aspects.put(aspectValue.getKey(), aspectValue.getConfiguredAspect());
}
}
- return aspects;
+ return aspects.buildOrThrow();
}
private static AnalysisFailedCause makeArtifactConflictAnalysisFailedCause(
