Skip to content

Commit

Permalink
Add new min_sdk_version attribute on android_binary and pipe to dex/d…
Browse files Browse the repository at this point in the history 
…esugar actions.

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
  • Loading branch information
@timpeut @copybara-github
timpeut authored and copybara-github committed May 28, 2022
1 parent e0e5896 commit 4c3219e
Show file tree
Hide file tree
Showing 6 changed files with 209 additions and 32 deletions.
103 changes: 76 additions & 27 deletions src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -1306,6 +1323,7 @@ private static DexingOutput dex(
common,
inclusionFilterJar,
dexopts,
minSdkVersion,
androidSemantics,
attributes,
derivedJarFunction,
Expand All @@ -1320,6 +1338,7 @@ private static DexingOutput dex(
proguardedJar,
classesDex,
dexopts,
minSdkVersion,
/*multidex=*/ false,
/*mainDexList=*/ null);
}
Expand Down Expand Up @@ -1356,6 +1375,7 @@ private static DexingOutput dex(
common,
inclusionFilterJar,
dexopts,
minSdkVersion,
androidSemantics,
attributes,
derivedJarFunction,
Expand All @@ -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();
Expand Down Expand Up @@ -1417,6 +1443,7 @@ private static DexingOutput dex(
common,
inclusionFilterJar,
dexopts,
minSdkVersion,
androidSemantics,
attributes,
derivedJarFunction,
Expand All @@ -1436,6 +1463,7 @@ private static DexingOutput dex(
proguardedJar,
classesDexIntermediate,
dexopts,
minSdkVersion,
/*multidex=*/ true,
mainDexList);
createCleanDexZipAction(ruleContext, classesDexIntermediate, classesDex);
Expand All @@ -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,
Expand All @@ -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()
Expand All @@ -1493,6 +1527,7 @@ private static void createIncrementalDexingActions(
"$dexbuilder_after_proguard",
proguardedJar,
DexArchiveAspect.topLevelDexbuilderDexopts(dexopts),
minSdkVersion,
dexArchives.get(0));
} else {
createShuffleJarActions(
Expand All @@ -1503,6 +1538,7 @@ private static void createIncrementalDexingActions(
common,
inclusionFilterJar,
dexopts,
minSdkVersion,
androidSemantics,
attributes,
derivedJarFunction,
Expand Down Expand Up @@ -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);
Expand All @@ -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();
Expand All @@ -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);
Expand All @@ -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,
Expand Down Expand Up @@ -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 {
Expand All @@ -1916,6 +1957,7 @@ private static Artifact createShuffleJarActions(
"$dexbuilder_after_proguard",
shuffleOutputs.get(i),
DexArchiveAspect.topLevelDexbuilderDexopts(dexopts),
minSdkVersion,
shards.get(i));
}
}
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinaryMobileInstall.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
4 changes: 4 additions & 0 deletions src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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) {
Expand Down
9 changes: 9 additions & 0 deletions src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading

0 comments on commit 4c3219e

Please sign in to comment.