Add new min_sdk_version attribute on android_binary and pipe to dex/desugar actions.

timpeut · copybara-github · commit 4c3219ec5fc2 · 2022-05-27T17:54:05.000-07:00
Also introduce an allowlist to restrict packages able to set this new attribute. Eventually we may roll this out more broadly, but there are outstanding concerns around useability and scalability.

RELNOTES: None
PiperOrigin-RevId: 451524093
Change-Id: I9383cac05a20c5115b2202828ca0cea951244d53
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
@@ -184,6 +184,13 @@ private static void validateRuleContext(RuleContext ruleContext, AndroidDataCont
       ruleContext.attributeError("multidex", "Multidex must be enabled");
     }
 
+    if (ruleContext.attributes().isAttributeValueExplicitlySpecified("min_sdk_version")
+        && Allowlist.hasAllowlist(ruleContext, "allow_min_sdk_version")
+        && !Allowlist.isAvailable(ruleContext, "allow_min_sdk_version")) {
+      ruleContext.attributeError(
+          "min_sdk_version", "Target is not permitted to set min_sdk_version");
+    }
+
     if (AndroidCommon.getAndroidConfig(ruleContext).desugarJava8Libs()
         && getMultidexMode(ruleContext) == MultidexMode.OFF) {
       // Multidex is required so we can include legacy libs as a separate .dex file.
@@ -263,30 +270,6 @@ private static RuleConfiguredTargetBuilder init(
                       : null);
     }
 
-    Artifact manifestValidation = null;
-    if (Allowlist.hasAllowlist(ruleContext, "android_multidex_native_min_sdk_allowlist")
-        && !Allowlist.isAvailable(ruleContext, "android_multidex_native_min_sdk_allowlist")
-        && getMultidexMode(ruleContext) == MultidexMode.NATIVE
-        && ruleContext.isAttrDefined("$validate_manifest", LABEL)) {
-      manifestValidation =
-          ruleContext.getPackageRelativeArtifact(
-              ruleContext.getLabel().getName() + "_manifest_validation_output",
-              ruleContext.getBinOrGenfilesDirectory());
-      ruleContext.registerAction(
-          createSpawnActionBuilder(ruleContext)
-              .setExecutable(ruleContext.getExecutablePrerequisite("$validate_manifest"))
-              .setProgressMessage("Validating %{input}")
-              .setMnemonic("ValidateManifest")
-              .addInput(manifest.getManifest())
-              .addOutput(manifestValidation)
-              .addCommandLine(
-                  CustomCommandLine.builder()
-                      .addExecPath("--manifest", manifest.getManifest())
-                      .addExecPath("--output", manifestValidation)
-                      .build())
-              .build(ruleContext));
-    }
-
     boolean shrinkResourceCycles =
         shouldShrinkResourceCycles(
             dataContext.getAndroidConfig(), ruleContext, dataContext.isResourceShrinkingEnabled());
@@ -415,6 +398,38 @@ && getMultidexMode(ruleContext) == MultidexMode.NATIVE
         AndroidBinaryMobileInstall.createMobileInstallResourceApks(
             ruleContext, dataContext, manifest);
 
+    Artifact manifestValidation = null;
+    boolean shouldValidateMultidex =
+        (Allowlist.hasAllowlist(ruleContext, "android_multidex_native_min_sdk_allowlist")
+            && !Allowlist.isAvailable(ruleContext, "android_multidex_native_min_sdk_allowlist")
+            && getMultidexMode(ruleContext) == MultidexMode.NATIVE);
+    boolean shouldValidateMinSdk = getMinSdkVersion(ruleContext) > 0;
+    if (ruleContext.isAttrDefined("$validate_manifest", LABEL)
+        && (shouldValidateMultidex || shouldValidateMinSdk)) {
+      manifestValidation =
+          ruleContext.getPackageRelativeArtifact(
+              ruleContext.getLabel().getName() + "_manifest_validation_output",
+              ruleContext.getBinOrGenfilesDirectory());
+      ruleContext.registerAction(
+          createSpawnActionBuilder(ruleContext)
+              .setExecutable(ruleContext.getExecutablePrerequisite("$validate_manifest"))
+              .setProgressMessage("Validating %{input}")
+              .setMnemonic("ValidateManifest")
+              .addInput(manifest.getManifest())
+              .addOutput(manifestValidation)
+              .addCommandLine(
+                  CustomCommandLine.builder()
+                      .addExecPath("--manifest", manifest.getManifest())
+                      .addExecPath("--output", manifestValidation)
+                      .addFormatted(
+                          "--validate_multidex=%s", Boolean.toString(shouldValidateMultidex))
+                      .add(
+                          "--expected_min_sdk_version",
+                          Integer.toString(getMinSdkVersion(ruleContext)))
+                      .build())
+              .build(ruleContext));
+    }
+
     return createAndroidBinary(
         ruleContext,
         dataContext,
@@ -1267,6 +1282,8 @@ private static DexingOutput dex(
               + "\" not supported by this version of the Android SDK");
     }
 
