Automated rollback of commit b98ad77.

*** Reason for rollback *** Broke Android build on Windows #9102 *** Original change description *** Default Bazel to use aapt2 and fix native tests to use aapt2. This CL sets --incompatible_use_aapt2_by_default=true and --android_aapt=aapt2. It also adds aapt2 to the mock SDK for Android analysis and integration tests. For the tests that assume and depend on AAPT, we explicitly pin them to use AAPT. Fixes #6907 Fixes #4103 RELNOTES: Bazel Android builds now use aapt2 by default. To revert to aapt, set `--an... *** ROLLBACK_OF=261424350 RELNOTES: None PiperOrigin-RevId: 262099198
bazelbuild · Aug 7, 2019 · 5d0dde0 · 5d0dde0
1 parent 5585169
commit 5d0dde0
Show file tree

Hide file tree

Showing 13 changed files with 181 additions and 92 deletions.
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
@@ -708,7 +708,7 @@ public static class Options extends FragmentOptions {
 
     @Option(
         name = "android_aapt",
-        defaultValue = "aapt2",
+        defaultValue = "auto",
         documentationCategory = OptionDocumentationCategory.TOOLCHAIN,
         effectTags = {
           OptionEffectTag.AFFECTS_OUTPUTS,
@@ -968,7 +968,7 @@ public static class Options extends FragmentOptions {
           OptionMetadataTag.INCOMPATIBLE_CHANGE,
           OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
         },
-        defaultValue = "true",
+        defaultValue = "false",
         help =
             "Switch the Android rules to use aapt2 by default for resource processing. "
                 + "To resolve issues when migrating your app to build with aapt2, see "

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
@@ -856,9 +856,6 @@ is exceeded. Assumes multidex classes are loaded through application code (i.e.
           .add(attr("manifest_values", STRING_DICT))
           /* <!-- #BLAZE_RULE(android_binary).ATTRIBUTE(aapt_version) -->
           Select the version of aapt for this rule.<br/>
-
-          This attribute only takes effect if you set `--android_aapt=auto`.<br/>
-
           Possible values:
           <ul>
               <li><code>aapt_version = "aapt"</code>: Use aapt (deprecated).</li>

diff --git a/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java b/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java
@@ -307,7 +307,6 @@ private ImmutableList<String> createAndroidBuildContents() {
         "android_sdk(",
         "    name = 'sdk',",
         "    aapt = ':static_aapt_tool',",
-        "    aapt2 = ':static_aapt2_tool',",
         "    adb = ':static_adb_tool',",
         "    aidl = ':static_aidl_tool',",
         "    android_jar = ':android_runtime_jar',",

diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidAssetsTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidAssetsTest.java
@@ -57,6 +57,9 @@ public void testParse() throws Exception {
 
   @Test
   public void testParseAapt2() throws Exception {
+    mockAndroidSdkWithAapt2();
+    useConfiguration("--android_sdk=//sdk:sdk");
+
     RuleContext ruleContext = getRuleContext();
     AndroidAssets assets = getLocalAssets();
 
@@ -155,6 +158,9 @@ public void testMerge() throws Exception {
 
   @Test
   public void testMergeAapt2() throws Exception {
+    mockAndroidSdkWithAapt2();
+    useConfiguration("--android_sdk=//sdk:sdk");
+
     RuleContext ruleContext = getRuleContext();
     ParsedAndroidAssets parsed =
         getLocalAssets().parse(AndroidDataContext.forNative(ruleContext), AndroidAaptVersion.AAPT2);

diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java
@@ -877,8 +877,6 @@ private void actualSignerToolTests(String apkSigningMethod, String signV1, Strin
 
   @Test
   public void testResourceShrinkingAction() throws Exception {
-    useConfiguration("--android_aapt=aapt");
-
     scratch.file("java/com/google/android/hello/BUILD",
         "android_binary(name = 'hello',",
         "               srcs = ['Foo.java'],",
@@ -935,9 +933,8 @@ public void testResourceShrinkingAction() throws Exception {
   }
 
   @Test
-  public void testResourceCycleShrinkingWithAapt() throws Exception {
-    useConfiguration("--android_aapt=aapt", "--experimental_android_resource_cycle_shrinking=true");
-
+  public void testResourceCycleShrinking() throws Exception {
+    useConfiguration("--experimental_android_resource_cycle_shrinking=true");
     checkError(
         "java/a",
         "a",
@@ -1329,9 +1326,6 @@ public void testResourceConfigurationFilters() throws Exception {
 
   @Test
   public void testFilteredResourcesInvalidFilter() throws Exception {
-    // This test is an analysis-time check with aapt.
-    useConfiguration("--android_aapt=aapt");
-
     String badQualifier = "invalid-qualifier";
 
     checkError(
@@ -1346,9 +1340,6 @@ public void testFilteredResourcesInvalidFilter() throws Exception {
 
   @Test
   public void testFilteredResourcesInvalidResourceDir() throws Exception {
-    // This test is an analysis-time check with aapt.
-    useConfiguration("--android_aapt=aapt");
-
     String badQualifierDir = "values-invalid-qualifier";
 
     checkError(
@@ -1368,6 +1359,10 @@ public void testFilteredResourcesFilteringAapt2() throws Exception {
         ImmutableList.of("res/values/foo.xml", "res/values-en/foo.xml", "res/values-fr/foo.xml");
     String dir = "java/r/android";
 
+    mockAndroidSdkWithAapt2();
+
+    useConfiguration("--android_sdk=//sdk:sdk");
+
     ConfiguredTarget binary =
         scratchConfiguredTarget(
             dir,
@@ -1395,8 +1390,6 @@ public void testFilteredResourcesFilteringAapt2() throws Exception {
 
   @Test
   public void testFilteredResourcesSimple() throws Exception {
-    useConfiguration("--android_aapt=aapt");
-
     testDirectResourceFiltering(
         "en",
         /* unexpectedQualifiers= */ ImmutableList.of("fr"),
@@ -1685,8 +1678,6 @@ private void testDirectResourceFiltering(
       String folderType,
       String suffix)
       throws Exception {
-    // Filtering is done at the analysis time for aapt.
-    useConfiguration("--android_aapt=aapt");
 
     List<String> unexpectedResources = new ArrayList<>();
     for (String qualifier : unexpectedQualifiers) {
@@ -1766,9 +1757,6 @@ private void testDirectResourceFiltering(
 
   @Test
   public void testFilteredTransitiveResources() throws Exception {
-    // Filtering is done at analysis time for aapt.
-    useConfiguration("--android_aapt=aapt");
-
     String matchingResource = "res/values-en/foo.xml";
     String unqualifiedResource = "res/values/foo.xml";
     String notMatchingResource = "res/values-fr/foo.xml";
@@ -1811,9 +1799,6 @@ public void testFilteredTransitiveResources() throws Exception {
 
   @Test
   public void testFilteredTransitiveResourcesDifferentDensities() throws Exception {
-    // Filtering is done at analysis time for aapt.
-    useConfiguration("--android_aapt=aapt");
-
     String dir = "java/r/android";
 
     ConfiguredTarget binary =
@@ -1861,9 +1846,6 @@ public void testFilteredTransitiveResourcesDifferentDensities() throws Exception
 
   @Test
   public void testFilteredResourcesAllFilteredOut() throws Exception {
-    // Filtering is done at analysis time for aapt.
-    useConfiguration("--android_aapt=aapt");
-
     String dir = "java/r/android";
 
     final String keptBaseDir = "partly_filtered_dir";
@@ -2099,9 +2081,6 @@ public void testUseRClassGeneratorCustomPackage() throws Exception {
 
   @Test
   public void testUseRClassGeneratorMultipleDeps() throws Exception {
-    // This test assumes using aapt.
-    useConfiguration("--android_aapt=aapt");
-
     scratch.file(
         "java/r/android/BUILD",
         "android_library(name = 'lib1',",
@@ -2536,7 +2515,6 @@ public void testMainDexListWithAndroidSdk() throws Exception {
         "android_sdk(",
         "    name = 'sdk',",
         "    aapt = 'aapt',",
-        "    aapt2 = 'aapt2',",
         "    adb = 'adb',",
         "    aidl = 'aidl',",
         "    android_jar = 'android.jar',",
@@ -2575,7 +2553,6 @@ public void testMainDexAaptGenerationSupported() throws Exception {
         "    name = 'sdk',",
         "    build_tools_version = '24.0.0',",
         "    aapt = 'aapt',",
-        "    aapt2 = 'aapt2',",
         "    adb = 'adb',",
         "    aidl = 'aidl',",
         "    android_jar = 'android.jar',",
@@ -3585,8 +3562,40 @@ public void testFeatureFlagPolicyIsNotUsedIfFlagValuesNotUsed() throws Exception
     assertContainsEvent("*super* busted package group");
   }
 
+  @Test
+  public void testAapt2WithoutAndroidSdk() throws Exception {
+    useConfiguration("--android_aapt=aapt2");
+    checkError(
+        "java/a",
+        "a",
+        "aapt2 processing requested but not available on the android_sdk",
+        "android_binary(",
+        "    name = 'a',",
+        "    srcs = ['A.java'],",
+        "    manifest = 'AndroidManifest.xml',",
+        "    resource_files = [ 'res/values/values.xml' ], ",
+        "    aapt_version = 'aapt2'",
+        ")");
+  }
+
+  @Test
+  public void testAapt2FlagWithoutAndroidSdk() throws Exception {
+    useConfiguration("--android_aapt=aapt2");
+    checkError(
+        "java/a",
+        "a",
+        "aapt2 processing requested but not available on the android_sdk",
+        "android_binary(",
+        "    name = 'a',",
+        "    srcs = ['A.java'],",
+        "    manifest = 'AndroidManifest.xml',",
+        "    resource_files = [ 'res/values/values.xml' ], ",
+        ")");
+  }
+
   @Test
   public void testAapt2WithAndroidSdk() throws Exception {
+    mockAndroidSdkWithAapt2();
     scratch.file(
         "java/a/BUILD",
         "android_binary(",
@@ -3597,20 +3606,17 @@ public void testAapt2WithAndroidSdk() throws Exception {
         "    aapt_version = 'aapt2'",
         ")");
 
+    useConfiguration("--android_sdk=//sdk:sdk");
     ConfiguredTarget a = getConfiguredTarget("//java/a:a");
     Artifact apk = getImplicitOutputArtifact(a, AndroidRuleClasses.ANDROID_RESOURCES_APK);
 
     assertThat(getGeneratingSpawnActionArgs(apk))
-        .containsAtLeast(
-            "--aapt2",
-            // The path to aapt2 is different between Blaze and Bazel, so we omit it here.
-            // It's safe to do so as we've already checked for the `--aapt2` flag.
-            "--tool",
-            "AAPT2_PACKAGE");
+        .containsAtLeast("--aapt2", "sdk/aapt2", "--tool", "AAPT2_PACKAGE");
   }
 
   @Test
   public void testAapt2WithAndroidSdkAndDependencies() throws Exception {
+    mockAndroidSdkWithAapt2();
     scratch.file(
         "java/b/BUILD",
         "android_library(",
@@ -3631,6 +3637,7 @@ public void testAapt2WithAndroidSdkAndDependencies() throws Exception {
         "    aapt_version = 'aapt2'",
         ")");
 
+    useConfiguration("--android_sdk=//sdk:sdk");
     ConfiguredTarget a = getConfiguredTarget("//java/a:a");
     ConfiguredTarget b = getDirectPrerequisite(a, "//java/b:b");
 
@@ -3641,12 +3648,7 @@ public void testAapt2WithAndroidSdkAndDependencies() throws Exception {
 
     SpawnAction apkAction = getGeneratingSpawnAction(apk);
     assertThat(getGeneratingSpawnActionArgs(apk))
-        .containsAtLeast(
-            "--aapt2",
-            // The path to aapt2 is different between Blaze and Bazel, so we omit it here.
-            // It's safe to do so as we've already checked for the `--aapt2` flag.
-            "--tool",
-            "AAPT2_PACKAGE");
+        .containsAtLeast("--aapt2", "sdk/aapt2", "--tool", "AAPT2_PACKAGE");
 
     assertThat(apkAction.getInputs())
         .contains(
@@ -3660,6 +3662,7 @@ public void testAapt2WithAndroidSdkAndDependencies() throws Exception {
 
   @Test
   public void testAapt2ResourceShrinkingAction() throws Exception {
+    mockAndroidSdkWithAapt2();
     scratch.file(
         "java/com/google/android/hello/BUILD",
         "android_binary(name = 'hello',",
@@ -3671,6 +3674,7 @@ public void testAapt2ResourceShrinkingAction() throws Exception {
         "               shrink_resources = 1,",
         "               proguard_specs = ['proguard-spec.pro'],)");
 
+    useConfiguration("--android_sdk=//sdk:sdk");
     ConfiguredTargetAndData targetAndData =
         getConfiguredTargetAndData("//java/com/google/android/hello:hello");
     ConfiguredTarget binary = targetAndData.getConfiguredTarget();
@@ -3722,7 +3726,9 @@ public void testAapt2ResourceShrinkingAction() throws Exception {
 
   @Test
   public void testAapt2ResourceCycleShrinking() throws Exception {
-    useConfiguration("--experimental_android_resource_cycle_shrinking=true");
+    mockAndroidSdkWithAapt2();
+    useConfiguration(
+        "--android_sdk=//sdk:sdk", "--experimental_android_resource_cycle_shrinking=true");
     scratch.file(
         "java/com/google/android/hello/BUILD",
         "android_binary(name = 'hello',",
@@ -3753,7 +3759,9 @@ public void testAapt2ResourceCycleShrinking() throws Exception {
 
   @Test
   public void testAapt2ResourceCycleShinkingWithoutResourceShrinking() throws Exception {
-    useConfiguration("--experimental_android_resource_cycle_shrinking=true");
+    mockAndroidSdkWithAapt2();
+    useConfiguration(
+        "--android_sdk=//sdk:sdk", "--experimental_android_resource_cycle_shrinking=true");
     checkError(
         "java/a",
         "a",
@@ -4316,6 +4324,7 @@ public void testConfigurableProguardSpecs() throws Exception {
 
   @Test
   public void testSkipParsingActionFlagGetsPropagated() throws Exception {
+    mockAndroidSdkWithAapt2();
     scratch.file(
         "java/b/BUILD",
         "android_library(",
@@ -4336,7 +4345,7 @@ public void testSkipParsingActionFlagGetsPropagated() throws Exception {
         "    aapt_version = 'aapt2'",
         ")");
 
-    useConfiguration("--experimental_skip_parsing_action");
+    useConfiguration("--android_sdk=//sdk:sdk", "--experimental_skip_parsing_action");
     ConfiguredTarget a = getConfiguredTarget("//java/a:a");
     ConfiguredTarget b = getDirectPrerequisite(a, "//java/b:b");
 
@@ -4359,6 +4368,7 @@ public void testSkipParsingActionFlagGetsPropagated() throws Exception {
 
   @Test
   public void alwaysSkipParsingActionWithAapt2() throws Exception {
+    mockAndroidSdkWithAapt2();
     scratch.file(
         "java/b/BUILD",
         "android_library(",
@@ -4379,6 +4389,7 @@ public void alwaysSkipParsingActionWithAapt2() throws Exception {
         "    aapt_version = 'aapt2'",
         ")");
 
+    useConfiguration("--android_sdk=//sdk:sdk");
     ConfiguredTarget a = getConfiguredTarget("//java/a:a");
     ConfiguredTarget b = getDirectPrerequisite(a, "//java/b:b");
 
@@ -4401,6 +4412,7 @@ public void alwaysSkipParsingActionWithAapt2() throws Exception {
 
   @Test
   public void testAapt1BuildsWithAapt2Sdk() throws Exception {
+    mockAndroidSdkWithAapt2();
     scratch.file(
         "java/b/BUILD",
         "android_library(",
@@ -4421,7 +4433,7 @@ public void testAapt1BuildsWithAapt2Sdk() throws Exception {
         "    aapt_version = 'aapt'",
         ")");
 
-    useConfiguration("--android_aapt=aapt", "--experimental_skip_parsing_action");
+    useConfiguration("--android_sdk=//sdk:sdk", "--experimental_skip_parsing_action");
     ConfiguredTarget a = getConfiguredTarget("//java/a:a");
     ConfiguredTarget b = getDirectPrerequisite(a, "//java/b:b");