From 25f37123a0b33484acedaab2ef7c78d50365c43a Mon Sep 17 00:00:00 2001 From: cnsun Date: Tue, 13 Mar 2018 12:27:50 -0700 Subject: Integrate import_deps_checker into aar_import. RELNOTES: Enable dependency checking for aar_import targets. PiperOrigin-RevId: 188912126 --- src/main/java/com/google/devtools/build/lib/BUILD | 1 + .../build/lib/rules/android/AarImport.java | 111 +++++++++++++++---- .../build/lib/rules/android/AarImportBaseRule.java | 2 + .../lib/rules/android/AndroidRuleClasses.java | 5 + .../rules/java/ImportDepsCheckActionBuilder.java | 118 +++++++++++++++++++++ 5 files changed, 216 insertions(+), 21 deletions(-) create mode 100644 src/main/java/com/google/devtools/build/lib/rules/java/ImportDepsCheckActionBuilder.java (limited to 'src/main/java/com/google') 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 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 bootclasspath = getBootclasspath(ruleContext); + depsCheckerResult = createAarArtifact(ruleContext, "aar_import_deps_checker_result.txt"); + ImportDepsCheckActionBuilder.newBuilder() + .bootcalsspath(bootclasspath) + .declareDeps(deps) + .checkJars(NestedSetBuilder.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 getBootclasspath(RuleContext ruleContext) { + if (AndroidCommon.getAndroidConfig(ruleContext).desugarJava8()) { + return NestedSetBuilder.stableOrder() + .addTransitive( + ruleContext + .getPrerequisite("$desugar_java8_extra_bootclasspath", Mode.HOST) + .getProvider(FileProvider.class) + .getFilesToBuild()) + .add(AndroidSdkProvider.fromRuleContext(ruleContext).getAndroidJar()) + .build(); + } else { + return NestedSetBuilder.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 jarsToCheck; + private NestedSet bootclasspath; + private NestedSet declaredDeps; + private ImportDepsCheckingLevel importDepsCheckingLevel; + + private ImportDepsCheckActionBuilder() {} + + public ImportDepsCheckActionBuilder checkJars(NestedSet 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 bootclasspath) { + checkState(this.bootclasspath == null); + this.bootclasspath = checkNotNull(bootclasspath); + return this; + } + + public ImportDepsCheckActionBuilder declareDeps(NestedSet 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)); + } +} -- cgit v1.2.3