+    int minSdkVersion = getMinSdkVersion(ruleContext);
+
     int dexShards = ruleContext.attributes().get("dex_shards", Type.INTEGER).toIntUnchecked();
     if (dexShards > 1) {
       if (multidexMode == MultidexMode.OFF) {
@@ -1306,6 +1323,7 @@ private static DexingOutput dex(
             common,
             inclusionFilterJar,
             dexopts,
+            minSdkVersion,
             androidSemantics,
             attributes,
             derivedJarFunction,
@@ -1320,6 +1338,7 @@ private static DexingOutput dex(
             proguardedJar,
             classesDex,
             dexopts,
+            minSdkVersion,
             /*multidex=*/ false,
             /*mainDexList=*/ null);
       }
@@ -1356,6 +1375,7 @@ private static DexingOutput dex(
                 common,
                 inclusionFilterJar,
                 dexopts,
+                minSdkVersion,
                 androidSemantics,
                 attributes,
                 derivedJarFunction,
@@ -1381,7 +1401,13 @@ private static DexingOutput dex(
                 dexopts);
           } else {
             AndroidCommon.createDexAction(
-                ruleContext, shard, shardDex, dexopts, /*multidex=*/ true, /*mainDexList=*/ null);
+                ruleContext,
+                shard,
+                shardDex,
+                dexopts,
+                minSdkVersion,
+                /*multidex=*/ true,
+                /*mainDexList=*/ null);
           }
         }
         ImmutableList<Artifact> shardDexes = shardDexesBuilder.build();
