BuildView - untangle more of the methods that are only for ide info.
In particular, don't immediately call into the ForTesting functions; I need to
refactor some code that is called from here, and the semantics when called
from ide info should not change. Changes to semantics when called from tests
are much less problematic - we can simply run all the tests.
--
MOS_MIGRATED_REVID=111846384
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 025d8b5..862aa0f 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
@@ -18,6 +18,7 @@
import com.google.common.base.Function;
import com.google.common.base.Predicate;
import com.google.common.cache.LoadingCache;
+import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.ImmutableMap;
@@ -775,29 +776,14 @@
public Iterable<ConfiguredTarget> getDirectPrerequisitesForIdeInfo(
EventHandler eventHandler, ConfiguredTarget ct, BuildConfigurationCollection configurations)
throws InterruptedException {
- return getDirectPrerequisitesForTesting(eventHandler, ct, configurations);
- }
-
- public Iterable<Dependency> getDirectPrerequisiteDependenciesForIdeInfo(
- EventHandler eventHandler, ConfiguredTarget ct,
- @Nullable final LoadingCache<Label, Target> targetCache,
- BuildConfigurationCollection configurations) throws InterruptedException {
- return getDirectPrerequisiteDependenciesForTesting(
- eventHandler, ct, targetCache, configurations);
- }
-
- // For testing
- @VisibleForTesting
- public Iterable<ConfiguredTarget> getDirectPrerequisitesForTesting(
- EventHandler eventHandler, ConfiguredTarget ct, BuildConfigurationCollection configurations)
- throws InterruptedException {
return skyframeExecutor.getConfiguredTargets(
eventHandler, ct.getConfiguration(),
- getDirectPrerequisiteDependenciesForTesting(eventHandler, ct, null, configurations), false);
+ getDirectPrerequisiteDependenciesForIdeInfo(eventHandler, ct, null, configurations),
+ false);
}
@VisibleForTesting
- public Iterable<Dependency> getDirectPrerequisiteDependenciesForTesting(
+ public Iterable<Dependency> getDirectPrerequisiteDependenciesForIdeInfo(
final EventHandler eventHandler, ConfiguredTarget ct,
@Nullable final LoadingCache<Label, Target> targetCache,
BuildConfigurationCollection configurations) throws InterruptedException {
@@ -817,10 +803,14 @@
}
@Override
- protected Target getTarget(Label label) throws NoSuchThingException {
+ protected Target getTarget(Label label) {
if (targetCache == null) {
- return LoadedPackageProvider.Bridge.getLoadedTarget(
- skyframeExecutor.getPackageManager(), eventHandler, label);
+ try {
+ return LoadedPackageProvider.Bridge.getLoadedTarget(
+ skyframeExecutor.getPackageManager(), eventHandler, label);
+ } catch (NoSuchThingException e) {
+ throw new IllegalStateException(e);
+ }
}
try {
@@ -835,7 +825,58 @@
DependencyResolver dependencyResolver = new SilentDependencyResolver();
TargetAndConfiguration ctgNode =
new TargetAndConfiguration(ct.getTarget(), ct.getConfiguration());
- return dependencyResolver.dependentNodes(ctgNode, configurations.getHostConfiguration(),
+ try {
+ return ImmutableSet.copyOf(dependencyResolver.dependentNodeMap(
+ ctgNode, configurations.getHostConfiguration(), /*aspect=*/ null,
+ getConfigurableAttributeKeysForTesting(eventHandler, ctgNode)).values());
+ } catch (EvalException e) {
+ throw new IllegalStateException(e);
+ }
+ }
+
+ // For testing
+ @VisibleForTesting
+ public Iterable<ConfiguredTarget> getDirectPrerequisitesForTesting(
+ EventHandler eventHandler, ConfiguredTarget ct, BuildConfigurationCollection configurations)
+ throws EvalException, InterruptedException {
+ return skyframeExecutor.getConfiguredTargets(
+ eventHandler, ct.getConfiguration(),
+ ImmutableSet.copyOf(
+ getDirectPrerequisiteDependenciesForTesting(eventHandler, ct, configurations).values()),
+ false);
+ }
+
+ @VisibleForTesting
+ public ListMultimap<Attribute, Dependency> getDirectPrerequisiteDependenciesForTesting(
+ final EventHandler eventHandler, ConfiguredTarget ct,
+ BuildConfigurationCollection configurations) throws EvalException, InterruptedException {
+ if (!(ct.getTarget() instanceof Rule)) {
+ return ArrayListMultimap.create();
+ }
+
+ class SilentDependencyResolver extends DependencyResolver {
+ @Override
+ protected void invalidVisibilityReferenceHook(TargetAndConfiguration node, Label label) {
+ // The error must have been reported already during analysis.
+ }
+
+ @Override
+ protected void invalidPackageGroupReferenceHook(TargetAndConfiguration node, Label label) {
+ // The error must have been reported already during analysis.
+ }
+
+ @Override
+ protected Target getTarget(Label label) throws NoSuchThingException {
+ return LoadedPackageProvider.Bridge.getLoadedTarget(
+ skyframeExecutor.getPackageManager(), eventHandler, label);
+ }
+ }
+
+ DependencyResolver dependencyResolver = new SilentDependencyResolver();
+ TargetAndConfiguration ctgNode =
+ new TargetAndConfiguration(ct.getTarget(), ct.getConfiguration());
+ return dependencyResolver.dependentNodeMap(
+ ctgNode, configurations.getHostConfiguration(), /*aspect=*/ null,
getConfigurableAttributeKeysForTesting(eventHandler, ctgNode));
}
@@ -866,40 +907,13 @@
private ListMultimap<Attribute, ConfiguredTarget> getPrerequisiteMapForTesting(
final EventHandler eventHandler, ConfiguredTarget target,
- BuildConfigurationCollection configurations) throws InterruptedException {
- DependencyResolver resolver = new DependencyResolver() {
- @Override
- protected void invalidVisibilityReferenceHook(TargetAndConfiguration node, Label label) {
- throw new RuntimeException("bad visibility on " + label + " during testing unexpected");
- }
-
- @Override
- protected void invalidPackageGroupReferenceHook(TargetAndConfiguration node, Label label) {
- throw new RuntimeException("bad package group on " + label + " during testing unexpected");
- }
-
- @Override
- protected Target getTarget(Label label) throws NoSuchThingException {
- return LoadedPackageProvider.Bridge.getLoadedTarget(
- skyframeExecutor.getPackageManager(), eventHandler, label);
- }
- };
- TargetAndConfiguration ctNode = new TargetAndConfiguration(target);
- ListMultimap<Attribute, Dependency> depNodeNames;
- try {
- depNodeNames =
- resolver.dependentNodeMap(
- ctNode,
- configurations.getHostConfiguration(),
- /*aspect=*/ null,
- getConfigurableAttributeKeysForTesting(eventHandler, ctNode));
- } catch (EvalException e) {
- throw new IllegalStateException(e);
- }
+ BuildConfigurationCollection configurations) throws EvalException, InterruptedException {
+ ListMultimap<Attribute, Dependency> depNodeNames = getDirectPrerequisiteDependenciesForTesting(
+ eventHandler, target, configurations);
ImmutableMap<Dependency, ConfiguredTarget> cts = skyframeExecutor.getConfiguredTargetMap(
eventHandler,
- ctNode.getConfiguration(), ImmutableSet.copyOf(depNodeNames.values()), false);
+ target.getConfiguration(), ImmutableSet.copyOf(depNodeNames.values()), false);
ImmutableListMultimap.Builder<Attribute, ConfiguredTarget> builder =
ImmutableListMultimap.builder();
@@ -925,7 +939,8 @@
@VisibleForTesting
public RuleContext getRuleContextForTesting(
ConfiguredTarget target, StoredEventHandler eventHandler,
- BuildConfigurationCollection configurations, BinTools binTools) throws InterruptedException {
+ BuildConfigurationCollection configurations, BinTools binTools)
+ throws EvalException, InterruptedException {
BuildConfiguration targetConfig = target.getConfiguration();
CachingAnalysisEnvironment env =
new CachingAnalysisEnvironment(getArtifactFactory(),
@@ -942,7 +957,7 @@
@VisibleForTesting
public RuleContext getRuleContextForTesting(EventHandler eventHandler, ConfiguredTarget target,
AnalysisEnvironment env, BuildConfigurationCollection configurations)
- throws InterruptedException {
+ throws EvalException, InterruptedException {
BuildConfiguration targetConfig = target.getConfiguration();
return new RuleContext.Builder(
env, (Rule) target.getTarget(), targetConfig, configurations.getHostConfiguration(),
@@ -964,7 +979,7 @@
public ConfiguredTarget getPrerequisiteConfiguredTargetForTesting(
EventHandler eventHandler, ConfiguredTarget dependentTarget, Label desiredTarget,
BuildConfigurationCollection configurations)
- throws InterruptedException {
+ throws EvalException, InterruptedException {
Collection<ConfiguredTarget> configuredTargets =
getPrerequisiteMapForTesting(eventHandler, dependentTarget, configurations).values();
for (ConfiguredTarget ct : configuredTargets) {
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java b/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java
index 452a2aa..d5a515a 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java
@@ -499,22 +499,6 @@
}
/**
- * A variant of {@link #dependentNodeMap} that only returns the values of the resulting map, and
- * also converts any internally thrown {@link EvalException} instances into {@link
- * IllegalStateException}.
- */
- public final Collection<Dependency> dependentNodes(
- TargetAndConfiguration node, BuildConfiguration hostConfig,
- Set<ConfigMatchingProvider> configConditions) throws InterruptedException {
- try {
- return ImmutableSet.copyOf(
- dependentNodeMap(node, hostConfig, /*aspect=*/ null, configConditions).values());
- } catch (EvalException e) {
- throw new IllegalStateException(e);
- }
- }
-
- /**
* Converts the given multimap of attributes to labels into a multi map of attributes to
* {@link Dependency} objects using the proper configuration transition for each attribute.
*
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java b/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java
index 23d0354..5a2d15f 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java
@@ -315,7 +315,7 @@
update("//package:top");
ConfiguredTarget top = getConfiguredTarget("//package:top", getTargetConfiguration());
Iterable<Dependency> targets = getView().getDirectPrerequisiteDependenciesForTesting(
- reporter, top, null, getBuildConfigurationCollection());
+ reporter, top, getBuildConfigurationCollection()).values();
Dependency innerDependency;
Dependency fileDependency;
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 623729f..32f8027 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
@@ -387,7 +387,7 @@
* the action graph.
*/
protected Iterable<ConfiguredTarget> getDirectPrerequisites(ConfiguredTarget target)
- throws InterruptedException {
+ throws Exception {
return view.getDirectPrerequisitesForTesting(reporter, target, masterConfig);
}
@@ -400,7 +400,7 @@
// requesting "//go:two" as a dependency. So the configured targets aren't considered "equal".
// Once we apply dynamic configs to top-level targets this discrepancy will go away.
protected void assertDirectPrerequisitesContain(ConfiguredTarget target, ConfiguredTarget dep)
- throws InterruptedException {
+ throws Exception {
Iterable<ConfiguredTarget> prereqs = getDirectPrerequisites(target);
BuildConfiguration depConfig = dep.getConfiguration();
for (ConfiguredTarget contained : prereqs) {
@@ -437,7 +437,7 @@
* Creates and returns a rule context that is equivalent to the one that was used to create the
* given configured target.
*/
- protected RuleContext getRuleContext(ConfiguredTarget target) throws InterruptedException {
+ protected RuleContext getRuleContext(ConfiguredTarget target) throws Exception {
return view.getRuleContextForTesting(
reporter, target, new StubAnalysisEnvironment(), masterConfig);
}
@@ -447,7 +447,7 @@
* that was used to create the given configured target.
*/
protected RuleContext getRuleContextForSkylark(ConfiguredTarget target)
- throws InterruptedException {
+ throws Exception {
// TODO(bazel-team): we need this horrible workaround because CachingAnalysisEnvironment
// only works with StoredErrorEventListener despite the fact it accepts the interface
// ErrorEventListener, so it's not possible to create it with reporter.
@@ -469,7 +469,7 @@
* the action graph.
*/
protected List<? extends TransitiveInfoCollection> getPrerequisites(ConfiguredTarget target,
- String attributeName) throws InterruptedException {
+ String attributeName) throws Exception {
return getRuleContext(target).getConfiguredTargetMap().get(attributeName);
}
@@ -480,7 +480,7 @@
* the action graph.
*/
protected <C extends TransitiveInfoProvider> Iterable<C> getPrerequisites(ConfiguredTarget target,
- String attributeName, Class<C> classType) throws InterruptedException {
+ String attributeName, Class<C> classType) throws Exception {
return AnalysisUtils.getProviders(getPrerequisites(target, attributeName), classType);
}
@@ -491,7 +491,7 @@
* the action graph.
*/
protected ImmutableList<Artifact> getPrerequisiteArtifacts(
- ConfiguredTarget target, String attributeName) throws InterruptedException {
+ ConfiguredTarget target, String attributeName) throws Exception {
Set<Artifact> result = new LinkedHashSet<>();
for (FileProvider provider : getPrerequisites(target, attributeName, FileProvider.class)) {
Iterables.addAll(result, provider.getFilesToBuild());
