diff options
Diffstat (limited to 'src')
8 files changed, 160 insertions, 2 deletions
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 index 1bfcac40af..4d389e94d8 100644 --- 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 @@ -217,6 +217,8 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { configurationMap, cppSemantics); + boolean shrinkResources = shouldShrinkResources(ruleContext); + // TODO(bazel-team): Resolve all the different cases of resource handling so this conditional // can go away: recompile from android_resources, and recompile from android_binary attributes. ApplicationManifest applicationManifest; @@ -250,6 +252,7 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { ruleContext.attributes().get("crunch_png", Type.BOOLEAN), ProguardHelper.getProguardConfigArtifact(ruleContext, ""), createMainDexProguardSpec(ruleContext), + shouldShrinkResourceCycles(ruleContext, shrinkResources), ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_PROCESSED_MANIFEST), ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_RESOURCES_ZIP), DataBinding.isEnabled(ruleContext) @@ -294,8 +297,6 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { } - boolean shrinkResources = shouldShrinkResources(ruleContext); - // Remove the library resource JARs from the binary's runtime classpath. // Resource classes from android_library dependencies are replaced by the binary's resource // class. We remove them only at the top level so that resources included by a library that is @@ -785,6 +786,18 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { return (state == TriState.YES); } + /** Returns {@code true} if resource shrinking should be performed. */ + private static boolean shouldShrinkResourceCycles( + RuleContext ruleContext, boolean shrinkResources) throws RuleErrorException { + boolean global = + ruleContext.getFragment(AndroidConfiguration.class).useAndroidResourceCycleShrinking(); + if (global && !shrinkResources) { + throw ruleContext.throwWithRuleError( + "resource cycle shrinking can only be enabled when resource shrinking is enabled"); + } + return global; + } + private static ResourceApk shrinkResources( RuleContext ruleContext, ResourceApk resourceApk, 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 41d9f50e02..6b48275d3f 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 @@ -524,6 +524,18 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { public boolean useAndroidResourceShrinking; @Option( + name = "experimental_android_resource_cycle_shrinking", + defaultValue = "false", + category = "semantics", + documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, + effectTags = {OptionEffectTag.UNKNOWN}, + help = + "Enables more shrinking of code and resources by instructing AAPT2 " + + "to emit conditional Proguard keep rules." + ) + public boolean useAndroidResourceCycleShrinking; + + @Option( name = "android_manifest_merger", defaultValue = "android", category = "semantics", @@ -727,6 +739,7 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { private final boolean useRexToCompressDexFiles; private final boolean allowAndroidLibraryDepsWithoutSrcs; private final boolean useAndroidResourceShrinking; + private final boolean useAndroidResourceCycleShrinking; private final AndroidManifestMerger manifestMerger; private final ApkSigningMethod apkSigningMethod; private final boolean useSingleJarApkBuilder; @@ -763,6 +776,7 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { this.allowAndroidLibraryDepsWithoutSrcs = options.allowAndroidLibraryDepsWithoutSrcs; this.useAndroidResourceShrinking = options.useAndroidResourceShrinking || options.useExperimentalAndroidResourceShrinking; + this.useAndroidResourceCycleShrinking = options.useAndroidResourceCycleShrinking; this.manifestMerger = options.manifestMerger; this.apkSigningMethod = options.apkSigningMethod; this.useSingleJarApkBuilder = options.useSingleJarApkBuilder; @@ -884,6 +898,10 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { return useAndroidResourceShrinking; } + public boolean useAndroidResourceCycleShrinking() { + return useAndroidResourceCycleShrinking; + } + public AndroidAaptVersion getAndroidAaptVersion() { return androidAaptVersion; } diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProcessorBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProcessorBuilder.java index 0adb8b52cd..368fee5d40 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProcessorBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProcessorBuilder.java @@ -77,6 +77,7 @@ public class AndroidResourcesProcessorBuilder { private ResourceDependencies dependencies; private Artifact proguardOut; private Artifact mainDexProguardOut; + private boolean conditionalKeepRules; private Artifact rTxtOut; private Artifact sourceJarOut; private boolean debug = false; @@ -165,6 +166,11 @@ public class AndroidResourcesProcessorBuilder { return this; } + public AndroidResourcesProcessorBuilder conditionalKeepRules(boolean conditionalKeepRules) { + this.conditionalKeepRules = conditionalKeepRules; + return this; + } + public AndroidResourcesProcessorBuilder setMainDexProguardOut(Artifact mainDexProguardCfg) { this.mainDexProguardOut = mainDexProguardCfg; return this; @@ -301,6 +307,10 @@ public class AndroidResourcesProcessorBuilder { builder.add("--useCompiledResourcesForMerge"); } + if (conditionalKeepRules) { + builder.add("--conditionalKeepRules"); + } + configureCommonFlags(outs, inputs, builder); ParamFileInfo.Builder paramFileInfo = ParamFileInfo.builder(ParameterFileType.SHELL_QUOTED); diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ApplicationManifest.java b/src/main/java/com/google/devtools/build/lib/rules/android/ApplicationManifest.java index e378f54fba..70ef13b9aa 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/ApplicationManifest.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/ApplicationManifest.java @@ -585,6 +585,7 @@ public final class ApplicationManifest { boolean crunchPng, Artifact proguardCfg, @Nullable Artifact mainDexProguardCfg, + boolean conditionalKeepRules, Artifact manifestOut, Artifact mergedResources, @Nullable Artifact dataBindingInfoZip, @@ -623,6 +624,11 @@ public final class ApplicationManifest { boolean skipParsingAction = targetAaptVersion == AndroidAaptVersion.AAPT2 && androidConfiguration.skipParsingAction(); + if (conditionalKeepRules && targetAaptVersion != AndroidAaptVersion.AAPT2) { + throw ruleContext.throwWithRuleError( + "resource cycle shrinking can only be enabled for builds with aapt2"); + } + ResourceContainer processed = new AndroidResourcesProcessorBuilder(ruleContext) .setLibrary(false) @@ -638,6 +644,7 @@ public final class ApplicationManifest { .withDependencies(resourceDeps) .setProguardOut(proguardCfg) .setMainDexProguardOut(mainDexProguardCfg) + .conditionalKeepRules(conditionalKeepRules) .setDataBindingInfoZip(dataBindingInfoZip) .setApplicationId(manifestValues.get("applicationId")) .setVersionCode(manifestValues.get("versionCode")) 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 index eebd097d06..8845f56714 100644 --- 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 @@ -885,6 +885,44 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { .isEqualTo(flagValue("--rOutput", processingArgs)); assertThat(flagValue("--primaryManifest", shrinkingArgs)) .isEqualTo(flagValue("--manifestOutput", processingArgs)); + + List<String> packageArgs = + getGeneratingSpawnActionArgs(getFirstArtifactEndingWith(artifacts, "_hello_proguard.cfg")); + + assertThat(flagValue("--tool", packageArgs)).isEqualTo("PACKAGE"); + assertThat(packageArgs).doesNotContain("--conditionalKeepRules"); + } + + @Test + public void testResourceCycleShrinking() throws Exception { + useConfiguration("--experimental_android_resource_cycle_shrinking=true"); + checkError( + "java/a", + "a", + "resource cycle shrinking can only be enabled for builds with aapt2", + "android_binary(", + " name = 'a',", + " srcs = ['A.java'],", + " manifest = 'AndroidManifest.xml',", + " resource_files = [ 'res/values/values.xml' ], ", + " shrink_resources = 1,", + ")"); + } + + @Test + public void testResourceCycleShrinkingWithoutResourceShinking() throws Exception { + useConfiguration("--experimental_android_resource_cycle_shrinking=true"); + checkError( + "java/a", + "a", + "resource cycle shrinking can only be enabled when resource shrinking is enabled", + "android_binary(", + " name = 'a',", + " srcs = ['A.java'],", + " manifest = 'AndroidManifest.xml',", + " resource_files = [ 'res/values/values.xml' ], ", + " shrink_resources = 0,", + ")"); } @Test @@ -3459,6 +3497,58 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { .isEqualTo(flagValue("--rOutput", processingArgs)); assertThat(flagValue("--primaryManifest", shrinkingArgs)) .isEqualTo(flagValue("--manifestOutput", processingArgs)); + + List<String> packageArgs = + getGeneratingSpawnActionArgs(getFirstArtifactEndingWith(artifacts, "_hello_proguard.cfg")); + + assertThat(flagValue("--tool", packageArgs)).isEqualTo("AAPT2_PACKAGE"); + assertThat(packageArgs).doesNotContain("--conditionalKeepRules"); + } + + @Test + public void testAapt2ResourceCycleShrinking() throws Exception { + mockAndroidSdkWithAapt2(); + useConfiguration( + "--android_sdk=//sdk:sdk", "--experimental_android_resource_cycle_shrinking=true"); + scratch.file( + "java/com/google/android/hello/BUILD", + "android_binary(name = 'hello',", + " srcs = ['Foo.java'],", + " manifest = 'AndroidManifest.xml',", + " inline_constants = 0,", + " aapt_version='aapt2',", + " resource_files = ['res/values/strings.xml'],", + " shrink_resources = 1,", + " proguard_specs = ['proguard-spec.pro'],)"); + + ConfiguredTarget binary = getConfiguredTarget("//java/com/google/android/hello:hello"); + + Set<Artifact> artifacts = actionsTestUtil().artifactClosureOf(getFilesToBuild(binary)); + + List<String> packageArgs = + getGeneratingSpawnActionArgs(getFirstArtifactEndingWith(artifacts, "_hello_proguard.cfg")); + + assertThat(flagValue("--tool", packageArgs)).isEqualTo("AAPT2_PACKAGE"); + assertThat(packageArgs).contains("--conditionalKeepRules"); + } + + @Test + public void testAapt2ResourceCycleShinkingWithoutResourceShrinking() throws Exception { + mockAndroidSdkWithAapt2(); + useConfiguration( + "--android_sdk=//sdk:sdk", "--experimental_android_resource_cycle_shrinking=true"); + checkError( + "java/a", + "a", + "resource cycle shrinking can only be enabled when resource shrinking is enabled", + "android_binary(", + " name = 'a',", + " srcs = ['A.java'],", + " manifest = 'AndroidManifest.xml',", + " resource_files = [ 'res/values/values.xml' ], ", + " shrink_resources = 0,", + " aapt_version = 'aapt2'", + ")"); } @Test diff --git a/src/tools/android/java/com/google/devtools/build/android/Aapt2ResourcePackagingAction.java b/src/tools/android/java/com/google/devtools/build/android/Aapt2ResourcePackagingAction.java index 9bdd1a098f..5739afa377 100644 --- a/src/tools/android/java/com/google/devtools/build/android/Aapt2ResourcePackagingAction.java +++ b/src/tools/android/java/com/google/devtools/build/android/Aapt2ResourcePackagingAction.java @@ -29,6 +29,7 @@ import com.google.devtools.build.android.aapt2.ResourceLinker; import com.google.devtools.build.android.aapt2.StaticLibrary; import com.google.devtools.common.options.OptionsParser; import com.google.devtools.common.options.ShellQuotedParamsFilePreProcessor; +import com.google.devtools.common.options.TriState; import java.io.Closeable; import java.io.IOException; import java.nio.file.FileSystems; @@ -186,6 +187,7 @@ public class Aapt2ResourcePackagingAction { .include(compiledResourceDeps) .withAssets(assetDirs) .buildVersion(aaptConfigOptions.buildToolsVersion) + .conditionalKeepRules(aaptConfigOptions.conditionalKeepRules == TriState.YES) .filterToDensity(densitiesToFilter) .includeOnlyConfigs(aaptConfigOptions.resourceConfigs) .link(compiled) diff --git a/src/tools/android/java/com/google/devtools/build/android/aapt2/Aapt2ConfigOptions.java b/src/tools/android/java/com/google/devtools/build/android/aapt2/Aapt2ConfigOptions.java index aa02712957..6d8d614116 100644 --- a/src/tools/android/java/com/google/devtools/build/android/aapt2/Aapt2ConfigOptions.java +++ b/src/tools/android/java/com/google/devtools/build/android/aapt2/Aapt2ConfigOptions.java @@ -90,6 +90,16 @@ public class Aapt2ConfigOptions extends OptionsBase { public TriState useAaptCruncher; @Option( + name = "conditionalKeepRules", + defaultValue = "auto", + category = "config", + documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, + effectTags = {OptionEffectTag.UNKNOWN}, + help = "Have AAPT2 produce conditional keep rules." + ) + public TriState conditionalKeepRules; + + @Option( name = "uncompressedExtensions", defaultValue = "", converter = CommaSeparatedOptionListConverter.class, diff --git a/src/tools/android/java/com/google/devtools/build/android/aapt2/ResourceLinker.java b/src/tools/android/java/com/google/devtools/build/android/aapt2/ResourceLinker.java index 309fa235ab..2787722c36 100644 --- a/src/tools/android/java/com/google/devtools/build/android/aapt2/ResourceLinker.java +++ b/src/tools/android/java/com/google/devtools/build/android/aapt2/ResourceLinker.java @@ -63,6 +63,7 @@ public class ResourceLinker { private Path baseApk; private List<CompiledResources> include = ImmutableList.of(); private List<Path> assetDirs = ImmutableList.of(); + private boolean conditionalKeepRules = false; private ResourceLinker(Path aapt2, Path workingDirectory) { this.aapt2 = aapt2; @@ -101,6 +102,11 @@ public class ResourceLinker { return this; } + public ResourceLinker conditionalKeepRules(boolean conditionalKeepRules) { + this.conditionalKeepRules = conditionalKeepRules; + return this; + } + public ResourceLinker baseApkToLinkAgainst(Path baseApk) { this.baseApk = baseApk; return this; @@ -243,6 +249,8 @@ public class ResourceLinker { .add("--java", javaSourceDirectory) .add("--proguard", proguardConfig) .add("--proguard-main-dex", mainDexProguard) + .when(conditionalKeepRules) + .thenAdd("--proguard-conditional-keep-rules") .add("-o", outPath) .execute(String.format("Linking %s", compiled.getManifest()))); profiler.recordEndOf("fulllink"); |