Skip to content

Commit

Permalink
Grab Java resources from _proguard.jar instead of _deploy.jar in Andr…
Browse files Browse the repository at this point in the history
…oidBinary.

Since bytecode optimizers may choose to keep or leave Java resources in some
cases, use _proguard.jar as the source of truth for resources to be copied into
the APKs. This allows the optimizers' changes to impact the final binary.

PiperOrigin-RevId: 345555308
  • Loading branch information
Googler authored and copybara-github committed Dec 4, 2020
1 parent b213637 commit 5a51814
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1067,6 +1067,10 @@ private static DexingOutput dex(
Artifact inclusionFilterJar =
isBinaryJarFiltered && Objects.equals(binaryJar, proguardedJar) ? binaryJar : null;
Artifact singleJarToDex = !Objects.equals(binaryJar, proguardedJar) ? proguardedJar : null;
Artifact javaResourceSourceJar =
AndroidCommon.getAndroidConfig(ruleContext).getJavaResourcesFromOptimizedJar()
? proguardedJar
: binaryJar;
if (multidexMode == MultidexMode.OFF) {
// Single dex mode: generate classes.dex directly from the input jar.
if (usesDexArchives) {
Expand All @@ -1083,7 +1087,7 @@ private static DexingOutput dex(
/*multidex=*/ false,
/*mainDexList=*/ null,
classesDex);
return new DexingOutput(classesDex, binaryJar, ImmutableList.of(classesDex));
return new DexingOutput(classesDex, javaResourceSourceJar, ImmutableList.of(classesDex));
} else {
// By *not* writing a zip we get dx to drop resources on the floor.
Artifact classesDex = getDxArtifact(ruleContext, "classes.dex");
Expand All @@ -1094,7 +1098,7 @@ private static DexingOutput dex(
dexopts,
/*multidex=*/ false,
/*mainDexList=*/ null);
return new DexingOutput(classesDex, binaryJar, ImmutableList.of(classesDex));
return new DexingOutput(classesDex, javaResourceSourceJar, ImmutableList.of(classesDex));
}
} else {
// Multidex mode: generate classes.dex.zip, where the zip contains [classes.dex,
Expand Down Expand Up @@ -1178,7 +1182,7 @@ private static DexingOutput dex(
// with incremental dexing b/c bazel can create the "incremental" and "split resource"
// APKs earlier (b/c these APKs don't depend on code being dexed here). This is also done
// for other multidex modes.
javaResourceJar = binaryJar;
javaResourceJar = javaResourceSourceJar;
}
return new DexingOutput(classesDex, javaResourceJar, shardDexes);
} else {
Expand Down Expand Up @@ -1212,7 +1216,7 @@ private static DexingOutput dex(
mainDexList);
createCleanDexZipAction(ruleContext, classesDexIntermediate, classesDex);
}
return new DexingOutput(classesDex, binaryJar, ImmutableList.of(classesDex));
return new DexingOutput(classesDex, javaResourceSourceJar, ImmutableList.of(classesDex));
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -916,6 +916,16 @@ public static class Options extends FragmentOptions {
+ "for instrumentation testing).")
public boolean disableInstrumentationManifestMerging;

@Option(
name = "experimental_get_android_java_resources_from_optimized_jar",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
effectTags = {OptionEffectTag.CHANGES_INPUTS},
help =
"Get Java resources from _proguard.jar instead of _deploy.jar in android_binary when "
+ "bundling the final APK.")
public boolean getJavaResourcesFromOptimizedJar;

@Override
public FragmentOptions getHost() {
Options host = (Options) super.getHost();
Expand Down Expand Up @@ -997,6 +1007,7 @@ public FragmentOptions getHost() {
private final Label legacyMainDexListGenerator;
private final boolean disableInstrumentationManifestMerging;
private final boolean incompatibleUseToolchainResolution;
private final boolean getJavaResourcesFromOptimizedJar;

public AndroidConfiguration(BuildOptions buildOptions) throws InvalidConfigurationException {
Options options = buildOptions.get(Options.class);
Expand Down Expand Up @@ -1055,6 +1066,7 @@ public AndroidConfiguration(BuildOptions buildOptions) throws InvalidConfigurati
this.legacyMainDexListGenerator = options.legacyMainDexListGenerator;
this.disableInstrumentationManifestMerging = options.disableInstrumentationManifestMerging;
this.incompatibleUseToolchainResolution = options.incompatibleUseToolchainResolution;
this.getJavaResourcesFromOptimizedJar = options.getJavaResourcesFromOptimizedJar;

if (incrementalDexingShardsAfterProguard < 0) {
throw new InvalidConfigurationException(
Expand Down Expand Up @@ -1321,6 +1333,10 @@ public boolean disableInstrumentationManifestMerging() {
return disableInstrumentationManifestMerging;
}

public boolean getJavaResourcesFromOptimizedJar() {
return getJavaResourcesFromOptimizedJar;
}

/** Returns the label provided with --legacy_main_dex_list_generator, if any. */
// TODO(b/147692286): Move R8's main dex list tool into tool repository.
@StarlarkConfigurationField(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4761,6 +4761,46 @@ public void testInstrumentsManifestMergeDisabled() throws Exception {
.isEmpty();
}

@Test
public void testOptimizedJavaResourcesDisabled() throws Exception {
useConfiguration("--noexperimental_get_android_java_resources_from_optimized_jar");
ConfiguredTarget ct =
scratchConfiguredTarget(
"java/a",
"a",
"android_binary(",
" name = 'a',",
" srcs = ['A.java'],",
" manifest = 'AndroidManifest.xml',",
" proguard_specs = ['proguard.cfg'],",
")");
Set<Artifact> artifacts = actionsTestUtil().artifactClosureOf(getFilesToBuild(ct));
Artifact extractedResources = getFirstArtifactEndingWith(artifacts, "extracted_a_deploy.jar");
String args = Joiner.on(" ").join(getGeneratingSpawnActionArgs(extractedResources));
assertThat(args).contains("/a_deploy.jar");
assertThat(args).doesNotContain("a_proguard.jar");
}

@Test
public void testOptimizedJavaResourcesEnabled() throws Exception {
useConfiguration("--experimental_get_android_java_resources_from_optimized_jar");
ConfiguredTarget ct =
scratchConfiguredTarget(
"java/a",
"a",
"android_binary(",
" name = 'a',",
" srcs = ['A.java'],",
" manifest = 'AndroidManifest.xml',",
" proguard_specs = ['proguard.cfg'],",
")");
Set<Artifact> artifacts = actionsTestUtil().artifactClosureOf(getFilesToBuild(ct));
Artifact extractedResources = getFirstArtifactEndingWith(artifacts, "extracted_a_proguard.jar");
String args = Joiner.on(" ").join(getGeneratingSpawnActionArgs(extractedResources));
assertThat(args).doesNotContain("a_deploy.jar");
assertThat(args).contains("/a_proguard.jar");
}

// DEPENDENCY order is not tested; the incorrect order of dependencies means the test would
// have to enforce incorrect behavior.
// TODO(b/117338320): Add a test when dependency order is fixed.
Expand Down

0 comments on commit 5a51814

Please sign in to comment.