diff options
Diffstat (limited to 'src')
5 files changed, 76 insertions, 28 deletions
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 ff521ecd3d..feda3f6a47 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 @@ -81,7 +81,7 @@ public class AndroidResourcesProcessorBuilder { private Artifact rTxtOut; private Artifact sourceJarOut; private boolean debug = false; - private ResourceFilterFactory resourceFilterFactory; + private ResourceFilterFactory resourceFilterFactory = ResourceFilterFactory.empty(); private List<String> uncompressedExtensions = Collections.emptyList(); private Artifact apkOut; private final AndroidSdkProvider sdk; @@ -113,7 +113,6 @@ public class AndroidResourcesProcessorBuilder { this.sdk = AndroidSdkProvider.fromRuleContext(ruleContext); this.ruleContext = ruleContext; this.spawnActionBuilder = new SpawnAction.Builder(); - this.resourceFilterFactory = ResourceFilterFactory.empty(ruleContext); } /** @@ -317,6 +316,10 @@ public class AndroidResourcesProcessorBuilder { builder.add("--conditionalKeepRules"); } + if (resourceFilterFactory.hasDensities()) { + builder.add("--densities", resourceFilterFactory.getDensityString()); + } + configureCommonFlags(outs, inputs, builder); ParamFileInfo.Builder paramFileInfo = ParamFileInfo.builder(ParameterFileType.SHELL_QUOTED); @@ -383,6 +386,21 @@ public class AndroidResourcesProcessorBuilder { builder.addExecPath("--aapt", sdk.getAapt().getExecutable()); configureCommonFlags(outs, inputs, builder); + if (resourceFilterFactory.hasDensities()) { + // If we did not filter by density in analysis, filter in execution. Otherwise, don't filter + // in execution, but still pass the densities so they can be added to the manifest. + if (resourceFilterFactory.isPrefiltering()) { + builder.add("--densitiesForManifest", resourceFilterFactory.getDensityString()); + } else { + builder.add("--densities", resourceFilterFactory.getDensityString()); + } + } + ImmutableList<String> filteredResources = + resourceFilterFactory.getResourcesToIgnoreInExecution(); + if (!filteredResources.isEmpty()) { + builder.addAll("--prefilteredResources", VectorArg.join(",").each(filteredResources)); + } + ParamFileInfo.Builder paramFileInfo = ParamFileInfo.builder(ParameterFileType.SHELL_QUOTED); // Some flags (e.g. --mainData) may specify lists (or lists of lists) separated by special // characters (colon, semicolon, hashmark, ampersand) that don't work on Windows, and quoting @@ -489,20 +507,6 @@ public class AndroidResourcesProcessorBuilder { // might remove resources that we previously accepted. builder.add("--resourceConfigs", resourceFilterFactory.getConfigurationFilterString()); } - if (resourceFilterFactory.hasDensities()) { - // If we did not filter by density in analysis, filter in execution. Otherwise, don't filter - // in execution, but still pass the densities so they can be added to the manifest. - if (resourceFilterFactory.isPrefiltering()) { - builder.add("--densitiesForManifest", resourceFilterFactory.getDensityString()); - } else { - builder.add("--densities", resourceFilterFactory.getDensityString()); - } - } - ImmutableList<String> filteredResources = - resourceFilterFactory.getResourcesToIgnoreInExecution(); - if (!filteredResources.isEmpty()) { - builder.addAll("--prefilteredResources", VectorArg.join(",").each(filteredResources)); - } if (!uncompressedExtensions.isEmpty()) { builder.addAll("--uncompressedExtensions", VectorArg.join(",").each(uncompressedExtensions)); } 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 df468bfa52..8b406cb82d 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 @@ -862,7 +862,7 @@ public final class ApplicationManifest { boolean createSource, Artifact proguardCfg, @Nullable Artifact mainDexProguardCfg) - throws InterruptedException { + throws InterruptedException, RuleErrorException { TransitiveInfoCollection resourcesPrerequisite = ruleContext.getPrerequisite("resources", Mode.TARGET); @@ -919,8 +919,7 @@ public final class ApplicationManifest { for (String extension : uncompressedExtensions) { additionalAaptOpts.add("-0").add(extension); } - if (resourceFilterFactory.hasConfigurationFilters() - && !resourceFilterFactory.isPrefiltering()) { + if (resourceFilterFactory.hasConfigurationFilters()) { additionalAaptOpts.add("-c").add(resourceFilterFactory.getConfigurationFilterString()); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceFilterFactory.java b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceFilterFactory.java index a77946803e..189a14a765 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceFilterFactory.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceFilterFactory.java @@ -34,6 +34,7 @@ import com.google.devtools.build.lib.analysis.config.PatchTransition; import com.google.devtools.build.lib.packages.AttributeMap; import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException; import com.google.devtools.build.lib.packages.RuleErrorConsumer; +import com.google.devtools.build.lib.rules.android.AndroidConfiguration.AndroidAaptVersion; import com.google.devtools.build.lib.syntax.Type; import com.google.devtools.common.options.EnumConverter; import com.google.devtools.common.options.OptionsParsingException; @@ -197,16 +198,22 @@ public class ResourceFilterFactory { return ImmutableList.sortedCopyOf(builder.build()); } - static ResourceFilterFactory fromRuleContext(RuleContext ruleContext) { + static ResourceFilterFactory fromRuleContext(RuleContext ruleContext) throws RuleErrorException { Preconditions.checkNotNull(ruleContext); if (!ruleContext.isLegalFragment(AndroidConfiguration.class)) { - return empty(DEFAULT_BEHAVIOR); + return empty(); } - return forBaseAndAttrs( - ruleContext.getFragment(AndroidConfiguration.class).getResourceFilterFactory(), - ruleContext.attributes()); + ResourceFilterFactory base; + if (AndroidAaptVersion.chooseTargetAaptVersion(ruleContext) == AndroidAaptVersion.AAPT2) { + // aapt2 must have access to all of the resources in execution, so don't filter in analysis. + base = empty(FilterBehavior.FILTER_IN_EXECUTION); + } else { + base = ruleContext.getFragment(AndroidConfiguration.class).getResourceFilterFactory(); + } + + return forBaseAndAttrs(base, ruleContext.attributes()); } @VisibleForTesting @@ -224,7 +231,7 @@ public class ResourceFilterFactory { */ ResourceFilterFactory withAttrsFrom(AttributeMap attrs) { if (!hasFilters(attrs)) { - return new ResourceFilterFactory(configFilters, densities, filterBehavior); + return this; } return new ResourceFilterFactory( @@ -382,8 +389,8 @@ public class ResourceFilterFactory { } } - static ResourceFilterFactory empty(RuleContext ruleContext) { - return empty(fromRuleContext(ruleContext).filterBehavior); + static ResourceFilterFactory empty() { + return empty(DEFAULT_BEHAVIOR); } @VisibleForTesting 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 8844451581..fb1fb56d67 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 @@ -58,7 +58,7 @@ public class ResourceShrinkerActionBuilder { this.ruleContext = ruleContext; this.spawnActionBuilder = new SpawnAction.Builder(); this.sdk = AndroidSdkProvider.fromRuleContext(ruleContext); - this.resourceFilterFactory = ResourceFilterFactory.empty(ruleContext); + this.resourceFilterFactory = ResourceFilterFactory.empty(); } public ResourceShrinkerActionBuilder setUncompressedExtensions( 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 c1df324407..7cdd63005e 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 @@ -1258,6 +1258,44 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { assertThat(resourceArguments(directResources)).contains("hdpi,xhdpi"); } + /** Test that resources are not filtered in analysis under aapt2. */ + @Test + public void testFilteredResourcesFilteringAapt2() throws Exception { + List<String> resources = + ImmutableList.of("res/values/foo.xml", "res/values-en/foo.xml", "res/values-fr/foo.xml"); + String dir = "java/r/android"; + + mockAndroidSdkWithAapt2(); + + useConfiguration( + "--android_sdk=//sdk:sdk", + "--experimental_android_resource_filtering_method", + "filter_in_analysis"); + + ConfiguredTarget binary = + scratchConfiguredTarget( + dir, + "r", + "android_binary(name = 'r',", + " manifest = 'AndroidManifest.xml',", + " resource_configuration_filters = ['', 'en, es, '],", + " aapt_version = 'aapt2',", + " densities = ['hdpi, , ', 'xhdpi'],", + " resource_files = ['" + Joiner.on("', '").join(resources) + "'])"); + ResourceContainer directResources = getResourceContainer(binary, /* transitive= */ false); + + // Validate that the AndroidResourceProvider for this binary contains all values. + assertThat(resourceContentsPaths(dir, directResources)).containsExactlyElementsIn(resources); + + // Validate that the input to resource processing contains all values. + assertThat(resourceInputPaths(dir, directResources)).containsAllIn(resources); + + // Validate that the filters are correctly passed to the resource processing action + // This includes trimming whitespace and ignoring empty filters. + assertThat(resourceArguments(directResources)).contains("en,es"); + assertThat(resourceArguments(directResources)).contains("hdpi,xhdpi"); + } + @Test public void testFilteredResourcesFilteringNotSpecified() throws Exception { // TODO(asteinb): Once prefiltering is run by default, remove this test and remove the |