Integrate import_deps_checker into aar_import.

RELNOTES: Enable dependency checking for aar_import targets.
PiperOrigin-RevId: 188912126

6 files changed, 241 insertions, 21 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/BUILD b/src/main/java/com/google/devtools/build/lib/BUILD
index 5dcca6bb65..f1833dc226 100644
--- a/src/main/java/com/google/devtools/build/lib/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/BUILD
@@ -878,6 +878,7 @@ java_library(
         "rules/java/BuildInfoPropertiesTranslator.java",
         "rules/java/ClasspathConfiguredFragment.java",
         "rules/java/DeployArchiveBuilder.java",
+        "rules/java/ImportDepsCheckActionBuilder.java",
         "rules/java/JavaBuildInfoFactory.java",
         "rules/java/JavaCommon.java",
         "rules/java/JavaCompilationArgs.java",
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AarImport.java b/src/main/java/com/google/devtools/build/lib/rules/android/AarImport.java
index d0e1f37621..d75adb4d95 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AarImport.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AarImport.java
@@ -18,6 +18,8 @@ import com.google.devtools.build.lib.actions.Action;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
+import com.google.devtools.build.lib.analysis.FileProvider;
+import com.google.devtools.build.lib.analysis.OutputGroupInfo;
 import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder;
 import com.google.devtools.build.lib.analysis.RuleConfiguredTargetFactory;
 import com.google.devtools.build.lib.analysis.RuleContext;
@@ -28,9 +30,12 @@ import com.google.devtools.build.lib.analysis.actions.SpawnAction;
 import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
+import com.google.devtools.build.lib.rules.java.ImportDepsCheckActionBuilder;
 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.JavaCompilationArtifacts;
+import com.google.devtools.build.lib.rules.java.JavaConfiguration;
+import com.google.devtools.build.lib.rules.java.JavaConfiguration.ImportDepsCheckingLevel;
 import com.google.devtools.build.lib.rules.java.JavaInfo;
 import com.google.devtools.build.lib.rules.java.JavaRuleOutputJarsProvider;
 import com.google.devtools.build.lib.rules.java.JavaRuntimeInfo;
@@ -77,9 +82,6 @@ public class AarImport implements RuleConfiguredTargetFactory {
 
     // AndroidManifest.xml is required in every AAR.
     Artifact androidManifestArtifact = createAarArtifact(ruleContext, ANDROID_MANIFEST);
-    ruleContext.registerAction(
-        createSingleFileExtractorActions(
-            ruleContext, aar, ANDROID_MANIFEST, androidManifestArtifact));
 
     Artifact resources = createAarTreeArtifact(ruleContext, "resources");
     Artifact assets = createAarTreeArtifact(ruleContext, "assets");
@@ -128,6 +130,14 @@ public class AarImport implements RuleConfiguredTargetFactory {
             /* compileDeps = */ targets,
             /* runtimeDeps = */ targets,
             /* bothDeps = */ targets);
+    // Need to compute the "deps" here, as mergedJar is put on the classpath later too.
+    NestedSet<Artifact> deps =
+        common
+            .collectJavaCompilationArgs(
+                /* recursive = */ true,
+                JavaCommon.isNeverLink(ruleContext),
+                /* srcLessDepsExport = */ false)
+            .getCompileTimeJars();
     common.setJavaCompilationArtifacts(
         new JavaCompilationArtifacts.Builder()
             .addRuntimeJar(mergedJar)
@@ -145,6 +155,28 @@ public class AarImport implements RuleConfiguredTargetFactory {
                 JavaCommon.isNeverLink(ruleContext),
                 /* srcLessDepsExport = */ false));
 
+    JavaConfiguration javaConfig = ruleContext.getFragment(JavaConfiguration.class);
+
+    Artifact depsCheckerResult = null;
+    if (javaConfig.getImportDepsCheckingLevel() != ImportDepsCheckingLevel.OFF) {
+      NestedSet<Artifact> bootclasspath = getBootclasspath(ruleContext);
+      depsCheckerResult = createAarArtifact(ruleContext, "aar_import_deps_checker_result.txt");
+      ImportDepsCheckActionBuilder.newBuilder()
+          .bootcalsspath(bootclasspath)
+          .declareDeps(deps)
+          .checkJars(NestedSetBuilder.<Artifact>stableOrder().add(mergedJar).build())
+          .outputArtifiact(depsCheckerResult)
+          .importDepsCheckingLevel(javaConfig.getImportDepsCheckingLevel())
+          .buildAndRegister(ruleContext);
+    }
+    // We pass depsCheckerResult to create the action of extracting ANDROID_MANIFEST. Note that
+    // this action does not need depsCheckerResult. The only reason is that we need to check the
+    // dependencies of this aar_import, and we need to put its result on the build graph so that the
+    // dependency checking action is called.
+    ruleContext.registerAction(
+        createSingleFileExtractorActions(
+            ruleContext, aar, ANDROID_MANIFEST, depsCheckerResult, androidManifestArtifact));
+
     JavaInfo.Builder javaInfoBuilder =
         JavaInfo.Builder.create()
             .addProvider(JavaCompilationArgsProvider.class, javaCompilationArgsProvider)
@@ -153,7 +185,7 @@ public class AarImport implements RuleConfiguredTargetFactory {
     common.addTransitiveInfoProviders(
         ruleBuilder, javaInfoBuilder, filesToBuild, /*classJar=*/ null);
 
-    return ruleBuilder
+    ruleBuilder
         .setFilesToBuild(filesToBuild)
         .addSkylarkTransitiveInfo(
             JavaSkylarkApiProvider.NAME, JavaSkylarkApiProvider.fromRuleContext())
@@ -164,26 +196,63 @@ public class AarImport implements RuleConfiguredTargetFactory {
                 AndroidCommon.collectTransitiveNativeLibs(ruleContext).add(nativeLibs).build()))
         .addProvider(
             JavaRuntimeJarProvider.class, new JavaRuntimeJarProvider(ImmutableList.of(mergedJar)))
-        .addNativeDeclaredProvider(javaInfoBuilder.build())
-        .build();
+        .addNativeDeclaredProvider(javaInfoBuilder.build());
+    if (depsCheckerResult != null) {
+      // Add the deps check result so that we can unit test it.
+      ruleBuilder.addOutputGroup(OutputGroupInfo.HIDDEN_TOP_LEVEL, depsCheckerResult);
+    }
+    return ruleBuilder.build();
+  }
+
+  private NestedSet<Artifact> getBootclasspath(RuleContext ruleContext) {
+    if (AndroidCommon.getAndroidConfig(ruleContext).desugarJava8()) {
+      return NestedSetBuilder.<Artifact>stableOrder()
+          .addTransitive(
+              ruleContext
+                  .getPrerequisite("$desugar_java8_extra_bootclasspath", Mode.HOST)
+                  .getProvider(FileProvider.class)
+                  .getFilesToBuild())
+          .add(AndroidSdkProvider.fromRuleContext(ruleContext).getAndroidJar())
+          .build();
+    } else {
+      return NestedSetBuilder.<Artifact>stableOrder()
+          .add(AndroidSdkProvider.fromRuleContext(ruleContext).getAndroidJar())
+          .build();
+    }
   }
 
+  /**
+   * Create an action to extract a file (specified by the parameter filename) from an AAR file. Note
+   * that the parameter depsCheckerResult is not necessary for this action. Conversely, the action
+   * of checking dependencies for aar_import needs this action instead. Therefore we add the output
+   * artifact of import_deps_checker to this extraction action as input. Therefore, the dependency
+   * checking will run each time.
+   */
   private static Action[] createSingleFileExtractorActions(
-      RuleContext ruleContext, Artifact aar, String filename, Artifact outputArtifact) {
-    return new SpawnAction.Builder()
-        .useDefaultShellEnvironment()
-        .setExecutable(ruleContext.getExecutablePrerequisite(AarImportBaseRule.ZIPPER, Mode.HOST))
-        .setMnemonic("AarFileExtractor")
-        .setProgressMessage("Extracting %s from %s", filename, aar.getFilename())
-        .addInput(aar)
-        .addOutput(outputArtifact)
-        .addCommandLine(
-            CustomCommandLine.builder()
-                .addExecPath("x", aar)
-                .addPath("-d", outputArtifact.getExecPath().getParentDirectory())
-                .addDynamicString(filename)
-                .build())
-        .build(ruleContext);
+      RuleContext ruleContext,
+      Artifact aar,
+      String filename,
+      Artifact depsCheckerResult,
+      Artifact outputArtifact) {
+    SpawnAction.Builder builder =
+        new SpawnAction.Builder()
+            .useDefaultShellEnvironment()
+            .setExecutable(
+                ruleContext.getExecutablePrerequisite(AarImportBaseRule.ZIPPER, Mode.HOST))
+            .setMnemonic("AarFileExtractor")
+            .setProgressMessage("Extracting %s from %s", filename, aar.getFilename())
+            .addInput(aar)
+            .addOutput(outputArtifact)
+            .addCommandLine(
+                CustomCommandLine.builder()
+                    .addExecPath("x", aar)
+                    .addPath("-d", outputArtifact.getExecPath().getParentDirectory())
+                    .addDynamicString(filename)
+                    .build());
+    if (depsCheckerResult != null) {
+      builder.addInput(depsCheckerResult);
+    }
+    return builder.build(ruleContext);
   }
 
   private static Action[] createAarResourcesExtractorActions(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AarImportBaseRule.java b/src/main/java/com/google/devtools/build/lib/rules/android/AarImportBaseRule.java
index 5a5b1bc568..d2e8811dfa 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AarImportBaseRule.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AarImportBaseRule.java
@@ -26,6 +26,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.SkylarkProviderIdentifier;
 import com.google.devtools.build.lib.rules.android.AndroidRuleClasses.AndroidBaseRule;
+import com.google.devtools.build.lib.rules.java.JavaConfiguration;
 import com.google.devtools.build.lib.rules.java.JavaInfo;
 import com.google.devtools.build.lib.util.FileType;
 
@@ -73,6 +74,7 @@ public class AarImportBaseRule implements RuleDefinition {
             .exec()
             .value(env.getToolsLabel("//tools/zip:zipper")))
         .advertiseSkylarkProvider(SkylarkProviderIdentifier.forKey(JavaInfo.PROVIDER.getKey()))
+        .requiresConfigurationFragments(JavaConfiguration.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 8c5ef76b2b..a37bae9334 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
@@ -636,6 +636,11 @@ public final class AndroidRuleClasses {
                   .cfg(HostTransition.INSTANCE)
                   .exec()
                   .value(env.getToolsLabel(DEFAULT_RESOURCES_BUSYBOX)))
+          .add(
+              attr("$import_deps_checker", LABEL)
+                  .cfg(HostTransition.INSTANCE)
+                  .exec()
+                  .value(env.getToolsLabel("//tools/android:aar_import_deps_checker")))
           .build();
     }
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/ImportDepsCheckActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/java/ImportDepsCheckActionBuilder.java
new file mode 100644
index 0000000000..e6d9f00ff1
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/ImportDepsCheckActionBuilder.java
@@ -0,0 +1,118 @@
+// 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.rules.java;
+
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.base.Preconditions.checkState;
+
+import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.analysis.RuleContext;
+import com.google.devtools.build.lib.analysis.actions.CustomCommandLine;
+import com.google.devtools.build.lib.analysis.actions.CustomCommandLine.VectorArg;
+import com.google.devtools.build.lib.analysis.actions.SpawnAction;
+import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode;
+import com.google.devtools.build.lib.collect.nestedset.NestedSet;
+import com.google.devtools.build.lib.rules.java.JavaConfiguration.ImportDepsCheckingLevel;
+import java.util.stream.Collectors;
+
+/** Utility for generating a call to the import_deps_checker. */
+public final class ImportDepsCheckActionBuilder {
+
+  public static ImportDepsCheckActionBuilder newBuilder() {
+    return new ImportDepsCheckActionBuilder();
+  }
+
+  private Artifact outputArtifact;
+  private NestedSet<Artifact> jarsToCheck;
+  private NestedSet<Artifact> bootclasspath;
+  private NestedSet<Artifact> declaredDeps;
+  private ImportDepsCheckingLevel importDepsCheckingLevel;
+
+  private ImportDepsCheckActionBuilder() {}
+
+  public ImportDepsCheckActionBuilder checkJars(NestedSet<Artifact> jarsToCheck) {
+    checkState(this.jarsToCheck == null);
+    this.jarsToCheck = checkNotNull(jarsToCheck);
+    return this;
+  }
+
+  public ImportDepsCheckActionBuilder outputArtifiact(Artifact outputArtifact) {
+    checkState(this.outputArtifact == null);
+    this.outputArtifact = checkNotNull(outputArtifact);
+    return this;
+  }
+
+  public ImportDepsCheckActionBuilder importDepsCheckingLevel(
+      ImportDepsCheckingLevel importDepsCheckingLevel) {
+    checkState(this.importDepsCheckingLevel == null);
+    checkArgument(importDepsCheckingLevel != ImportDepsCheckingLevel.OFF);
+    this.importDepsCheckingLevel = checkNotNull(importDepsCheckingLevel);
+    return this;
+  }
+
+  public ImportDepsCheckActionBuilder bootcalsspath(NestedSet<Artifact> bootclasspath) {
+    checkState(this.bootclasspath == null);
+    this.bootclasspath = checkNotNull(bootclasspath);
+    return this;
+  }
+
+  public ImportDepsCheckActionBuilder declareDeps(NestedSet<Artifact> declaredDeps) {
+    checkState(this.declaredDeps == null);
+    this.declaredDeps = checkNotNull(declaredDeps);
+    return this;
+  }
+
+  public void buildAndRegister(RuleContext ruleContext) {
+    checkNotNull(outputArtifact);
+    checkNotNull(jarsToCheck);
+    checkNotNull(bootclasspath);
+    checkNotNull(declaredDeps);
+    checkState(
+        importDepsCheckingLevel == ImportDepsCheckingLevel.ERROR
+            || importDepsCheckingLevel == ImportDepsCheckingLevel.WARNING,
+        "%s",
+        importDepsCheckingLevel);
+
+    CustomCommandLine args =
+        CustomCommandLine.builder()
+            .addExecPath("--output", outputArtifact)
+            .addExecPaths(VectorArg.addBefore("--input").each(jarsToCheck))
+            .addExecPaths(VectorArg.addBefore("--classpath_entry").each(declaredDeps))
+            .addExecPaths(VectorArg.addBefore("--bootclasspath_entry").each(bootclasspath))
+            .add(
+                importDepsCheckingLevel == ImportDepsCheckingLevel.ERROR
+                    ? "--fail_on_errors"
+                    : "--nofail_on_errors")
+            .build();
+    ruleContext.registerAction(
+        new SpawnAction.Builder()
+            .useDefaultShellEnvironment()
+            .setExecutable(ruleContext.getExecutablePrerequisite("$import_deps_checker", Mode.HOST))
+            .addTransitiveInputs(jarsToCheck)
+            .addTransitiveInputs(declaredDeps)
+            .addTransitiveInputs(bootclasspath)
+            .addOutput(outputArtifact)
+            .setMnemonic("ImportDepsChecker")
+            .setProgressMessage(
+                "Checking the completeness of the deps for %s",
+                jarsToCheck
+                    .toList()
+                    .stream()
+                    .map(Artifact::prettyPrint)
+                    .collect(Collectors.joining(", ")))
+            .addCommandLine(args)
+            .build(ruleContext));
+  }
+}
diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/AarImportTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/AarImportTest.java
index 26c5487219..7a17eeaf52 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/android/AarImportTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/android/AarImportTest.java
@@ -22,6 +22,7 @@ import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.FilesToRunProvider;
+import com.google.devtools.build.lib.analysis.OutputGroupInfo;
 import com.google.devtools.build.lib.analysis.actions.SpawnAction;
 import com.google.devtools.build.lib.analysis.configuredtargets.FileConfiguredTarget;
 import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
@@ -31,6 +32,7 @@ import com.google.devtools.build.lib.rules.java.JavaCompilationArgsProvider;
 import com.google.devtools.build.lib.rules.java.JavaInfo;
 import com.google.devtools.build.lib.rules.java.JavaRuleOutputJarsProvider;
 import com.google.devtools.build.lib.rules.java.JavaRuleOutputJarsProvider.OutputJar;
+import java.util.List;
 import java.util.Set;
 import org.junit.Before;
 import org.junit.Test;
@@ -42,6 +44,7 @@ import org.junit.runners.JUnit4;
 public class AarImportTest extends BuildViewTestCase {
   @Before
   public void setup() throws Exception {
+    useConfiguration("--experimental_import_deps_checking=ERROR");
     scratch.file("a/BUILD",
         "aar_import(",
         "    name = 'foo',",
@@ -124,6 +127,28 @@ public class AarImportTest extends BuildViewTestCase {
   }
 
   @Test
+  public void testDepsCheckerActionExists() throws Exception {
+    ConfiguredTarget aarImportTarget = getConfiguredTarget("//a:bar");
+    OutputGroupInfo outputGroupInfo = aarImportTarget.get(OutputGroupInfo.SKYLARK_CONSTRUCTOR);
+    NestedSet<Artifact> outputGroup =
+        outputGroupInfo.getOutputGroup(OutputGroupInfo.HIDDEN_TOP_LEVEL);
+    Artifact artifact = Iterables.getOnlyElement(outputGroup);
+    assertThat(artifact.isTreeArtifact()).isFalse();
+    assertThat(artifact.getExecPathString())
+        .endsWith("_aar/bar/aar_import_deps_checker_result.txt");
+
+    SpawnAction checkerAction = getGeneratingSpawnAction(artifact);
+    List<String> arguments = checkerAction.getArguments();
+    assertThat(arguments)
+        .containsAllOf(
+            "--bootclasspath_entry",
+            "--classpath_entry",
+            "--input",
+            "--output",
+            "--fail_on_errors");
+  }
+
+  @Test
   public void testNativeLibsProvided() throws Exception {
     ConfiguredTarget androidLibraryTarget = getConfiguredTarget("//java:lib");