Use an aspect to calculate the Android neverlink libraries.

This cuts the .java -> .android dependency between Java packages, which is nice at the cost of some cavalier over-estimation of the direct dependencies in AndroidNeverlinkAspect.

--
MOS_MIGRATED_REVID=94745544

5 files changed, 125 insertions, 23 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java
index f0a4778e00..2a36d8df5f 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java
@@ -294,7 +294,6 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
         deployJar,
         filesBuilder,
         resourceApk,
-        applicationManifest,
         ruleContext.getPrerequisiteArtifacts(PROGUARD_SPECS, Mode.TARGET).list(),
         proguardMapping,
         tools);
@@ -629,12 +628,11 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
   }
 
   /** Applies the proguard specifications, and creates a ProguardedJar. */
-  public static ProguardOutput applyProguard(RuleContext ruleContext,
+  private static ProguardOutput applyProguard(RuleContext ruleContext,
       AndroidCommon common,
       Artifact deployJarArtifact,
       NestedSetBuilder<Artifact> filesBuilder,
       ResourceApk resourceApk,
-      ApplicationManifest applicationManifest,
       ImmutableList<Artifact> proguardSpecs,
       Artifact proguardMapping,
       AndroidTools tools) {
@@ -658,8 +656,9 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
         proguardSpecs, proguardMapping, tools.getAndroidJar(), proguardOutputJar, filesBuilder);
   }
 
