From dbe76ceade3ea4561072fac9b4252b06293ee32a Mon Sep 17 00:00:00 2001 From: dchai Date: Wed, 20 Nov 2019 16:01:12 -0800 Subject: [PATCH] Remove a usage of aapt1 when --incompatible_prohibit_aapt1 is specified Bazel previously invoked both aapt1 and aapt2 separately to validate resources from an android_library, but the former is unnecessary if we don't support aapt1. Since we can't remove any plumbing yet, we just label the same triplet of rTxt/sourceJar/apk artifacts as both "aapt2" and "aapt1". PiperOrigin-RevId: 281624756 --- .../rules/android/AndroidConfiguration.java | 4 +++ .../android/ValidatedAndroidResources.java | 35 +++++++++++++++++++ .../rules/android/AndroidResourcesTest.java | 26 ++++++++++++++ 3 files changed, 65 insertions(+) diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java index 0bae26352df666..c0c01e3ceaa65e 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java @@ -1406,4 +1406,8 @@ public boolean filterLibraryJarWithProgramJar() { boolean useRTxtFromMergedResources() { return useRTxtFromMergedResources; } + + boolean incompatibleProhibitAapt1() { + return incompatibleProhibitAapt1; + } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ValidatedAndroidResources.java b/src/main/java/com/google/devtools/build/lib/rules/android/ValidatedAndroidResources.java index dfa8f9c6b57539..16540639ba7ef7 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/ValidatedAndroidResources.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/ValidatedAndroidResources.java @@ -60,6 +60,39 @@ public class ValidatedAndroidResources extends MergedAndroidResources public static ValidatedAndroidResources validateFrom( AndroidDataContext dataContext, MergedAndroidResources merged, AndroidAaptVersion aaptVersion) throws InterruptedException { + if (dataContext.getAndroidConfig().incompatibleProhibitAapt1()) { + Artifact rTxtOut = dataContext.createOutputArtifact(AndroidRuleClasses.ANDROID_R_TXT); + Artifact sourceJarOut = + dataContext.createOutputArtifact(AndroidRuleClasses.ANDROID_JAVA_SOURCE_JAR); + Artifact apkOut = dataContext.createOutputArtifact(AndroidRuleClasses.ANDROID_LIBRARY_APK); + + BusyBoxActionBuilder.create(dataContext, "LINK_STATIC_LIBRARY") + .addAapt(AndroidAaptVersion.AAPT2) + .addInput("--libraries", dataContext.getSdk().getAndroidJar()) + .addInput("--compiled", merged.getCompiledSymbols()) + .addInput("--manifest", merged.getManifest()) + // Sets an alternative java package for the generated R.java + // this allows android rules to generate resources outside of the java{,tests} tree. + .maybeAddFlag("--packageForR", merged.getJavaPackage()) + .addTransitiveVectoredInput( + "--compiledDep", merged.getResourceDependencies().getTransitiveCompiledSymbols()) + .addOutput("--sourceJarOut", sourceJarOut) + .addOutput("--rTxtOut", rTxtOut) + .addOutput("--staticLibraryOut", apkOut) + .buildAndRegister("Linking static android resource library", "AndroidResourceLink"); + + return of( + merged, + rTxtOut, + sourceJarOut, + apkOut, + // TODO: remove below three when incompatibleProhibitAapt1 is on by default. + rTxtOut, + sourceJarOut, + apkOut, + dataContext.getAndroidConfig().useRTxtFromMergedResources()); + } + AndroidResourceValidatorActionBuilder builder = new AndroidResourceValidatorActionBuilder() .setJavaPackage(merged.getJavaPackage()) @@ -150,6 +183,8 @@ public Artifact getJavaSourceJar() { return sourceJar; } + // TODO(b/30307842,b/119560471): remove this; it was added for no reason, but persists because + // the Starlark API is not noneable. @Override public Artifact getApk() { return apk; diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidResourcesTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidResourcesTest.java index 9f2f19060c2250..4c63ccabe86e40 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidResourcesTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidResourcesTest.java @@ -20,6 +20,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; +import com.google.devtools.build.lib.actions.ActionAnalysisMetadata; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.RuleContext; @@ -30,6 +31,7 @@ import com.google.devtools.build.lib.testutil.MoreAsserts; import com.google.devtools.build.lib.vfs.PathFragment; import java.util.Optional; +import java.util.Set; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -375,6 +377,10 @@ public void testValidateAapt() throws Exception { MergedAndroidResources merged = makeMergedResources(ruleContext, AndroidAaptVersion.AAPT); ValidatedAndroidResources validated = merged.validate(AndroidDataContext.forNative(ruleContext), AndroidAaptVersion.AAPT); + Set actionMnemonics = + ruleContext.getAnalysisEnvironment().getRegisteredActions().stream() + .map(ActionAnalysisMetadata::getMnemonic) + .collect(ImmutableSet.toImmutableSet()); // Inherited values should be equal assertThat(merged).isEqualTo(new MergedAndroidResources(validated)); @@ -385,12 +391,14 @@ public void testValidateAapt() throws Exception { /* inputs = */ ImmutableList.of(validated.getMergedResources(), validated.getManifest()), /* outputs = */ ImmutableList.of( validated.getRTxt(), validated.getJavaSourceJar(), validated.getApk())); + assertThat(actionMnemonics).contains("AndroidResourceValidator"); // aapt1 validation // aapt2 artifacts should not be generated assertThat(validated.getCompiledSymbols()).isNull(); assertThat(validated.getAapt2RTxt()).isNull(); assertThat(validated.getAapt2SourceJar()).isNull(); assertThat(validated.getStaticLibrary()).isNull(); + assertThat(actionMnemonics).doesNotContain("AndroidResourceLink"); // aapt2 validation } @Test @@ -431,6 +439,24 @@ public void testValidateAapt2() throws Exception { validated.getAapt2RTxt(), validated.getAapt2SourceJar(), validated.getStaticLibrary())); } + @Test + public void testValidateNoAapt1() throws Exception { + useConfiguration("--incompatible_prohibit_aapt1"); + RuleContext ruleContext = getRuleContext(); + + makeMergedResources(ruleContext, AndroidAaptVersion.AAPT2) + .validate(AndroidDataContext.forNative(ruleContext), AndroidAaptVersion.AAPT2); + + Set actionMnemonics = + ruleContext.getAnalysisEnvironment().getRegisteredActions().stream() + .map(ActionAnalysisMetadata::getMnemonic) + .collect(ImmutableSet.toImmutableSet()); + // These are unfortunately the mnemonics used in Bazel; these should be changed once aapt1 is + // removed. + assertThat(actionMnemonics).contains("AndroidResourceLink"); // aapt2 validation + assertThat(actionMnemonics).doesNotContain("AndroidResourceValidator"); // aapt1 validation + } + @Test public void testGenerateRClass() throws Exception { RuleContext ruleContext = getRuleContext();