diff options
4 files changed, 105 insertions, 80 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 f09d09fdbd..f93299c273 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 @@ -394,14 +394,6 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { ImmutableList<Artifact> proguardSpecs = ProguardHelper.collectTransitiveProguardSpecs( ruleContext, ImmutableList.of(resourceApk.getResourceProguardConfig())); - if (shrinkResources) { - resourceApk = shrinkResources( - ruleContext, - androidCommon, - resourceApk, - binaryJar, - proguardSpecs); - } ProguardOutput proguardOutput = applyProguard( ruleContext, @@ -411,6 +403,15 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { filesBuilder, proguardSpecs, proguardMapping); + + if (shrinkResources) { + resourceApk = shrinkResources( + ruleContext, + resourceApk, + proguardSpecs, + proguardOutput); + } + Artifact jarToDex = proguardOutput.getOutputJar(); DexingOutput dexingOutput = shouldDexWithJack(ruleContext) @@ -949,55 +950,14 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { private static ResourceApk shrinkResources( RuleContext ruleContext, - AndroidCommon androidCommon, ResourceApk resourceApk, - Artifact deployJar, - ImmutableList<Artifact> proguardSpecs) throws InterruptedException { + ImmutableList<Artifact> proguardSpecs, + ProguardOutput proguardOutput) throws InterruptedException { if (ruleContext.getFragment(AndroidConfiguration.class).useAndroidResourceShrinking() && LocalResourceContainer.definesAndroidResources(ruleContext.attributes()) && !proguardSpecs.isEmpty()) { - // TODO(apell): Once ProGuard is split into multiple runs, use the Artifact from the shrinking - // pass here instead. - Artifact shrunkJar = ruleContext.getImplicitOutputArtifact( - AndroidRuleClasses.ANDROID_BINARY_SHRUNK_JAR); - AndroidSdkProvider sdk = AndroidSdkProvider.fromRuleContext(ruleContext); - - Iterable<Artifact> libraryJars = NestedSetBuilder.<Artifact>naiveLinkOrder() - .add(sdk.getAndroidJar()) - .addTransitive(androidCommon.getTransitiveNeverLinkLibraries()) - .build(); - Builder builder = new SpawnAction.Builder() - .addInput(deployJar) - .addInputs(libraryJars) - .addInputs(proguardSpecs) - .setExecutable(sdk.getProguard()) - .setProgressMessage("Finding Resource References With Proguard") - .setMnemonic("ProguardResourceMapping") - .addArgument("-injars") - .addArgument(deployJar.getExecPathString()); - - for (Artifact libraryJar : libraryJars) { - builder.addArgument("-libraryjars") - .addArgument(libraryJar.getExecPathString()); - } - - for (Artifact proguardSpec : proguardSpecs) { - builder.addArgument("@" + proguardSpec.getExecPathString()); - } - - builder.addArgument("-ignorewarnings") - .addArgument("-dontnote") - .addArgument("-forceprocessing") - .addArgument("-dontoptimize") - .addArgument("-dontobfuscate") - .addArgument("-dontpreverify") - .addArgument("-outjars") - .addOutputArgument(shrunkJar); - - ruleContext.registerAction(builder.build(ruleContext)); - Artifact apk = new ResourceShrinkerActionBuilder(ruleContext) .setResourceApkOut(ruleContext.getImplicitOutputArtifact( AndroidRuleClasses.ANDROID_RESOURCES_SHRUNK_APK)) @@ -1007,7 +967,8 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { AndroidRuleClasses.ANDROID_RESOURCE_SHRINKER_LOG)) .withResourceFiles(ruleContext.getImplicitOutputArtifact( AndroidRuleClasses.ANDROID_RESOURCES_ZIP)) - .withShrunkJar(shrunkJar) + .withShrunkJar(proguardOutput.getOutputJar()) + .withProguardMapping(proguardOutput.getMapping()) .withPrimary(resourceApk.getPrimaryResource()) .withDependencies(resourceApk.getResourceDependencies()) .setConfigurationFilters( diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceShrinkerActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceShrinkerActionBuilder.java index 8ec9d3c9cf..4feb65b218 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceShrinkerActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceShrinkerActionBuilder.java @@ -33,6 +33,7 @@ import java.util.List; public class ResourceShrinkerActionBuilder { private Artifact resourceFilesZip; private Artifact shrunkJar; + private Artifact proguardMapping; private ResourceContainer primaryResources; private ResourceDependencies dependencyResources; private Artifact resourceApkOut; @@ -89,6 +90,14 @@ public class ResourceShrinkerActionBuilder { } /** + * @param proguardMapping The Proguard mapping between obfuscated and original code. + */ + public ResourceShrinkerActionBuilder withProguardMapping(Artifact proguardMapping) { + this.proguardMapping = proguardMapping; + return this; + } + + /** * @param primary The fully processed {@link ResourceContainer} of the resources to be shrunk. * Must contain both an R.txt and merged manifest. */ @@ -174,6 +183,11 @@ public class ResourceShrinkerActionBuilder { commandLine.addExecPath("--shrunkJar", shrunkJar); inputs.add(shrunkJar); + if (proguardMapping != null) { + commandLine.addExecPath("--proguardMapping", proguardMapping); + inputs.add(proguardMapping); + } + commandLine.addExecPath("--rTxt", primaryResources.getRTxt()); inputs.add(primaryResources.getRTxt()); diff --git a/src/tools/android/java/com/google/devtools/build/android/ResourceShrinker.java b/src/tools/android/java/com/google/devtools/build/android/ResourceShrinker.java index 03accccec6..9d12f06dec 100644 --- a/src/tools/android/java/com/google/devtools/build/android/ResourceShrinker.java +++ b/src/tools/android/java/com/google/devtools/build/android/ResourceShrinker.java @@ -93,40 +93,46 @@ import javax.xml.parsers.ParserConfigurationException; /** * Class responsible for searching through a Gradle built tree (after resource merging, compilation * and ProGuarding has been completed, but before final .apk assembly), which figures out which - * resources if any are unused, and removes them. <p> It does this by examining <ul> <li>The merged - * manifest, to find root resource references (such as drawables used for activity icons)</li> - * <li>The merged R class (to find the actual integer constants assigned to resources)</li> <li>The - * ProGuard log files (to find the mapping from original symbol names to short names)</li>* <li>The - * merged resources (to find which resources reference other resources, e.g. drawable state lists - * including other drawables, or layouts including other layouts, or styles referencing other - * drawables, or menus items including action layouts, etc.)</li> <li>The ProGuard output classes - * (to find resource references in code that are actually reachable)</li> </ul> From all this, it - * builds up a reference graph, and based on the root references (e.g. from the manifest and from - * the remaining code) it computes which resources are actually reachable in the app, and anything - * that is not reachable is then marked for deletion. <p> A resource is referenced in code if either - * the field R.type.name is referenced (which is the case for non-final resource references, e.g. in - * libraries), or if the corresponding int value is referenced (for final resource values). We check - * this by looking at the ProGuard output classes with an ASM visitor. One complication is that code - * can also call {@code Resources#getIdentifier(String,String,String)} where they can pass in the - * names of resources to look up. To handle this scenario, we use the ClassVisitor to see if there - * are any calls to the specific {@code Resources#getIdentifier} method. If not, great, the usage - * analysis is completely accurate. If we <b>do</b> find one, we check <b>all</b> the string - * constants found anywhere in the app, and look to see if any look relevant. For example, if we - * find the string "string/foo" or "my.pkg:string/foo", we will then mark the string resource named - * foo (if any) as potentially used. Similarly, if we find just "foo" or "/foo", we will mark - * <b>all</b> resources named "foo" as potentially used. However, if the string is "bar/foo" or " - * foo " these strings are ignored. This means we can potentially miss resources usages where the - * resource name is completed computed (e.g. by concatenating individual characters or taking - * substrings of strings that do not look like resource names), but that seems extremely unlikely to - * be a real-world scenario. <p> For now, for reasons detailed in the code, this only applies to - * file-based resources like layouts, menus and drawables, not value-based resources like strings - * and dimensions. + * resources if any are unused, and removes them. + * <p>It does this by examining + * <ul> + * <li>The merged manifest, to find root resource references (such as drawables used for activity + * icons)</li> + * <li>The R.txt file (to find the actual integer constants assigned to resources)</li> + * <li>The ProGuard log files (to find the mapping from original symbol names to short names)</li> + * <li>The merged resources (to find which resources reference other resources, e.g. drawable + * state lists including other drawables, or layouts including other layouts, or styles + * referencing other drawables, or menus items including action layouts, etc.)</li> + * <li>The ProGuard output classes (to find resource references in code that are actually + * reachable)</li> + * </ul> + * From all this, it builds up a reference graph, and based on the root references (e.g. from the + * manifest and from the remaining code) it computes which resources are actually reachable in the + * app, and anything that is not reachable is then marked for deletion. + * <p>A resource is referenced in code if either the field R.type.name is referenced (which is the + * case for non-final resource references, e.g. in libraries), or if the corresponding int value is + * referenced (for final resource values). We check this by looking at the ProGuard output classes + * with an ASM visitor. One complication is that code can also call + * {@code Resources#getIdentifier(String,String,String)} where they can pass in the names of + * resources to look up. To handle this scenario, we use the ClassVisitor to see if there are any + * calls to the specific {@code Resources#getIdentifier} method. If not, great, the usage analysis + * is completely accurate. If we <b>do</b> find one, we check <b>all</b> the string constants found + * anywhere in the app, and look to see if any look relevant. For example, if we find the string + * "string/foo" or "my.pkg:string/foo", we will then mark the string resource named foo (if any) as + * potentially used. Similarly, if we find just "foo" or "/foo", we will mark <b>all</b> resources + * named "foo" as potentially used. However, if the string is "bar/foo" or " foo " these strings are + * ignored. This means we can potentially miss resources usages where the resource name is completed + * computed (e.g. by concatenating individual characters or taking substrings of strings that do not + * look like resource names), but that seems extremely unlikely to be a real-world scenario. <p> For + * now, for reasons detailed in the code, this only applies to file-based resources like layouts, + * menus and drawables, not value-based resources like strings and dimensions. */ public class ResourceShrinker { public static final int TYPICAL_RESOURCE_COUNT = 200; private final Set<String> resourcePackages; private final Path rTxt; + private final Path proguardMapping; private final Path classesJar; private final Path mergedManifest; private final Path mergedResourceDir; @@ -162,10 +168,12 @@ public class ResourceShrinker { @NonNull Path rTxt, @NonNull Path classesJar, @NonNull Path manifest, + @Nullable Path mapping, @NonNull Path resources, Path logFile) { this.resourcePackages = resourcePackages; this.rTxt = rTxt; + this.proguardMapping = mapping; this.classesJar = classesJar; this.mergedManifest = manifest; this.mergedResourceDir = resources; @@ -191,6 +199,7 @@ public class ResourceShrinker { public void shrink(Path destinationDir) throws IOException, ParserConfigurationException, SAXException { parseResourceTxtFile(rTxt, resourcePackages); + recordMapping(proguardMapping); recordUsages(classesJar); recordManifestUsages(mergedManifest); recordResources(mergedResourceDir); @@ -665,6 +674,39 @@ public class ResourceShrinker { } } + private void recordMapping(@Nullable Path mapping) throws IOException { + if (mapping == null || !mapping.toFile().exists()) { + return; + } + final String arrowIndicator = " -> "; + final String resourceIndicator = ".R$"; + for (String line : Files.readLines(mapping.toFile(), UTF_8)) { + if (line.startsWith(" ") || line.startsWith("\t")) { + continue; + } + int index = line.indexOf(resourceIndicator); + if (index == -1) { + continue; + } + int arrow = line.indexOf(arrowIndicator, index + 3); + if (arrow == -1) { + continue; + } + String typeName = line.substring(index + resourceIndicator.length(), arrow); + ResourceType type = ResourceType.getEnum(typeName); + if (type == null) { + continue; + } + int end = line.indexOf(':', arrow + arrowIndicator.length()); + if (end == -1) { + end = line.length(); + } + String target = line.substring(arrow + arrowIndicator.length(), end).trim(); + String ownerName = target.replace('.', '/'); + resourceClassOwners.put(ownerName, type); + } + } + private void recordManifestUsages(Path manifest) throws IOException, ParserConfigurationException, SAXException { String xml = Files.toString(manifest.toFile(), UTF_8); diff --git a/src/tools/android/java/com/google/devtools/build/android/ResourceShrinkerAction.java b/src/tools/android/java/com/google/devtools/build/android/ResourceShrinkerAction.java index 1994e8abb9..ea5f1225ff 100644 --- a/src/tools/android/java/com/google/devtools/build/android/ResourceShrinkerAction.java +++ b/src/tools/android/java/com/google/devtools/build/android/ResourceShrinkerAction.java @@ -81,6 +81,13 @@ public class ResourceShrinkerAction { help = "Path to the shrunk jar from a Proguard run with shrinking enabled.") public Path shrunkJar; + @Option(name = "proguardMapping", + defaultValue = "null", + category = "input", + converter = PathConverter.class, + help = "Path to the Proguard obfuscation mapping of shrunkJar.") + public Path proguardMapping; + @Option(name = "resources", defaultValue = "null", category = "input", @@ -201,6 +208,7 @@ public class ResourceShrinkerAction { options.rTxt, options.shrunkJar, options.primaryManifest, + options.proguardMapping, resourceFiles.resolve("res"), options.log); |