@@ -1417,6 +1443,7 @@ private static DexingOutput dex(
               common,
               inclusionFilterJar,
               dexopts,
+              minSdkVersion,
               androidSemantics,
               attributes,
               derivedJarFunction,
@@ -1436,6 +1463,7 @@ private static DexingOutput dex(
               proguardedJar,
               classesDexIntermediate,
               dexopts,
+              minSdkVersion,
               /*multidex=*/ true,
               mainDexList);
           createCleanDexZipAction(ruleContext, classesDexIntermediate, classesDex);
@@ -1455,6 +1483,7 @@ private static void createIncrementalDexingActions(
       AndroidCommon common,
       @Nullable Artifact inclusionFilterJar,
       List<String> dexopts,
+      int minSdkVersion,
       AndroidSemantics androidSemantics,
       JavaTargetAttributes attributes,
       Function<Artifact, Artifact> derivedJarFunction,
@@ -1471,7 +1500,12 @@ private static void createIncrementalDexingActions(
               ruleContext,
               collectRuntimeJars(common, attributes),
               collectDexArchives(
-                  ruleContext, common, dexopts, androidSemantics, derivedJarFunction));
+                  ruleContext,
+                  common,
+                  dexopts,
+                  minSdkVersion,
+                  androidSemantics,
+                  derivedJarFunction));
     } else {
       if (proguardedJar != null
           && AndroidCommon.getAndroidConfig(ruleContext).incrementalDexingShardsAfterProguard()
@@ -1493,6 +1527,7 @@ private static void createIncrementalDexingActions(
             "$dexbuilder_after_proguard",
             proguardedJar,
             DexArchiveAspect.topLevelDexbuilderDexopts(dexopts),
+            minSdkVersion,
             dexArchives.get(0));
       } else {
         createShuffleJarActions(
@@ -1503,6 +1538,7 @@ private static void createIncrementalDexingActions(
             common,
             inclusionFilterJar,
             dexopts,
+            minSdkVersion,
             androidSemantics,
             attributes,
             derivedJarFunction,
@@ -1771,6 +1807,7 @@ public static Function<Artifact, Artifact> collectDesugaredJars(
               jar,
               attributes.getBootClassPath().bootclasspath(),
               attributes.getCompileTimeClassPath(),
+              getMinSdkVersion(ruleContext),
               ruleContext.getDerivedArtifact(
                   jarPath.replaceName(jarPath.getBaseName() + "_desugared.jar"), jar.getRoot()));
       result.addDesugaredJar(jar, desugared);
@@ -1797,6 +1834,7 @@ private static Map<Artifact, Artifact> collectDexArchives(
       RuleContext ruleContext,
       AndroidCommon common,
       List<String> dexopts,
+      int minSdkVersion,
       AndroidSemantics semantics,
       Function<Artifact, Artifact> derivedJarFunction) {
     DexArchiveProvider.Builder result = new DexArchiveProvider.Builder();
@@ -1820,6 +1858,7 @@ private static Map<Artifact, Artifact> collectDexArchives(
               "$dexbuilder",
               derivedJarFunction.apply(jar),
               incrementalDexopts,
+              minSdkVersion,
               ruleContext.getDerivedArtifact(
                   jarPath.replaceName(jarPath.getBaseName() + ".dex.zip"), jar.getRoot()));
       result.addDexArchive(incrementalDexopts, dexArchive, jar);
@@ -1835,6 +1874,7 @@ private static Artifact createShuffleJarActions(
       AndroidCommon common,
       @Nullable Artifact inclusionFilterJar,
       List<String> dexopts,
+      int minSdkVersion,
       AndroidSemantics semantics,
       JavaTargetAttributes attributes,
       Function<Artifact, Artifact> derivedJarFunction,
@@ -1889,7 +1929,8 @@ private static Artifact createShuffleJarActions(
         // there should be very few or no Jar files that still end up in shards.  The dexing
         // step below will have to deal with those in addition to merging .dex files together.
         Map<Artifact, Artifact> dexArchives =
-            collectDexArchives(ruleContext, common, dexopts, semantics, derivedJarFunction);
+            collectDexArchives(
+                ruleContext, common, dexopts, minSdkVersion, semantics, derivedJarFunction);
         classpath = toDexedClasspath(ruleContext, classpath, dexArchives);
         shardCommandLine.add("--split_dexed_classes");
       } else {
@@ -1916,6 +1957,7 @@ private static Artifact createShuffleJarActions(
             "$dexbuilder_after_proguard",
             shuffleOutputs.get(i),
             DexArchiveAspect.topLevelDexbuilderDexopts(dexopts),
+            minSdkVersion,
             shards.get(i));
       }
     }
@@ -2178,6 +2220,13 @@ public static MultidexMode getMultidexMode(RuleContext ruleContext) {
     }
   }
 
+  private static int getMinSdkVersion(RuleContext ruleContext) {
+    if (ruleContext.getRule().isAttrDefined("min_sdk_version", Type.INTEGER)) {
+      return ruleContext.attributes().get("min_sdk_version", Type.INTEGER).toIntUnchecked();
+    }
+    return 0;
+  }
+
   /**
    * List of Android SDKs that contain runtimes that do not support the native multidexing
    * introduced in Android L. If someone tries to build an android_binary that has multidex=native
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinaryMobileInstall.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinaryMobileInstall.java
@@ -369,7 +369,7 @@ private static Artifact getStubDex(
             ruleContext,
             split ? "split_stub_application/classes.dex" : "stub_application/classes.dex");
     AndroidCommon.createDexAction(
-        ruleContext, stubDeployJar, stubDex, ImmutableList.<String>of(), false, null);
+        ruleContext, stubDeployJar, stubDex, ImmutableList.<String>of(), 0, false, null);
 
     return stubDex;
   }
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
@@ -187,6 +187,7 @@ public static void createDexAction(
       Artifact jarToDex,
       Artifact classesDex,
       List<String> dexOptions,
+      int minSdkVersion,
       boolean multidex,
       Artifact mainDexList) {
     CustomCommandLine.Builder commandLine = CustomCommandLine.builder();
@@ -201,6 +202,9 @@ public static void createDexAction(
     }
 
     commandLine.addAll(dexOptions);
+    if (minSdkVersion > 0) {
+      commandLine.add("--min_sdk_version", Integer.toString(minSdkVersion));
+    }
     if (multidex) {
       commandLine.add("--multi-dex");
       if (mainDexList != null) {
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
@@ -571,6 +571,15 @@ Do PNG crunching (or not). This is independent of nine-patch processing, which i
           <code>en_XA</code> and/or <code>ar_XB</code> pseudo-locales.
           <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
           .add(attr(ResourceFilterFactory.RESOURCE_CONFIGURATION_FILTERS_NAME, STRING_LIST))
+          /* <!-- #BLAZE_RULE($android_binary_base).ATTRIBUTE(min_sdk_version) -->
+          The minSdkVersion. This must match the minSdkVersion specified in the AndroidManifest.xml.
+          If set, will be used for dex/desugaring.
+          <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
+          .add(
+              attr("min_sdk_version", INTEGER)
+                  .undocumented("experimental")
+                  .value(StarlarkInt.of(0))
+                  .nonconfigurable("AspectParameters don't support configurations."))
           /* <!-- #BLAZE_RULE($android_binary_base).ATTRIBUTE(shrink_resources) -->
           Whether to perform resource shrinking. Resources that are not used by the binary will be
           removed from the APK. This is only supported for rules using local resources (i.e. the
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/DexArchiveAspect.java b/src/main/java/com/google/devtools/build/lib/rules/android/DexArchiveAspect.java
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

Original file line number	Diff line number	Diff line change
`@@ -369,7 +369,7 @@ private static Artifact getStubDex(`
`369`	`369`	`ruleContext,`
`370`	`370`	`split ? "split_stub_application/classes.dex" : "stub_application/classes.dex");`
`371`	`371`	`AndroidCommon.createDexAction(`
`372`		`- ruleContext, stubDeployJar, stubDex, ImmutableList.<String>of(), false, null);`
	`372`	`+ ruleContext, stubDeployJar, stubDex, ImmutableList.<String>of(), 0, false, null);`
`373`	`373`
`374`	`374`	`return stubDex;`
`375`	`375`	`}`