Delete code associated with `--experimental_dynamic_configs=retroactive`.
This flag value is unused and untested outside of a couple unit tests for individual components. Removing it simplifies work on b/185778053, specifically unblocking the removal of `BuildOptions.DiffForReconstruction`.
Also took this opportunity to apply fixes to warnings in the affected files.
PiperOrigin-RevId: 369970226
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BUILD b/src/main/java/com/google/devtools/build/lib/analysis/BUILD
index 367f5da..472f6a8 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BUILD
@@ -1581,10 +1581,8 @@
":config/fragment_options",
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
- "//src/main/java/com/google/devtools/build/lib/skyframe/trimming:trimmed_configuration_cache",
"//src/main/java/com/google/devtools/build/lib/util",
"//src/main/java/com/google/devtools/common/options",
- "//third_party:flogger",
"//third_party:guava",
"//third_party:jsr305",
"//third_party/protobuf:protobuf_java",
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 077e652..0801c60 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
@@ -100,49 +100,44 @@
import javax.annotation.Nullable;
/**
- * The BuildView presents a semantically-consistent and transitively-closed
- * dependency graph for some set of packages.
+ * The BuildView presents a semantically-consistent and transitively-closed dependency graph for
+ * some set of packages.
*
* <h2>Package design</h2>
*
- * <p>This package contains the Blaze dependency analysis framework (aka
- * "analysis phase"). The goal of this code is to perform semantic analysis of
- * all of the build targets required for a given build, to report
- * errors/warnings for any problems in the input, and to construct an "action
- * graph" (see {@code lib.actions} package) correctly representing the work to
- * be done during the execution phase of the build.
+ * <p>This package contains the Blaze dependency analysis framework (aka "analysis phase"). The goal
+ * of this code is to perform semantic analysis of all of the build targets required for a given
+ * build, to report errors/warnings for any problems in the input, and to construct an "action
+ * graph" (see {@code lib.actions} package) correctly representing the work to be done during the
+ * execution phase of the build.
*
- * <p><b>Configurations</b> the inputs to a build come from two sources: the
- * intrinsic inputs, specified in the BUILD file, are called <em>targets</em>.
- * The environmental inputs, coming from the build tool, the command-line, or
- * configuration files, are called the <em>configuration</em>. Only when a
- * target and a configuration are combined is there sufficient information to
- * perform a build. </p>
+ * <p><b>Configurations</b> the inputs to a build come from two sources: the intrinsic inputs,
+ * specified in the BUILD file, are called <em>targets</em>. The environmental inputs, coming from
+ * the build tool, the command-line, or configuration files, are called the <em>configuration</em>.
+ * Only when a target and a configuration are combined is there sufficient information to perform a
+ * build.
*
- * <p>Targets are implemented by the {@link Target} hierarchy in the {@code
- * lib.packages} code. Configurations are implemented by {@link
- * BuildConfiguration}. The pair of these together is represented by an
- * instance of class {@link ConfiguredTarget}; this is the root of a hierarchy
- * with different implementations for each kind of target: source file, derived
- * file, rules, etc.
+ * <p>Targets are implemented by the {@link Target} hierarchy in the {@code lib.packages} code.
+ * Configurations are implemented by {@link BuildConfiguration}. The pair of these together is
+ * represented by an instance of class {@link ConfiguredTarget}; this is the root of a hierarchy
+ * with different implementations for each kind of target: source file, derived file, rules, etc.
*
- * <p>The framework code in this package (as opposed to its subpackages) is
- * responsible for constructing the {@code ConfiguredTarget} graph for a given
- * target and configuration, taking care of such issues as:
+ * <p>The framework code in this package (as opposed to its subpackages) is responsible for
+ * constructing the {@code ConfiguredTarget} graph for a given target and configuration, taking care
+ * of such issues as:
+ *
* <ul>
* <li>caching common subgraphs.
* <li>detecting and reporting cycles.
* <li>correct propagation of errors through the graph.
- * <li>reporting universal errors, such as dependencies from production code
- * to tests, or to experimental branches.
+ * <li>reporting universal errors, such as dependencies from production code to tests, or to
+ * experimental branches.
* <li>capturing and replaying errors.
- * <li>maintaining the graph from one build to the next to
- * avoid unnecessary recomputation.
+ * <li>maintaining the graph from one build to the next to avoid unnecessary recomputation.
* <li>checking software licenses.
* </ul>
*
- * <p>See also {@link ConfiguredTarget} which documents some important
- * invariants.
+ * <p>See also {@link ConfiguredTarget} which documents some important invariants.
*/
public class BuildView {
private static final GoogleLogger logger = GoogleLogger.forEnclosingClass();
@@ -154,12 +149,11 @@
private final ConfiguredRuleClassProvider ruleClassProvider;
- /**
- * A factory class to create the coverage report action. May be null.
- */
+ /** A factory class to create the coverage report action. May be null. */
@Nullable private final CoverageReportActionFactory coverageReportActionFactory;
- public BuildView(BlazeDirectories directories,
+ public BuildView(
+ BlazeDirectories directories,
ConfiguredRuleClassProvider ruleClassProvider,
SkyframeExecutor skyframeExecutor,
CoverageReportActionFactory coverageReportActionFactory) {
@@ -191,8 +185,7 @@
@VisibleForTesting
static LinkedHashSet<ConfiguredTarget> filterTestsByTargets(
Collection<ConfiguredTarget> targets, Set<Label> allowedTargetLabels) {
- return targets
- .stream()
+ return targets.stream()
.filter(ct -> allowedTargetLabels.contains(ct.getLabel()))
.collect(Collectors.toCollection(LinkedHashSet::new));
}
@@ -228,8 +221,9 @@
if (viewOptions.skyframePrepareAnalysis) {
PrepareAnalysisPhaseValue prepareAnalysisPhaseValue;
try (SilentCloseable c = Profiler.instance().profile("Prepare analysis phase")) {
- prepareAnalysisPhaseValue = skyframeExecutor.prepareAnalysisPhase(
- eventHandler, targetOptions, multiCpu, loadingResult.getTargetLabels());
+ prepareAnalysisPhaseValue =
+ skyframeExecutor.prepareAnalysisPhase(
+ eventHandler, targetOptions, multiCpu, loadingResult.getTargetLabels());
// Determine the configurations
configurations =
@@ -243,12 +237,7 @@
// needed. This requires cleaning up the invalidation in SkyframeBuildView.setConfigurations.
try (SilentCloseable c = Profiler.instance().profile("createConfigurations")) {
configurations =
- skyframeExecutor
- .createConfigurations(
- eventHandler,
- targetOptions,
- multiCpu,
- keepGoing);
+ skyframeExecutor.createConfigurations(eventHandler, targetOptions, multiCpu, keepGoing);
}
try (SilentCloseable c = Profiler.instance().profile("AnalysisUtils.getTargetsWithConfigs")) {
topLevelTargetsWithConfigsResult =
@@ -261,10 +250,9 @@
eventHandler, configurations, viewOptions.maxConfigChangesToShow);
if (configurations.getTargetConfigurations().size() == 1) {
- eventBus
- .post(
- new MakeEnvironmentEvent(
- configurations.getTargetConfigurations().get(0).getMakeEnvironment()));
+ eventBus.post(
+ new MakeEnvironmentEvent(
+ configurations.getTargetConfigurations().get(0).getMakeEnvironment()));
}
for (BuildConfiguration targetConfig : configurations.getTargetConfigurations()) {
eventBus.post(targetConfig.toBuildEvent());
@@ -274,8 +262,7 @@
topLevelTargetsWithConfigsResult.getTargetsAndConfigs();
// Report the generated association of targets to configurations
- Multimap<Label, BuildConfiguration> byLabel =
- ArrayListMultimap.<Label, BuildConfiguration>create();
+ Multimap<Label, BuildConfiguration> byLabel = ArrayListMultimap.create();
for (TargetAndConfiguration pair : topLevelTargetsWithConfigs) {
byLabel.put(pair.getLabel(), pair.getConfiguration());
}
@@ -334,13 +321,6 @@
String starlarkFunctionName = aspect.substring(delimiterPosition + 1);
for (TargetAndConfiguration targetSpec : topLevelTargetsWithConfigs) {
- if (targetSpec.getConfiguration() != null
- && targetSpec.getConfiguration().trimConfigurationsRetroactively()) {
- String errorMessage =
- "Aspects were requested, but are not supported in retroactive trimming mode.";
- throw new ViewCreationFailedException(
- errorMessage, createFailureDetail(errorMessage, Analysis.Code.ASPECT_PREREQ_UNMET));
- }
aspectConfigurations.put(
Pair.of(targetSpec.getLabel(), aspect), targetSpec.getConfiguration());
aspectKeys.add(
@@ -359,14 +339,6 @@
if (aspectFactoryClass != null) {
for (TargetAndConfiguration targetSpec : topLevelTargetsWithConfigs) {
- if (targetSpec.getConfiguration() != null
- && targetSpec.getConfiguration().trimConfigurationsRetroactively()) {
- String errorMessage =
- "Aspects were requested, but are not supported in retroactive trimming mode.";
- throw new ViewCreationFailedException(
- errorMessage,
- createFailureDetail(errorMessage, Analysis.Code.ASPECT_PREREQ_UNMET));
- }
// For invoking top-level aspects, use the top-level configuration for both the
// aspect and the base target while the top-level configuration is untrimmed.
BuildConfiguration configuration = targetSpec.getConfiguration();
@@ -445,8 +417,10 @@
int numTargetsToAnalyze = topLevelTargetsWithConfigs.size();
int numSuccessful = skyframeAnalysisResult.getConfiguredTargets().size();
if (0 < numSuccessful && numSuccessful < numTargetsToAnalyze) {
- String msg = String.format("Analysis succeeded for only %d of %d top-level targets",
- numSuccessful, numTargetsToAnalyze);
+ String msg =
+ String.format(
+ "Analysis succeeded for only %d of %d top-level targets",
+ numSuccessful, numTargetsToAnalyze);
eventHandler.handle(Event.info(msg));
logger.atInfo().log(msg);
}
@@ -765,7 +739,7 @@
ImmutableSet.Builder<ConfiguredTarget> targetsToTest = ImmutableSet.builder();
ImmutableSet.Builder<ConfiguredTarget> targetsToTestExclusive = ImmutableSet.builder();
for (ConfiguredTarget configuredTarget : allTestTargets) {
- Target target = null;
+ Target target;
try {
target = packageManager.getTarget(eventHandler, configuredTarget.getLabel());
} catch (NoSuchTargetException | NoSuchPackageException e) {
@@ -795,10 +769,10 @@
}
/**
- * Tests and clears the current thread's pending "interrupted" status, and
- * throws InterruptedException iff it was set.
+ * Tests and clears the current thread's pending "interrupted" status, and throws
+ * InterruptedException iff it was set.
*/
- private final void pollInterruptedStatus() throws InterruptedException {
+ private static void pollInterruptedStatus() throws InterruptedException {
if (Thread.interrupted()) {
throw new InterruptedException();
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/CachingAnalysisEnvironment.java b/src/main/java/com/google/devtools/build/lib/analysis/CachingAnalysisEnvironment.java
index 35b3fc0..aa53a09 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/CachingAnalysisEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/CachingAnalysisEnvironment.java
@@ -55,22 +55,15 @@
/**
* The implementation of AnalysisEnvironment used for analysis. It tracks metadata for each
- * configured target, such as the errors and warnings emitted by that target. It is intended that
- * a separate instance is used for each configured target, so that these don't mix up.
+ * configured target, such as the errors and warnings emitted by that target. It is intended that a
+ * separate instance is used for each configured target, so that these don't mix up.
*/
-public class CachingAnalysisEnvironment implements AnalysisEnvironment {
- private final ArtifactFactory artifactFactory;
+public final class CachingAnalysisEnvironment implements AnalysisEnvironment {
+ private final ArtifactFactory artifactFactory;
private final ActionLookupKey owner;
- /**
- * If this is the system analysis environment, then errors and warnings are directly reported
- * to the global reporter, rather than stored, i.e., we don't track here whether there are any
- * errors.
- */
- private final boolean isSystemEnv;
private final boolean extendedSanityChecks;
private final boolean allowAnalysisFailures;
-
private final ActionKeyContext actionKeyContext;
private boolean enabled = true;
@@ -88,19 +81,18 @@
*
* <p>The artifact is stored so that we can deduplicate artifacts created multiple times.
*/
- private Map<Artifact, Object> artifacts;
+ private Map<Artifact, Object> artifacts = new HashMap<>();
/**
* The list of actions registered by the configured target this analysis environment is
* responsible for. May get cleared out at the end of the analysis of said target.
*/
- final List<ActionAnalysisMetadata> actions = new ArrayList<>();
+ private final List<ActionAnalysisMetadata> actions = new ArrayList<>();
public CachingAnalysisEnvironment(
ArtifactFactory artifactFactory,
ActionKeyContext actionKeyContext,
ActionLookupKey owner,
- boolean isSystemEnv,
boolean extendedSanityChecks,
boolean allowAnalysisFailures,
ExtendedEventHandler errorEventListener,
@@ -109,14 +101,12 @@
this.artifactFactory = artifactFactory;
this.actionKeyContext = actionKeyContext;
this.owner = Preconditions.checkNotNull(owner);
- this.isSystemEnv = isSystemEnv;
this.extendedSanityChecks = extendedSanityChecks;
this.allowAnalysisFailures = allowAnalysisFailures;
this.errorEventListener = errorEventListener;
this.skyframeEnv = env;
this.starlarkBuiltinsValue = starlarkBuiltinsValue;
middlemanFactory = new MiddlemanFactory(artifactFactory, this);
- artifacts = new HashMap<>();
}
public void disable(Target target) {
@@ -146,7 +136,7 @@
*/
public void verifyGeneratedArtifactHaveActions(Target target) {
Collection<String> orphanArtifacts = getOrphanArtifactMap().values();
- List<String> checkedActions = null;
+ List<String> checkedActions;
if (!orphanArtifacts.isEmpty()) {
checkedActions = Lists.newArrayListWithCapacity(actions.size());
for (ActionAnalysisMetadata action : actions) {
@@ -244,10 +234,6 @@
@Override
public boolean hasErrors() {
- // The system analysis environment never has errors.
- if (isSystemEnv) {
- return false;
- }
Preconditions.checkState(enabled);
return ((StoredEventHandler) errorEventListener).hasErrors();
}
@@ -295,7 +281,7 @@
PathFragment rootRelativePath, ArtifactRoot root, boolean contentBasedPath) {
Preconditions.checkState(enabled);
return dedupAndTrackArtifactAndOrigin(
- artifactFactory.getDerivedArtifact(rootRelativePath, root, getOwner(), contentBasedPath),
+ artifactFactory.getDerivedArtifact(rootRelativePath, root, owner, contentBasedPath),
extendedSanityChecks ? new Throwable() : null);
}
@@ -304,7 +290,7 @@
Preconditions.checkState(enabled);
return (SpecialArtifact)
dedupAndTrackArtifactAndOrigin(
- artifactFactory.getTreeArtifact(rootRelativePath, root, getOwner()),
+ artifactFactory.getTreeArtifact(rootRelativePath, root, owner),
extendedSanityChecks ? new Throwable() : null);
}
@@ -313,7 +299,7 @@
Preconditions.checkState(enabled);
return (SpecialArtifact)
dedupAndTrackArtifactAndOrigin(
- artifactFactory.getSymlinkArtifact(rootRelativePath, root, getOwner()),
+ artifactFactory.getSymlinkArtifact(rootRelativePath, root, owner),
extendedSanityChecks ? new Throwable() : null);
}
@@ -327,13 +313,13 @@
PathFragment rootRelativePath, ArtifactRoot root) {
Preconditions.checkState(enabled);
return dedupAndTrackArtifactAndOrigin(
- artifactFactory.getFilesetArtifact(rootRelativePath, root, getOwner()),
+ artifactFactory.getFilesetArtifact(rootRelativePath, root, owner),
extendedSanityChecks ? new Throwable() : null);
}
@Override
public Artifact getConstantMetadataArtifact(PathFragment rootRelativePath, ArtifactRoot root) {
- return artifactFactory.getConstantMetadataArtifact(rootRelativePath, root, getOwner());
+ return artifactFactory.getConstantMetadataArtifact(rootRelativePath, root, owner);
}
@Override
@@ -363,12 +349,12 @@
}
@Override
- public StarlarkSemantics getStarlarkSemantics() throws InterruptedException {
+ public StarlarkSemantics getStarlarkSemantics() {
return starlarkBuiltinsValue.starlarkSemantics;
}
@Override
- public ImmutableMap<String, Object> getStarlarkDefinedBuiltins() throws InterruptedException {
+ public ImmutableMap<String, Object> getStarlarkDefinedBuiltins() {
return starlarkBuiltinsValue.exportedToJava;
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
index ad63bb9..fbd8e11 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
@@ -769,14 +769,6 @@
}
/**
- * Returns whether we should trim configurations to only include the fragments needed to correctly
- * analyze a rule.
- */
- public boolean trimConfigurationsRetroactively() {
- return options.configsMode == CoreOptions.ConfigsMode.RETROACTIVE;
- }
-
- /**
* <b>>Experimental feature:</b> if true, qualifying outputs use path prefixes based on their
* content instead of the traditional <code>blaze-out/$CPU-$COMPILATION_MODE</code>.
*
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildOptions.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildOptions.java
index 92ef550..bb123e8 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildOptions.java
@@ -14,7 +14,6 @@
package com.google.devtools.build.lib.analysis.config;
-import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
import com.google.common.annotations.VisibleForTesting;
@@ -38,7 +37,6 @@
import com.google.devtools.build.lib.skyframe.serialization.ObjectCodec;
import com.google.devtools.build.lib.skyframe.serialization.SerializationContext;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
-import com.google.devtools.build.lib.skyframe.trimming.ConfigurationComparer;
import com.google.devtools.build.lib.util.Fingerprint;
import com.google.devtools.build.lib.util.OrderedSetMultimap;
import com.google.devtools.common.options.OptionDefinition;
@@ -87,12 +85,10 @@
public static BuildOptions getDefaultBuildOptionsForFragments(
List<Class<? extends FragmentOptions>> fragmentClasses) {
- ArrayList<String> collector = new ArrayList<>();
try {
- String[] stringCollector = new String[collector.size()];
- return BuildOptions.of(fragmentClasses, collector.toArray(stringCollector));
+ return BuildOptions.of(fragmentClasses);
} catch (OptionsParsingException e) {
- throw new IllegalArgumentException("Failed to parse default options", e);
+ throw new IllegalArgumentException("Failed to parse empty options", e);
}
}
@@ -531,11 +527,6 @@
return this;
}
- /** Returns whether the builder contains a particular Starlark option. */
- boolean contains(Label key) {
- return starlarkOptions.containsKey(key);
- }
-
/** Removes the value for the {@link FragmentOptions} with the given FragmentOptions class. */
public Builder removeFragmentOptions(Class<? extends FragmentOptions> key) {
fragmentOptions.remove(key);
@@ -975,82 +966,6 @@
}
/**
- * Compares the fragment sets in the options described by two diffs with the same base.
- *
- * @see ConfigurationComparer
- */
- public static ConfigurationComparer.Result compareFragments(
- OptionsDiffForReconstruction left, OptionsDiffForReconstruction right) {
- // TODO: Add support for marking Starlark options as known default when trimming
- // (sentinel object?)
- checkArgument(
- Arrays.equals(left.baseFingerprint, right.baseFingerprint),
- "Can't compare diffs with different bases: %s and %s",
- left,
- right);
- // This code effectively looks up each piece of data (native fragment or Starlark option) in
- // this table (numbers reference comments in the code below):
- // ▼left right▶ (none) extraSecond extraFirst differing
- // (none) equal right only (#4) left only (#4) different (#1)
- // extraSecond left only (#4) compare (#3) (impossible) (impossible)
- // extraFirst right only (#4) (impossible) equal right only (#4)
- // differing different (#1) (impossible) left only (#4) compare (#2)
-
- // Any difference in shared data is grounds to return DIFFERENT, which happens if:
- // 1a. any starlark option was changed by one diff, but is neither changed nor removed by
- // the other
- if (left.hasChangeToStarlarkOptionUnchangedIn(right)
- || right.hasChangeToStarlarkOptionUnchangedIn(left)) {
- return ConfigurationComparer.Result.DIFFERENT;
- }
- // 1b. any native fragment was changed by one diff, but is neither changed nor removed by
- // the other
- if (left.hasChangeToNativeFragmentUnchangedIn(right)
- || right.hasChangeToNativeFragmentUnchangedIn(left)) {
- return ConfigurationComparer.Result.DIFFERENT;
- }
- // 2a. any starlark option was changed by both diffs, but to different values
- if (!commonKeysHaveEqualValues(
- left.differingStarlarkOptions, right.differingStarlarkOptions)) {
- return ConfigurationComparer.Result.DIFFERENT;
- }
- // 2b. any native fragment was changed by both diffs, but to different values
- if (!commonKeysHaveEqualValues(left.differingOptions, right.differingOptions)) {
- return ConfigurationComparer.Result.DIFFERENT;
- }
- // 3a. any starlark option was added by both diffs, but with different values
- if (!commonKeysHaveEqualValues(
- left.extraSecondStarlarkOptions, right.extraSecondStarlarkOptions)) {
- return ConfigurationComparer.Result.DIFFERENT;
- }
- // 3b. any native fragment was added by both diffs, but with different values
- if (!commonKeysHaveEqualValues(
- left.getExtraSecondFragmentsByClass(), right.getExtraSecondFragmentsByClass())) {
- return ConfigurationComparer.Result.DIFFERENT;
- }
-
- // At this point DIFFERENT is definitely not the result, so depending on which side(s) have
- // extra data, we can decide which of the remaining choices to return. (#4)
- boolean leftHasExtraData = left.hasExtraNativeFragmentsOrStarlarkOptionsNotIn(right);
- boolean rightHasExtraData = right.hasExtraNativeFragmentsOrStarlarkOptionsNotIn(left);
-
- if (leftHasExtraData && rightHasExtraData) {
- // If both have data that the other does not, all-shared-fragments-are-equal is all
- // that can be said.
- return ConfigurationComparer.Result.ALL_SHARED_FRAGMENTS_EQUAL;
- } else if (leftHasExtraData) {
- // If only the left instance has extra data, left is a superset of right.
- return ConfigurationComparer.Result.SUPERSET;
- } else if (rightHasExtraData) {
- // If only the right instance has extra data, left is a subset of right.
- return ConfigurationComparer.Result.SUBSET;
- } else {
- // If there is no extra data, the two options described by these diffs are equal.
- return ConfigurationComparer.Result.EQUAL;
- }
- }
-
- /**
* Clears {@link #cachedReconstructed} so that tests can cover the core logic of {@link
* #applyDiff}.
*/
@@ -1059,67 +974,6 @@
cachedReconstructed = new SoftReference<>(null);
}
- private boolean hasChangeToStarlarkOptionUnchangedIn(OptionsDiffForReconstruction that) {
- Set<Label> starlarkOptionsChangedOrRemovedInThat =
- Sets.union(
- that.differingStarlarkOptions.keySet(),
- ImmutableSet.copyOf(that.extraFirstStarlarkOptions));
- return !starlarkOptionsChangedOrRemovedInThat.containsAll(
- this.differingStarlarkOptions.keySet());
- }
-
- private boolean hasChangeToNativeFragmentUnchangedIn(OptionsDiffForReconstruction that) {
- Set<Class<? extends FragmentOptions>> nativeFragmentsChangedOrRemovedInThat =
- Sets.union(that.differingOptions.keySet(), that.extraFirstFragmentClasses);
- return !nativeFragmentsChangedOrRemovedInThat.containsAll(this.differingOptions.keySet());
- }
-
- private Map<Class<? extends FragmentOptions>, FragmentOptions>
- getExtraSecondFragmentsByClass() {
- ImmutableMap.Builder<Class<? extends FragmentOptions>, FragmentOptions> builder =
- new ImmutableMap.Builder<>();
- for (FragmentOptions options : extraSecondFragments) {
- builder.put(options.getClass(), options);
- }
- return builder.build();
- }
-
- private static <K> boolean commonKeysHaveEqualValues(Map<K, ?> left, Map<K, ?> right) {
- Set<K> commonKeys = Sets.intersection(left.keySet(), right.keySet());
- for (K commonKey : commonKeys) {
- if (!Objects.equals(left.get(commonKey), right.get(commonKey))) {
- return false;
- }
- }
- return true;
- }
-
- private boolean hasExtraNativeFragmentsOrStarlarkOptionsNotIn(
- OptionsDiffForReconstruction that) {
- // extra fragments/options can be...
- // starlark options added by this diff, but not that one
- if (!that.extraSecondStarlarkOptions
- .keySet()
- .containsAll(this.extraSecondStarlarkOptions.keySet())) {
- return true;
- }
- // native fragments added by this diff, but not that one
- if (!that.getExtraSecondFragmentsByClass()
- .keySet()
- .containsAll(this.getExtraSecondFragmentsByClass().keySet())) {
- return true;
- }
- // starlark options removed by that diff, but not this one
- if (!this.extraFirstStarlarkOptions.containsAll(that.extraFirstStarlarkOptions)) {
- return true;
- }
- // native fragments removed by that diff, but not this one
- if (!this.extraFirstFragmentClasses.containsAll(that.extraFirstFragmentClasses)) {
- return true;
- }
- return false;
- }
-
@Override
public boolean equals(Object o) {
if (this == o) {
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationResolver.java b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationResolver.java
index c0a2d63..5f67920 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationResolver.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationResolver.java
@@ -44,7 +44,6 @@
import com.google.devtools.build.lib.packages.AttributeTransitionData;
import com.google.devtools.build.lib.packages.ConfiguredAttributeMapper;
import com.google.devtools.build.lib.packages.Target;
-import com.google.devtools.build.lib.packages.TargetUtils;
import com.google.devtools.build.lib.skyframe.BuildConfigurationValue;
import com.google.devtools.build.lib.skyframe.PackageValue;
import com.google.devtools.build.lib.skyframe.PlatformMappingValue;
@@ -58,7 +57,6 @@
import com.google.devtools.common.options.OptionsParsingException;
import java.util.ArrayList;
import java.util.Collection;
-import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
import java.util.HashSet;
@@ -216,24 +214,7 @@
}
private Dependency resolveHostTransition(
- Dependency.Builder dependencyBuilder, DependencyKey dependencyKey)
- throws DependencyEvaluationException {
- // The current rule's host configuration can also be used for the dep. We short-circuit
- // the standard transition logic for host transitions because these transitions are
- // uniquely frequent. It's possible, e.g., for every node in the configured target graph
- // to incur multiple host transitions. So we aggressively optimize to avoid hurting
- // analysis time.
- if (hostConfiguration.trimConfigurationsRetroactively()
- && !dependencyKey.getAspects().isEmpty()) {
- String message =
- ctgValue.getLabel()
- + " has aspects attached, but these are not supported in retroactive"
- + " trimming mode.";
- env.getListener()
- .handle(Event.error(TargetUtils.getLocationMaybe(ctgValue.getTarget()), message));
- throw new DependencyEvaluationException(new InvalidConfigurationException(message));
- }
-
+ Dependency.Builder dependencyBuilder, DependencyKey dependencyKey) {
return dependencyBuilder
.setConfiguration(hostConfiguration)
.setAspects(dependencyKey.getAspects())
@@ -332,8 +313,7 @@
throw new DependencyEvaluationException(e);
}
- Collections.sort(dependencies, SPLIT_DEP_ORDERING);
- return ImmutableList.copyOf(dependencies);
+ return ImmutableList.sortedCopyOf(SPLIT_DEP_ORDERING, dependencies);
}
private ImmutableList<String> collectTransitionKeys(Attribute attribute)
@@ -566,13 +546,8 @@
LinkedHashSet<TargetAndConfiguration> result = new LinkedHashSet<>();
for (TargetAndConfiguration originalInput : defaultContext) {
- if (successfullyEvaluatedTargets.containsKey(originalInput)) {
- // The configuration was successfully evaluated.
- result.add(successfullyEvaluatedTargets.get(originalInput));
- } else {
- // Either the configuration couldn't be determined (e.g. loading phase error) or it's null.
- result.add(originalInput);
- }
+ // If the configuration couldn't be determined (e.g. loading phase error), use the original.
+ result.add(successfullyEvaluatedTargets.getOrDefault(originalInput, originalInput));
}
return new TopLevelTargetsAndConfigsResult(result, hasError);
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java b/src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java
index 0097e18..6d501e3 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java
@@ -622,13 +622,7 @@
*/
ON,
/** Default mode: Each configured target is evaluated with all fragments known to Blaze. */
- NOTRIM,
- /**
- * Experimental mode: Each configured target is evaluated with only the configuration fragments
- * it needs by visiting them with a full configuration to begin with and collapsing the
- * configuration down to the fragments which were actually used.
- */
- RETROACTIVE;
+ NOTRIM
}
/** Converter for --experimental_dynamic_configs. */
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
index 971e2e1..91b121a 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
@@ -128,7 +128,7 @@
* @throws AspectCreationException if the value loaded is not a {@link StarlarkDefinedAspect}.
*/
@Nullable
- static StarlarkDefinedAspect loadStarlarkDefinedAspect(
+ private static StarlarkDefinedAspect loadStarlarkDefinedAspect(
Environment env, StarlarkAspectClass starlarkAspectClass)
throws AspectCreationException, InterruptedException {
Label extensionLabel = starlarkAspectClass.getExtensionLabel();
@@ -269,9 +269,6 @@
throw new IllegalStateException("Unexpected exception from BuildConfigurationFunction when "
+ "computing " + key.getAspectConfigurationKey(), e);
}
- if (aspectConfiguration.trimConfigurationsRetroactively()) {
- throw new AssertionError("Aspects should NEVER be evaluated in retroactive trimming mode.");
- }
}
ConfiguredTarget associatedTarget = baseConfiguredTargetValue.getConfiguredTarget();
@@ -301,9 +298,6 @@
configuration =
((BuildConfigurationValue) result.get(associatedTarget.getConfigurationKey()))
.getConfiguration();
- if (configuration.trimConfigurationsRetroactively()) {
- throw new AssertionError("Aspects should NEVER be evaluated in retroactive trimming mode.");
- }
}
try {
associatedConfiguredTargetAndData =
@@ -389,8 +383,6 @@
ToolchainContextKey.key()
.configurationKey(BuildConfigurationValue.key(configuration))
.requiredToolchainTypeLabels(requiredToolchains)
- .shouldSanityCheckConfiguration(
- configuration.trimConfigurationsRetroactively())
.build(),
ToolchainException.class);
} catch (ToolchainException e) {
@@ -553,14 +545,13 @@
}
/**
- * Collect all SkyKeys that are needed for a given list of AspectKeys,
- * including transitive dependencies.
+ * Collect all SkyKeys that are needed for a given list of AspectKeys, including transitive
+ * dependencies.
*
- * Also collects all propagating aspects in correct order.
+ * <p>Also collects all propagating aspects in correct order.
*/
- private ImmutableMap<AspectDescriptor, SkyKey> getSkyKeysForAspectsAndCollectAspectPath(
- ImmutableList<AspectKey> keys,
- ImmutableList.Builder<SkyKey> aspectPathBuilder) {
+ private static ImmutableMap<AspectDescriptor, SkyKey> getSkyKeysForAspectsAndCollectAspectPath(
+ ImmutableList<AspectKey> keys, ImmutableList.Builder<SkyKey> aspectPathBuilder) {
HashMap<AspectDescriptor, SkyKey> result = new HashMap<>();
for (AspectKey key : keys) {
buildSkyKeys(key, result, aspectPathBuilder);
@@ -568,7 +559,9 @@
return ImmutableMap.copyOf(result);
}
- private void buildSkyKeys(AspectKey key, HashMap<AspectDescriptor, SkyKey> result,
+ private static void buildSkyKeys(
+ AspectKey key,
+ HashMap<AspectDescriptor, SkyKey> result,
ImmutableList.Builder<SkyKey> aspectPathBuilder) {
if (result.containsKey(key.getAspectDescriptor())) {
return;
@@ -657,7 +650,7 @@
StoredEventHandler events = new StoredEventHandler();
CachingAnalysisEnvironment analysisEnvironment =
view.createAnalysisEnvironment(
- key, false, events, env, aspectConfiguration, starlarkBuiltinsValue);
+ key, events, env, aspectConfiguration, starlarkBuiltinsValue);
ConfiguredAspect configuredAspect;
if (aspect.getDefinition().applyToGeneratingRules()
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
index 165cf98..0ad6577 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
@@ -8,7 +8,6 @@
"//src/main/java/com/google/devtools/build/lib/skyframe/packages:srcs",
"//src/main/java/com/google/devtools/build/lib/skyframe/proto:srcs",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization:srcs",
- "//src/main/java/com/google/devtools/build/lib/skyframe/trimming:srcs",
],
visibility = ["//src:__subpackages__"],
)
@@ -78,7 +77,6 @@
"TopLevelActionLookupConflictFindingFunction.java",
"ToplevelStarlarkAspectFunction.java",
"TransitiveTargetFunction.java",
- "TrimmedConfigurationProgressReceiver.java",
"WorkspaceFileFunction.java",
"actiongraph/ActionGraphDump.java",
"actiongraph/v2/ActionGraphDump.java",
@@ -299,7 +297,6 @@
"//src/main/java/com/google/devtools/build/lib/rules/cpp:cpp_interface",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec:serialization-constant",
- "//src/main/java/com/google/devtools/build/lib/skyframe/trimming:trimmed_configuration_cache",
"//src/main/java/com/google/devtools/build/lib/util",
"//src/main/java/com/google/devtools/build/lib/util:TestType",
"//src/main/java/com/google/devtools/build/lib/util:abrupt_exit_exception",
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
index 950693a..5f03dc0 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
@@ -317,7 +317,7 @@
env,
resolver,
ctgValue,
- ImmutableList.<Aspect>of(),
+ ImmutableList.of(),
configConditions.asProviders(),
unloadedToolchainContexts == null
? null
@@ -521,8 +521,7 @@
ToolchainContextKey.key()
.configurationKey(toolchainConfig)
.requiredToolchainTypeLabels(requiredDefaultToolchains)
- .execConstraintLabels(defaultExecConstraintLabels)
- .shouldSanityCheckConfiguration(configuration.trimConfigurationsRetroactively());
+ .execConstraintLabels(defaultExecConstraintLabels);
if (parentToolchainContextKey != null) {
// Find out what execution platform the parent used, and force that.
@@ -550,7 +549,6 @@
.configurationKey(toolchainConfig)
.requiredToolchainTypeLabels(execGroup.requiredToolchains())
.execConstraintLabels(execGroup.execCompatibleWith())
- .shouldSanityCheckConfiguration(configuration.trimConfigurationsRetroactively())
.build());
}
@@ -610,7 +608,6 @@
* @param env the Skyframe environment
* @param resolver the dependency resolver
* @param ctgValue the label and the configuration of the node
- * @param aspects
* @param configConditions the configuration conditions for evaluating the attributes of the node
* @param toolchainContexts the toolchain context for this target
* @param ruleClassProvider rule class provider for determining the right configuration fragments
@@ -739,15 +736,6 @@
.collect(Collectors.toList());
if (configLabels.isEmpty()) {
return ConfigConditions.EMPTY;
- } else if (ctgValue.getConfiguration().trimConfigurationsRetroactively()) {
- String message =
- target.getLabel()
- + " has configurable attributes, but these are not supported in retroactive trimming "
- + "mode.";
- env.getListener().handle(Event.error(TargetUtils.getLocationMaybe(target), message));
- throw new DependencyEvaluationException(
- new ConfiguredValueCreationException(
- message, ctgValue.getLabel(), ctgValue.getConfiguration()));
}
// Collect the actual deps without a configuration transition (since by definition config
@@ -905,20 +893,7 @@
BuildConfiguration depConfiguration = dep.getConfiguration();
BuildConfigurationValue.Key depKey =
depValue.getConfiguredTarget().getConfigurationKey();
- // Retroactive trimming may change the configuration associated with the dependency.
- // If it does, we need to get that instance.
- // TODO(b/140632978): doing these individually instead of doing them all at once may
- // end up being wasteful use of Skyframe. Although these configurations are guaranteed
- // to be in the Skyframe cache (because the dependency would have had to retrieve them
- // to be created in the first place), looking them up repeatedly may be slower than
- // just keeping a local cache and assigning the same configuration to all the CTs
- // which need it. Profile this and see if there's a better way.
if (depKey != null && !depKey.equals(BuildConfigurationValue.key(depConfiguration))) {
- if (!depConfiguration.trimConfigurationsRetroactively()) {
- throw new AssertionError(
- "Loading configurations mid-dependency resolution should ONLY happen when "
- + "retroactive trimming is enabled.");
- }
depConfiguration =
((BuildConfigurationValue) env.getValue(depKey)).getConfiguration();
}
@@ -973,7 +948,7 @@
}
@Nullable
- private ConfiguredTargetValue createConfiguredTarget(
+ private static ConfiguredTargetValue createConfiguredTarget(
SkyframeBuildView view,
Environment env,
Target target,
@@ -994,7 +969,7 @@
StoredEventHandler events = new StoredEventHandler();
CachingAnalysisEnvironment analysisEnvironment =
view.createAnalysisEnvironment(
- configuredTargetKey, false, events, env, configuration, starlarkBuiltinsValue);
+ configuredTargetKey, events, env, configuration, starlarkBuiltinsValue);
Preconditions.checkNotNull(depValueMap);
ConfiguredTarget configuredTarget;
@@ -1032,8 +1007,7 @@
configuration == null
? null
: configuration.getEventId().getConfiguration(),
- createDetailedExitCode(
- event.getMessage(), Code.CONFIGURED_VALUE_CREATION_FAILED)))
+ createDetailedExitCode(event.getMessage())))
.collect(Collectors.toList()));
throw new ConfiguredTargetFunctionException(
new ConfiguredValueCreationException(
@@ -1070,11 +1044,11 @@
}
}
- private static DetailedExitCode createDetailedExitCode(String message, Code code) {
+ private static DetailedExitCode createDetailedExitCode(String message) {
return DetailedExitCode.of(
FailureDetail.newBuilder()
.setMessage(message)
- .setAnalysis(Analysis.newBuilder().setCode(code))
+ .setAnalysis(Analysis.newBuilder().setCode(Code.CONFIGURED_VALUE_CREATION_FAILED))
.build());
}
@@ -1093,16 +1067,16 @@
* Used to declare all the exception types that can be wrapped in the exception thrown by {@link
* ConfiguredTargetFunction#compute}.
*/
- static final class ConfiguredTargetFunctionException extends SkyFunctionException {
- private ConfiguredTargetFunctionException(ConfiguredValueCreationException e) {
+ private static final class ConfiguredTargetFunctionException extends SkyFunctionException {
+ ConfiguredTargetFunctionException(ConfiguredValueCreationException e) {
super(e, Transience.PERSISTENT);
}
- private ConfiguredTargetFunctionException(ActionConflictException e) {
+ ConfiguredTargetFunctionException(ActionConflictException e) {
super(e, Transience.PERSISTENT);
}
- private ConfiguredTargetFunctionException(InvalidExecGroupException e) {
+ ConfiguredTargetFunctionException(InvalidExecGroupException e) {
super(e, Transience.PERSISTENT);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PlatformLookupUtil.java b/src/main/java/com/google/devtools/build/lib/skyframe/PlatformLookupUtil.java
index e49675f..f0317b0 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PlatformLookupUtil.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PlatformLookupUtil.java
@@ -14,17 +14,13 @@
package com.google.devtools.build.lib.skyframe;
-import static com.google.common.base.Predicates.equalTo;
-import static com.google.common.base.Predicates.not;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
-import static java.util.stream.Collectors.joining;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.ConfiguredTargetValue;
-import com.google.devtools.build.lib.analysis.PlatformConfiguration;
import com.google.devtools.build.lib.analysis.platform.PlatformInfo;
import com.google.devtools.build.lib.analysis.platform.PlatformProviderUtils;
import com.google.devtools.build.lib.cmdline.Label;
@@ -49,11 +45,8 @@
@Nullable
public static Map<ConfiguredTargetKey, PlatformInfo> getPlatformInfo(
- ImmutableList<ConfiguredTargetKey> platformKeys,
- Environment env,
- boolean sanityCheckConfiguration)
+ ImmutableList<ConfiguredTargetKey> platformKeys, Environment env)
throws InterruptedException, InvalidPlatformException {
-
validatePlatformKeys(platformKeys, env);
if (env.valuesMissing()) {
return null;
@@ -72,7 +65,7 @@
boolean valuesMissing = env.valuesMissing();
Map<ConfiguredTargetKey, PlatformInfo> platforms = valuesMissing ? null : new HashMap<>();
for (ConfiguredTargetKey key : platformKeys) {
- PlatformInfo platformInfo = findPlatformInfo(key, values.get(key), sanityCheckConfiguration);
+ PlatformInfo platformInfo = findPlatformInfo(key, values.get(key));
if (!valuesMissing && platformInfo != null) {
platforms.put(key, platformInfo);
}
@@ -142,8 +135,7 @@
ConfiguredTargetKey key,
ValueOrException3<
ConfiguredValueCreationException, NoSuchThingException, ActionConflictException>
- valueOrException,
- boolean sanityCheckConfiguration)
+ valueOrException)
throws InvalidPlatformException {
try {
@@ -153,29 +145,6 @@
}
ConfiguredTarget configuredTarget = ctv.getConfiguredTarget();
- BuildConfigurationValue.Key configurationKey = configuredTarget.getConfigurationKey();
- // This check is necessary because trimming for other rules assumes that platform resolution
- // uses the platform fragment and _only_ the platform fragment. Without this check, it's
- // possible another fragment could slip in without us realizing, and thus break this
- // assumption.
- if (sanityCheckConfiguration
- && configurationKey.getFragments().stream()
- .anyMatch(not(equalTo(PlatformConfiguration.class)))) {
- // Only the PlatformConfiguration fragment may be present on a platform rule in retroactive
- // trimming mode.
- String extraFragmentDescription =
- configurationKey.getFragments().stream()
- .filter(not(equalTo(PlatformConfiguration.class)))
- .map(Class::getSimpleName)
- .collect(joining(","));
- throw new InvalidPlatformException(
- configuredTarget.getLabel(),
- "has fragments other than PlatformConfiguration, "
- + "which is forbidden in retroactive trimming mode: "
- + "extra fragments are ["
- + extraFragmentDescription
- + "]");
- }
PlatformInfo platformInfo = PlatformProviderUtils.platform(configuredTarget);
if (platformInfo == null) {
throw new InvalidPlatformException(configuredTarget.getLabel());
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RegisteredExecutionPlatformsFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/RegisteredExecutionPlatformsFunction.java
index b4ca46d..65d7cb1 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/RegisteredExecutionPlatformsFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/RegisteredExecutionPlatformsFunction.java
@@ -14,8 +14,6 @@
package com.google.devtools.build.lib.skyframe;
-import static com.google.common.base.Predicates.equalTo;
-import static com.google.common.base.Predicates.not;
import static com.google.common.collect.ImmutableList.toImmutableList;
import com.google.auto.value.AutoValue;
@@ -97,8 +95,7 @@
// Load the configured target for each, and get the declared execution platforms providers.
ImmutableList<ConfiguredTargetKey> registeredExecutionPlatformKeys =
- configureRegisteredExecutionPlatforms(
- env, configuration, configuration.trimConfigurationsRetroactively(), platformLabels);
+ configureRegisteredExecutionPlatforms(env, configuration, platformLabels);
if (env.valuesMissing()) {
return null;
}
@@ -125,13 +122,9 @@
return externalPackage.getRegisteredExecutionPlatforms();
}
- private ImmutableList<ConfiguredTargetKey> configureRegisteredExecutionPlatforms(
- Environment env,
- BuildConfiguration configuration,
- boolean sanityCheckConfiguration,
- List<Label> labels)
+ private static ImmutableList<ConfiguredTargetKey> configureRegisteredExecutionPlatforms(
+ Environment env, BuildConfiguration configuration, List<Label> labels)
throws InterruptedException, RegisteredExecutionPlatformsFunctionException {
-
ImmutableList<ConfiguredTargetKey> keys =
labels.stream()
.map(
@@ -157,22 +150,6 @@
}
ConfiguredTarget target =
((ConfiguredTargetValue) valueOrException.get()).getConfiguredTarget();
- // This check is necessary because trimming for other rules assumes that platform resolution
- // uses the platform fragment and _only_ the platform fragment. Without this check, it's
- // possible another fragment could slip in without us realizing, and thus break this
- // assumption.
- if (sanityCheckConfiguration
- && target.getConfigurationKey().getFragments().stream()
- .anyMatch(not(equalTo(PlatformConfiguration.class)))) {
- // Only the PlatformConfiguration fragment may be present on a platform rule in
- // retroactive trimming mode.
- throw new RegisteredExecutionPlatformsFunctionException(
- new InvalidPlatformException(
- target.getLabel(),
- "has fragments other than PlatformConfiguration, "
- + "which is forbidden in retroactive trimming mode"),
- Transience.PERSISTENT);
- }
PlatformInfo platformInfo = PlatformProviderUtils.platform(target);
if (platformInfo == null) {
throw new RegisteredExecutionPlatformsFunctionException(
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RegisteredToolchainsFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/RegisteredToolchainsFunction.java
index 024601d..e90f6c8 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/RegisteredToolchainsFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/RegisteredToolchainsFunction.java
@@ -14,8 +14,8 @@
package com.google.devtools.build.lib.skyframe;
+import static com.google.common.base.MoreObjects.firstNonNull;
import static com.google.common.collect.ImmutableList.toImmutableList;
-import static java.util.stream.Collectors.joining;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableList;
@@ -97,13 +97,9 @@
return RegisteredToolchainsValue.create(registeredToolchains);
}
- private Iterable<? extends String> getWorkspaceToolchains(Environment env)
+ private static ImmutableList<String> getWorkspaceToolchains(Environment env)
throws InterruptedException {
- List<String> patterns = getRegisteredToolchains(env);
- if (patterns == null) {
- return ImmutableList.of();
- }
- return patterns;
+ return firstNonNull(getRegisteredToolchains(env), ImmutableList.of());
}
/**
@@ -113,7 +109,8 @@
*/
@Nullable
@VisibleForTesting
- public static List<String> getRegisteredToolchains(Environment env) throws InterruptedException {
+ public static ImmutableList<String> getRegisteredToolchains(Environment env)
+ throws InterruptedException {
PackageValue externalPackageValue =
(PackageValue) env.getValue(PackageValue.key(LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER));
if (externalPackageValue == null) {
@@ -124,10 +121,8 @@
return externalPackage.getRegisteredToolchains();
}
- private ImmutableList<DeclaredToolchainInfo> configureRegisteredToolchains(
- Environment env,
- BuildConfiguration configuration,
- List<Label> labels)
+ private static ImmutableList<DeclaredToolchainInfo> configureRegisteredToolchains(
+ Environment env, BuildConfiguration configuration, List<Label> labels)
throws InterruptedException, RegisteredToolchainsFunctionException {
ImmutableList<SkyKey> keys =
labels.stream()
@@ -155,28 +150,6 @@
ConfiguredTarget target =
((ConfiguredTargetValue) valueOrException.get()).getConfiguredTarget();
- if (configuration.trimConfigurationsRetroactively()
- && !target.getConfigurationKey().getFragments().isEmpty()) {
- // No fragment may be present on a toolchain rule in retroactive trimming mode.
- // This is because trimming expects that platform and toolchain resolution uses only
- // the platform configuration. In theory, this means toolchains could use platforms, but
- // the current expectation is that toolchains should not use anything at all, so better
- // to go with the stricter expectation for now.
- String extraFragmentDescription =
- target.getConfigurationKey().getFragments().stream()
- .map(cl -> cl.getSimpleName())
- .collect(joining(","));
-
- throw new RegisteredToolchainsFunctionException(
- new InvalidToolchainLabelException(
- toolchainLabel,
- "this toolchain uses configuration, which is forbidden in retroactive trimming "
- + "mode: "
- + "extra fragments are ["
- + extraFragmentDescription
- + "]"),
- Transience.PERSISTENT);
- }
DeclaredToolchainInfo toolchainInfo = PlatformProviderUtils.declaredToolchainInfo(target);
if (toolchainInfo == null) {
throw new RegisteredToolchainsFunctionException(
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SingleToolchainResolutionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/SingleToolchainResolutionFunction.java
index 8d92213..ee67d95 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SingleToolchainResolutionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SingleToolchainResolutionFunction.java
@@ -88,7 +88,6 @@
key.toolchainTypeLabel(),
key.availableExecutionPlatformKeys(),
key.targetPlatformKey(),
- configuration.trimConfigurationsRetroactively(),
toolchains.registeredToolchains(),
env,
debug ? env.getListener() : null);
@@ -104,7 +103,6 @@
Label toolchainTypeLabel,
List<ConfiguredTargetKey> availableExecutionPlatformKeys,
ConfiguredTargetKey targetPlatformKey,
- boolean sanityCheckConfigurations,
ImmutableList<DeclaredToolchainInfo> toolchains,
Environment env,
@Nullable EventHandler eventHandler)
@@ -113,15 +111,14 @@
// Load the PlatformInfo needed to check constraints.
Map<ConfiguredTargetKey, PlatformInfo> platforms;
try {
-
platforms =
PlatformLookupUtil.getPlatformInfo(
- new ImmutableList.Builder<ConfiguredTargetKey>()
+ ImmutableList.<ConfiguredTargetKey>builderWithExpectedSize(
+ availableExecutionPlatformKeys.size() + 1)
.add(targetPlatformKey)
.addAll(availableExecutionPlatformKeys)
.build(),
- env,
- sanityCheckConfigurations);
+ env);
if (env.valuesMissing()) {
return null;
}
@@ -318,15 +315,15 @@
* Used to indicate errors during the computation of an {@link SingleToolchainResolutionValue}.
*/
private static final class ToolchainResolutionFunctionException extends SkyFunctionException {
- public ToolchainResolutionFunctionException(NoToolchainFoundException e) {
+ ToolchainResolutionFunctionException(NoToolchainFoundException e) {
super(e, Transience.PERSISTENT);
}
- public ToolchainResolutionFunctionException(InvalidToolchainLabelException e) {
+ ToolchainResolutionFunctionException(InvalidToolchainLabelException e) {
super(e, Transience.PERSISTENT);
}
- public ToolchainResolutionFunctionException(InvalidPlatformException e) {
+ ToolchainResolutionFunctionException(InvalidPlatformException e) {
super(e, Transience.PERSISTENT);
}
}
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 1554ccf..713d543 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
@@ -332,12 +332,7 @@
skyframeExecutor.handleAnalysisInvalidatingChange();
}
}
- if (configurations.getTargetConfigurations().stream()
- .anyMatch(BuildConfiguration::trimConfigurationsRetroactively)) {
- skyframeExecutor.activateRetroactiveTrimming();
- } else {
- skyframeExecutor.deactivateRetroactiveTrimming();
- }
+
skyframeAnalysisWasDiscarded = false;
this.configurations = configurations;
setTopLevelHostConfiguration(configurations.getHostConfiguration());
@@ -981,7 +976,6 @@
CachingAnalysisEnvironment createAnalysisEnvironment(
ActionLookupKey owner,
- boolean isSystemEnv,
ExtendedEventHandler eventHandler,
Environment env,
BuildConfiguration config,
@@ -992,7 +986,6 @@
artifactFactory,
skyframeExecutor.getActionKeyContext(),
owner,
- isSystemEnv,
extendedSanityChecks,
allowAnalysisFailures,
eventHandler,
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
index 86fe88d..56715ae 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -87,7 +87,6 @@
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollection;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
-import com.google.devtools.build.lib.analysis.config.BuildOptions.OptionsDiffForReconstruction;
import com.google.devtools.build.lib.analysis.config.BuildOptionsView;
import com.google.devtools.build.lib.analysis.config.ConfigurationResolver;
import com.google.devtools.build.lib.analysis.config.CoreOptions;
@@ -171,7 +170,6 @@
import com.google.devtools.build.lib.skyframe.PackageLookupFunction.CrossRepositoryLabelViolationStrategy;
import com.google.devtools.build.lib.skyframe.SkyframeActionExecutor.ActionCompletedReceiver;
import com.google.devtools.build.lib.skyframe.SkyframeActionExecutor.ProgressSupplier;
-import com.google.devtools.build.lib.skyframe.trimming.TrimmedConfigurationCache;
import com.google.devtools.build.lib.util.AbruptExitException;
import com.google.devtools.build.lib.util.DetailedExitCode;
import com.google.devtools.build.lib.util.ResourceUsage;
@@ -375,11 +373,6 @@
private final PathResolverFactory pathResolverFactory = new PathResolverFactoryImpl();
@Nullable private final NonexistentFileReceiver nonexistentFileReceiver;
- private final TrimmedConfigurationCache<SkyKey, Label, OptionsDiffForReconstruction>
- trimmingCache = TrimmedConfigurationProgressReceiver.buildCache();
- private final TrimmedConfigurationProgressReceiver trimmingListener =
- new TrimmedConfigurationProgressReceiver(trimmingCache);
-
private boolean siblingRepositoryLayout = false;
private Map<String, String> lastRemoteDefaultExecProperties;
@@ -855,7 +848,6 @@
public void resetEvaluator() {
init();
emittedEventState.clear();
- clearTrimmingCache();
skyframeBuildView.reset();
}
@@ -885,26 +877,11 @@
public void handleAnalysisInvalidatingChange() {
logger.atInfo().log("Dropping configured target data");
analysisCacheDiscarded = true;
- clearTrimmingCache();
skyframeBuildView.clearInvalidatedActionLookupKeys();
skyframeBuildView.clearLegacyData();
ArtifactNestedSetFunction.getInstance().resetArtifactNestedSetFunctionMaps();
}
- /** Activates retroactive trimming (idempotently, so has no effect if already active). */
- void activateRetroactiveTrimming() {
- trimmingListener.activate();
- }
-
- /** Deactivates retroactive trimming (idempotently, so has no effect if already inactive). */
- void deactivateRetroactiveTrimming() {
- trimmingListener.deactivate();
- }
-
- protected void clearTrimmingCache() {
- trimmingCache.clear();
- }
-
/** Used with dump --rules. */
public static class RuleStat {
private final String key;
@@ -1901,19 +1878,6 @@
// when depConfig is non-null, so we need to explicitly override it in that case.
resolvedConfig = null;
} else if (!configKey.equals(BuildConfigurationValue.key(depConfig))) {
- // Retroactive trimming may change the configuration associated with the dependency.
- // If it does, we need to get that instance.
- // TODO(b/140632978): doing these individually instead of doing them all at once may
- // end up being wasteful use of Skyframe. Although these configurations are guaranteed
- // to be in the Skyframe cache (because the dependency would have had to retrieve
- // them to be created in the first place), looking them up repeatedly may be slower
- // than just keeping a local cache and assigning the same configuration to all the
- // CTs which need it. Profile this and see if there's a better way.
- if (!depConfig.trimConfigurationsRetroactively()) {
- throw new AssertionError(
- "Loading configurations mid-dependency resolution should ONLY happen when "
- + "retroactive trimming is enabled.");
- }
resolvedConfig = getConfiguration(eventHandler, mergedTarget.getConfigurationKey());
}
cts.put(
@@ -1923,7 +1887,6 @@
packageValue.getPackage().getTarget(configuredTarget.getLabel().getName()),
resolvedConfig,
null));
-
} catch (DuplicateException | NoSuchTargetException e) {
throw new IllegalStateException(
String.format("Error creating %s", configuredTarget.getLabel()), e);
@@ -3104,7 +3067,6 @@
/** This receiver is only needed for loading, so it is null otherwise. */
@Override
public void invalidated(SkyKey skyKey, InvalidationState state) {
- trimmingListener.invalidated(skyKey, state);
if (ignoreInvalidations) {
return;
}
@@ -3113,7 +3075,6 @@
@Override
public void enqueueing(SkyKey skyKey) {
- trimmingListener.enqueueing(skyKey);
if (ignoreInvalidations) {
return;
}
@@ -3130,7 +3091,6 @@
@Nullable ErrorInfo newError,
Supplier<EvaluationSuccessState> evaluationSuccessState,
EvaluationState state) {
- trimmingListener.evaluated(skyKey, newValue, newError, evaluationSuccessState, state);
if (ignoreInvalidations) {
return;
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ToolchainContextKey.java b/src/main/java/com/google/devtools/build/lib/skyframe/ToolchainContextKey.java
index f82b27a..1062675 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ToolchainContextKey.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ToolchainContextKey.java
@@ -33,7 +33,6 @@
return new AutoValue_ToolchainContextKey.Builder()
.requiredToolchainTypeLabels(ImmutableSet.of())
.execConstraintLabels(ImmutableSet.of())
- .shouldSanityCheckConfiguration(false)
.forceExecutionPlatform(Optional.empty());
}
@@ -48,8 +47,6 @@
abstract ImmutableSet<Label> execConstraintLabels();
- abstract boolean shouldSanityCheckConfiguration();
-
abstract Optional<Label> forceExecutionPlatform();
/** Builder for {@link ToolchainContextKey}. */
@@ -65,8 +62,6 @@
Builder execConstraintLabels(Label... execConstraintLabels);
- Builder shouldSanityCheckConfiguration(boolean shouldSanityCheckConfiguration);
-
ToolchainContextKey build();
Builder forceExecutionPlatform(Optional<Label> execPlatform);
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ToolchainResolutionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ToolchainResolutionFunction.java
index 470f9fd..714cfb4 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ToolchainResolutionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ToolchainResolutionFunction.java
@@ -87,11 +87,7 @@
// Load the configured target for the toolchain types to ensure that they are valid and
// resolve aliases.
ImmutableMap<Label, ToolchainTypeInfo> resolvedToolchainTypes =
- loadToolchainTypes(
- env,
- configuration,
- key.requiredToolchainTypeLabels(),
- key.shouldSanityCheckConfiguration());
+ loadToolchainTypes(env, configuration, key.requiredToolchainTypeLabels());
builder.setRequestedLabelToToolchainType(resolvedToolchainTypes);
ImmutableSet<Label> resolvedToolchainTypeLabels =
resolvedToolchainTypes.values().stream()
@@ -106,8 +102,7 @@
key.configurationKey(),
configuration,
platformConfiguration,
- key.execConstraintLabels(),
- key.shouldSanityCheckConfiguration());
+ key.execConstraintLabels());
if (env.valuesMissing()) {
return null;
}
@@ -119,8 +114,7 @@
resolvedToolchainTypeLabels,
key.forceExecutionPlatform().map(platformKeys::find),
builder,
- platformKeys,
- key.shouldSanityCheckConfiguration());
+ platformKeys);
UnloadedToolchainContext unloadedToolchainContext = builder.build();
if (debug) {
@@ -150,11 +144,10 @@
}
/** Returns a map from the requested toolchain type to the {@link ToolchainTypeInfo} provider. */
- private ImmutableMap<Label, ToolchainTypeInfo> loadToolchainTypes(
+ private static ImmutableMap<Label, ToolchainTypeInfo> loadToolchainTypes(
Environment environment,
BuildConfiguration configuration,
- ImmutableSet<Label> requestedToolchainTypeLabels,
- boolean shouldSanityCheckConfiguration)
+ ImmutableSet<Label> requestedToolchainTypeLabels)
throws InvalidToolchainTypeException, InterruptedException, ValueMissingException {
ImmutableSet<ConfiguredTargetKey> toolchainTypeKeys =
requestedToolchainTypeLabels.stream()
@@ -167,8 +160,7 @@
.collect(toImmutableSet());
ImmutableMap<Label, ToolchainTypeInfo> resolvedToolchainTypes =
- ToolchainTypeLookupUtil.resolveToolchainTypes(
- environment, toolchainTypeKeys, shouldSanityCheckConfiguration);
+ ToolchainTypeLookupUtil.resolveToolchainTypes(environment, toolchainTypeKeys);
if (environment.valuesMissing()) {
throw new ValueMissingException();
}
@@ -210,14 +202,13 @@
}
}
- private PlatformKeys loadPlatformKeys(
+ private static PlatformKeys loadPlatformKeys(
SkyFunction.Environment environment,
boolean debug,
BuildConfigurationValue.Key configurationKey,
BuildConfiguration configuration,
PlatformConfiguration platformConfiguration,
- ImmutableSet<Label> execConstraintLabels,
- boolean shouldSanityCheckConfiguration)
+ ImmutableSet<Label> execConstraintLabels)
throws InterruptedException, ValueMissingException, InvalidConstraintValueException,
InvalidPlatformException {
// Determine the target and host platform keys.
@@ -237,9 +228,7 @@
// Load the host and target platforms early, to check for errors.
PlatformLookupUtil.getPlatformInfo(
- ImmutableList.of(hostPlatformKey, targetPlatformKey),
- environment,
- shouldSanityCheckConfiguration);
+ ImmutableList.of(hostPlatformKey, targetPlatformKey), environment);
if (environment.valuesMissing()) {
throw new ValueMissingException();
}
@@ -251,20 +240,18 @@
configurationKey,
configuration,
hostPlatformKey,
- execConstraintLabels,
- shouldSanityCheckConfiguration);
+ execConstraintLabels);
return PlatformKeys.create(hostPlatformKey, targetPlatformKey, executionPlatformKeys);
}
- private ImmutableList<ConfiguredTargetKey> loadExecutionPlatformKeys(
+ private static ImmutableList<ConfiguredTargetKey> loadExecutionPlatformKeys(
SkyFunction.Environment environment,
boolean debug,
BuildConfigurationValue.Key configurationKey,
BuildConfiguration configuration,
ConfiguredTargetKey defaultPlatformKey,
- ImmutableSet<Label> execConstraintLabels,
- boolean shouldSanityCheckConfiguration)
+ ImmutableSet<Label> execConstraintLabels)
throws InterruptedException, ValueMissingException, InvalidConstraintValueException,
InvalidPlatformException {
RegisteredExecutionPlatformsValue registeredExecutionPlatforms =
@@ -294,20 +281,15 @@
.collect(toImmutableList());
return filterAvailablePlatforms(
- environment,
- debug,
- availableExecutionPlatformKeys,
- execConstraintKeys,
- shouldSanityCheckConfiguration);
+ environment, debug, availableExecutionPlatformKeys, execConstraintKeys);
}
/** Returns only the platform keys that match the given constraints. */
- private ImmutableList<ConfiguredTargetKey> filterAvailablePlatforms(
+ private static ImmutableList<ConfiguredTargetKey> filterAvailablePlatforms(
SkyFunction.Environment environment,
boolean debug,
ImmutableList<ConfiguredTargetKey> platformKeys,
- ImmutableList<ConfiguredTargetKey> constraintKeys,
- boolean shouldSanityCheckConfiguration)
+ ImmutableList<ConfiguredTargetKey> constraintKeys)
throws InterruptedException, ValueMissingException, InvalidConstraintValueException,
InvalidPlatformException {
@@ -323,8 +305,7 @@
// platform is the host platform), Skyframe will return the correct results immediately without
// need of a restart.
Map<ConfiguredTargetKey, PlatformInfo> platformInfoMap =
- PlatformLookupUtil.getPlatformInfo(
- platformKeys, environment, shouldSanityCheckConfiguration);
+ PlatformLookupUtil.getPlatformInfo(platformKeys, environment);
if (platformInfoMap == null) {
throw new ValueMissingException();
}
@@ -340,7 +321,7 @@
}
/** Returns {@code true} if the given platform has all of the constraints. */
- private boolean filterPlatform(
+ private static boolean filterPlatform(
SkyFunction.Environment environment,
boolean debug,
PlatformInfo platformInfo,
@@ -365,14 +346,13 @@
return missingConstraints.isEmpty();
}
- private void determineToolchainImplementations(
+ private static void determineToolchainImplementations(
Environment environment,
BuildConfigurationValue.Key configurationKey,
ImmutableSet<Label> requiredToolchainTypeLabels,
Optional<ConfiguredTargetKey> forcedExecutionPlatform,
UnloadedToolchainContextImpl.Builder builder,
- PlatformKeys platformKeys,
- boolean shouldSanityCheckConfiguration)
+ PlatformKeys platformKeys)
throws InterruptedException, ValueMissingException, InvalidPlatformException,
NoMatchingPlatformException, UnresolvedToolchainsException,
InvalidToolchainLabelException {
@@ -452,8 +432,7 @@
Map<ConfiguredTargetKey, PlatformInfo> platforms =
PlatformLookupUtil.getPlatformInfo(
ImmutableList.of(selectedExecutionPlatformKey.get(), platformKeys.targetPlatformKey()),
- environment,
- shouldSanityCheckConfiguration);
+ environment);
if (platforms == null) {
throw new ValueMissingException();
}
@@ -521,13 +500,11 @@
return false;
}
- Map<ToolchainTypeInfo, Label> toolchains = resolvedToolchains.row(executionPlatformKey);
- if (!toolchains.keySet().containsAll(requiredToolchainTypes)) {
- // Not all toolchains are present, ignore this execution platform.
- return false;
- }
-
- return true;
+ // Unless all toolchains are present, ignore this execution platform.
+ return resolvedToolchains
+ .row(executionPlatformKey)
+ .keySet()
+ .containsAll(requiredToolchainTypes);
}
@@ -614,7 +591,7 @@
/** Used to indicate errors during the computation of an {@link UnloadedToolchainContextImpl}. */
private static final class ToolchainResolutionFunctionException extends SkyFunctionException {
- public ToolchainResolutionFunctionException(ToolchainException e) {
+ ToolchainResolutionFunctionException(ToolchainException e) {
super(e, Transience.PERSISTENT);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ToolchainTypeLookupUtil.java b/src/main/java/com/google/devtools/build/lib/skyframe/ToolchainTypeLookupUtil.java
index c77da68..df229c1 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ToolchainTypeLookupUtil.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ToolchainTypeLookupUtil.java
@@ -14,7 +14,6 @@
package com.google.devtools.build.lib.skyframe;
-import static java.util.stream.Collectors.joining;
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
@@ -37,9 +36,7 @@
@Nullable
public static ImmutableMap<Label, ToolchainTypeInfo> resolveToolchainTypes(
- Environment env,
- Iterable<ConfiguredTargetKey> toolchainTypeKeys,
- boolean sanityCheckConfiguration)
+ Environment env, Iterable<ConfiguredTargetKey> toolchainTypeKeys)
throws InterruptedException, InvalidToolchainTypeException {
Map<
SkyKey,
@@ -55,8 +52,7 @@
Map<Label, ToolchainTypeInfo> results = valuesMissing ? null : new HashMap<>();
for (ConfiguredTargetKey key : toolchainTypeKeys) {
Label originalLabel = key.getLabel();
- ToolchainTypeInfo toolchainTypeInfo =
- findToolchainTypeInfo(key, values.get(key), sanityCheckConfiguration);
+ ToolchainTypeInfo toolchainTypeInfo = findToolchainTypeInfo(key, values.get(key));
if (!valuesMissing && toolchainTypeInfo != null) {
// These are only different if the toolchain type was aliased.
results.put(originalLabel, toolchainTypeInfo);
@@ -75,8 +71,7 @@
ConfiguredTargetKey key,
ValueOrException3<
ConfiguredValueCreationException, NoSuchThingException, ActionConflictException>
- valueOrException,
- boolean sanityCheckConfiguration)
+ valueOrException)
throws InvalidToolchainTypeException {
try {
@@ -86,26 +81,6 @@
}
ConfiguredTarget configuredTarget = ctv.getConfiguredTarget();
- BuildConfigurationValue.Key configurationKey = configuredTarget.getConfigurationKey();
- // This check is necessary because trimming for other rules assumes that platform resolution
- // uses the platform fragment and _only_ the platform fragment. Without this check, it's
- // possible another fragment could slip in without us realizing, and thus break this
- // assumption.
- if (sanityCheckConfiguration && !configurationKey.getFragments().isEmpty()) {
- // No fragments may be present on a toolchain_type rule in retroactive
- // trimming mode.
- String extraFragmentDescription =
- configurationKey.getFragments().stream()
- .map(cl -> cl.getSimpleName())
- .collect(joining(","));
- throw new InvalidToolchainTypeException(
- configuredTarget.getLabel(),
- "has configuration fragments, "
- + "which is forbidden in retroactive trimming mode: "
- + "extra fragments are ["
- + extraFragmentDescription
- + "]");
- }
ToolchainTypeInfo toolchainTypeInfo = PlatformProviderUtils.toolchainType(configuredTarget);
if (toolchainTypeInfo == null) {
if (PlatformProviderUtils.declaredToolchainInfo(configuredTarget) != null) {
@@ -120,9 +95,9 @@
return toolchainTypeInfo;
} catch (ConfiguredValueCreationException e) {
- throw new InvalidToolchainTypeException(key.getLabel(), e);
+ throw new InvalidToolchainTypeException(e);
} catch (NoSuchThingException e) {
- throw new InvalidToolchainTypeException(key.getLabel(), e);
+ throw new InvalidToolchainTypeException(e);
} catch (ActionConflictException e) {
throw new InvalidToolchainTypeException(key.getLabel(), e);
}
@@ -136,12 +111,12 @@
super(formatError(label, DEFAULT_ERROR));
}
- InvalidToolchainTypeException(Label label, ConfiguredValueCreationException e) {
+ InvalidToolchainTypeException(ConfiguredValueCreationException e) {
// Just propagate the inner exception, because it's directly actionable.
super(e);
}
- public InvalidToolchainTypeException(Label label, NoSuchThingException e) {
+ public InvalidToolchainTypeException(NoSuchThingException e) {
// Just propagate the inner exception, because it's directly actionable.
super(e);
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TrimmedConfigurationProgressReceiver.java b/src/main/java/com/google/devtools/build/lib/skyframe/TrimmedConfigurationProgressReceiver.java
deleted file mode 100644
index 0480cf7..0000000
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TrimmedConfigurationProgressReceiver.java
+++ /dev/null
@@ -1,133 +0,0 @@
-// Copyright 2019 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.skyframe;
-
-import com.google.common.base.Preconditions;
-import com.google.devtools.build.lib.analysis.config.BuildOptions;
-import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.skyframe.trimming.TrimmedConfigurationCache;
-import com.google.devtools.build.skyframe.ErrorInfo;
-import com.google.devtools.build.skyframe.EvaluationProgressReceiver;
-import com.google.devtools.build.skyframe.SkyKey;
-import com.google.devtools.build.skyframe.SkyValue;
-import java.util.function.Supplier;
-import javax.annotation.Nullable;
-
-/**
- * Skyframe progress receiver which keeps a {@link TrimmedConfigurationCache} in sync with Skyframe
- * invalidations and revalidations.
- */
-public final class TrimmedConfigurationProgressReceiver implements EvaluationProgressReceiver {
-
- private final TrimmedConfigurationCache<SkyKey, Label, BuildOptions.OptionsDiffForReconstruction>
- cache;
-
- private boolean enabled = true;
-
- public TrimmedConfigurationProgressReceiver(
- TrimmedConfigurationCache<SkyKey, Label, BuildOptions.OptionsDiffForReconstruction> cache) {
- this.cache = cache;
- }
-
- public static TrimmedConfigurationCache<SkyKey, Label, BuildOptions.OptionsDiffForReconstruction>
- buildCache() {
- return new TrimmedConfigurationCache<>(
- TrimmedConfigurationProgressReceiver::extractLabel,
- TrimmedConfigurationProgressReceiver::extractOptionsDiff,
- BuildOptions.OptionsDiffForReconstruction::compareFragments);
- }
-
- public TrimmedConfigurationCache<SkyKey, Label, BuildOptions.OptionsDiffForReconstruction>
- getCache() {
- return this.cache;
- }
-
- public static Label extractLabel(SkyKey key) {
- Preconditions.checkArgument(isKeyCacheable(key));
- ConfiguredTargetKey configuredTargetKey = (ConfiguredTargetKey) key;
- return configuredTargetKey.getLabel();
- }
-
- public static BuildOptions.OptionsDiffForReconstruction extractOptionsDiff(SkyKey key) {
- Preconditions.checkArgument(isKeyCacheable(key));
- ConfiguredTargetKey configuredTargetKey = (ConfiguredTargetKey) key;
- BuildConfigurationValue.Key configurationKey = configuredTargetKey.getConfigurationKey();
- return configurationKey.getOptionsDiff();
- }
-
- public static boolean isKeyCacheable(SkyKey key) {
- if (!(key instanceof ConfiguredTargetKey)) {
- // Only configured targets go in the cache.
- // TODO(b/129286648): add aspect support
- return false;
- }
- ConfiguredTargetKey configuredTargetKey = (ConfiguredTargetKey) key;
- if (configuredTargetKey.getConfigurationKey() == null) {
- // Null-configured targets do not go in the cache.
- return false;
- }
- return true;
- }
-
- public void activate() {
- if (this.enabled) {
- return;
- }
- this.enabled = true;
- }
-
- public void deactivate() {
- if (!this.enabled) {
- return;
- }
- this.enabled = false;
- this.cache.clear();
- }
-
- @Override
- public void invalidated(SkyKey key, InvalidationState state) {
- if (!enabled || !isKeyCacheable(key)) {
- return;
- }
- switch (state) {
- case DIRTY:
- cache.invalidate(key);
- break;
- case DELETED:
- cache.remove(key);
- break;
- }
- }
-
- @Override
- public void evaluated(
- SkyKey key,
- @Nullable SkyValue newValue,
- @Nullable ErrorInfo newError,
- Supplier<EvaluationSuccessState> evaluationSuccessState,
- EvaluationState state) {
- if (!enabled || !isKeyCacheable(key)) {
- return;
- }
- switch (state) {
- case BUILT:
- // Do nothing; the evaluation would have handled putting itself (back) in the cache.
- break;
- case CLEAN:
- cache.revalidate(key);
- break;
- }
- }
-}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/trimming/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/trimming/BUILD
deleted file mode 100644
index fb13104..0000000
--- a/src/main/java/com/google/devtools/build/lib/skyframe/trimming/BUILD
+++ /dev/null
@@ -1,26 +0,0 @@
-# Classes which provide support for automatically trimming configuration to avoid wasted work during a build.
-
-load("@rules_java//java:defs.bzl", "java_library")
-
-package(
- default_visibility = ["//src:__subpackages__"],
-)
-
-filegroup(
- name = "srcs",
- srcs = glob(["**"]),
- visibility = ["//src:__subpackages__"],
-)
-
-java_library(
- name = "trimmed_configuration_cache",
- srcs = [
- "ConfigurationComparer.java",
- "KeyAndState.java",
- "TrimmedConfigurationCache.java",
- ],
- deps = [
- "//third_party:auto_value",
- "//third_party:guava",
- ],
-)
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/trimming/ConfigurationComparer.java b/src/main/java/com/google/devtools/build/lib/skyframe/trimming/ConfigurationComparer.java
deleted file mode 100644
index 91b63b4..0000000
--- a/src/main/java/com/google/devtools/build/lib/skyframe/trimming/ConfigurationComparer.java
+++ /dev/null
@@ -1,72 +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.skyframe.trimming;
-
-import java.util.function.BiFunction;
-
-/**
- * Interface describing a function which compares two configurations and determines their
- * relationship.
- */
-@FunctionalInterface
-public interface ConfigurationComparer<ConfigurationT>
- extends BiFunction<ConfigurationT, ConfigurationT, ConfigurationComparer.Result> {
- /** The outcome of comparing two configurations. */
- public enum Result {
- /**
- * All fragments in the first configuration are present and equal in the second, and vice versa.
- */
- EQUAL(true, true, true),
- /**
- * All shared fragments are equal, but the second configuration has additional fragments the
- * first does not.
- */
- SUBSET(true, false, true),
- /**
- * All shared fragments are equal, but the first configuration has additional fragments the
- * second does not.
- */
- SUPERSET(false, true, true),
- /**
- * The two configurations each have fragments the other does not, but the shared fragments are
- * equal.
- */
- ALL_SHARED_FRAGMENTS_EQUAL(false, false, true),
- /** At least one fragment shared between the two configurations is unequal. */
- DIFFERENT(false, false, false);
-
- private final boolean isSubsetOrEqual;
- private final boolean isSupersetOrEqual;
- private final boolean hasEqualSharedFragments;
-
- Result(boolean isSubsetOrEqual, boolean isSupersetOrEqual, boolean hasEqualSharedFragments) {
- this.isSubsetOrEqual = isSubsetOrEqual;
- this.isSupersetOrEqual = isSupersetOrEqual;
- this.hasEqualSharedFragments = hasEqualSharedFragments;
- }
-
- public boolean isSubsetOrEqual() {
- return isSubsetOrEqual;
- }
-
- public boolean isSupersetOrEqual() {
- return isSupersetOrEqual;
- }
-
- public boolean hasEqualSharedFragments() {
- return hasEqualSharedFragments;
- }
- }
-}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/trimming/KeyAndState.java b/src/main/java/com/google/devtools/build/lib/skyframe/trimming/KeyAndState.java
deleted file mode 100644
index b30f42c..0000000
--- a/src/main/java/com/google/devtools/build/lib/skyframe/trimming/KeyAndState.java
+++ /dev/null
@@ -1,64 +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.skyframe.trimming;
-
-import com.google.auto.value.AutoValue;
-
-/**
- * A pair of some key type and a valid/invalid state, for use in {@link TrimmedConfigurationCache}.
- */
-@AutoValue
-abstract class KeyAndState<KeyT> {
- enum State {
- VALID(true),
- POSSIBLY_INVALID(false);
-
- private final boolean isKnownValid;
-
- State(boolean isKnownValid) {
- this.isKnownValid = isKnownValid;
- }
-
- boolean isKnownValid() {
- return isKnownValid;
- }
- }
-
- abstract KeyT getKey();
-
- abstract State getState();
-
- static <KeyT> KeyAndState<KeyT> create(KeyT key) {
- return create(key, State.VALID);
- }
-
- private static <KeyT> KeyAndState<KeyT> create(KeyT key, State state) {
- return new AutoValue_KeyAndState<>(key, state);
- }
-
- KeyAndState<KeyT> asValidated() {
- if (State.VALID.equals(getState())) {
- return this;
- }
- return create(getKey(), State.VALID);
- }
-
- KeyAndState<KeyT> asInvalidated() {
- if (State.POSSIBLY_INVALID.equals(getState())) {
- return this;
- }
- return create(getKey(), State.POSSIBLY_INVALID);
- }
-}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/trimming/TrimmedConfigurationCache.java b/src/main/java/com/google/devtools/build/lib/skyframe/trimming/TrimmedConfigurationCache.java
deleted file mode 100644
index ea82f13..0000000
--- a/src/main/java/com/google/devtools/build/lib/skyframe/trimming/TrimmedConfigurationCache.java
+++ /dev/null
@@ -1,392 +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.skyframe.trimming;
-
-import com.google.common.base.Preconditions;
-import java.util.Map.Entry;
-import java.util.Optional;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.function.Function;
-import java.util.function.UnaryOperator;
-
-/**
- * Cache which tracks canonical invocations and matches keys to equivalent keys (after trimming).
- *
- * <p>This cache can be built independently of the massive build dependency that is build-base
- * (SkyFunctions and BuildConfiguration and so on), and so it is - thus, it uses type parameters to
- * speak more abstractly about what it cares about.
- *
- * <p>Consider a {@code <KeyT>} as a pair of {@code <DescriptorT>} and {@code <ConfigurationT>}. The
- * descriptor describes what the key builds, while the configuration describes how to build it.
- *
- * <p>For example, a ConfiguredTargetKey is made up of a Label, which is its descriptor, and a
- * BuildConfiguration, which is its configuration. An AspectKey is made up of a Label and a set of
- * AspectDescriptors describing the aspect and the aspects it depends on, all of which are part of
- * the AspectKey's descriptor, and also has a BuildConfiguration, which is its configuration.
- *
- * <p>A key always uses all of its descriptor, but it may only use part of its configuration. A Java
- * configured target may have no use for Python configuration, for example. Thus, it would produce
- * the same result to evaluate that target with a configuration which doesn't include Python data.
- * Reducing the configuration to the subset configuration which only includes the bits the target
- * actually needs is called trimming the configuration.
- *
- * <p>If this trimmed configuration is a subset of another configuration, then building whatever the
- * descriptor refers to with that other configuration will produce the same result as the trimmed
- * configuration, which is the same result as the configuration that the trimmed configuration was
- * trimmed from.
- *
- * <p>This cache provides methods for matching keys which would evaluate to the same result because
- * they have the same descriptor and trim to the same configuration, allowing callers to avoid doing
- * work that has already been done. It also permits invalidating, revalidating, and removing these
- * keys, as might happen during their lifecycle (if something they depend on has changed, etc.).
- *
- * <p>Internally, this cache is essentially a very sparse table. Each row, headed by a descriptor,
- * describes the possible configurations of that descriptor. Columns, headed by a trimmed
- * configuration, represent minimal configurations that descriptors can be invoked with. And a cell
- * contains the key which corresponds to the canonical invocation of that descriptor with that
- * configuration.
- *
- * <p>This class expects to be used in ways which are consistent with trimming. That is to say:
- *
- * <ul>
- * <li>If the same key is put in the cache twice with different trimmed configurations, it must be
- * invalidated between the two puts. Afterward, the original trimmed configuration is no
- * longer valid for the rest of this build.
- * <li>No trimmed configuration must be put in the cache which has equal values for every fragment
- * it shares with another trimmed configuration already in the cache, unless the key
- * associated with the other configuration has been invalidated. Afterward, the configuration
- * which had previously been invalidated is no longer valid for the rest of this build.
- * <li>Methods which read and add to the cache - {@link #get(KeyT)}, {@link #revalidate(KeyT)},
- * and {@link #putIfAbsent(KeyT, ConfigurationT)} - may be used together in one phase of the
- * build. Methods which remove from the cache - {@link #invalidate(KeyT)}, {@link
- * #remove(KeyT)}, and {@link #clear()} - may be used together in another phase of the build.
- * Calls to these groups of methods must never be interleaved.
- * </ul>
- *
- * <p>If used as described above, this class is thread-safe.
- */
-public final class TrimmedConfigurationCache<KeyT, DescriptorT, ConfigurationT> {
-
- // ======== Tuning parameters ==========
- /** The initial capacity of the cache of descriptors. */
- private static final int CACHE_INITIAL_SIZE = 100;
- /** The table density for the cache of descriptors. */
- private static final float CACHE_LOAD_FACTOR = 0.9f;
- /** The number of threads expected to be writing to the descriptor cache at a time. */
- private static final int CACHE_CONCURRENCY_LEVEL = 16;
- /**
- * The number of configurations to expect in a single descriptor - that is, the initial capacity
- * of descriptors' maps.
- */
- private static final int EXPECTED_CONFIGURATIONS_PER_DESCRIPTOR = 4;
- /**
- * The table density for the {@link ConcurrentHashMap ConcurrentHashMaps} created for tracking
- * configurations of each descriptor.
- */
- private static final float DESCRIPTOR_LOAD_FACTOR = 0.9f;
- /** The number of threads expected to be writing to a single descriptor at a time. */
- private static final int DESCRIPTOR_CONCURRENCY_LEVEL = 1;
-
- private final Function<KeyT, DescriptorT> descriptorExtractor;
- private final Function<KeyT, ConfigurationT> configurationExtractor;
-
- private final ConfigurationComparer<ConfigurationT> configurationComparer;
-
- private volatile ConcurrentHashMap<
- DescriptorT, ConcurrentHashMap<ConfigurationT, KeyAndState<KeyT>>>
- descriptors;
-
- /**
- * Constructs a new TrimmedConfigurationCache with the given methods of extracting descriptors and
- * configurations from keys, and uses the given predicate to determine the relationship between
- * two configurations.
- *
- * <p>{@code configurationComparer} should be consistent with equals - that is,
- * {@code a.equals(b) == b.equals(a) == configurationComparer.compare(a, b).equals(Result.EQUAL)}
- */
- public TrimmedConfigurationCache(
- Function<KeyT, DescriptorT> descriptorExtractor,
- Function<KeyT, ConfigurationT> configurationExtractor,
- ConfigurationComparer<ConfigurationT> configurationComparer) {
- this.descriptorExtractor = descriptorExtractor;
- this.configurationExtractor = configurationExtractor;
- this.configurationComparer = configurationComparer;
- this.descriptors = newCacheMap();
- }
-
- /**
- * Looks for a key with the same descriptor as the input key, which has a configuration that
- * trimmed to a subset of the input key's.
- *
- * <p>Note that this is not referring to a <em>proper</em> subset; it's quite possible for a key
- * to "trim" to a configuration equal to its configuration. That is, without anything being
- * removed.
- *
- * <p>If such a key has been added to this cache, it is returned in a present {@link Optional}.
- * Invoking this key will produce the same result as invoking the input key.
- *
- * <p>If no such key has been added to this cache, or if a key has been added to the cache and
- * subsequently been the subject of an {@link #invalidate(KeyT)}, an absent Optional will be
- * returned instead. No currently-valid key has trimmed to an equivalent configuration, and so the
- * input key should be executed.
- */
- public Optional<KeyT> get(KeyT input) {
- DescriptorT descriptor = getDescriptorFor(input);
- ConcurrentHashMap<ConfigurationT, KeyAndState<KeyT>> trimmingsOfDescriptor =
- descriptors.get(descriptor);
- if (trimmingsOfDescriptor == null) {
- // There are no entries at all for this descriptor.
- return Optional.empty();
- }
- ConfigurationT candidateConfiguration = getConfigurationFor(input);
- for (Entry<ConfigurationT, KeyAndState<KeyT>> entry : trimmingsOfDescriptor.entrySet()) {
- ConfigurationT trimmedConfig = entry.getKey();
- KeyAndState<KeyT> canonicalKeyAndState = entry.getValue();
- if (canSubstituteFor(candidateConfiguration, trimmedConfig, canonicalKeyAndState)) {
- return Optional.of(canonicalKeyAndState.getKey());
- }
- }
- return Optional.empty();
- }
-
- /**
- * Returns whether the given trimmed configuration and key are a suitable substitute for the
- * candidate configuration.
- */
- private boolean canSubstituteFor(
- ConfigurationT candidateConfiguration,
- ConfigurationT trimmedConfiguration,
- KeyAndState<KeyT> canonicalKeyAndState) {
- return canonicalKeyAndState.getState().isKnownValid()
- && compareConfigurations(trimmedConfiguration, candidateConfiguration).isSubsetOrEqual();
- }
-
- /**
- * Attempts to record the given key as the canonical invocation for its descriptor and the
- * passed-in trimmed configuration.
- *
- * <p>The trimmed configuration must be a subset of the input key's configuration. Otherwise,
- * {@link IllegalArgumentException} will be thrown.
- *
- * <p>If another key matching this configuration is found, that key will be returned. That key
- * represents the canonical invocation, which should produce the same result as the input key. It
- * may have been previously invalidated, but will be considered revalidated at this point.
- *
- * <p>Otherwise, if the input key is the first to trim to this configuration, the input key is
- * returned.
- */
- public KeyT putIfAbsent(KeyT canonicalKey, ConfigurationT trimmedConfiguration) {
- ConfigurationT fullConfiguration = getConfigurationFor(canonicalKey);
- Preconditions.checkArgument(
- compareConfigurations(trimmedConfiguration, fullConfiguration).isSubsetOrEqual());
- ConcurrentHashMap<ConfigurationT, KeyAndState<KeyT>> trimmingsOfDescriptor =
- descriptors.computeIfAbsent(getDescriptorFor(canonicalKey), unused -> newDescriptorMap());
- KeyAndState<KeyT> currentMapping =
- trimmingsOfDescriptor.compute(
- trimmedConfiguration,
- (configuration, currentValue) -> {
- if (currentValue == null) {
- return KeyAndState.create(canonicalKey);
- } else {
- return currentValue.asValidated();
- }
- });
- boolean newlyAdded = currentMapping.getKey().equals(canonicalKey);
- int failedRemoves;
- do {
- failedRemoves = 0;
- for (Entry<ConfigurationT, KeyAndState<KeyT>> entry : trimmingsOfDescriptor.entrySet()) {
- if (entry.getValue().getState().equals(KeyAndState.State.POSSIBLY_INVALID)) {
- // Remove invalidated keys where:
- // * the same key evaluated to a different configuration than it does now
- // * (for trimmed configurations not yet seen) the new trimmed configuration has equal
- // values for every fragment it shares with the old configuration (including subsets
- // or supersets).
- // These are keys we know will not be revalidated as part of the current build.
- // Although it also ensures that we don't remove the entry we just added, the check for
- // invalidation is mainly to avoid wasting time checking entries that are still valid for
- // the current build and therefore will not match either of these properties.
- if (entry.getValue().getKey().equals(canonicalKey)
- || (newlyAdded
- && compareConfigurations(trimmedConfiguration, entry.getKey())
- .hasEqualSharedFragments())) {
- if (!trimmingsOfDescriptor.remove(entry.getKey(), entry.getValue())) {
- // It's possible that this entry was removed by another thread in the meantime.
- failedRemoves += 1;
- }
- }
- }
- }
- } while (failedRemoves > 0);
- return currentMapping.getKey();
- }
-
- /**
- * Marks the given key as invalidated.
- *
- * <p>An invalidated key will not be returned from {@link #get(KeyT)}, as it cannot be proven that
- * the key will still trim to the same configuration.
- *
- * <p>This invalidation is undone if the input key is passed to {@link #revalidate(KeyT)}, or if
- * the configuration it originally trimmed to is passed to a call of {@link putIfAbsent(KeyT,
- * ConfigurationT)}. This is true regardless of whether the key passed to putIfAbsent is the same
- * as the input to this method.
- *
- * <p>If the key is not currently canonical for any descriptor/configuration pair, or if the key
- * had previously been invalidated and not revalidated, this method has no effect.
- */
- public void invalidate(KeyT key) {
- updateEntryWithRetries(key, KeyAndState::asInvalidated);
- }
-
- /**
- * Unmarks the given key as invalidated.
- *
- * <p>This undoes the effects of {@link #invalidate(KeyT)}, allowing the key to be returned from
- * {@link #get(KeyT)} again.
- *
- * <p>If the key is not currently canonical for any descriptor/configuration pair, or if the key
- * had not previously been invalidated or had since been revalidated, this method has no effect.
- */
- public void revalidate(KeyT key) {
- updateEntryWithRetries(key, KeyAndState::asValidated);
- }
-
- /**
- * Completely removes the given key from the cache.
- *
- * <p>After this call, {@link #get(KeyT)} and {@link #putIfAbsent(KeyT, ConfigurationT)} will no
- * longer return this key unless it is put back in the cache with putIfAbsent.
- *
- * <p>If the key is not currently canonical for any descriptor/configuration pair, this method has
- * no effect.
- */
- public void remove(KeyT key) {
- // Return null from the transformer to remove the key from the map.
- updateEntryWithRetries(key, unused -> null);
-
- DescriptorT descriptor = getDescriptorFor(key);
- ConcurrentHashMap<ConfigurationT, KeyAndState<KeyT>> trimmingsOfDescriptor =
- descriptors.get(descriptor);
- if (trimmingsOfDescriptor != null && trimmingsOfDescriptor.isEmpty()) {
- descriptors.remove(descriptor, trimmingsOfDescriptor);
- }
- }
-
- /**
- * Finds the entry in the cache where the given key is canonical and updates or removes it.
- *
- * <p>The transformation is applied transactionally; that is, if another change has happened since
- * the value was first looked up, the new value is retrieved and the transformation is applied
- * again. This repeats until there are no conflicts.
- *
- * <p>This method has no effect if this key is currently not canonical.
- *
- * @param transformation The transformation to apply to the given entry. The entry will be
- * replaced with the value returned from invoking this on the original value. If it returns
- * null, the entry will be removed instead. If it returns the same instance, nothing will be
- * done to the entry.
- */
- private void updateEntryWithRetries(KeyT key, UnaryOperator<KeyAndState<KeyT>> transformation) {
- while (!updateEntryIfNoConflicts(key, transformation)) {}
- }
-
- /**
- * Finds the entry in the cache where the given key is canonical and updates or removes it.
- *
- * <p>Only one attempt is made, and if there's a collision with another change, false is returned
- * and the map is not changed.
- *
- * <p>This method succeeds (returns {@code true}) without doing anything if this key is currently
- * not canonical.
- *
- * @param transformation The transformation to apply to the given entry. The entry will be
- * replaced with the value returned from invoking this on the original value. If it returns
- * null, the entry will be removed instead. If it returns the same instance, nothing will be
- * done to the entry.
- */
- private boolean updateEntryIfNoConflicts(
- KeyT key, UnaryOperator<KeyAndState<KeyT>> transformation) {
- DescriptorT descriptor = getDescriptorFor(key);
- ConcurrentHashMap<ConfigurationT, KeyAndState<KeyT>> trimmingsOfDescriptor =
- descriptors.get(descriptor);
- if (trimmingsOfDescriptor == null) {
- // There are no entries at all for this descriptor.
- return true;
- }
-
- for (Entry<ConfigurationT, KeyAndState<KeyT>> entry : trimmingsOfDescriptor.entrySet()) {
- KeyAndState<KeyT> currentValue = entry.getValue();
- if (currentValue.getKey().equals(key)) {
- KeyAndState<KeyT> newValue = transformation.apply(currentValue);
- if (newValue == null) {
- return trimmingsOfDescriptor.remove(entry.getKey(), currentValue);
- } else if (newValue != currentValue) {
- return trimmingsOfDescriptor.replace(entry.getKey(), currentValue, newValue);
- } else {
- // newValue == currentValue, there's nothing to do
- return true;
- }
- }
- }
- // The key requested wasn't in the map, so there's nothing to do
- return true;
- }
-
- /**
- * Removes all keys from this cache, resetting it to its empty state.
- *
- * <p>This is equivalent to calling {@link #remove(KeyT)} on every key which had ever been passed
- * to {@link #putIfAbsent(KeyT, ConfigurationT)}.
- */
- public void clear() {
- // Getting a brand new instance lets the old map be garbage collected, reducing its memory
- // footprint from its previous expansions.
- this.descriptors = newCacheMap();
- }
-
- /** Retrieves the descriptor by calling the descriptorExtractor. */
- private DescriptorT getDescriptorFor(KeyT key) {
- return descriptorExtractor.apply(key);
- }
-
- /** Retrieves the configuration by calling the configurationExtractor. */
- private ConfigurationT getConfigurationFor(KeyT key) {
- return configurationExtractor.apply(key);
- }
-
- /**
- * Checks whether the first configuration is equal to or a subset of the second by calling the
- * configurationComparer.
- */
- private ConfigurationComparer.Result compareConfigurations(
- ConfigurationT left, ConfigurationT right) {
- return configurationComparer.apply(left, right);
- }
-
- /** Generates a new map suitable for storing the cache as a whole. */
- private ConcurrentHashMap<DescriptorT, ConcurrentHashMap<ConfigurationT, KeyAndState<KeyT>>>
- newCacheMap() {
- return new ConcurrentHashMap<>(CACHE_INITIAL_SIZE, CACHE_LOAD_FACTOR, CACHE_CONCURRENCY_LEVEL);
- }
-
- /** Generates a new map suitable for storing the cache of configurations for a descriptor. */
- private ConcurrentHashMap<ConfigurationT, KeyAndState<KeyT>> newDescriptorMap() {
- return new ConcurrentHashMap<>(
- EXPECTED_CONFIGURATIONS_PER_DESCRIPTOR,
- DESCRIPTOR_LOAD_FACTOR,
- DESCRIPTOR_CONCURRENCY_LEVEL);
- }
-}
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 a7c1bef..f7c236a 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/analysis/BUILD
@@ -137,7 +137,6 @@
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/testutils",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/testutils:depsutils",
- "//src/main/java/com/google/devtools/build/lib/skyframe/trimming:trimmed_configuration_cache",
"//src/main/java/com/google/devtools/build/lib/util",
"//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code",
"//src/main/java/com/google/devtools/build/lib/util:filetype",
@@ -160,12 +159,10 @@
"//src/test/java/com/google/devtools/build/lib/packages:testutil",
"//src/test/java/com/google/devtools/build/lib/rules/platform:testutil",
"//src/test/java/com/google/devtools/build/lib/skyframe:testutil",
- "//src/test/java/com/google/devtools/build/lib/skyframe/trimming:trimmable_test_fragments",
"//src/test/java/com/google/devtools/build/lib/testutil",
"//src/test/java/com/google/devtools/build/lib/testutil:JunitUtils",
"//src/test/java/com/google/devtools/build/lib/testutil:TestConstants",
"//src/test/java/com/google/devtools/build/skyframe:testutil",
- "//third_party:auto_value",
"//third_party:guava",
"//third_party:guava-testlib",
"//third_party:jsr305",
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/config/BuildOptionsCompareFragmentsTest.java b/src/test/java/com/google/devtools/build/lib/analysis/config/BuildOptionsCompareFragmentsTest.java
deleted file mode 100644
index 3464e5f..0000000
--- a/src/test/java/com/google/devtools/build/lib/analysis/config/BuildOptionsCompareFragmentsTest.java
+++ /dev/null
@@ -1,474 +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.config;
-
-import static com.google.common.truth.Truth.assertThat;
-import static org.junit.Assert.assertThrows;
-
-import com.google.auto.value.AutoValue;
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
-import com.google.devtools.build.lib.analysis.config.BuildOptions.OptionsDiffForReconstruction;
-import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.skyframe.trimming.ConfigurationComparer;
-import com.google.devtools.build.lib.skyframe.trimming.TrimmableTestConfigurationFragments.AOptions;
-import com.google.devtools.build.lib.skyframe.trimming.TrimmableTestConfigurationFragments.BOptions;
-import java.util.Objects;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.JUnit4;
-import org.junit.runners.Parameterized;
-import org.junit.runners.Parameterized.Parameters;
-
-/** Tests of compareFragments in BuildOptions.OptionsDiffForReconstruction. */
-public final class BuildOptionsCompareFragmentsTest {
-
- /** Test cases for BuildOptionsCompareFragmentsTest. */
- @AutoValue
- public abstract static class Case {
- public abstract String getName();
-
- public abstract BuildOptions getBase();
-
- public abstract BuildOptions getLeft();
-
- public abstract BuildOptions getRight();
-
- public abstract ConfigurationComparer.Result getLeftToRightResult();
-
- public abstract ConfigurationComparer.Result getRightToLeftResult();
-
- public static Builder named(String name) {
- return new AutoValue_BuildOptionsCompareFragmentsTest_Case.Builder().setName(name);
- }
-
- /** Quick builder for test cases. */
- @AutoValue.Builder
- public abstract static class Builder {
- public abstract Builder setName(String name);
-
- public abstract Builder setBase(BuildOptions base);
-
- public Builder setBase(OptionsBuilder base) throws Exception {
- return this.setBase(base.build());
- }
-
- public abstract Builder setLeft(BuildOptions left);
-
- public Builder setLeft(OptionsBuilder left) throws Exception {
- return this.setLeft(left.build());
- }
-
- public abstract Builder setRight(BuildOptions right);
-
- public Builder setRight(OptionsBuilder right) throws Exception {
- return this.setRight(right.build());
- }
-
- public Builder setResult(ConfigurationComparer.Result result) {
- return this.setLeftToRightResult(result).setRightToLeftResult(result);
- }
-
- public abstract Builder setLeftToRightResult(ConfigurationComparer.Result result);
-
- public abstract Builder setRightToLeftResult(ConfigurationComparer.Result result);
-
- public abstract Case build();
- }
-
- @Override
- public final String toString() {
- if (Objects.equals(this.getLeftToRightResult(), this.getRightToLeftResult())) {
- return String.format("%s [result = %s]", this.getName(), this.getLeftToRightResult());
- } else {
- return String.format(
- "%s [compare(left, right) = %s; compare(right, left) = %s]",
- this.getName(), this.getLeftToRightResult(), this.getRightToLeftResult());
- }
- }
- }
-
- /** Quick builder for BuildOptions instances. */
- public static final class OptionsBuilder {
- private final ImmutableMap.Builder<Label, Object> starlarkOptions =
- new ImmutableMap.Builder<>();
- private final ImmutableList.Builder<Class<? extends FragmentOptions>> fragments =
- new ImmutableList.Builder<>();
- private final ImmutableList.Builder<String> nativeOptions = new ImmutableList.Builder<>();
-
- public OptionsBuilder withNativeFragment(
- Class<? extends FragmentOptions> fragment, String... flags) {
- this.fragments.add(fragment);
- this.nativeOptions.add(flags);
- return this;
- }
-
- public OptionsBuilder withStarlarkOption(String setting, Object value) {
- this.starlarkOptions.put(Label.parseAbsoluteUnchecked(setting), value);
- return this;
- }
-
- public OptionsBuilder withStarlarkOption(String setting) {
- return this.withStarlarkOption(setting, setting);
- }
-
- public BuildOptions build() throws Exception {
- return BuildOptions.builder()
- .addStarlarkOptions(this.starlarkOptions.build())
- .merge(
- BuildOptions.of(
- this.fragments.build(), this.nativeOptions.build().toArray(new String[0])))
- .build();
- }
- }
-
- /** Test cases for compareFragments which produce an ordinary result. */
- @RunWith(Parameterized.class)
- public static final class NormalCases {
-
- @Parameters(name = "{index}: {0}")
- public static Iterable<Case> cases() throws Exception {
- return ImmutableList.of(
- Case.named("both options equal to the base")
- .setResult(ConfigurationComparer.Result.EQUAL)
- .setBase(
- new OptionsBuilder()
- .withNativeFragment(AOptions.class)
- .withStarlarkOption("//alpha"))
- .setLeft(
- new OptionsBuilder()
- .withNativeFragment(AOptions.class)
- .withStarlarkOption("//alpha"))
- .setRight(
- new OptionsBuilder()
- .withNativeFragment(AOptions.class)
- .withStarlarkOption("//alpha"))
- .build(),
- Case.named("both sides change native fragment to same value")
- .setResult(ConfigurationComparer.Result.EQUAL)
- .setBase(new OptionsBuilder().withNativeFragment(AOptions.class, "--alpha=base"))
- .setLeft(new OptionsBuilder().withNativeFragment(AOptions.class, "--alpha=new"))
- .setRight(new OptionsBuilder().withNativeFragment(AOptions.class, "--alpha=new"))
- .build(),
- Case.named("both sides add native fragment with same value")
- .setResult(ConfigurationComparer.Result.EQUAL)
- .setBase(new OptionsBuilder())
- .setLeft(new OptionsBuilder().withNativeFragment(AOptions.class, "--alpha=new"))
- .setRight(new OptionsBuilder().withNativeFragment(AOptions.class, "--alpha=new"))
- .build(),
- Case.named("both sides remove same native fragment")
- .setResult(ConfigurationComparer.Result.EQUAL)
- .setBase(new OptionsBuilder().withNativeFragment(AOptions.class))
- .setLeft(new OptionsBuilder())
- .setRight(new OptionsBuilder())
- .build(),
- Case.named("both sides change Starlark option to same value")
- .setResult(ConfigurationComparer.Result.EQUAL)
- .setBase(new OptionsBuilder().withStarlarkOption("//alpha", "base"))
- .setLeft(new OptionsBuilder().withStarlarkOption("//alpha", "new"))
- .setRight(new OptionsBuilder().withStarlarkOption("//alpha", "new"))
- .build(),
- Case.named("both sides add Starlark option with same value")
- .setResult(ConfigurationComparer.Result.EQUAL)
- .setBase(new OptionsBuilder())
- .setLeft(new OptionsBuilder().withStarlarkOption("//alpha", "new"))
- .setRight(new OptionsBuilder().withStarlarkOption("//alpha", "new"))
- .build(),
- Case.named("both sides remove same Starlark option")
- .setResult(ConfigurationComparer.Result.EQUAL)
- .setBase(new OptionsBuilder().withStarlarkOption("//alpha"))
- .setLeft(new OptionsBuilder())
- .setRight(new OptionsBuilder())
- .build(),
- Case.named("native fragment removed from right")
- .setLeftToRightResult(ConfigurationComparer.Result.SUPERSET)
- .setRightToLeftResult(ConfigurationComparer.Result.SUBSET)
- .setBase(new OptionsBuilder().withNativeFragment(AOptions.class))
- .setLeft(new OptionsBuilder().withNativeFragment(AOptions.class))
- .setRight(new OptionsBuilder())
- .build(),
- Case.named("native fragment added to right")
- .setLeftToRightResult(ConfigurationComparer.Result.SUBSET)
- .setRightToLeftResult(ConfigurationComparer.Result.SUPERSET)
- .setBase(new OptionsBuilder())
- .setLeft(new OptionsBuilder())
- .setRight(new OptionsBuilder().withNativeFragment(AOptions.class))
- .build(),
- Case.named("native fragment changed in left and removed from right")
- .setLeftToRightResult(ConfigurationComparer.Result.SUPERSET)
- .setRightToLeftResult(ConfigurationComparer.Result.SUBSET)
- .setBase(new OptionsBuilder().withNativeFragment(AOptions.class, "--alpha=base"))
- .setLeft(new OptionsBuilder().withNativeFragment(AOptions.class, "--alpha=left"))
- .setRight(new OptionsBuilder())
- .build(),
- Case.named("native fragment added to left and another fragment removed from right")
- .setLeftToRightResult(ConfigurationComparer.Result.SUPERSET)
- .setRightToLeftResult(ConfigurationComparer.Result.SUBSET)
- .setBase(new OptionsBuilder().withNativeFragment(AOptions.class))
- .setLeft(
- new OptionsBuilder()
- .withNativeFragment(AOptions.class)
- .withNativeFragment(BOptions.class))
- .setRight(new OptionsBuilder())
- .build(),
- Case.named("Starlark option removed from right")
- .setLeftToRightResult(ConfigurationComparer.Result.SUPERSET)
- .setRightToLeftResult(ConfigurationComparer.Result.SUBSET)
- .setBase(new OptionsBuilder().withStarlarkOption("//alpha"))
- .setLeft(new OptionsBuilder().withStarlarkOption("//alpha"))
- .setRight(new OptionsBuilder())
- .build(),
- Case.named("Starlark option added to right")
- .setLeftToRightResult(ConfigurationComparer.Result.SUBSET)
- .setRightToLeftResult(ConfigurationComparer.Result.SUPERSET)
- .setBase(new OptionsBuilder())
- .setLeft(new OptionsBuilder())
- .setRight(new OptionsBuilder().withStarlarkOption("//alpha"))
- .build(),
- Case.named("Starlark option changed in left and removed from right")
- .setLeftToRightResult(ConfigurationComparer.Result.SUPERSET)
- .setRightToLeftResult(ConfigurationComparer.Result.SUBSET)
- .setBase(new OptionsBuilder().withStarlarkOption("//alpha", "base"))
- .setLeft(new OptionsBuilder().withStarlarkOption("//alpha", "left"))
- .setRight(new OptionsBuilder())
- .build(),
- Case.named("Starlark option added to left and another option removed from right")
- .setLeftToRightResult(ConfigurationComparer.Result.SUPERSET)
- .setRightToLeftResult(ConfigurationComparer.Result.SUBSET)
- .setBase(new OptionsBuilder().withStarlarkOption("//alpha"))
- .setLeft(
- new OptionsBuilder().withStarlarkOption("//alpha").withStarlarkOption("//bravo"))
- .setRight(new OptionsBuilder())
- .build(),
- Case.named("different native fragment added to each side")
- .setResult(ConfigurationComparer.Result.ALL_SHARED_FRAGMENTS_EQUAL)
- .setBase(new OptionsBuilder())
- .setLeft(new OptionsBuilder().withNativeFragment(AOptions.class))
- .setRight(new OptionsBuilder().withNativeFragment(BOptions.class))
- .build(),
- Case.named("different native fragment removed from each side")
- .setResult(ConfigurationComparer.Result.ALL_SHARED_FRAGMENTS_EQUAL)
- .setBase(
- new OptionsBuilder()
- .withNativeFragment(AOptions.class)
- .withNativeFragment(BOptions.class))
- .setLeft(new OptionsBuilder().withNativeFragment(AOptions.class))
- .setRight(new OptionsBuilder().withNativeFragment(BOptions.class))
- .build(),
- Case.named("native fragment added and different fragment removed on left")
- .setResult(ConfigurationComparer.Result.ALL_SHARED_FRAGMENTS_EQUAL)
- .setBase(new OptionsBuilder().withNativeFragment(BOptions.class))
- .setLeft(new OptionsBuilder().withNativeFragment(AOptions.class))
- .setRight(new OptionsBuilder().withNativeFragment(BOptions.class))
- .build(),
- Case.named(
- "native fragment added to right; "
- + "other fragment changed on left and removed from right")
- .setResult(ConfigurationComparer.Result.ALL_SHARED_FRAGMENTS_EQUAL)
- .setBase(new OptionsBuilder().withNativeFragment(AOptions.class, "--alpha=base"))
- .setLeft(new OptionsBuilder().withNativeFragment(AOptions.class, "--alpha=left"))
- .setRight(new OptionsBuilder().withNativeFragment(BOptions.class))
- .build(),
- Case.named("native fragment changed on each side, removed from the other")
- .setResult(ConfigurationComparer.Result.ALL_SHARED_FRAGMENTS_EQUAL)
- .setBase(
- new OptionsBuilder()
- .withNativeFragment(AOptions.class, "--alpha=base")
- .withNativeFragment(BOptions.class, "--bravo=base"))
- .setLeft(new OptionsBuilder().withNativeFragment(AOptions.class, "--alpha=left"))
- .setRight(new OptionsBuilder().withNativeFragment(BOptions.class, "--bravo=right"))
- .build(),
- Case.named(
- "native fragment changed on left, removed from right; "
- + "other fragment removed from left")
- .setResult(ConfigurationComparer.Result.ALL_SHARED_FRAGMENTS_EQUAL)
- .setBase(
- new OptionsBuilder()
- .withNativeFragment(AOptions.class, "--alpha=base")
- .withNativeFragment(BOptions.class))
- .setLeft(new OptionsBuilder().withNativeFragment(AOptions.class, "--alpha=left"))
- .setRight(new OptionsBuilder().withNativeFragment(BOptions.class))
- .build(),
- Case.named("different Starlark option added to each side")
- .setResult(ConfigurationComparer.Result.ALL_SHARED_FRAGMENTS_EQUAL)
- .setBase(new OptionsBuilder())
- .setLeft(new OptionsBuilder().withStarlarkOption("//alpha"))
- .setRight(new OptionsBuilder().withStarlarkOption("//bravo"))
- .build(),
- Case.named("different Starlark option removed from each side")
- .setResult(ConfigurationComparer.Result.ALL_SHARED_FRAGMENTS_EQUAL)
- .setBase(
- new OptionsBuilder().withStarlarkOption("//alpha").withStarlarkOption("//bravo"))
- .setLeft(new OptionsBuilder().withStarlarkOption("//alpha"))
- .setRight(new OptionsBuilder().withStarlarkOption("//bravo"))
- .build(),
- Case.named("Starlark option added and different option removed on left")
- .setResult(ConfigurationComparer.Result.ALL_SHARED_FRAGMENTS_EQUAL)
- .setBase(new OptionsBuilder().withStarlarkOption("//bravo"))
- .setLeft(new OptionsBuilder().withStarlarkOption("//alpha"))
- .setRight(new OptionsBuilder().withStarlarkOption("//bravo"))
- .build(),
- Case.named(
- "Starlark option added to right; "
- + "other option changed on left and removed from right")
- .setResult(ConfigurationComparer.Result.ALL_SHARED_FRAGMENTS_EQUAL)
- .setBase(new OptionsBuilder().withStarlarkOption("//alpha", "base"))
- .setLeft(new OptionsBuilder().withStarlarkOption("//alpha", "left"))
- .setRight(new OptionsBuilder().withStarlarkOption("//bravo"))
- .build(),
- Case.named("Starlark option changed on each side, removed from the other")
- .setResult(ConfigurationComparer.Result.ALL_SHARED_FRAGMENTS_EQUAL)
- .setBase(
- new OptionsBuilder()
- .withStarlarkOption("//alpha", "base")
- .withStarlarkOption("//bravo", "base"))
- .setLeft(new OptionsBuilder().withStarlarkOption("//alpha", "left"))
- .setRight(new OptionsBuilder().withStarlarkOption("//bravo", "right"))
- .build(),
- Case.named(
- "Starlark option changed on left, removed from right; "
- + "other option removed from left")
- .setResult(ConfigurationComparer.Result.ALL_SHARED_FRAGMENTS_EQUAL)
- .setBase(
- new OptionsBuilder()
- .withStarlarkOption("//alpha", "base")
- .withStarlarkOption("//bravo"))
- .setLeft(new OptionsBuilder().withStarlarkOption("//alpha", "left"))
- .setRight(new OptionsBuilder().withStarlarkOption("//bravo"))
- .build(),
- Case.named("Starlark option removed from left, native option removed from right")
- .setResult(ConfigurationComparer.Result.ALL_SHARED_FRAGMENTS_EQUAL)
- .setBase(
- new OptionsBuilder()
- .withNativeFragment(AOptions.class)
- .withStarlarkOption("//bravo"))
- .setLeft(new OptionsBuilder().withNativeFragment(AOptions.class))
- .setRight(new OptionsBuilder().withStarlarkOption("//bravo"))
- .build(),
- Case.named("Starlark option added to left, native option added to right")
- .setResult(ConfigurationComparer.Result.ALL_SHARED_FRAGMENTS_EQUAL)
- .setBase(new OptionsBuilder())
- .setLeft(new OptionsBuilder().withStarlarkOption("//alpha"))
- .setRight(new OptionsBuilder().withNativeFragment(BOptions.class))
- .build(),
- Case.named("native fragment is unchanged in left, changes in right")
- .setResult(ConfigurationComparer.Result.DIFFERENT)
- .setBase(new OptionsBuilder().withNativeFragment(AOptions.class, "--alpha=base"))
- .setLeft(new OptionsBuilder().withNativeFragment(AOptions.class, "--alpha=base"))
- .setRight(new OptionsBuilder().withNativeFragment(AOptions.class, "--alpha=right"))
- .build(),
- Case.named("native fragment is changed to different values")
- .setResult(ConfigurationComparer.Result.DIFFERENT)
- .setBase(new OptionsBuilder().withNativeFragment(AOptions.class, "--alpha=base"))
- .setLeft(new OptionsBuilder().withNativeFragment(AOptions.class, "--alpha=left"))
- .setRight(new OptionsBuilder().withNativeFragment(AOptions.class, "--alpha=right"))
- .build(),
- Case.named("native fragment is added with different values")
- .setResult(ConfigurationComparer.Result.DIFFERENT)
- .setBase(new OptionsBuilder())
- .setLeft(new OptionsBuilder().withNativeFragment(AOptions.class, "--alpha=left"))
- .setRight(new OptionsBuilder().withNativeFragment(AOptions.class, "--alpha=right"))
- .build(),
- Case.named("Starlark option is unchanged in left, changes in right")
- .setResult(ConfigurationComparer.Result.DIFFERENT)
- .setBase(new OptionsBuilder().withStarlarkOption("//alpha", "base"))
- .setLeft(new OptionsBuilder().withStarlarkOption("//alpha", "base"))
- .setRight(new OptionsBuilder().withStarlarkOption("//alpha", "right"))
- .build(),
- Case.named("Starlark option is changed to different values")
- .setResult(ConfigurationComparer.Result.DIFFERENT)
- .setBase(new OptionsBuilder().withStarlarkOption("//alpha", "base"))
- .setLeft(new OptionsBuilder().withStarlarkOption("//alpha", "left"))
- .setRight(new OptionsBuilder().withStarlarkOption("//alpha", "right"))
- .build(),
- Case.named("Starlark option is added with different values")
- .setResult(ConfigurationComparer.Result.DIFFERENT)
- .setBase(new OptionsBuilder())
- .setLeft(new OptionsBuilder().withStarlarkOption("//alpha", "left"))
- .setRight(new OptionsBuilder().withStarlarkOption("//alpha", "right"))
- .build());
- }
-
- private final Case testCase;
-
- public NormalCases(Case testCase) {
- this.testCase = testCase;
- }
-
- @Test
- public void compareLeftToRight() throws Exception {
- OptionsDiffForReconstruction diffLeft =
- BuildOptions.diffForReconstruction(testCase.getBase(), testCase.getLeft());
- OptionsDiffForReconstruction diffRight =
- BuildOptions.diffForReconstruction(testCase.getBase(), testCase.getRight());
-
- assertThat(OptionsDiffForReconstruction.compareFragments(diffLeft, diffRight))
- .isEqualTo(testCase.getLeftToRightResult());
- }
-
- @Test
- public void compareRightToLeft() throws Exception {
- OptionsDiffForReconstruction diffLeft =
- BuildOptions.diffForReconstruction(testCase.getBase(), testCase.getLeft());
- OptionsDiffForReconstruction diffRight =
- BuildOptions.diffForReconstruction(testCase.getBase(), testCase.getRight());
-
- assertThat(OptionsDiffForReconstruction.compareFragments(diffRight, diffLeft))
- .isEqualTo(testCase.getRightToLeftResult());
- }
- }
-
- /** Test cases for compareFragments which produce errors. */
- @RunWith(JUnit4.class)
- public static final class ExceptionalCases {
- @Test
- public void withDifferentBases_throwsError() throws Exception {
- BuildOptions baseA =
- new OptionsBuilder()
- .withNativeFragment(AOptions.class, "--alpha=A")
- .withNativeFragment(BOptions.class, "--bravo=base")
- .build();
- BuildOptions newA =
- new OptionsBuilder()
- .withNativeFragment(AOptions.class, "--alpha=A")
- .withNativeFragment(BOptions.class, "--bravo=new")
- .build();
- BuildOptions baseB =
- new OptionsBuilder()
- .withNativeFragment(AOptions.class, "--alpha=B")
- .withNativeFragment(BOptions.class, "--bravo=base")
- .build();
- BuildOptions newB =
- new OptionsBuilder()
- .withNativeFragment(AOptions.class, "--alpha=B")
- .withNativeFragment(BOptions.class, "--bravo=old")
- .build();
-
- OptionsDiffForReconstruction diffA = BuildOptions.diffForReconstruction(baseA, newA);
- OptionsDiffForReconstruction diffB = BuildOptions.diffForReconstruction(baseB, newB);
-
- IllegalArgumentException forwardException =
- assertThrows(
- IllegalArgumentException.class,
- () -> OptionsDiffForReconstruction.compareFragments(diffA, diffB));
- assertThat(forwardException).hasMessageThat().contains("diffs with different bases");
-
- IllegalArgumentException reverseException =
- assertThrows(
- IllegalArgumentException.class,
- () -> OptionsDiffForReconstruction.compareFragments(diffB, diffA));
- assertThat(reverseException).hasMessageThat().contains("diffs with different bases");
- }
- }
-}
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java
index 3f8b306..293a0b7 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java
@@ -18,7 +18,6 @@
import static com.google.common.collect.ImmutableMap.toImmutableMap;
import static com.google.common.collect.Streams.stream;
-import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Functions;
import com.google.common.base.Preconditions;
import com.google.common.collect.Collections2;
@@ -32,7 +31,6 @@
import com.google.common.collect.Maps;
import com.google.common.collect.Multimap;
import com.google.common.collect.Sets;
-import com.google.common.collect.Streams;
import com.google.common.eventbus.EventBus;
import com.google.devtools.build.lib.actions.ActionLookupKey;
import com.google.devtools.build.lib.actions.ArtifactFactory;
@@ -160,7 +158,6 @@
this.skyframeBuildView = skyframeExecutor.getSkyframeBuildView();
}
- @VisibleForTesting
public Set<ActionLookupKey> getSkyframeEvaluatedActionLookupKeyCountForTesting() {
Set<ActionLookupKey> actionLookupKeys = populateActionLookupKeyMapAndGetDiff();
Preconditions.checkState(
@@ -188,7 +185,6 @@
/**
* Returns whether the given configured target has errors.
*/
- @VisibleForTesting
public boolean hasErrors(ConfiguredTarget configuredTarget) {
return configuredTarget == null;
}
@@ -223,8 +219,7 @@
eventBus);
}
- /** Sets the configurations. Not thread-safe. DO NOT CALL except from tests! */
- @VisibleForTesting
+ /** Sets the configurations. Not thread-safe. */
public void setConfigurationsForTesting(
EventHandler eventHandler, BuildConfigurationCollection configurations) {
skyframeBuildView.setConfigurations(
@@ -242,7 +237,6 @@
* the fragments needed by the fragment and its transitive closure. Else unconditionally includes
* all fragments.
*/
- @VisibleForTesting
public BuildConfiguration getConfigurationForTesting(
Target target, BuildConfiguration config, ExtendedEventHandler eventHandler)
throws InvalidConfigurationException, InterruptedException {
@@ -262,22 +256,19 @@
* Sets the possible artifact roots in the artifact factory. This allows the factory to resolve
* paths with unknown roots to artifacts.
*/
- @VisibleForTesting // for BuildViewTestCase
public void setArtifactRoots(PackageRoots packageRoots) {
getArtifactFactory().setPackageRoots(packageRoots.getPackageRootLookup());
}
- @VisibleForTesting
// TODO(janakr): pass the configuration in as a parameter here.
public Collection<ConfiguredTarget> getDirectPrerequisitesForTesting(
ExtendedEventHandler eventHandler,
ConfiguredTarget ct,
BuildConfigurationCollection configurations)
- throws DependencyResolver.Failure, InvalidConfigurationException, InterruptedException,
+ throws DependencyResolver.Failure, InvalidConfigurationException,
InconsistentAspectOrderException, StarlarkTransition.TransitionException {
return Collections2.transform(
- getConfiguredTargetAndDataDirectPrerequisitesForTesting(
- eventHandler, ct, ct.getConfigurationKey(), configurations),
+ getConfiguredTargetAndDataDirectPrerequisitesForTesting(eventHandler, ct, configurations),
ConfiguredTargetAndData::getConfiguredTarget);
}
@@ -285,9 +276,8 @@
getConfiguredTargetAndDataDirectPrerequisitesForTesting(
ExtendedEventHandler eventHandler,
ConfiguredTarget ct,
- BuildConfigurationValue.Key configuration,
BuildConfigurationCollection configurations)
- throws DependencyResolver.Failure, InvalidConfigurationException, InterruptedException,
+ throws DependencyResolver.Failure, InvalidConfigurationException,
InconsistentAspectOrderException, StarlarkTransition.TransitionException {
SkyframeExecutorWrappingWalkableGraph walkableGraph =
@@ -313,23 +303,18 @@
walkableGraph.getDirectDeps(
ConfiguredTargetKey.builder().setConfiguredTarget(ct).build());
- // Turn the keys back into ConfiguredTarget instances, possibly merging in aspects that
- // were propagated from the original target.
- Collection<ConfiguredTargetAndData> cts =
- Streams.stream(directPrerequisites)
- .filter(dep -> dep instanceof ConfiguredTargetKey)
- .map(dep -> (ConfiguredTargetKey) dep)
- .map(configuredTargetKey -> getConfiguredTarget(walkableGraph, configuredTargetKey))
- // For each configured target, add in any aspects from depNodeNames.
- .map(
- configuredTarget ->
- mergeAspects(
- walkableGraph,
- configuredTarget,
- findDependencyKey(dependencyKeys, configuredTarget)))
- .collect(toImmutableList());
-
- return cts;
+ // Turn the keys back into ConfiguredTarget instances, possibly merging in aspects that were
+ // propagated from the original target.
+ return stream(Iterables.filter(directPrerequisites, ConfiguredTargetKey.class))
+ .map(configuredTargetKey -> getConfiguredTarget(walkableGraph, configuredTargetKey))
+ // For each configured target, add in any aspects from depNodeNames.
+ .map(
+ configuredTarget ->
+ mergeAspects(
+ walkableGraph,
+ configuredTarget,
+ findDependencyKey(dependencyKeys, configuredTarget)))
+ .collect(toImmutableList());
} catch (InterruptedException e) {
return ImmutableList.of();
}
@@ -363,7 +348,7 @@
}
@Nullable
- private DependencyKey findDependencyKey(
+ private static DependencyKey findDependencyKey(
Multimap<Label, DependencyKey> dependencyKeys, ConfiguredTargetAndData configuredTarget) {
// TODO(blaze-configurability): Figure out how to map the ConfiguredTarget back to the correct
// DependencyKey when there are more than one.
@@ -371,7 +356,7 @@
}
// Helper method to find the aspects needed for a target and merge them.
- protected ConfiguredTargetAndData mergeAspects(
+ protected static ConfiguredTargetAndData mergeAspects(
WalkableGraph graph, ConfiguredTargetAndData ctd, @Nullable DependencyKey dependencyKey) {
if (dependencyKey == null || dependencyKey.getAspects().getUsedAspects().isEmpty()) {
return ctd;
@@ -400,7 +385,6 @@
}
}
- @VisibleForTesting
public OrderedSetMultimap<DependencyKind, DependencyKey>
getDirectPrerequisiteDependenciesForTesting(
final ExtendedEventHandler eventHandler,
@@ -410,7 +394,7 @@
throws DependencyResolver.Failure, InterruptedException, InconsistentAspectOrderException,
StarlarkTransition.TransitionException, InvalidConfigurationException {
- Target target = null;
+ Target target;
try {
target = skyframeExecutor.getPackageManager().getTarget(eventHandler, ct.getLabel());
} catch (NoSuchPackageException | NoSuchTargetException | InterruptedException e) {
@@ -558,7 +542,6 @@
*
* <p>Returns {@code null} if something goes wrong.
*/
- @VisibleForTesting
public ConfiguredTarget getConfiguredTargetForTesting(
ExtendedEventHandler eventHandler, Label label, BuildConfiguration config)
throws StarlarkTransition.TransitionException, InvalidConfigurationException,
@@ -571,7 +554,6 @@
return skyframeExecutor.getConfiguredTargetForTesting(eventHandler, label, config, transition);
}
- @VisibleForTesting
ConfiguredTargetAndData getConfiguredTargetAndDataForTesting(
ExtendedEventHandler eventHandler, Label label, BuildConfiguration config)
throws StarlarkTransition.TransitionException, InvalidConfigurationException,
@@ -588,7 +570,6 @@
/**
* Returns a RuleContext which is the same as the original RuleContext of the target parameter.
*/
- @VisibleForTesting
public RuleContext getRuleContextForTesting(
ConfiguredTarget target,
StoredEventHandler eventHandler,
@@ -611,7 +592,6 @@
.setLabel(target.getLabel())
.setConfiguration(targetConfig)
.build(),
- /*isSystemEnv=*/ false,
targetConfig.extendedSanityChecks(),
targetConfig.allowAnalysisFailures(),
eventHandler,
@@ -624,7 +604,6 @@
* Creates and returns a rule context that is equivalent to the one that was used to create the
* given configured target.
*/
- @VisibleForTesting
public RuleContext getRuleContextForTesting(
ExtendedEventHandler eventHandler,
ConfiguredTarget configuredTarget,
@@ -635,7 +614,7 @@
StarlarkTransition.TransitionException, InvalidExecGroupException {
BuildConfiguration targetConfig =
skyframeExecutor.getConfiguration(eventHandler, configuredTarget.getConfigurationKey());
- Target target = null;
+ Target target;
try {
target =
skyframeExecutor.getPackageManager().getTarget(eventHandler, configuredTarget.getLabel());
@@ -740,7 +719,6 @@
}
/** Clears the analysis cache as in --discard_analysis_cache. */
- @VisibleForTesting
void clearAnalysisCache(
Collection<ConfiguredTarget> topLevelTargets, ImmutableSet<AspectKey> topLevelAspects) {
skyframeBuildView.clearAnalysisCache(topLevelTargets, topLevelAspects);
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 b1da3f9..aac74b6 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
@@ -338,9 +338,9 @@
packageOptions,
buildLanguageOptions,
UUID.randomUUID(),
- ImmutableMap.<String, String>of(),
+ ImmutableMap.of(),
tsgm);
- skyframeExecutor.setActionEnv(ImmutableMap.<String, String>of());
+ skyframeExecutor.setActionEnv(ImmutableMap.of());
useConfiguration();
setUpSkyframe();
this.actionLogBufferPathGenerator =
@@ -411,7 +411,7 @@
}
protected Iterable<EnvironmentExtension> getEnvironmentExtensions() {
- return ImmutableList.<EnvironmentExtension>of();
+ return ImmutableList.of();
}
protected StarlarkSemantics getStarlarkSemantics() {
@@ -452,8 +452,7 @@
optionsParser.setStarlarkOptions(starlarkOptions);
BuildOptions buildOptions = ruleClassProvider.createBuildOptions(optionsParser);
- return skyframeExecutor.createConfigurations(
- reporter, buildOptions, ImmutableSet.<String>of(), false);
+ return skyframeExecutor.createConfigurations(reporter, buildOptions, ImmutableSet.of(), false);
}
protected Target getTarget(String label)
@@ -559,8 +558,6 @@
* Invalidates all existing packages. Optionally invalidates configurations too.
*
* <p>Tests should invalidate both unless they have specific reason not to.
- *
- * @throws InterruptedException
*/
protected void invalidatePackages(boolean alsoConfigs) throws InterruptedException {
skyframeExecutor.invalidateFilesUnderPathForTesting(
@@ -605,7 +602,6 @@
* @param starlarkOptions map of Starlark-defined options where the keys are option names (in the
* form of label-like strings) and the values are option values
* @param args native option name/pair descriptions in command line form (e.g. "--cpu=k8")
- * @throws IllegalArgumentException
*/
protected void useConfiguration(ImmutableMap<String, Object> starlarkOptions, String... args)
throws Exception {
@@ -624,11 +620,10 @@
}
/**
- * Creates BuildView using current hostConfig/targetConfig values.
- * Ensures that hostConfig is either identical to the targetConfig or has
- * 'host' short name.
+ * Creates BuildView using current hostConfig/targetConfig values. Ensures that hostConfig is
+ * either identical to the targetConfig or has 'host' short name.
*/
- protected final void createBuildView() throws Exception {
+ protected final void createBuildView() {
Preconditions.checkNotNull(masterConfig);
Preconditions.checkState(getHostConfiguration().equals(getTargetConfiguration())
|| getHostConfiguration().isHostConfiguration(),
@@ -664,7 +659,6 @@
return null;
}
},
- /*isSystemEnv=*/ true,
/*extendedSanityChecks=*/ false,
/*allowAnalysisFailures=*/ false,
reporter,
@@ -680,7 +674,7 @@
*/
protected final Collection<ConfiguredTarget> getDirectPrerequisites(ConfiguredTarget target)
throws TransitionException, InvalidConfigurationException, InconsistentAspectOrderException,
- Failure, InterruptedException {
+ Failure {
return view.getDirectPrerequisitesForTesting(reporter, target, masterConfig);
}
@@ -703,7 +697,6 @@
view.getConfiguredTargetAndDataDirectPrerequisitesForTesting(
reporter,
ctad.getConfiguredTarget(),
- ctad.getConfiguredTarget().getConfigurationKey(),
masterConfig)) {
if (candidate.getConfiguredTarget().getLabel().equals(candidateLabel)) {
return candidate;
@@ -733,10 +726,10 @@
* comparison.
*
* <p>Generally, this means they share the same checksum, which is computed by iterating over all
- * the individual @Option annotated values contained within the {@link FragmentOption} classes
+ * the individual @Option annotated values contained within the {@link FragmentOptions} classes
* contained within the {@link BuildOptions} inside the given configurations.
*/
- protected void assertConfigurationsEqual(
+ protected static void assertConfigurationsEqual(
BuildConfiguration config1,
BuildConfiguration config2,
Set<Class<? extends FragmentOptions>> excludeFragmentOptions) {
@@ -747,8 +740,9 @@
.isEqualTo(trimConfiguration(config1.cloneOptions(), excludeFragmentOptions));
}
- protected void assertConfigurationsEqual(BuildConfiguration config1, BuildConfiguration config2) {
- assertConfigurationsEqual(config1, config2, ImmutableSet.of());
+ protected static void assertConfigurationsEqual(
+ BuildConfiguration config1, BuildConfiguration config2) {
+ assertConfigurationsEqual(config1, config2, /*excludeFragmentOptions=*/ ImmutableSet.of());
}
/**
@@ -1099,11 +1093,9 @@
* @param ruleName the name of the rule.
* @param lines the text of the rule.
* @return the configured target instance for the created rule.
- * @throws IOException
- * @throws Exception
*/
protected ConfiguredTarget scratchConfiguredTarget(
- String packageName, String ruleName, String... lines) throws IOException, Exception {
+ String packageName, String ruleName, String... lines) throws Exception {
return scratchConfiguredTarget(packageName, ruleName, targetConfig, lines);
}
@@ -1115,12 +1107,10 @@
* @param config the configuration to use to construct the configured rule.
* @param lines the text of the rule.
* @return the configured target instance for the created rule.
- * @throws IOException
- * @throws Exception
*/
protected ConfiguredTarget scratchConfiguredTarget(
String packageName, String ruleName, BuildConfiguration config, String... lines)
- throws IOException, Exception {
+ throws Exception {
ConfiguredTargetAndData ctad =
scratchConfiguredTargetAndData(packageName, ruleName, config, lines);
return ctad == null ? null : ctad.getConfiguredTarget();
@@ -1133,7 +1123,6 @@
* @param rulename the name of the rule.
* @param lines the text of the rule.
* @return the configured tatarget and target instance for the created rule.
- * @throws Exception
*/
protected ConfiguredTargetAndData scratchConfiguredTargetAndData(
String packageName, String rulename, String... lines) throws Exception {
@@ -1148,8 +1137,6 @@
* @param config the configuration to use to construct the configured rule.
* @param lines the text of the rule.
* @return the ConfiguredTargetAndData instance for the created rule.
- * @throws IOException
- * @throws Exception
*/
protected ConfiguredTargetAndData scratchConfiguredTargetAndData(
String packageName, String ruleName, BuildConfiguration config, String... lines)
@@ -1165,8 +1152,6 @@
* @param ruleName the name of the rule.
* @param lines the text of the rule.
* @return the rule instance for the created rule.
- * @throws IOException
- * @throws Exception
*/
protected Rule scratchRule(String packageName, String ruleName, String... lines)
throws Exception {
@@ -1427,7 +1412,7 @@
}
/** Returns the input {@link Artifact}s to the given {@link Action} with the given exec paths. */
- protected List<Artifact> getInputs(Action owner, Collection<String> execPaths) {
+ protected static List<Artifact> getInputs(Action owner, Collection<String> execPaths) {
Set<String> expectedPaths = new HashSet<>(execPaths);
List<Artifact> result = new ArrayList<>();
for (Artifact output : owner.getInputs().toList()) {
@@ -1664,11 +1649,11 @@
: outputPath;
}
- protected String fileName(Artifact artifact) {
+ protected static String fileName(Artifact artifact) {
return artifact.getExecPathString();
}
- protected String fileName(FileConfiguredTarget target) {
+ protected static String fileName(FileConfiguredTarget target) {
return fileName(target.getArtifact());
}
@@ -1722,8 +1707,9 @@
"'" + packageName + ":a.foo' does not produce any " + descriptionPlural);
}
- protected void assertSrcsValidity(String ruleType, String targetName, boolean expectedError,
- String... expectedMessages) throws Exception{
+ protected void assertSrcsValidity(
+ String ruleType, String targetName, boolean expectedError, String... expectedMessages)
+ throws Exception {
ConfiguredTarget target = getConfiguredTarget(targetName);
if (expectedError) {
assertThat(view.hasErrors(target)).isTrue();
@@ -1757,10 +1743,10 @@
.build();
}
- protected static List<String> actionInputsToPaths(NestedSet<? extends ActionInput> actionInputs) {
+ protected static ImmutableList<String> actionInputsToPaths(
+ NestedSet<? extends ActionInput> actionInputs) {
return ImmutableList.copyOf(
- Iterables.transform(
- actionInputs.toList(), (actionInput) -> actionInput.getExecPathString()));
+ Lists.transform(actionInputs.toList(), ActionInput::getExecPathString));
}
/**
@@ -1774,20 +1760,19 @@
}
/**
- * Utility method for asserting that a list contains the elements of a
- * sublist. This is useful for checking that a list of arguments contains a
- * particular set of arguments.
+ * Utility method for asserting that a list contains the elements of a sublist. This is useful for
+ * checking that a list of arguments contains a particular set of arguments.
*/
- protected void assertContainsSublist(List<String> list, List<String> sublist) {
+ protected static void assertContainsSublist(List<String> list, List<String> sublist) {
assertContainsSublist(null, list, sublist);
}
/**
- * Utility method for asserting that a list contains the elements of a
- * sublist. This is useful for checking that a list of arguments contains a
- * particular set of arguments.
+ * Utility method for asserting that a list contains the elements of a sublist. This is useful for
+ * checking that a list of arguments contains a particular set of arguments.
*/
- protected void assertContainsSublist(String message, List<String> list, List<String> sublist) {
+ protected static void assertContainsSublist(
+ String message, List<String> list, List<String> sublist) {
if (Collections.indexOfSubList(list, sublist) == -1) {
fail((message == null ? "" : (message + ' '))
+ "expected: <" + list + "> to contain sublist: <" + sublist + ">");
@@ -1795,10 +1780,10 @@
}
protected void assertContainsSelfEdgeEvent(String label) {
- assertContainsEvent(Pattern.compile(label + " \\([a-f0-9]+\\) \\[self-edge\\]"));
+ assertContainsEvent(Pattern.compile(label + " \\([a-f0-9]+\\) \\[self-edge]"));
}
- protected NestedSet<Artifact> collectRunfiles(ConfiguredTarget target) {
+ protected static NestedSet<Artifact> collectRunfiles(ConfiguredTarget target) {
RunfilesProvider runfilesProvider = target.getProvider(RunfilesProvider.class);
if (runfilesProvider != null) {
return runfilesProvider.getDefaultRunfiles().getAllArtifacts();
@@ -1807,7 +1792,7 @@
}
}
- protected NestedSet<Artifact> getFilesToBuild(TransitiveInfoCollection target) {
+ protected static NestedSet<Artifact> getFilesToBuild(TransitiveInfoCollection target) {
return target.getProvider(FileProvider.class).getFilesToBuild();
}
@@ -1851,15 +1836,16 @@
return ImmutableList.copyOf(result);
}
- protected NestedSet<Artifact> getOutputGroup(
+ protected static NestedSet<Artifact> getOutputGroup(
TransitiveInfoCollection target, String outputGroup) {
OutputGroupInfo provider = OutputGroupInfo.get(target);
return provider == null
- ? NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER)
+ ? NestedSetBuilder.emptySet(Order.STABLE_ORDER)
: provider.getOutputGroup(outputGroup);
}
- protected NestedSet<Artifact.DerivedArtifact> getExtraActionArtifacts(ConfiguredTarget target) {
+ protected static NestedSet<Artifact.DerivedArtifact> getExtraActionArtifacts(
+ ConfiguredTarget target) {
return target.getProvider(ExtraActionArtifactsProvider.class).getExtraActionArtifacts();
}
@@ -1867,15 +1853,15 @@
return getConfiguredTarget(label).getProvider(FilesToRunProvider.class).getExecutable();
}
- protected Artifact getExecutable(TransitiveInfoCollection target) {
+ protected static Artifact getExecutable(TransitiveInfoCollection target) {
return target.getProvider(FilesToRunProvider.class).getExecutable();
}
- protected NestedSet<Artifact> getFilesToRun(TransitiveInfoCollection target) {
+ protected static NestedSet<Artifact> getFilesToRun(TransitiveInfoCollection target) {
return target.getProvider(FilesToRunProvider.class).getFilesToRun();
}
- protected NestedSet<Artifact> getFilesToRun(Label label) throws Exception {
+ protected NestedSet<Artifact> getFilesToRun(Label label) {
return getConfiguredTarget(label, targetConfig)
.getProvider(FilesToRunProvider.class).getFilesToRun();
}
@@ -1888,7 +1874,7 @@
return getConfiguredTarget(label).getProvider(FilesToRunProvider.class).getRunfilesSupport();
}
- protected RunfilesSupport getRunfilesSupport(TransitiveInfoCollection target) {
+ protected static RunfilesSupport getRunfilesSupport(TransitiveInfoCollection target) {
return target.getProvider(FilesToRunProvider.class).getRunfilesSupport();
}
@@ -1984,7 +1970,7 @@
boolean doAnalysis,
EventBus eventBus) throws Exception {
return update(
- targets, ImmutableList.<String>of(), keepGoing, loadingPhaseThreads, doAnalysis, eventBus);
+ targets, ImmutableList.of(), keepGoing, loadingPhaseThreads, doAnalysis, eventBus);
}
protected AnalysisResult update(
@@ -2058,51 +2044,53 @@
return result;
}
- protected String getErrorMsgNoGoodFiles(String attrName, String ruleType, String ruleName,
- String depRuleName) {
+ protected static String getErrorMsgNoGoodFiles(
+ String attrName, String ruleType, String ruleName, String depRuleName) {
return "in " + attrName + " attribute of " + ruleType + " rule " + ruleName + ": '"
+ depRuleName + "' does not produce any " + ruleType + " " + attrName + " files";
}
- protected String getErrorMsgMisplacedFiles(String attrName, String ruleType, String ruleName,
- String fileName) {
+ protected static String getErrorMsgMisplacedFiles(
+ String attrName, String ruleType, String ruleName, String fileName) {
return "in " + attrName + " attribute of " + ruleType + " rule " + ruleName + ": source file '"
+ fileName + "' is misplaced here";
}
- protected String getErrorNonExistingTarget(String attrName, String ruleType, String ruleName,
- String targetName) {
+ protected static String getErrorNonExistingTarget(
+ String attrName, String ruleType, String ruleName, String targetName) {
return "in " + attrName + " attribute of " + ruleType + " rule " + ruleName + ": target '"
+ targetName + "' does not exist";
}
- protected String getErrorNonExistingRule(String attrName, String ruleType, String ruleName,
- String targetName) {
+ protected static String getErrorNonExistingRule(
+ String attrName, String ruleType, String ruleName, String targetName) {
return "in " + attrName + " attribute of " + ruleType + " rule " + ruleName + ": rule '"
+ targetName + "' does not exist";
}
- protected String getErrorMsgMisplacedRules(String attrName, String ruleType, String ruleName,
- String depRuleType, String depRuleName) {
+ protected static String getErrorMsgMisplacedRules(
+ String attrName, String ruleType, String ruleName, String depRuleType, String depRuleName) {
return "in " + attrName + " attribute of " + ruleType + " rule " + ruleName + ": "
+ depRuleType + " rule '" + depRuleName + "' is misplaced here";
}
- protected String getErrorMsgNonEmptyList(String attrName, String ruleType, String ruleName) {
+ protected static String getErrorMsgNonEmptyList(
+ String attrName, String ruleType, String ruleName) {
return "in " + attrName + " attribute of " + ruleType + " rule " + ruleName + ": attribute "
+ "must be non empty";
}
- protected String getErrorMsgMandatoryMissing(String attrName, String ruleType) {
+ protected static String getErrorMsgMandatoryMissing(String attrName, String ruleType) {
return "missing value for mandatory attribute '" + attrName + "' in '" + ruleType + "' rule";
}
- protected String getErrorMsgWrongAttributeValue(String value, String... expected) {
+ protected static String getErrorMsgWrongAttributeValue(String value, String... expected) {
return String.format("has to be one of %s instead of '%s'",
StringUtil.joinEnglishList(ImmutableSet.copyOf(expected), "or", "'"), value);
}
- protected String getErrorMsgMandatoryProviderMissing(String offendingRule, String providerName) {
+ protected static String getErrorMsgMandatoryProviderMissing(
+ String offendingRule, String providerName) {
return String.format("'%s' does not have mandatory providers: '%s'",
offendingRule, providerName);
}
@@ -2122,7 +2110,7 @@
String packageName, String... lines) throws Exception {
eventCollector.clear();
reporter.removeHandler(failFastHandler);
- scratch.file("" + packageName + "/BUILD", lines);
+ scratch.file(packageName + "/BUILD", lines);
return getPackageManager()
.getPackage(reporter, PackageIdentifier.createInMainRepo(packageName));
}
@@ -2193,7 +2181,7 @@
}
@Override
- public ImmutableMap<String, Object> getStarlarkDefinedBuiltins() throws InterruptedException {
+ public ImmutableMap<String, Object> getStarlarkDefinedBuiltins() {
throw new UnsupportedOperationException();
}
@@ -2427,7 +2415,7 @@
/** Creates instances of {@link ActionExecutionContext} consistent with test case. */
public class ActionExecutionContextBuilder {
private MetadataProvider actionInputFileCache = null;
- private TreeMap<String, String> clientEnv = new TreeMap<>();
+ private final TreeMap<String, String> clientEnv = new TreeMap<>();
private ArtifactExpander artifactExpander = null;
private Executor executor = new DummyExecutor(fileSystem, getExecRoot());
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/CompileOnlyTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/CompileOnlyTestCase.java
index a3c196d..84f760c 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/CompileOnlyTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/CompileOnlyTestCase.java
@@ -22,7 +22,7 @@
*/
public abstract class CompileOnlyTestCase extends BuildViewTestCase {
- protected Artifact getArtifactByExecPathSuffix(ConfiguredTarget target, String path) {
+ protected static Artifact getArtifactByExecPathSuffix(ConfiguredTarget target, String path) {
for (Artifact artifact : getOutputGroup(target, OutputGroupInfo.FILES_TO_COMPILE).toList()) {
if (artifact.getExecPathString().endsWith(path)) {
return artifact;
diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/ResourceTestBase.java b/src/test/java/com/google/devtools/build/lib/rules/android/ResourceTestBase.java
index 2fe0e5d..d81d1fe 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/android/ResourceTestBase.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/android/ResourceTestBase.java
@@ -210,10 +210,6 @@
return getArtifact(RESOURCE_ROOT, pathString);
}
- Artifact getOutput(String pathString) {
- return getArtifact("outputs", pathString);
- }
-
private Artifact getArtifact(String subdir, String pathString) {
Path path = fileSystem.getPath("/" + subdir + "/" + pathString);
return new Artifact.SourceArtifact(
@@ -244,7 +240,6 @@
.setLabel(dummyTarget.getLabel())
.setConfiguration(targetConfig)
.build(),
- /*isSystemEnv=*/ false,
targetConfig.extendedSanityChecks(),
targetConfig.allowAnalysisFailures(),
eventHandler,
@@ -262,7 +257,7 @@
/**
* Assets that the action used to generate the given outputs has the expected inputs and outputs.
*/
- void assertActionArtifacts(
+ static void assertActionArtifacts(
RuleContext ruleContext, ImmutableList<Artifact> inputs, ImmutableList<Artifact> outputs) {
// Actions must have at least one output
assertThat(outputs).isNotEmpty();
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/StarlarkCcCommonTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/StarlarkCcCommonTest.java
index 173b3ea..194998b 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/StarlarkCcCommonTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/StarlarkCcCommonTest.java
@@ -1479,10 +1479,7 @@
assertThat(userLinkFlags.getImmutableList())
.containsExactly("-la", "-lc2", "-DEP2_LINKOPT", "-lc1", "-lc2", "-DEP1_LINKOPT");
Depset additionalInputs = info.getValue("additional_inputs", Depset.class);
- assertThat(
- additionalInputs.toList(Artifact.class).stream()
- .map(x -> x.getFilename())
- .collect(ImmutableList.toImmutableList()))
+ assertThat(additionalInputs.toList(Artifact.class).stream().map(Artifact::getFilename))
.containsExactly("b.lds", "d.lds");
Collection<LibraryToLink> librariesToLink =
info.getValue("libraries_to_link", Depset.class).toList(LibraryToLink.class);
@@ -4661,11 +4658,11 @@
(CcToolchainConfigInfo) target.get(CcToolchainConfigInfo.PROVIDER.getKey());
ImmutableSet<String> featureNames =
ccToolchainConfigInfo.getFeatures().stream()
- .map(feature -> feature.getName())
+ .map(Feature::getName)
.collect(ImmutableSet.toImmutableSet());
ImmutableSet<String> actionConfigNames =
ccToolchainConfigInfo.getActionConfigs().stream()
- .map(actionConfig -> actionConfig.getActionName())
+ .map(ActionConfig::getActionName)
.collect(ImmutableSet.toImmutableSet());
assertThat(featureNames).containsExactly("no_legacy_features", "custom_feature");
assertThat(actionConfigNames).containsExactly("custom_action");
@@ -4730,11 +4727,11 @@
(CcToolchainConfigInfo) target.get(CcToolchainConfigInfo.PROVIDER.getKey());
ImmutableList<String> featureNames =
ccToolchainConfigInfo.getFeatures().stream()
- .map(feature -> feature.getName())
+ .map(Feature::getName)
.collect(ImmutableList.toImmutableList());
ImmutableSet<String> actionConfigNames =
ccToolchainConfigInfo.getActionConfigs().stream()
- .map(actionConfig -> actionConfig.getActionName())
+ .map(ActionConfig::getActionName)
.collect(ImmutableSet.toImmutableSet());
// fdo_optimize should not be re-added to the list of features by legacy behavior
assertThat(featureNames).containsNoDuplicates();
@@ -4882,7 +4879,7 @@
assertThat(toolchain.getCcTargetOs()).isEqualTo("os");
assertThat(
toolchain.getFeatureList().stream()
- .map(feature -> feature.getName())
+ .map(CToolchain.Feature::getName)
.collect(ImmutableList.toImmutableList()))
.containsAtLeast("featureone", "sysroot")
.inOrder();
@@ -4925,7 +4922,7 @@
assertThat(
toolchain.getActionConfigList().stream()
- .map(actionConfig -> actionConfig.getActionName())
+ .map(CToolchain.ActionConfig::getActionName)
.collect(ImmutableList.toImmutableList()))
.containsAtLeast("action_one", "action_two")
.inOrder();
@@ -5318,7 +5315,7 @@
ConfiguredTarget target = getConfiguredTarget("//foo:starlark_lib");
assertThat(
getFilesToBuild(target).toList().stream()
- .map(x -> x.getFilename())
+ .map(Artifact::getFilename)
.collect(ImmutableList.toImmutableList()))
.contains("libstarlark_lib.a");
}
@@ -5330,12 +5327,12 @@
ConfiguredTarget target = getConfiguredTarget("//foo:starlark_lib");
assertThat(
getFilesToBuild(target).toList().stream()
- .map(x -> x.getFilename())
+ .map(Artifact::getFilename)
.collect(ImmutableList.toImmutableList()))
.doesNotContain("libstarlark_lib.a");
}
- private List<String> getFilenamesToBuild(ConfiguredTarget target) {
+ private static ImmutableList<String> getFilenamesToBuild(ConfiguredTarget target) {
return getFilesToBuild(target).toList().stream()
.map(Artifact::getFilename)
.collect(ImmutableList.toImmutableList());
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/BUILD b/src/test/java/com/google/devtools/build/lib/skyframe/BUILD
index fef712a..7739ef0 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/BUILD
@@ -8,7 +8,7 @@
filegroup(
name = "srcs",
testonly = 0,
- srcs = glob(["**"]) + ["//src/test/java/com/google/devtools/build/lib/skyframe/trimming:srcs"],
+ srcs = glob(["**"]),
visibility = ["//src/test/java/com/google/devtools/build/lib:__pkg__"],
)
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/PlatformLookupUtilTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/PlatformLookupUtilTest.java
index 1c588ac..085ba2e 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/PlatformLookupUtilTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/PlatformLookupUtilTest.java
@@ -158,7 +158,7 @@
}
}
- EvaluationResult<GetPlatformInfoValue> getPlatformInfo(GetPlatformInfoKey key)
+ private EvaluationResult<GetPlatformInfoValue> getPlatformInfo(GetPlatformInfoKey key)
throws InterruptedException {
try {
// Must re-enable analysis for Skyframe functions that create configured targets.
@@ -188,7 +188,7 @@
GetPlatformInfoKey key = (GetPlatformInfoKey) skyKey;
try {
Map<ConfiguredTargetKey, PlatformInfo> platforms =
- PlatformLookupUtil.getPlatformInfo(key.platformKeys(), env, false);
+ PlatformLookupUtil.getPlatformInfo(key.platformKeys(), env);
if (env.valuesMissing()) {
return null;
}
@@ -205,8 +205,8 @@
}
}
- private static class GetPlatformInfoFunctionException extends SkyFunctionException {
- public GetPlatformInfoFunctionException(InvalidPlatformException e) {
+ private static final class GetPlatformInfoFunctionException extends SkyFunctionException {
+ GetPlatformInfoFunctionException(InvalidPlatformException e) {
super(e, Transience.PERSISTENT);
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ToolchainTypeLookupUtilTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ToolchainTypeLookupUtilTest.java
index f85600c..47999e7 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/ToolchainTypeLookupUtilTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/ToolchainTypeLookupUtilTest.java
@@ -182,8 +182,8 @@
}
}
- EvaluationResult<GetToolchainTypeInfoValue> getToolchainTypeInfo(GetToolchainTypeInfoKey key)
- throws InterruptedException {
+ private EvaluationResult<GetToolchainTypeInfoValue> getToolchainTypeInfo(
+ GetToolchainTypeInfoKey key) throws InterruptedException {
try {
// Must re-enable analysis for Skyframe functions that create configured targets.
skyframeExecutor.getSkyframeBuildView().enableAnalysis(true);
@@ -212,7 +212,7 @@
GetToolchainTypeInfoKey key = (GetToolchainTypeInfoKey) skyKey;
try {
Map<Label, ToolchainTypeInfo> toolchainTypes =
- ToolchainTypeLookupUtil.resolveToolchainTypes(env, key.toolchainTypeKeys(), false);
+ ToolchainTypeLookupUtil.resolveToolchainTypes(env, key.toolchainTypeKeys());
if (env.valuesMissing()) {
return null;
}
@@ -229,8 +229,8 @@
}
}
- private static class GetToolchainTypeInfoFunctionException extends SkyFunctionException {
- public GetToolchainTypeInfoFunctionException(InvalidToolchainTypeException e) {
+ private static final class GetToolchainTypeInfoFunctionException extends SkyFunctionException {
+ GetToolchainTypeInfoFunctionException(InvalidToolchainTypeException e) {
super(e, Transience.PERSISTENT);
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/trimming/BUILD b/src/test/java/com/google/devtools/build/lib/skyframe/trimming/BUILD
deleted file mode 100644
index 9636055..0000000
--- a/src/test/java/com/google/devtools/build/lib/skyframe/trimming/BUILD
+++ /dev/null
@@ -1,85 +0,0 @@
-# Tests for the trimming support classes.
-
-load("@rules_java//java:defs.bzl", "java_library", "java_test")
-
-package(
- default_testonly = 1,
-)
-
-filegroup(
- name = "srcs",
- testonly = 0,
- srcs = glob(["**"]),
- visibility = ["//src:__subpackages__"],
-)
-
-java_library(
- name = "trimmable_test_fragments",
- srcs = ["TrimmableTestConfigurationFragments.java"],
- visibility = ["//src/test:__subpackages__"],
- deps = [
- "//src/main/java/com/google/devtools/build/lib/actions",
- "//src/main/java/com/google/devtools/build/lib/actions:artifacts",
- "//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",
- "//src/main/java/com/google/devtools/build/lib/analysis:config/build_options",
- "//src/main/java/com/google/devtools/build/lib/analysis:config/core_options",
- "//src/main/java/com/google/devtools/build/lib/analysis:config/fragment",
- "//src/main/java/com/google/devtools/build/lib/analysis:config/fragment_options",
- "//src/main/java/com/google/devtools/build/lib/analysis:config/fragment_provider",
- "//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/patch_transition",
- "//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/transition_factory",
- "//src/main/java/com/google/devtools/build/lib/analysis:configured_target",
- "//src/main/java/com/google/devtools/build/lib/analysis:file_provider",
- "//src/main/java/com/google/devtools/build/lib/analysis:platform_configuration",
- "//src/main/java/com/google/devtools/build/lib/analysis:platform_options",
- "//src/main/java/com/google/devtools/build/lib/analysis:resolved_toolchain_context",
- "//src/main/java/com/google/devtools/build/lib/analysis:test/test_configuration",
- "//src/main/java/com/google/devtools/build/lib/analysis/platform",
- "//src/main/java/com/google/devtools/build/lib/cmdline",
- "//src/main/java/com/google/devtools/build/lib/collect/nestedset",
- "//src/main/java/com/google/devtools/build/lib/events",
- "//src/main/java/com/google/devtools/build/lib/packages",
- "//src/main/java/com/google/devtools/build/lib/rules:core_rules",
- "//src/main/java/com/google/devtools/build/lib/rules:repository/bind_rule",
- "//src/main/java/com/google/devtools/build/lib/rules:repository/workspace_base_rule",
- "//src/main/java/com/google/devtools/build/lib/rules:toolchain_type",
- "//src/main/java/com/google/devtools/build/lib/util:filetype",
- "//src/main/java/com/google/devtools/common/options",
- "//src/main/java/net/starlark/java/annot",
- "//src/main/java/net/starlark/java/eval",
- "//src/main/java/net/starlark/java/syntax",
- "//src/test/java/com/google/devtools/build/lib/analysis/util",
- "//src/test/java/com/google/devtools/build/lib/testutil",
- "//third_party:guava",
- "//third_party:jsr305",
- ],
-)
-
-java_library(
- name = "test_key",
- srcs = ["TestKey.java"],
- deps = [
- "//src/main/java/com/google/devtools/build/lib/skyframe/trimming:trimmed_configuration_cache",
- "//third_party:auto_value",
- "//third_party:guava",
- ],
-)
-
-java_test(
- name = "TrimmedConfigurationCacheTests",
- size = "small",
- srcs = [
- "TestKeyTest.java",
- "TrimmedConfigurationCacheTest.java",
- ],
- test_class = "com.google.devtools.build.lib.AllTests",
- runtime_deps = ["//src/test/java/com/google/devtools/build/lib:test_runner"],
- deps = [
- ":test_key",
- "//src/main/java/com/google/devtools/build/lib/skyframe/trimming:trimmed_configuration_cache",
- "//third_party:guava",
- "//third_party:guava-testlib",
- "//third_party:junit4",
- "//third_party:truth",
- ],
-)
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/trimming/TestKey.java b/src/test/java/com/google/devtools/build/lib/skyframe/trimming/TestKey.java
deleted file mode 100644
index 45316c1..0000000
--- a/src/test/java/com/google/devtools/build/lib/skyframe/trimming/TestKey.java
+++ /dev/null
@@ -1,91 +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.skyframe.trimming;
-
-import com.google.auto.value.AutoValue;
-import com.google.common.base.Preconditions;
-import com.google.common.base.Splitter;
-import com.google.common.base.Strings;
-import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Sets;
-import java.util.Set;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
-
-/** A simple key suitable for putting into a TrimmedConfigurationCache for testing it. */
-@AutoValue
-abstract class TestKey {
- abstract String descriptor();
-
- abstract ImmutableMap<String, String> configuration();
-
- static TestKey create(String descriptor, ImmutableMap<String, String> configuration) {
- return new AutoValue_TestKey(descriptor, configuration);
- }
-
- // Test keys look like <config: value, config: value> descriptor
- private static final Pattern TEST_KEY_SHAPE =
- Pattern.compile("<(?<config>[^>]*)>(?<descriptor>.+)");
-
- static TestKey parse(String input) {
- Matcher matcher = TEST_KEY_SHAPE.matcher(input.trim());
- Preconditions.checkArgument(matcher.matches());
- return create(
- matcher.group("descriptor").trim(), parseConfiguration(matcher.group("config").trim()));
- }
-
- static ImmutableMap<String, String> parseConfiguration(String input) {
- if (Strings.isNullOrEmpty(input)) {
- return ImmutableMap.of();
- }
- return ImmutableMap.copyOf(
- Splitter.on(',')
- .trimResults()
- .withKeyValueSeparator(Splitter.on(':').trimResults())
- .split(input));
- }
-
- static ConfigurationComparer.Result compareConfigurations(
- ImmutableMap<String, String> left,
- ImmutableMap<String, String> right) {
- Set<String> sharedKeys = Sets.intersection(left.keySet(), right.keySet());
- for (String key : sharedKeys) {
- if (!left.get(key).equals(right.get(key))) {
- return ConfigurationComparer.Result.DIFFERENT;
- }
- }
- boolean hasLeftOnlyKeys = !Sets.difference(left.keySet(), right.keySet()).isEmpty();
- boolean hasRightOnlyKeys = !Sets.difference(right.keySet(), left.keySet()).isEmpty();
- if (hasLeftOnlyKeys) {
- if (hasRightOnlyKeys) {
- return ConfigurationComparer.Result.ALL_SHARED_FRAGMENTS_EQUAL;
- } else {
- return ConfigurationComparer.Result.SUPERSET;
- }
- } else {
- if (hasRightOnlyKeys) {
- return ConfigurationComparer.Result.SUBSET;
- } else {
- return ConfigurationComparer.Result.EQUAL;
- }
- }
- }
-
- /** Produces a cache suitable for storing TestKeys. */
- static TrimmedConfigurationCache<TestKey, String, ImmutableMap<String, String>> newCache() {
- return new TrimmedConfigurationCache<>(
- TestKey::descriptor, TestKey::configuration, TestKey::compareConfigurations);
- }
-}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/trimming/TestKeyTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/trimming/TestKeyTest.java
deleted file mode 100644
index ede356f..0000000
--- a/src/test/java/com/google/devtools/build/lib/skyframe/trimming/TestKeyTest.java
+++ /dev/null
@@ -1,152 +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.skyframe.trimming;
-
-import static com.google.common.truth.Truth.assertThat;
-
-import com.google.common.collect.ImmutableMap;
-import com.google.common.testing.EqualsTester;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.JUnit4;
-
-/** Tests for TestKey's parsing functionality. */
-@RunWith(JUnit4.class)
-public final class TestKeyTest {
-
- @Test
- public void parseEmptyConfig() throws Exception {
- assertThat(TestKey.parse("<> //foo").configuration()).isEmpty();
- assertThat(TestKey.parse("< > //foo").configuration()).isEmpty();
- assertThat(TestKey.parse(" < > //foo").configuration()).isEmpty();
- }
-
- @Test
- public void parseOneElementConfig() throws Exception {
- assertThat(TestKey.parse("<A:1> //foo").configuration()).containsExactly("A", "1");
- assertThat(TestKey.parse("< A : 1 > //foo").configuration()).containsExactly("A", "1");
- assertThat(TestKey.parse(" < A : 1 > //foo").configuration())
- .containsExactly("A", "1");
- }
-
- @Test
- public void parseMultiElementConfig() throws Exception {
- assertThat(TestKey.parse("<A:1,B:6,C:90> //foo").configuration())
- .containsExactly("A", "1", "B", "6", "C", "90");
- assertThat(TestKey.parse("< A : 1 , B : 6 , C : 90 > //foo").configuration())
- .containsExactly("A", "1", "B", "6", "C", "90");
- assertThat(TestKey.parse(" < A : 1, B: 6, C :90 > //foo").configuration())
- .containsExactly("A", "1", "B", "6", "C", "90");
- }
-
- @Test
- public void parseConfigWithSpaces() throws Exception {
- assertThat(TestKey.parse("<An Item: A Value> //foo").configuration())
- .containsExactly("An Item", "A Value");
- }
-
- @Test
- public void parseDescriptor() throws Exception {
- assertThat(TestKey.parse("<>//foo").descriptor()).isEqualTo("//foo");
- assertThat(TestKey.parse("<> //foo ").descriptor()).isEqualTo("//foo");
- assertThat(TestKey.parse(" < A : 1, B: 6, C :90 > //foo ").descriptor())
- .isEqualTo("//foo");
- }
-
- @Test
- public void parseDescriptorWithSpaces() throws Exception {
- assertThat(TestKey.parse("<>//foo with space").descriptor()).isEqualTo("//foo with space");
- assertThat(TestKey.parse("<> //foo with space ").descriptor())
- .isEqualTo("//foo with space");
- }
-
- @Test
- public void parseMissingConfiguration() throws Exception {
- assertThat(TestKey.parse("<>//foo with space").descriptor()).isEqualTo("//foo with space");
- assertThat(TestKey.parse("<> //foo with space ").descriptor())
- .isEqualTo("//foo with space");
- }
-
- @Test
- public void equality() throws Exception {
- new EqualsTester()
- .addEqualityGroup(
- TestKey.parse("<>//foo"),
- TestKey.parse(" < > //foo "),
- TestKey.create("//foo", ImmutableMap.of()))
- .addEqualityGroup(
- TestKey.parse("<A:1>//foo"),
- TestKey.parse(" < A : 1 > //foo "),
- TestKey.create("//foo", ImmutableMap.of("A", "1")))
- .addEqualityGroup(
- TestKey.parse("<>//bar"),
- TestKey.parse(" < > //bar "),
- TestKey.create("//bar", ImmutableMap.of()))
- .addEqualityGroup(
- TestKey.parse("<A:1>//bar"),
- TestKey.parse(" < A : 1 > //bar "),
- TestKey.create("//bar", ImmutableMap.of("A", "1")))
- .testEquals();
- }
-
- @Test
- public void compareConfigurations_EqualCases() throws Exception {
- assertThat(TestKey.compareConfigurations(ImmutableMap.of(), ImmutableMap.of()))
- .isEqualTo(ConfigurationComparer.Result.EQUAL);
- assertThat(TestKey.compareConfigurations(ImmutableMap.of("A", "1"), ImmutableMap.of("A", "1")))
- .isEqualTo(ConfigurationComparer.Result.EQUAL);
- }
-
- @Test
- public void compareConfigurations_SubsetCases() throws Exception {
- assertThat(TestKey.compareConfigurations(ImmutableMap.of(), ImmutableMap.of("A", "1")))
- .isEqualTo(ConfigurationComparer.Result.SUBSET);
- assertThat(
- TestKey.compareConfigurations(
- ImmutableMap.of("A", "1"), ImmutableMap.of("A", "1", "B", "2")))
- .isEqualTo(ConfigurationComparer.Result.SUBSET);
- }
-
- @Test
- public void compareConfigurations_SupersetCases() throws Exception {
- assertThat(TestKey.compareConfigurations(ImmutableMap.of("A", "1"), ImmutableMap.of()))
- .isEqualTo(ConfigurationComparer.Result.SUPERSET);
- assertThat(
- TestKey.compareConfigurations(
- ImmutableMap.of("A", "1", "B", "2"), ImmutableMap.of("A", "1")))
- .isEqualTo(ConfigurationComparer.Result.SUPERSET);
- }
-
- @Test
- public void compareConfigurations_AllSharedFragmentsEqualCases() throws Exception {
- assertThat(TestKey.compareConfigurations(ImmutableMap.of("A", "1"), ImmutableMap.of("B", "1")))
- .isEqualTo(ConfigurationComparer.Result.ALL_SHARED_FRAGMENTS_EQUAL);
- assertThat(
- TestKey.compareConfigurations(
- ImmutableMap.of("A", "1", "B", "2"), ImmutableMap.of("A", "1", "C", "3")))
- .isEqualTo(ConfigurationComparer.Result.ALL_SHARED_FRAGMENTS_EQUAL);
- }
-
- @Test
- public void compareConfigurations_DifferentCases() throws Exception {
- assertThat(TestKey.compareConfigurations(ImmutableMap.of("A", "1"), ImmutableMap.of("A", "2")))
- .isEqualTo(ConfigurationComparer.Result.DIFFERENT);
- assertThat(
- TestKey.compareConfigurations(
- ImmutableMap.of("A", "1", "B", "2", "C", "3"),
- ImmutableMap.of("A", "2", "B", "2", "D", "4")))
- .isEqualTo(ConfigurationComparer.Result.DIFFERENT);
- }
-}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/trimming/TrimmableTestConfigurationFragments.java b/src/test/java/com/google/devtools/build/lib/skyframe/trimming/TrimmableTestConfigurationFragments.java
deleted file mode 100644
index 8218e76..0000000
--- a/src/test/java/com/google/devtools/build/lib/skyframe/trimming/TrimmableTestConfigurationFragments.java
+++ /dev/null
@@ -1,652 +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.skyframe.trimming;
-
-import static com.google.devtools.build.lib.packages.Attribute.attr;
-
-import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.ImmutableSet;
-import com.google.devtools.build.lib.actions.Artifact;
-import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
-import com.google.devtools.build.lib.analysis.BaseRuleClasses;
-import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
-import com.google.devtools.build.lib.analysis.ConfiguredTarget;
-import com.google.devtools.build.lib.analysis.FileProvider;
-import com.google.devtools.build.lib.analysis.PlatformConfiguration;
-import com.google.devtools.build.lib.analysis.PlatformOptions;
-import com.google.devtools.build.lib.analysis.ResolvedToolchainContext;
-import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder;
-import com.google.devtools.build.lib.analysis.RuleConfiguredTargetFactory;
-import com.google.devtools.build.lib.analysis.RuleContext;
-import com.google.devtools.build.lib.analysis.Runfiles;
-import com.google.devtools.build.lib.analysis.RunfilesProvider;
-import com.google.devtools.build.lib.analysis.actions.FileWriteAction;
-import com.google.devtools.build.lib.analysis.config.BuildOptions;
-import com.google.devtools.build.lib.analysis.config.BuildOptionsView;
-import com.google.devtools.build.lib.analysis.config.CoreOptions;
-import com.google.devtools.build.lib.analysis.config.Fragment;
-import com.google.devtools.build.lib.analysis.config.FragmentOptions;
-import com.google.devtools.build.lib.analysis.config.RequiresOptions;
-import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition;
-import com.google.devtools.build.lib.analysis.config.transitions.TransitionFactory;
-import com.google.devtools.build.lib.analysis.platform.ToolchainInfo;
-import com.google.devtools.build.lib.analysis.platform.ToolchainTypeInfo;
-import com.google.devtools.build.lib.analysis.test.TestConfiguration;
-import com.google.devtools.build.lib.analysis.util.MockRule;
-import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
-import com.google.devtools.build.lib.collect.nestedset.Depset;
-import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
-import com.google.devtools.build.lib.collect.nestedset.Order;
-import com.google.devtools.build.lib.events.EventHandler;
-import com.google.devtools.build.lib.packages.AttributeMap;
-import com.google.devtools.build.lib.packages.BuildType;
-import com.google.devtools.build.lib.packages.ImplicitOutputsFunction;
-import com.google.devtools.build.lib.packages.NonconfigurableAttributeMapper;
-import com.google.devtools.build.lib.packages.Rule;
-import com.google.devtools.build.lib.packages.RuleClass.ToolchainResolutionMode;
-import com.google.devtools.build.lib.packages.Type;
-import com.google.devtools.build.lib.rules.ToolchainType.ToolchainTypeRule;
-import com.google.devtools.build.lib.rules.core.CoreRules;
-import com.google.devtools.build.lib.rules.repository.BindRule;
-import com.google.devtools.build.lib.rules.repository.WorkspaceBaseRule;
-import com.google.devtools.build.lib.testutil.Scratch;
-import com.google.devtools.build.lib.util.FileTypeSet;
-import com.google.devtools.common.options.Option;
-import com.google.devtools.common.options.OptionDefinition;
-import com.google.devtools.common.options.OptionDocumentationCategory;
-import com.google.devtools.common.options.OptionEffectTag;
-import com.google.devtools.common.options.OptionsParser;
-import java.io.IOException;
-import java.util.List;
-import javax.annotation.Nullable;
-import net.starlark.java.annot.StarlarkBuiltin;
-import net.starlark.java.eval.EvalException;
-import net.starlark.java.eval.Starlark;
-import net.starlark.java.eval.StarlarkValue;
-
-/** Set of trimmable fragments for testing automatic trimming. */
-public final class TrimmableTestConfigurationFragments {
-
- private TrimmableTestConfigurationFragments() {
- // Utility class, non-instantiable
- }
-
- public static void installStarlarkRules(Scratch scratch, String path)
- throws LabelSyntaxException, IOException {
- installStarlarkRules(
- scratch, path, Label.parseAbsolute("//:undefined_toolchain_type", ImmutableMap.of()));
- }
-
- public static void installStarlarkRules(Scratch scratch, String path, Label toolchainTypeLabel)
- throws LabelSyntaxException, IOException {
- scratch.file(
- path,
- "toolchainTypeLabel = " + Starlark.repr(toolchainTypeLabel),
- "def _impl(ctx):",
- " ctx.actions.write(ctx.outputs.main, '')",
- " files = depset(",
- " direct = [ctx.outputs.main],",
- " transitive = [dep.files for dep in ctx.attr.deps])",
- " return [DefaultInfo(files=files)]",
- "alpha_starlark = rule(",
- " implementation = _impl,",
- " attrs = {'deps': attr.label_list(allow_files=True)},",
- " fragments = ['alpha'],",
- " outputs = {'main': '%{name}.sa'},",
- ")",
- "bravo_starlark = rule(",
- " implementation = _impl,",
- " attrs = {'deps': attr.label_list(allow_files=True)},",
- " fragments = ['bravo'],",
- " outputs = {'main': '%{name}.sb'},",
- ")",
- "charlie_starlark = rule(",
- " implementation = _impl,",
- " attrs = {'deps': attr.label_list(allow_files=True)},",
- " fragments = ['charlie'],",
- " outputs = {'main': '%{name}.sc'},",
- ")",
- "delta_starlark = rule(",
- " implementation = _impl,",
- " attrs = {'deps': attr.label_list(allow_files=True)},",
- " fragments = ['delta'],",
- " outputs = {'main': '%{name}.sd'},",
- ")",
- "echo_starlark = rule(",
- " implementation = _impl,",
- " attrs = {'deps': attr.label_list(allow_files=True)},",
- " fragments = ['echo'],",
- " outputs = {'main': '%{name}.se'},",
- ")",
- "platformer_starlark = rule(",
- " implementation = _impl,",
- " attrs = {'deps': attr.label_list(allow_files=True)},",
- " outputs = {'main': '%{name}.sp'},",
- ")",
- "def _uses_toolchains_impl(ctx):",
- " ctx.actions.write(ctx.outputs.main, '')",
- " transitive_depsets = [dep.files for dep in ctx.attr.deps]",
- " toolchain_deps = ctx.toolchains[toolchainTypeLabel].files",
- " files = depset(",
- " direct = [ctx.outputs.main],",
- " transitive = transitive_depsets + [toolchain_deps])",
- " return [DefaultInfo(files=files)]",
- "uses_toolchains_starlark = rule(",
- " implementation = _uses_toolchains_impl,",
- " attrs = {'deps': attr.label_list(allow_files=True)},",
- " outputs = {'main': '%{name}.su'},",
- " toolchains = [str(toolchainTypeLabel)],",
- ")",
- "def _toolchain_impl(ctx):",
- " ctx.actions.write(ctx.outputs.main, '')",
- " files = depset(",
- " direct = [ctx.outputs.main],",
- " transitive = [dep.files for dep in ctx.attr.deps])",
- " return [DefaultInfo(files=files), platform_common.ToolchainInfo(files=files)]",
- "toolchain_starlark = rule(",
- " implementation = _toolchain_impl,",
- " attrs = {'deps': attr.label_list(allow_files=True)},",
- " outputs = {'main': '%{name}.st'},",
- ")",
- "def _group_impl(ctx):",
- " files = depset(transitive = [dep.files for dep in ctx.attr.deps])",
- " return [DefaultInfo(files=files)]",
- "group = rule(",
- " implementation = _group_impl,",
- " attrs = {'deps': attr.label_list(allow_files=True)},",
- ")");
- }
-
- public static void installFragmentsAndNativeRules(ConfiguredRuleClassProvider.Builder builder) {
- installFragmentsAndNativeRules(builder, null);
- }
-
- public static void installFragmentsAndNativeRules(
- ConfiguredRuleClassProvider.Builder builder, @Nullable Label toolchainTypeLabel) {
- // boilerplate:
- builder
- // must be set, but it doesn't matter here what it's set to
- .setToolsRepository("@")
- // must be set, but it doesn't matter here what it's set to
- .setRunfilesPrefix("runfiles")
- // must be set, but it doesn't matter here what it's set to
- .setPrerequisiteValidator((contextBuilder, prerequisite, attribute) -> {})
- // must be set, but it doesn't matter here what it's set to
- .setPrelude("//:prelude.bzl")
- // must be part of BuildOptions for various reasons e.g. dynamic configs
- .addConfigurationOptions(CoreOptions.class)
- .addConfigurationFragment(TestConfiguration.class)
- // needed for the default workspace
- .addRuleDefinition(new WorkspaceBaseRule())
- .addRuleDefinition(new BindRule())
- // needed for our native rules
- .addRuleDefinition(new BaseRuleClasses.NativeBuildRule())
- // needed to define toolchains
- .addRuleDefinition(new ToolchainTypeRule())
- // needs to be set to something
- .addUniversalConfigurationFragment(TestConfiguration.class);
-
- CoreRules.INSTANCE.init(builder);
-
- MockRule transitionRule =
- () ->
- MockRule.ancestor(BaseRuleClasses.NativeBuildRule.class)
- .factory(DepsCollectingFactory.class)
- .define(
- "with_configuration",
- (ruleBuilder, env) ->
- ruleBuilder
- .requiresConfigurationFragments(
- AConfig.class,
- BConfig.class,
- CConfig.class,
- DConfig.class,
- EConfig.class,
- PlatformConfiguration.class)
- .cfg(new TestFragmentTransitionFactory())
- .add(
- attr("deps", BuildType.LABEL_LIST)
- .allowedFileTypes(FileTypeSet.ANY_FILE))
- .add(
- attr("alpha", Type.STRING)
- .value((String) null)
- .nonconfigurable("used in transition"))
- .add(
- attr("bravo", Type.STRING)
- .value((String) null)
- .nonconfigurable("used in transition"))
- .add(
- attr("charlie", Type.STRING)
- .value((String) null)
- .nonconfigurable("used in transition"))
- .add(
- attr("delta", Type.STRING)
- .value((String) null)
- .nonconfigurable("used in transition"))
- .add(
- attr("echo", Type.STRING)
- .value((String) null)
- .nonconfigurable("used in transition"))
- .add(
- attr("platforms", BuildType.NODEP_LABEL_LIST)
- .value((List<Label>) null)
- .nonconfigurable("used in transition"))
- .add(
- attr("extra_execution_platforms", Type.STRING_LIST)
- .value((List<String>) null)
- .nonconfigurable("used in transition"))
- .add(
- attr("extra_toolchains", Type.STRING_LIST)
- .value((List<String>) null)
- .nonconfigurable("used in transition")));
-
- MockRule alphaRule =
- () ->
- MockRule.ancestor(BaseRuleClasses.NativeBuildRule.class)
- .factory(DepsCollectingFactory.class)
- .define(
- "alpha_native",
- (ruleBuilder, env) ->
- ruleBuilder
- .add(
- attr("deps", BuildType.LABEL_LIST)
- .allowedFileTypes(FileTypeSet.ANY_FILE))
- .requiresConfigurationFragments(AConfig.class)
- .setImplicitOutputsFunction(
- ImplicitOutputsFunction.fromTemplates("%{name}.a")));
-
- MockRule bravoRule =
- () ->
- MockRule.ancestor(BaseRuleClasses.NativeBuildRule.class)
- .factory(DepsCollectingFactory.class)
- .define(
- "bravo_native",
- (ruleBuilder, env) ->
- ruleBuilder
- .add(
- attr("deps", BuildType.LABEL_LIST)
- .allowedFileTypes(FileTypeSet.ANY_FILE))
- .requiresConfigurationFragments(BConfig.class)
- .setImplicitOutputsFunction(
- ImplicitOutputsFunction.fromTemplates("%{name}.b")));
-
- MockRule charlieRule =
- () ->
- MockRule.ancestor(BaseRuleClasses.NativeBuildRule.class)
- .factory(DepsCollectingFactory.class)
- .define(
- "charlie_native",
- (ruleBuilder, env) ->
- ruleBuilder
- .add(
- attr("deps", BuildType.LABEL_LIST)
- .allowedFileTypes(FileTypeSet.ANY_FILE))
- .requiresConfigurationFragments(CConfig.class)
- .setImplicitOutputsFunction(
- ImplicitOutputsFunction.fromTemplates("%{name}.c")));
-
- MockRule deltaRule =
- () ->
- MockRule.ancestor(BaseRuleClasses.NativeBuildRule.class)
- .factory(DepsCollectingFactory.class)
- .define(
- "delta_native",
- (ruleBuilder, env) ->
- ruleBuilder
- .add(
- attr("deps", BuildType.LABEL_LIST)
- .allowedFileTypes(FileTypeSet.ANY_FILE))
- .requiresConfigurationFragments(DConfig.class)
- .setImplicitOutputsFunction(
- ImplicitOutputsFunction.fromTemplates("%{name}.d")));
-
- MockRule echoRule =
- () ->
- MockRule.ancestor(BaseRuleClasses.NativeBuildRule.class)
- .factory(DepsCollectingFactory.class)
- .define(
- "echo_native",
- (ruleBuilder, env) ->
- ruleBuilder
- .add(
- attr("deps", BuildType.LABEL_LIST)
- .allowedFileTypes(FileTypeSet.ANY_FILE))
- .requiresConfigurationFragments(EConfig.class)
- .setImplicitOutputsFunction(
- ImplicitOutputsFunction.fromTemplates("%{name}.e")));
-
- MockRule platformlessRule =
- () ->
- MockRule.ancestor(BaseRuleClasses.NativeBuildRule.class)
- .factory(DepsCollectingFactory.class)
- .define(
- "platformless_native",
- (ruleBuilder, env) ->
- ruleBuilder
- .add(
- attr("deps", BuildType.LABEL_LIST)
- .allowedFileTypes(FileTypeSet.ANY_FILE))
- .useToolchainResolution(ToolchainResolutionMode.DISABLED)
- .setImplicitOutputsFunction(
- ImplicitOutputsFunction.fromTemplates("%{name}.np")));
-
- MockRule platformerRule =
- () ->
- MockRule.ancestor(BaseRuleClasses.NativeBuildRule.class)
- .factory(DepsCollectingFactory.class)
- .define(
- "platformer_native",
- (ruleBuilder, env) ->
- ruleBuilder
- .add(
- attr("deps", BuildType.LABEL_LIST)
- .allowedFileTypes(FileTypeSet.ANY_FILE))
- .useToolchainResolution(ToolchainResolutionMode.ENABLED)
- .setImplicitOutputsFunction(
- ImplicitOutputsFunction.fromTemplates("%{name}.p")));
-
- builder
- .addConfigurationFragment(AConfig.class)
- .addConfigurationFragment(BConfig.class)
- .addConfigurationFragment(CConfig.class)
- .addConfigurationFragment(DConfig.class)
- .addConfigurationFragment(EConfig.class)
- .addRuleDefinition(transitionRule)
- .addRuleDefinition(alphaRule)
- .addRuleDefinition(bravoRule)
- .addRuleDefinition(charlieRule)
- .addRuleDefinition(deltaRule)
- .addRuleDefinition(echoRule)
- .addRuleDefinition(platformlessRule)
- .addRuleDefinition(platformerRule);
-
- if (toolchainTypeLabel != null) {
- MockRule usesToolchainsRule =
- () ->
- MockRule.ancestor(BaseRuleClasses.NativeBuildRule.class)
- .factory(DepsCollectingFactory.class)
- .define(
- "uses_toolchains_native",
- (ruleBuilder, env) ->
- ruleBuilder
- .add(
- attr("deps", BuildType.LABEL_LIST)
- .allowedFileTypes(FileTypeSet.ANY_FILE))
- .useToolchainResolution(ToolchainResolutionMode.ENABLED)
- .addRequiredToolchains(toolchainTypeLabel)
- .setImplicitOutputsFunction(
- ImplicitOutputsFunction.fromTemplates("%{name}.u")));
- builder.addRuleDefinition(usesToolchainsRule);
- }
- }
-
- /** Set of test options. */
- public static final class AOptions extends FragmentOptions {
- @Option(
- name = "alpha",
- documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
- effectTags = {OptionEffectTag.NO_OP},
- defaultValue = "0")
- public String alpha;
- }
-
- /** Test configuration fragment. */
- @StarlarkBuiltin(name = "alpha", doc = "Test config fragment.")
- @RequiresOptions(options = {AOptions.class})
- public static final class AConfig extends Fragment implements StarlarkValue {
- private final String value;
-
- public AConfig(BuildOptions buildOptions) {
- this.value = buildOptions.get(AOptions.class).alpha;
- }
-
- @Override
- public String getOutputDirectoryName() {
- return "A" + value;
- }
- }
-
- /** Set of test options. */
- public static final class BOptions extends FragmentOptions {
- @Option(
- name = "bravo",
- documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
- effectTags = {OptionEffectTag.NO_OP},
- defaultValue = "0")
- public String bravo;
- }
-
- /** Test configuration fragment. */
- @StarlarkBuiltin(name = "bravo", doc = "Test config fragment.")
- @RequiresOptions(options = {BOptions.class})
- public static final class BConfig extends Fragment implements StarlarkValue {
- private final String value;
-
- public BConfig(BuildOptions buildOptions) {
- this.value = buildOptions.get(BOptions.class).bravo;
- }
-
- @Override
- public String getOutputDirectoryName() {
- return "B" + value;
- }
- }
-
- /** Set of test options. */
- public static final class COptions extends FragmentOptions {
- @Option(
- name = "charlie",
- documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
- effectTags = {OptionEffectTag.NO_OP},
- defaultValue = "0")
- public String charlie;
- }
-
- /** Test configuration fragment. */
- @StarlarkBuiltin(name = "charlie", doc = "Test config fragment.")
- @RequiresOptions(options = {COptions.class})
- public static final class CConfig extends Fragment implements StarlarkValue {
- private final String value;
-
- public CConfig(BuildOptions buildOptions) {
- this.value = buildOptions.get(COptions.class).charlie;
- }
-
- @Override
- public String getOutputDirectoryName() {
- return "C" + value;
- }
- }
-
- /** Set of test options. */
- public static final class DOptions extends FragmentOptions {
- @Option(
- name = "delta",
- documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
- effectTags = {OptionEffectTag.NO_OP},
- defaultValue = "0")
- public String delta;
- }
-
- /** Test configuration fragment. */
- @StarlarkBuiltin(name = "delta", doc = "Test config fragment.")
- @RequiresOptions(options = {DOptions.class})
- public static final class DConfig extends Fragment implements StarlarkValue {
- private final String value;
-
- public DConfig(BuildOptions buildOptions) {
- this.value = buildOptions.get(DOptions.class).delta;
- }
-
- @Override
- public String getOutputDirectoryName() {
- return "D" + value;
- }
- }
-
- /** Set of test options. */
- public static final class EOptions extends FragmentOptions {
- public static final OptionDefinition ECHO =
- OptionsParser.getOptionDefinitionByName(EOptions.class, "echo");
-
- @Option(
- name = "echo",
- documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
- effectTags = {OptionEffectTag.NO_OP},
- defaultValue = "0")
- public String echo;
- }
-
- /** Test configuration fragment. */
- @StarlarkBuiltin(name = "echo", doc = "Test config fragment.")
- @RequiresOptions(options = {EOptions.class})
- public static final class EConfig extends Fragment implements StarlarkValue {
- private final String value;
-
- public EConfig(BuildOptions buildOptions) {
- this.value = buildOptions.get(EOptions.class).echo;
- }
-
- @Override
- public String getOutputDirectoryName() {
- return "E" + value;
- }
- }
-
- private static final class TestFragmentTransitionFactory implements TransitionFactory<Rule> {
- private static final class SetValuesTransition implements PatchTransition {
- private final String alpha;
- private final String bravo;
- private final String charlie;
- private final String delta;
- private final String echo;
- private final List<Label> platforms;
- private final List<String> extraExecutionPlatforms;
- private final List<String> extraToolchains;
-
- public SetValuesTransition(
- String alpha,
- String bravo,
- String charlie,
- String delta,
- String echo,
- List<Label> platforms,
- List<String> extraExecutionPlatforms,
- List<String> extraToolchains) {
- this.alpha = alpha;
- this.bravo = bravo;
- this.charlie = charlie;
- this.delta = delta;
- this.echo = echo;
- this.platforms = platforms;
- this.extraExecutionPlatforms = extraExecutionPlatforms;
- this.extraToolchains = extraToolchains;
- }
-
- @Override
- public ImmutableSet<Class<? extends FragmentOptions>> requiresOptionFragments() {
- return ImmutableSet.of(
- AOptions.class,
- BOptions.class,
- COptions.class,
- DOptions.class,
- EOptions.class,
- PlatformOptions.class);
- }
-
- @Override
- public BuildOptions patch(BuildOptionsView target, EventHandler eventHandler) {
- BuildOptionsView output = target.clone();
- if (alpha != null) {
- output.get(AOptions.class).alpha = alpha;
- }
- if (bravo != null) {
- output.get(BOptions.class).bravo = bravo;
- }
- if (charlie != null) {
- output.get(COptions.class).charlie = charlie;
- }
- if (delta != null) {
- output.get(DOptions.class).delta = delta;
- }
- if (echo != null) {
- output.get(EOptions.class).echo = echo;
- }
- if (platforms != null) {
- output.get(PlatformOptions.class).platforms = platforms;
- }
- if (extraExecutionPlatforms != null) {
- output.get(PlatformOptions.class).extraExecutionPlatforms = extraExecutionPlatforms;
- }
- if (extraToolchains != null) {
- output.get(PlatformOptions.class).extraToolchains = extraToolchains;
- }
- return output.underlying();
- }
- }
-
- @Override
- public PatchTransition create(Rule rule) {
- AttributeMap attributes = NonconfigurableAttributeMapper.of(rule);
- return new SetValuesTransition(
- attributes.get("alpha", Type.STRING),
- attributes.get("bravo", Type.STRING),
- attributes.get("charlie", Type.STRING),
- attributes.get("delta", Type.STRING),
- attributes.get("echo", Type.STRING),
- attributes.get("platforms", BuildType.NODEP_LABEL_LIST),
- attributes.get("extra_execution_platforms", Type.STRING_LIST),
- attributes.get("extra_toolchains", Type.STRING_LIST));
- }
- }
-
- /** RuleConfiguredTargetFactory which collects dependencies. */
- public static final class DepsCollectingFactory implements RuleConfiguredTargetFactory {
- @Override
- public ConfiguredTarget create(RuleContext ruleContext)
- throws InterruptedException, RuleErrorException, ActionConflictException {
- NestedSetBuilder<Artifact> filesToBuild = NestedSetBuilder.stableOrder();
- filesToBuild.addAll(ruleContext.getOutputArtifacts());
- for (FileProvider dep : ruleContext.getPrerequisites("deps", FileProvider.class)) {
- filesToBuild.addTransitive(dep.getFilesToBuild());
- }
- for (Artifact artifact : ruleContext.getOutputArtifacts()) {
- ruleContext.registerAction(
- FileWriteAction.createEmptyWithInputs(
- ruleContext.getActionOwner(),
- NestedSetBuilder.emptySet(Order.STABLE_ORDER),
- artifact));
- }
- if (ruleContext.getToolchainContext() != null) {
- ResolvedToolchainContext toolchainContext = ruleContext.getToolchainContext();
- for (ToolchainTypeInfo toolchainType : toolchainContext.requiredToolchainTypes()) {
- ToolchainInfo toolchainInfo = toolchainContext.forToolchainType(toolchainType);
- try {
- filesToBuild.addTransitive(
- ((Depset) toolchainInfo.getValue("files")).getSet(Artifact.class));
- } catch (EvalException | Depset.TypeException ex) {
- throw new AssertionError(ex);
- }
- }
- }
- return new RuleConfiguredTargetBuilder(ruleContext)
- .setFilesToBuild(filesToBuild.build())
- .setRunfilesSupport(null, null)
- .add(RunfilesProvider.class, RunfilesProvider.simple(Runfiles.EMPTY))
- .build();
- }
- }
-}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/trimming/TrimmedConfigurationCacheTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/trimming/TrimmedConfigurationCacheTest.java
deleted file mode 100644
index e3ae994..0000000
--- a/src/test/java/com/google/devtools/build/lib/skyframe/trimming/TrimmedConfigurationCacheTest.java
+++ /dev/null
@@ -1,401 +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.skyframe.trimming;
-
-import static com.google.common.truth.Truth.assertThat;
-import static com.google.common.truth.Truth8.assertThat;
-
-import com.google.common.collect.ImmutableMap;
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.JUnit4;
-
-/** Tests for the TrimmedConfigurationCache. */
-@RunWith(JUnit4.class)
-public final class TrimmedConfigurationCacheTest {
-
- private TrimmedConfigurationCache<TestKey, String, ImmutableMap<String, String>> cache;
-
- @Before
- public void initializeCache() {
- cache = TestKey.newCache();
- }
-
- @Test
- public void get_onFreshCache_returnsEmpty() throws Exception {
- assertThat(cache.get(TestKey.parse("<A: 1> //foo"))).isEmpty();
- }
-
- @Test
- public void get_afterAddingSubsetCacheEntry_returnsMatchingValue() throws Exception {
- TestKey canonicalKey = TestKey.parse("<A: 1, B: 1> //foo");
- cache.putIfAbsent(canonicalKey, TestKey.parseConfiguration("A: 1"));
-
- assertThat(cache.get(TestKey.parse("<A: 1, C: 1> //foo"))).hasValue(canonicalKey);
- }
-
- @Test
- public void get_afterRemovingMatchingCacheEntry_returnsEmpty() throws Exception {
- TestKey canonicalKey = TestKey.parse("<A: 1, B: 1> //foo");
- cache.putIfAbsent(canonicalKey, TestKey.parseConfiguration("A: 1"));
- cache.remove(canonicalKey);
-
- assertThat(cache.get(TestKey.parse("<A: 1, B: 2> //foo"))).isEmpty();
- }
-
- @Test
- public void get_afterRemovingCacheEntryWithDifferentConfig_returnsOriginalKey() throws Exception {
- TestKey canonicalKey = TestKey.parse("<A: 1, B: 1> //foo");
- cache.putIfAbsent(canonicalKey, TestKey.parseConfiguration("A: 1"));
- TestKey removedOtherKey = TestKey.parse("<A: 1, B: 2> //foo");
- cache.remove(removedOtherKey);
-
- assertThat(cache.get(canonicalKey)).hasValue(canonicalKey);
- }
-
- @Test
- public void get_afterRemovingCacheEntryWithDifferentDescriptor_returnsOriginalKey()
- throws Exception {
- TestKey canonicalKey = TestKey.parse("<A: 1, B: 1> //foo");
- cache.putIfAbsent(canonicalKey, TestKey.parseConfiguration("A: 1"));
- TestKey removedOtherKey = TestKey.parse("<A: 1, B: 1> //bar");
- cache.remove(removedOtherKey);
-
- assertThat(cache.get(canonicalKey)).hasValue(canonicalKey);
- }
-
- @Test
- public void get_afterClearingMatchingCacheEntry_returnsEmpty() throws Exception {
- cache.putIfAbsent(TestKey.parse("<A: 1, B: 1> //foo"), TestKey.parseConfiguration("A: 1"));
- cache.clear();
-
- assertThat(cache.get(TestKey.parse("<A: 1, B: 2> //foo"))).isEmpty();
- }
-
- @Test
- public void get_afterRemovingAndReAddingCacheEntry_returnsNewValue() throws Exception {
- TestKey oldKey = TestKey.parse("<A: 1, B: 1> //foo");
- ImmutableMap<String, String> trimmedConfiguration = TestKey.parseConfiguration("A: 1");
- cache.putIfAbsent(oldKey, trimmedConfiguration);
- cache.remove(oldKey);
- TestKey newKey = TestKey.parse("<A: 1, C: 1> //foo");
- cache.putIfAbsent(newKey, trimmedConfiguration);
-
- assertThat(cache.get(TestKey.parse("<A: 1, D: 1> //foo"))).hasValue(newKey);
- }
-
- @Test
- public void get_afterAddingMatchingConfigurationForDifferentDescriptor_returnsEmpty()
- throws Exception {
- cache.putIfAbsent(TestKey.parse("<A: 1, B: 1> //bar"), TestKey.parseConfiguration("A: 1"));
-
- assertThat(cache.get(TestKey.parse("<A: 1, B: 1> //foo"))).isEmpty();
- }
-
- @Test
- public void get_afterAddingNonMatchingConfigurationForSameDescriptor_returnsEmpty()
- throws Exception {
- cache.putIfAbsent(TestKey.parse("<A: 1, B: 1> //foo"), TestKey.parseConfiguration("A: 1"));
-
- assertThat(cache.get(TestKey.parse("<A: 2, B: 1> //foo"))).isEmpty();
- }
-
- @Test
- public void get_afterAddingAndInvalidatingMatchingCacheEntry_returnsEmpty() throws Exception {
- TestKey canonicalKey = TestKey.parse("<A: 1, B: 1> //foo");
- cache.putIfAbsent(canonicalKey, TestKey.parseConfiguration("A: 1"));
- cache.invalidate(canonicalKey);
-
- assertThat(cache.get(TestKey.parse("<A: 1, C: 1> //foo"))).isEmpty();
- }
-
- @Test
- public void get_afterAddingAndInvalidatingAndReAddingMatchingCacheEntry_returnsOriginalValue()
- throws Exception {
- TestKey oldKey = TestKey.parse("<A: 1, B: 1> //foo");
- ImmutableMap<String, String> trimmedConfiguration = TestKey.parseConfiguration("A: 1");
- cache.putIfAbsent(oldKey, trimmedConfiguration);
- cache.invalidate(oldKey);
- TestKey newKey = TestKey.parse("<A: 1, C: 1> //foo");
- cache.putIfAbsent(newKey, trimmedConfiguration);
-
- assertThat(cache.get(TestKey.parse("<A: 1, D: 1> //foo"))).hasValue(oldKey);
- }
-
- @Test
- public void get_afterAddingAndInvalidatingAndRevalidatingMatchingCacheEntry_returnsOriginalValue()
- throws Exception {
- TestKey oldKey = TestKey.parse("<A: 1, B: 1> //foo");
- ImmutableMap<String, String> trimmedConfiguration = TestKey.parseConfiguration("A: 1");
- cache.putIfAbsent(oldKey, trimmedConfiguration);
- cache.invalidate(oldKey);
- cache.revalidate(oldKey);
-
- assertThat(cache.get(TestKey.parse("<A: 1, D: 1> //foo"))).hasValue(oldKey);
- }
-
- @Test
- public void get_afterMovingKeyToDifferentTrimming_returnsEmpty() throws Exception {
- TestKey oldKey = TestKey.parse("<A: 1, B: 1> //foo");
- cache.putIfAbsent(oldKey, TestKey.parseConfiguration("A: 1"));
- cache.invalidate(oldKey);
- cache.putIfAbsent(oldKey, TestKey.parseConfiguration("B: 1"));
-
- assertThat(cache.get(TestKey.parse("<A: 1, B: 2> //foo"))).isEmpty();
- }
-
- @Test(expected = IllegalArgumentException.class)
- public void putIfAbsent_forNonSubsetConfiguration_throwsIllegalArgumentException()
- throws Exception {
- cache.putIfAbsent(TestKey.parse("<A: 1> //foo"), TestKey.parseConfiguration("A: 2"));
- }
-
- @Test
- public void putIfAbsent_onFreshCache_returnsInputKey() throws Exception {
- TestKey inputKey = TestKey.parse("<A: 1, B: 1> //foo");
-
- assertThat(cache.putIfAbsent(inputKey, TestKey.parseConfiguration("A: 1"))).isEqualTo(inputKey);
- }
-
- @Test
- public void putIfAbsent_afterRemoving_returnsNewKey() throws Exception {
- TestKey oldKey = TestKey.parse("<A: 1, B: 1> //foo");
- ImmutableMap<String, String> trimmedConfiguration = TestKey.parseConfiguration("A: 1");
- cache.putIfAbsent(oldKey, trimmedConfiguration);
- cache.remove(oldKey);
- TestKey newKey = TestKey.parse("<A: 1, B: 2> //foo");
-
- assertThat(cache.putIfAbsent(newKey, trimmedConfiguration)).isEqualTo(newKey);
- }
-
- @Test
- public void putIfAbsent_afterAddingEqualConfigurationForDifferentDescriptor_returnsInputKey()
- throws Exception {
- TestKey oldKey = TestKey.parse("<A: 1, B: 1> //foo");
- ImmutableMap<String, String> trimmedConfiguration = TestKey.parseConfiguration("A: 1");
- cache.putIfAbsent(oldKey, trimmedConfiguration);
- TestKey newKey = TestKey.parse("<A: 1, B: 1> //bar");
-
- assertThat(cache.putIfAbsent(newKey, trimmedConfiguration)).isEqualTo(newKey);
- }
-
- @Test
- public void putIfAbsent_afterAddingNonEqualConfigurationForSameDescriptor_returnsInputKey()
- throws Exception {
- cache.putIfAbsent(TestKey.parse("<A: 1, B: 1> //foo"), TestKey.parseConfiguration("A: 1"));
- TestKey newKey = TestKey.parse("<A: 2, B: 2> //foo");
-
- assertThat(cache.putIfAbsent(newKey, TestKey.parseConfiguration("A: 2"))).isEqualTo(newKey);
- }
-
- @Test
- public void putIfAbsent_afterAddingEqualConfigurationForSameDescriptor_returnsOriginalKey()
- throws Exception {
- TestKey oldKey = TestKey.parse("<A: 1, B: 1> //foo");
- ImmutableMap<String, String> trimmedConfiguration = TestKey.parseConfiguration("A: 1");
- cache.putIfAbsent(oldKey, trimmedConfiguration);
-
- assertThat(cache.putIfAbsent(TestKey.parse("<A: 1, B: 2> //foo"), trimmedConfiguration))
- .isEqualTo(oldKey);
- }
-
- @Test
- public void putIfAbsent_afterInvalidatingEqualEntry_returnsOriginalKey() throws Exception {
- TestKey oldKey = TestKey.parse("<A: 1, B: 1> //foo");
- ImmutableMap<String, String> trimmedConfiguration = TestKey.parseConfiguration("A: 1");
- cache.putIfAbsent(oldKey, trimmedConfiguration);
- cache.invalidate(oldKey);
-
- assertThat(cache.putIfAbsent(TestKey.parse("<A: 1, B: 2> //foo"), trimmedConfiguration))
- .isEqualTo(oldKey);
- }
-
- @Test
- public void putIfAbsent_forKeyAssociatedWithDifferentTrimming_returnsOldKey() throws Exception {
- TestKey oldKey = TestKey.parse("<A: 1, B: 1> //foo");
- cache.putIfAbsent(oldKey, TestKey.parseConfiguration("A: 1"));
- cache.invalidate(oldKey);
-
- assertThat(cache.putIfAbsent(oldKey, TestKey.parseConfiguration("B: 1"))).isEqualTo(oldKey);
- }
-
- @Test
- public void putIfAbsent_afterMovingPreviousAssociatedKeyToNewTrimming_returnsNewKey()
- throws Exception {
- TestKey oldKey = TestKey.parse("<A: 1, B: 1> //foo");
- ImmutableMap<String, String> trimmedA1 = TestKey.parseConfiguration("A: 1");
- ImmutableMap<String, String> trimmedB1 = TestKey.parseConfiguration("B: 1");
- cache.putIfAbsent(oldKey, trimmedA1);
- cache.invalidate(oldKey);
- cache.putIfAbsent(oldKey, trimmedB1);
- cache.invalidate(oldKey);
- TestKey newKey = TestKey.parse("<A: 1, B: 2> //foo");
-
- // This is testing that oldKey is not still associated with trimmedA1, because now it's
- // associated with trimmedB1 instead.
- assertThat(cache.putIfAbsent(newKey, trimmedA1)).isEqualTo(newKey);
- }
-
- @Test
- public void putIfAbsent_afterInvalidatingAndReAddingEqualEntry_returnsOriginalKey()
- throws Exception {
- TestKey oldKey = TestKey.parse("<A: 1, B: 1> //foo");
- ImmutableMap<String, String> trimmedConfiguration = TestKey.parseConfiguration("A: 1");
- cache.putIfAbsent(oldKey, trimmedConfiguration);
- cache.invalidate(oldKey);
- cache.putIfAbsent(TestKey.parse("<A: 1, B: 2> //foo"), trimmedConfiguration);
-
- assertThat(cache.putIfAbsent(TestKey.parse("<A: 1, B: 3> //foo"), trimmedConfiguration))
- .isEqualTo(oldKey);
- }
-
- @Test
- public void putIfAbsent_afterInvalidatingAndRevalidatingEqualEntry_returnsOriginalKey()
- throws Exception {
- TestKey oldKey = TestKey.parse("<A: 1, B: 1> //foo");
- ImmutableMap<String, String> trimmedConfiguration = TestKey.parseConfiguration("A: 1");
- cache.putIfAbsent(oldKey, trimmedConfiguration);
- cache.invalidate(oldKey);
- cache.revalidate(oldKey);
-
- assertThat(cache.putIfAbsent(TestKey.parse("<A: 1, B: 2> //foo"), trimmedConfiguration))
- .isEqualTo(oldKey);
- }
-
- @Test
- public void putIfAbsent_afterAddingAndInvalidatingSubsetConfiguration_returnsInputKey()
- throws Exception {
- TestKey oldKey = TestKey.parse("<A: 1, B: 1> //foo");
- ImmutableMap<String, String> trimmedConfiguration = TestKey.parseConfiguration("A: 1");
- cache.putIfAbsent(oldKey, trimmedConfiguration);
- cache.invalidate(oldKey);
- TestKey newKey = TestKey.parse("<A: 1, B: 2> //foo");
-
- assertThat(cache.putIfAbsent(newKey, TestKey.parseConfiguration("A: 1, B: 2")))
- .isEqualTo(newKey);
- }
-
- @Test
- public void putIfAbsent_afterAddingAndInvalidatingSupersetConfiguration_returnsInputKey()
- throws Exception {
- TestKey oldKey = TestKey.parse("<A: 1, B: 1, C: 1> //foo");
- cache.putIfAbsent(oldKey, TestKey.parseConfiguration("A: 1, B: 1"));
- cache.invalidate(oldKey);
- TestKey newKey = TestKey.parse("<A: 1, B: 1, C: 2> //foo");
-
- assertThat(cache.putIfAbsent(newKey, TestKey.parseConfiguration("A: 1"))).isEqualTo(newKey);
- }
-
- @Test
- public void invalidate_onEntryNotInCache_doesNotThrow() throws Exception {
- // Expect no exception here.
- cache.invalidate(TestKey.parse("<A: 1> //foo"));
- }
-
- @Test
- public void invalidate_onAlreadyInvalidatedEntry_doesNotThrow() throws Exception {
- TestKey oldKey = TestKey.parse("<A: 1, B: 1> //foo");
- cache.putIfAbsent(oldKey, TestKey.parseConfiguration("A: 1"));
- cache.invalidate(oldKey);
-
- // Expect no exception here.
- cache.invalidate(oldKey);
- }
-
- @Test
- public void invalidate_onRemovedEntry_doesNotThrow() throws Exception {
- TestKey oldKey = TestKey.parse("<A: 1, B: 1> //foo");
- cache.putIfAbsent(oldKey, TestKey.parseConfiguration("A: 1"));
- cache.remove(oldKey);
-
- // Expect no exception here.
- cache.invalidate(oldKey);
- }
-
- @Test
- public void revalidate_onEntryNotInCache_doesNotThrow() throws Exception {
- // Expect no exception here.
- cache.revalidate(TestKey.parse("<A: 1> //foo"));
- }
-
- @Test
- public void revalidate_onAlreadyInvalidatedAndRevalidatedEntry_doesNotThrow() throws Exception {
- TestKey oldKey = TestKey.parse("<A: 1, B: 1> //foo");
- cache.putIfAbsent(oldKey, TestKey.parseConfiguration("A: 1"));
- cache.invalidate(oldKey);
- cache.revalidate(oldKey);
-
- // Expect no exception here.
- cache.revalidate(oldKey);
- }
-
- @Test
- public void revalidate_onRemovedEntry_doesNotThrow() throws Exception {
- TestKey oldKey = TestKey.parse("<A: 1, B: 1> //foo");
- cache.putIfAbsent(oldKey, TestKey.parseConfiguration("A: 1"));
- cache.remove(oldKey);
-
- // Expect no exception here.
- cache.revalidate(oldKey);
- }
-
- @Test
- public void remove_onEntryNotInCache_doesNotThrow() throws Exception {
- // Expect no exception here.
- cache.remove(TestKey.parse("<A: 1> //foo"));
- }
-
- @Test
- public void remove_onAlreadyInvalidatedEntry_doesNotThrow() throws Exception {
- TestKey oldKey = TestKey.parse("<A: 1, B: 1> //foo");
- cache.putIfAbsent(oldKey, TestKey.parseConfiguration("A: 1"));
- cache.invalidate(oldKey);
-
- // Expect no exception here.
- cache.remove(oldKey);
- }
-
- @Test
- public void remove_onEntryWithDifferentConfiguration_doesNotThrow() throws Exception {
- cache.putIfAbsent(TestKey.parse("<A: 1, B: 1> //foo"), TestKey.parseConfiguration("A: 1"));
-
- // Expect no exception here.
- cache.remove(TestKey.parse("<A: 1, B: 2> //foo"));
- }
-
- @Test
- public void remove_onEntryWithDifferentDescriptor_doesNotThrow() throws Exception {
- cache.putIfAbsent(TestKey.parse("<A: 1, B: 1> //foo"), TestKey.parseConfiguration("A: 1"));
-
- // Expect no exception here.
- cache.remove(TestKey.parse("<A: 1, B: 1> //bar"));
- }
-
- @Test
- public void remove_onRemovedEntry_doesNotThrow() throws Exception {
- TestKey oldKey = TestKey.parse("<A: 1, B: 1> //foo");
- cache.putIfAbsent(oldKey, TestKey.parseConfiguration("A: 1"));
- cache.remove(oldKey);
-
- // Expect no exception here.
- cache.remove(oldKey);
- }
-
- @Test
- public void clear_onEmptyCache_doesNotThrow() throws Exception {
- cache.clear();
- }
-}
