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 19238c9e008120..696dedfbbb7219 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 @@ -54,6 +54,7 @@ import java.util.LinkedHashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.stream.Collectors; import javax.annotation.Nullable; @@ -75,7 +76,8 @@ private interface DependencyKind { /** * The attribute through which a dependency arises. * - *

Should only be called for dependency kinds representing an attribute. + *

Returns {@code null} for visibility, the dependency pointing from an output file to its + * generating rule and toolchain dependencies. */ @Nullable Attribute getAttribute(); @@ -95,7 +97,7 @@ private NonAttributeDependencyKind() {} @Override public Attribute getAttribute() { - throw new IllegalStateException(); + return null; } @Nullable @@ -337,6 +339,29 @@ public final OrderedSetMultimap dependentNodeMap( PartiallyResolvedDependency.of(toLabel, attributeTransition, propagatingAspects.build())); } + Set illegalVisibilityDeps = new LinkedHashSet<>(); + for (PartiallyResolvedDependency dep : partiallyResolvedDeps.get(VISIBILITY_DEPENDENCY)) { + Target toTarget = targetMap.get(dep.getLabel()); + if (toTarget == null) { + // Dependency pointing to non-existent target. This error was reported above, so we can just + // ignore this dependency. + } + + if (!(toTarget instanceof PackageGroup)) { + // Note that this error could also be caught in + // AbstractConfiguredTarget.convertVisibility(), but we have an + // opportunity here to avoid dependency cycles that result from + // the visibility attribute of a rule referring to a rule that + // depends on it (instead of its package) + invalidPackageGroupReferenceHook(node, dep.getLabel()); + illegalVisibilityDeps.add(dep); + } + } + + for (PartiallyResolvedDependency illegalVisibilityDep : illegalVisibilityDeps) { + partiallyResolvedDeps.remove(VISIBILITY_DEPENDENCY, illegalVisibilityDep); + } + OrderedSetMultimap outgoingEdges = OrderedSetMultimap.create(); for (Map.Entry entry : @@ -361,29 +386,6 @@ public final OrderedSetMultimap dependentNodeMap( continue; } - if (entry.getKey() == VISIBILITY_DEPENDENCY) { - if (toTarget instanceof PackageGroup) { - outgoingEdges.put(null, Dependency.withNullConfiguration(dep.getLabel())); - } else { - // Note that this error could also be caught in - // AbstractConfiguredTarget.convertVisibility(), but we have an - // opportunity here to avoid dependency cycles that result from - // the visibility attribute of a rule referring to a rule that - // depends on it (instead of its package) - invalidPackageGroupReferenceHook(node, dep.getLabel()); - } - - continue; - } - - if (entry.getKey() == OUTPUT_FILE_RULE_DEPENDENCY) { - outgoingEdges.put( - null, - Dependency.withTransitionAndAspects( - dep.getLabel(), NoTransition.INSTANCE, AspectCollection.EMPTY)); - continue; - } - ConfigurationTransition transition = TransitionResolver.evaluateTransition( node.getConfiguration(), dep.getTransition(), toTarget, trimmingTransitionFactory);