-  public static ProguardOutput createProguardAction(RuleContext ruleContext, AndroidCommon common,
-      FilesToRunProvider proguard, Artifact jar, ImmutableList<Artifact> proguardSpecs,
+  private static ProguardOutput createProguardAction(RuleContext ruleContext, AndroidCommon common,
+      FilesToRunProvider proguard,
+      Artifact jar, ImmutableList<Artifact> proguardSpecs,
       Artifact proguardMapping, Artifact androidJar, Artifact proguardOutputJar,
       NestedSetBuilder<Artifact> filesBuilder) {
     Iterable<Artifact> libraryJars = NestedSetBuilder.<Artifact>naiveLinkOrder()
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java
index 08984b49ce..093a7fe9f3 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java
@@ -23,6 +23,7 @@ import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.MiddlemanFactory;
 import com.google.devtools.build.lib.actions.ResourceSet;
+import com.google.devtools.build.lib.analysis.AnalysisUtils;
 import com.google.devtools.build.lib.analysis.FilesToRunProvider;
 import com.google.devtools.build.lib.analysis.OutputGroupProvider;
 import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
@@ -54,6 +55,7 @@ import com.google.devtools.build.lib.rules.java.JavaCompilationArgsProvider;
 import com.google.devtools.build.lib.rules.java.JavaCompilationArtifacts;
 import com.google.devtools.build.lib.rules.java.JavaCompilationHelper;
 import com.google.devtools.build.lib.rules.java.JavaNativeLibraryProvider;
+import com.google.devtools.build.lib.rules.java.JavaRuntimeJarProvider;
 import com.google.devtools.build.lib.rules.java.JavaSemantics;
 import com.google.devtools.build.lib.rules.java.JavaSourceJarsProvider;
 import com.google.devtools.build.lib.rules.java.JavaTargetAttributes;
@@ -102,7 +104,7 @@ public class AndroidCommon {
   public AndroidCommon(RuleContext ruleContext, JavaCommon javaCommon) {
     this.ruleContext = ruleContext;
     this.javaCommon = javaCommon;
-    this.asNeverLink = this.javaCommon.isNeverLink();
+    this.asNeverLink = JavaCommon.isNeverLink(ruleContext);
   }
 
   /**
@@ -123,24 +125,31 @@ public class AndroidCommon {
 
   /**
    * Collects the transitive neverlink dependencies.
+   *
+   * @param ruleContext the context of the rule neverlink deps are to be computed for
+   * @param deps the targets to be treated as dependencies
+   * @param runtimeJars the runtime jars produced by the rule (non-transitive)
+   *
+   * @return a nested set of the neverlink deps.
    */
   public static NestedSet<Artifact> collectTransitiveNeverlinkLibraries(
-      RuleContext ruleContext, JavaCommon javaCommon) {
+      RuleContext ruleContext, Iterable<? extends TransitiveInfoCollection> deps,
+      ImmutableList<Artifact> runtimeJars) {
     NestedSetBuilder<Artifact> builder = NestedSetBuilder.naiveLinkOrder();
-    for (AndroidNeverLinkLibrariesProvider dep :
-        javaCommon.getDependencies(AndroidNeverLinkLibrariesProvider.class)) {
-      builder.addTransitive(dep.getTransitiveNeverLinkLibraries());
+
+    for (AndroidNeverLinkLibrariesProvider provider : AnalysisUtils.getProviders(deps,
+        AndroidNeverLinkLibrariesProvider.class)) {
+      builder.addTransitive(provider.getTransitiveNeverLinkLibraries());
     }
-    if (javaCommon.isNeverLink()) {
-      builder.addAll(javaCommon.getJavaCompilationArtifacts().getRuntimeJars());
-      for (TransitiveInfoCollection dep : javaCommon.getDependencies()) {
-        if (dep.getProvider(AndroidNeverLinkLibrariesProvider.class) != null) {
-          JavaCompilationArgsProvider javaArgs = dep.getProvider(JavaCompilationArgsProvider.class);
-          Preconditions.checkState(javaArgs != null, ruleContext.getLabel());
-          builder.addTransitive(javaArgs.getRecursiveJavaCompilationArgs().getRuntimeJars());
-        }
+
+    if (JavaCommon.isNeverLink(ruleContext)) {
+      builder.addAll(runtimeJars);
+      for (JavaCompilationArgsProvider provider : AnalysisUtils.getProviders(
+          deps, JavaCompilationArgsProvider.class)) {
+        builder.addTransitive(provider.getRecursiveJavaCompilationArgs().getRuntimeJars());
       }
     }
+
     return builder.build();
   }
 
@@ -419,7 +428,10 @@ public class AndroidCommon {
     javaCommon.setClassPathFragment(new ClasspathConfiguredFragment(
         javaCommon.getJavaCompilationArtifacts(), attributes, asNeverLink));
     
-    transitiveNeverlinkLibraries = collectTransitiveNeverlinkLibraries(ruleContext, javaCommon);
+    transitiveNeverlinkLibraries = collectTransitiveNeverlinkLibraries(
+        ruleContext,
+        javaCommon.getDependencies(),
+        javaCommon.getJavaCompilationArtifacts().getRuntimeJars());
     topLevelSourceJars = ImmutableList.of(srcJar);
     transitiveSourceJars = javaCommon.collectTransitiveSourceJars(srcJar);
 
@@ -446,6 +458,8 @@ public class AndroidCommon {
 
     return builder
         .setFilesToBuild(filesToBuild)
+        .add(JavaRuntimeJarProvider.class,
+            new JavaRuntimeJarProvider(javaCommon.getJavaCompilationArtifacts().getRuntimeJars()))
         .add(RunfilesProvider.class, RunfilesProvider.simple(runfiles))
         .add(AndroidResourcesProvider.class, new AndroidResourcesProvider(
             ruleContext.getLabel(), transitiveResources))
@@ -454,8 +468,6 @@ public class AndroidCommon {
             javaCompilationArgs, recursiveJavaCompilationArgs,
             compileTimeDependencyArtifacts,
             NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER)))
-        .add(AndroidNeverLinkLibrariesProvider.class, new AndroidNeverLinkLibrariesProvider(
-            transitiveNeverlinkLibraries))
         .addOutputGroup(OutputGroupProvider.HIDDEN_TOP_LEVEL,
             collectHiddenTopLevelArtifacts(ruleContext))
         .addOutputGroup(JavaSemantics.SOURCE_JARS_OUTPUT_GROUP, transitiveSourceJars);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibraryBaseRule.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibraryBaseRule.java
index 0d3ebb8fd1..06e9b0eb6d 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibraryBaseRule.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibraryBaseRule.java
@@ -29,6 +29,7 @@ import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
 import com.google.devtools.build.lib.rules.android.AndroidRuleClasses.AndroidAaptBaseRule;
 import com.google.devtools.build.lib.rules.android.AndroidRuleClasses.AndroidBaseRule;
 import com.google.devtools.build.lib.rules.android.AndroidRuleClasses.AndroidResourceSupportRule;
+import com.google.devtools.build.lib.rules.java.JavaCompilationArgsProvider;
 import com.google.devtools.build.lib.rules.java.JavaSemantics;
 
 /**
@@ -156,6 +157,7 @@ public final class AndroidLibraryBaseRule implements RuleDefinition {
         }))
         .add(attr("$android_manifest_merge_tool", LABEL).cfg(HOST).exec().value(env.getLabel(
             AndroidRuleClasses.MANIFEST_MERGE_TOOL_LABEL)))
+        .advertiseProvider(JavaCompilationArgsProvider.class)
         .build();
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidNeverlinkAspect.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidNeverlinkAspect.java
new file mode 100644
index 0000000000..189b6fb736
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidNeverlinkAspect.java
@@ -0,0 +1,84 @@
+// Copyright 2015 Google Inc. 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.rules.android;
+
+import com.google.common.collect.ImmutableList;
+import com.google.devtools.build.lib.analysis.Aspect;
+import com.google.devtools.build.lib.analysis.ConfiguredAspectFactory;
+import com.google.devtools.build.lib.analysis.ConfiguredTarget;
+import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
+import com.google.devtools.build.lib.analysis.RuleContext;
+import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
+import com.google.devtools.build.lib.packages.AspectDefinition;
+import com.google.devtools.build.lib.packages.Type;
+import com.google.devtools.build.lib.rules.java.JavaCommon;
+import com.google.devtools.build.lib.rules.java.JavaCompilationArgsProvider;
+import com.google.devtools.build.lib.rules.java.JavaRuntimeJarProvider;
+
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * An aspect that collects neverlink libraries in the transitive closure.
+ *
+ * <p>Used for determining the -libraryjars argument of the Proguard invocation in Android binaries.
+ *
+ * <p>One would think that using the compile time classpath would be enough, but alas, those are
+ * ijars,
+ */
+public class AndroidNeverlinkAspect implements ConfiguredAspectFactory {
+  private static final ImmutableList<String> ATTRIBUTES = ImmutableList.of(
+      "deps", "exports", "runtime_deps", "binary_under_test", "$instrumentation_test_runner");
+
+  @Override
+  public Aspect create(ConfiguredTarget base, RuleContext ruleContext) {
+    if (!JavaCommon.getConstraints(ruleContext).contains("android")
+        && !ruleContext.getRule().getRuleClass().startsWith("android_")) {
+      return new Aspect.Builder().build();
+    }
+
+    List<TransitiveInfoCollection> deps = new ArrayList<>();
+
+    // This is probably an overestimate, but it's fine -- Proguard doesn't care if there are more
+    // jars in -libraryjars than is required. The alternative would be somehow getting
+    // JavaCommon.getDependencies() here, which would be fugly.
+    for (String attribute : ATTRIBUTES) {
+      if (!ruleContext.getRule().getRuleClassObject().hasAttr(attribute, Type.LABEL_LIST)) {
+        continue;
+      }
+
+      deps.addAll(ruleContext.getPrerequisites(attribute, Mode.TARGET));
+    }
+
+    return new Aspect.Builder()
+        .addProvider(AndroidNeverLinkLibrariesProvider.class,
+            new AndroidNeverLinkLibrariesProvider(AndroidCommon.collectTransitiveNeverlinkLibraries(
+                ruleContext,
+                deps,
+                base.getProvider(JavaRuntimeJarProvider.class).getRuntimeJars())))
+        .build();
+  }
+
+  @Override
+  public AspectDefinition getDefinition() {
+    AspectDefinition.Builder builder = new AspectDefinition.Builder("AndroidNeverlinkAspect");
+    for (String attribute : ATTRIBUTES) {
+      builder.attributeAspect(attribute, AndroidNeverlinkAspect.class);
+    }
+
+    return builder
+        .requireProvider(JavaCompilationArgsProvider.class)
+        .build();
+  }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java
index 6664288934..f859fc47e7 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java
@@ -45,6 +45,7 @@ import com.google.devtools.build.lib.packages.RuleClass.Builder;
 import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
 import com.google.devtools.build.lib.packages.TriState;
 import com.google.devtools.build.lib.packages.Type;
+import com.google.devtools.build.lib.rules.java.JavaCompilationArgsProvider;
 import com.google.devtools.build.lib.rules.java.JavaSemantics;
 import com.google.devtools.build.lib.syntax.Label;
 import com.google.devtools.build.lib.util.FileType;
@@ -551,8 +552,11 @@ public final class AndroidRuleClasses {
           <code>cc_library</code> wrapping or producing <code>.so</code> native libraries for the
           Android target platform.
           <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
-          .override(builder.copy("deps").cfg(ANDROID_SPLIT_TRANSITION)
-              .allowedRuleClasses(ALLOWED_DEPENDENCIES).allowedFileTypes())
+          .override(builder.copy("deps")
+              .cfg(ANDROID_SPLIT_TRANSITION)
+              .allowedRuleClasses(ALLOWED_DEPENDENCIES)
+              .allowedFileTypes()
+              .aspect(AndroidNeverlinkAspect.class))
           // Proguard rule specifying master list of classes to keep during legacy multidexing.
           .add(attr("$build_incremental_dexmanifest", LABEL).cfg(HOST).exec()
               .value(env.getLabel(AndroidRuleClasses.BUILD_INCREMENTAL_DEXMANIFEST_LABEL)))
@@ -664,6 +668,7 @@ com/google/common/base/Objects.class
             </ul>
           <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
           .add(attr("legacy_native_support", TRISTATE).value(TriState.AUTO))
+          .advertiseProvider(JavaCompilationArgsProvider.class)
           .build();
       }