aboutsummaryrefslogtreecommitdiffhomepage
path: root/src
diff options
context:
space:
mode:
authorGravatar Googler <noreply@google.com>2017-03-24 15:32:53 +0000
committerGravatar Philipp Wollermann <philwo@google.com>2017-03-27 11:35:01 +0000
commit74e90608de56759247c12f0eabfdcf26b0cfd19f (patch)
tree666c65461da650ced96242ceffb047083641e8d0 /src
parent28d378bb6d62a59930142d618af882b257491c05 (diff)
Un-rollback and fix bugs in resource density filtering
Rollback of commit df366408188f0451bae9b2ed079c795a4beb2e2b. In addition to undoing the rollback of my previous change, fix the bugs it introduced and add tests for those bugs. Always ignore empty filters. Empty filters are always useless or counterproductive. Before the original change, empty filters as a single item within the list of filters (e.g., ["en", ""]) were ignored, but empty filters as a portion of a string in the list (e.g., ["en,"]) were not. I can't imagine any reason people would actually want the empty filter (if it were handled correctly, it would effectively tell Bazel to just ignore every other filter the user passed in). Since it makes more sense with the new code, represent the stringified filters as a list of strings, rather than a single string of comma-seperated values. Manually trim whitespace from each token. Before the original change, the code trimmed whitespace following commas (e.g., ["en, es"] -> ["en,es"]) but not otherwise. If we're allowing whitespace in filter strings anyway, there doesn't seem to be any reason to allow it in some places but not others. -- PiperOrigin-RevId: 151128685 MOS_MIGRATED_REVID=151128685
Diffstat (limited to 'src')
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java21
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinaryOnlyRule.java4
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibrary.java3
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProcessorBuilder.java30
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/android/ApplicationManifest.java39
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/android/ResourceConfigurationFilter.java203
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/android/ResourceContainer.java2
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/android/ResourceDependencies.java8
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/android/ResourceFilter.java502
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/android/ResourceShrinkerActionBuilder.java19
-rw-r--r--src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java183
11 files changed, 737 insertions, 277 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 ad872d4e0e..32e2aa5b70 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
@@ -237,10 +237,9 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
resourceDeps,
ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_R_TXT),
null, /* Artifact symbolsTxt */
- ResourceConfigurationFilter.fromRuleContext(ruleContext),
+ ResourceFilter.fromRuleContext(ruleContext),
ruleContext.getTokenizedStringListAttr("nocompress_extensions"),
ruleContext.attributes().get("crunch_png", Type.BOOLEAN),
- ruleContext.getTokenizedStringListAttr("densities"),
false, /* incremental */
ProguardHelper.getProguardConfigArtifact(ruleContext, ""),
createMainDexProguardSpec(ruleContext),
@@ -261,10 +260,9 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
resourceDeps,
null, /* Artifact rTxt */
null, /* Artifact symbolsTxt */
- ResourceConfigurationFilter.fromRuleContext(ruleContext),
+ ResourceFilter.fromRuleContext(ruleContext),
ruleContext.getTokenizedStringListAttr("nocompress_extensions"),
ruleContext.attributes().get("crunch_png", Type.BOOLEAN),
- ruleContext.getTokenizedStringListAttr("densities"),
true, /* incremental */
ProguardHelper.getProguardConfigArtifact(ruleContext, "incremental"),
null, /* mainDexProguardCfg */
@@ -284,10 +282,9 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
resourceDeps,
null, /* Artifact rTxt */
null, /* Artifact symbolsTxt */
- ResourceConfigurationFilter.fromRuleContext(ruleContext),
+ ResourceFilter.fromRuleContext(ruleContext),
ruleContext.getTokenizedStringListAttr("nocompress_extensions"),
ruleContext.attributes().get("crunch_png", Type.BOOLEAN),
- ruleContext.getTokenizedStringListAttr("densities"),
true, /* incremental */
ProguardHelper.getProguardConfigArtifact(ruleContext, "instant_run"),
null, /* mainDexProguardCfg */
@@ -307,10 +304,9 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
resourceDeps,
null, /* Artifact rTxt */
null, /* Artifact symbolsTxt */
- ResourceConfigurationFilter.fromRuleContext(ruleContext),
+ ResourceFilter.fromRuleContext(ruleContext),
ruleContext.getTokenizedStringListAttr("nocompress_extensions"),
ruleContext.attributes().get("crunch_png", Type.BOOLEAN),
- ruleContext.getTokenizedStringListAttr("densities"),
true, /* incremental */
ProguardHelper.getProguardConfigArtifact(ruleContext, "incremental_split"),
null, /* mainDexProguardCfg */
@@ -1164,7 +1160,8 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
.withProguardMapping(proguardOutput.getMapping())
.withPrimary(resourceApk.getPrimaryResource())
.withDependencies(resourceApk.getResourceDependencies())
- .setConfigurationFilters(ResourceConfigurationFilter.fromRuleContext(ruleContext))
+ .setResourceFilter(ResourceFilter.fromRuleContext(ruleContext))
+
.setUncompressedExtensions(
ruleContext.getTokenizedStringListAttr("nocompress_extensions"))
.build();
@@ -1773,16 +1770,14 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
* <p>The resources should be regenerated (using aapt) if any of the following are true:
* <ul>
* <li>There is more than one resource container
- * <li>There are densities to filter by.
- * <li>There are resource configuration filters.
+ * <li>There are resource filters.
* <li>There are extensions that should be compressed.
* </ul>
*/
public static boolean shouldRegenerate(RuleContext ruleContext,
ResourceDependencies resourceDeps) {
return Iterables.size(resourceDeps.getResources()) > 1
- || ruleContext.attributes().isAttributeValueExplicitlySpecified("densities")
- || ResourceConfigurationFilter.hasFilters(ruleContext)
+ || ResourceFilter.hasFilters(ruleContext)
|| ruleContext.attributes().isAttributeValueExplicitlySpecified("nocompress_extensions");
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinaryOnlyRule.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinaryOnlyRule.java
index d4bb19487f..3d8efbaade 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinaryOnlyRule.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinaryOnlyRule.java
@@ -84,7 +84,7 @@ public final class AndroidBinaryOnlyRule implements RuleDefinition {
A list of resource configuration filters, such 'en' that will limit the resources in the
apk to only the ones in the 'en' configuration.
<!-- #END_BLAZE_RULE.ATTRIBUTE --> */
- .add(attr(ResourceConfigurationFilter.ATTR_NAME, STRING_LIST))
+ .add(attr(ResourceFilter.RESOURCE_CONFIGURATION_FILTERS_NAME, STRING_LIST))
/* <!-- #BLAZE_RULE(android_binary).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
@@ -116,7 +116,7 @@ public final class AndroidBinaryOnlyRule implements RuleDefinition {
section will also be added to the manifest if it does not already contain a superset
listing.
<!-- #END_BLAZE_RULE.ATTRIBUTE --> */
- .add(attr("densities", STRING_LIST))
+ .add(attr(ResourceFilter.DENSITIES_NAME, STRING_LIST))
.add(attr("$android_manifest_merge_tool", LABEL)
.cfg(HOST)
.exec()
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibrary.java
index f573132b03..ef4ff5295b 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibrary.java
@@ -97,10 +97,9 @@ public abstract class AndroidLibrary implements RuleConfiguredTargetFactory {
ResourceDependencies.fromRuleDeps(ruleContext, JavaCommon.isNeverLink(ruleContext)),
ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_R_TXT),
ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_MERGED_SYMBOLS),
- ResourceConfigurationFilter.empty(ruleContext),
+ ResourceFilter.empty(ruleContext),
ImmutableList.<String>of(), /* uncompressedExtensions */
false, /* crunchPng */
- ImmutableList.<String>of(), /* densities */
false, /* incremental */
null, /* proguardCfgOut */
null, /* mainDexProguardCfg */
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 75fbfa697a..6bee442d83 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
@@ -69,13 +69,12 @@ public class AndroidResourcesProcessorBuilder {
private Artifact rTxtOut;
private Artifact sourceJarOut;
private boolean debug = false;
- private ResourceConfigurationFilter resourceConfigs;
+ private ResourceFilter resourceFilter;
private List<String> uncompressedExtensions = Collections.emptyList();
private Artifact apkOut;
private final AndroidSdkProvider sdk;
private List<String> assetsToIgnore = Collections.emptyList();
private SpawnAction.Builder spawnActionBuilder;
- private List<String> densities = Collections.emptyList();
private String customJavaPackage;
private final RuleContext ruleContext;
private String versionCode;
@@ -98,7 +97,7 @@ public class AndroidResourcesProcessorBuilder {
this.sdk = AndroidSdkProvider.fromRuleContext(ruleContext);
this.ruleContext = ruleContext;
this.spawnActionBuilder = new SpawnAction.Builder();
- this.resourceConfigs = ResourceConfigurationFilter.empty(ruleContext);
+ this.resourceFilter = ResourceFilter.empty(ruleContext);
}
/**
@@ -136,14 +135,9 @@ public class AndroidResourcesProcessorBuilder {
return this;
}
- public AndroidResourcesProcessorBuilder setDensities(List<String> densities) {
- this.densities = densities;
- return this;
- }
-
- public AndroidResourcesProcessorBuilder setConfigurationFilters(
- ResourceConfigurationFilter resourceConfigs) {
- this.resourceConfigs = resourceConfigs;
+ public AndroidResourcesProcessorBuilder setResourceFilter(
+ ResourceFilter resourceFilter) {
+ this.resourceFilter = resourceFilter;
return this;
}
@@ -215,7 +209,7 @@ public class AndroidResourcesProcessorBuilder {
public ResourceContainer build(ActionConstructionContext context) {
List<Artifact> outs = new ArrayList<>();
CustomCommandLine.Builder builder = new CustomCommandLine.Builder();
-
+
// Set the busybox tool.
builder.add("--tool").add("PACKAGE").add("--");
@@ -285,11 +279,15 @@ public class AndroidResourcesProcessorBuilder {
builder.addExecPath("--packagePath", apkOut);
outs.add(apkOut);
}
- if (!resourceConfigs.isEmpty()) {
- builder.add("--resourceConfigs").add(resourceConfigs.getFilterString());
+ if (resourceFilter.hasConfigurationFilters() && !resourceFilter.isPrefiltering()) {
+ builder.add("--resourceConfigs").add(resourceFilter.getConfigurationFilterString());
+ }
+ if (resourceFilter.hasDensities() && !resourceFilter.isPrefiltering()) {
+ builder.add("--densities").add(resourceFilter.getDensityString());
}
- if (!densities.isEmpty()) {
- builder.addJoinStrings("--densities", ",", densities);
+ ImmutableList<String> filteredResources = resourceFilter.getFilteredResources();
+ if (!filteredResources.isEmpty()) {
+ builder.addJoinStrings("--prefilteredResources", ",", filteredResources);
}
if (!uncompressedExtensions.isEmpty()) {
builder.addJoinStrings("--uncompressedExtensions", ",", 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 32e78d86e4..441b0c4b12 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
@@ -300,10 +300,9 @@ public final class ApplicationManifest {
ruleContext,
false, /* isLibrary */
resourceDeps,
- ResourceConfigurationFilter.empty(ruleContext),
+ ResourceFilter.empty(ruleContext),
ImmutableList.<String>of(), /* uncompressedExtensions */
true, /* crunchPng */
- ImmutableList.<String>of(), /* densities */
incremental,
ResourceContainer.builderFromRule(ruleContext)
.setAssetsAndResourcesFrom(data)
@@ -348,10 +347,9 @@ public final class ApplicationManifest {
ruleContext,
true, /* isLibrary */
resourceDeps,
- ResourceConfigurationFilter.empty(ruleContext),
+ ResourceFilter.empty(ruleContext),
ImmutableList.<String>of(), /* List<String> uncompressedExtensions */
false, /* crunchPng */
- ImmutableList.<String>of(), /* List<String> densities */
false, /* incremental */
resourceContainer.build(),
data,
@@ -373,10 +371,9 @@ public final class ApplicationManifest {
ResourceDependencies resourceDeps,
Artifact rTxt,
Artifact symbols,
- ResourceConfigurationFilter configurationFilters,
+ ResourceFilter resourceFilter,
List<String> uncompressedExtensions,
boolean crunchPng,
- List<String> densities,
boolean incremental,
Artifact proguardCfg,
@Nullable Artifact mainDexProguardCfg,
@@ -405,10 +402,9 @@ public final class ApplicationManifest {
ruleContext,
isLibrary,
resourceDeps,
- configurationFilters,
+ resourceFilter,
uncompressedExtensions,
crunchPng,
- densities,
incremental,
ResourceContainer.builderFromRule(ruleContext)
.setAssetsAndResourcesFrom(data)
@@ -431,10 +427,9 @@ public final class ApplicationManifest {
RuleContext ruleContext,
boolean isLibrary,
ResourceDependencies resourceDeps,
- ResourceConfigurationFilter configurationFilters,
+ ResourceFilter resourceFilter,
List<String> uncompressedExtensions,
boolean crunchPng,
- List<String> densities,
boolean incremental,
ResourceContainer maybeInlinedResourceContainer,
LocalResourceContainer data,
@@ -455,11 +450,11 @@ public final class ApplicationManifest {
if (ruleContext.hasErrors()) {
return null;
}
-
+
// Filter the resources during analysis to prevent processing of and dependencies on unwanted
// resources during execution.
- resourceContainer = resourceContainer.filter(configurationFilters);
- resourceDeps = resourceDeps.filter(configurationFilters);
+ resourceContainer = resourceContainer.filter(resourceFilter);
+ resourceDeps = resourceDeps.filter(resourceFilter);
ResourceContainer processed;
if (isLibrary && AndroidCommon.getAndroidConfig(ruleContext).useParallelResourceProcessing()) {
@@ -507,7 +502,7 @@ public final class ApplicationManifest {
new AndroidResourcesProcessorBuilder(ruleContext)
.setLibrary(isLibrary)
.setApkOut(resourceContainer.getApk())
- .setConfigurationFilters(configurationFilters)
+ .setResourceFilter(resourceFilter)
.setUncompressedExtensions(uncompressedExtensions)
.setCrunchPng(crunchPng)
.setJavaPackage(resourceContainer.getJavaPackage())
@@ -516,7 +511,6 @@ public final class ApplicationManifest {
.setMergedResourcesOut(mergedResources)
.withPrimary(resourceContainer)
.withDependencies(resourceDeps)
- .setDensities(densities)
.setProguardOut(proguardCfg)
.setMainDexProguardOut(mainDexProguardCfg)
.setDataBindingInfoZip(dataBindingInfoZip)
@@ -623,7 +617,8 @@ public final class ApplicationManifest {
AndroidAaptActionHelper aaptActionHelper = new AndroidAaptActionHelper(ruleContext,
getManifest(), Lists.newArrayList(resourceContainers));
- String resourceConfigurationFilters = ResourceConfigurationFilter.extractFilters(ruleContext);
+ ResourceFilter resourceFilter = ResourceFilter.fromRuleContext(ruleContext);
+
List<String> uncompressedExtensions =
ruleContext.getTokenizedStringListAttr("nocompress_extensions");
@@ -632,8 +627,8 @@ public final class ApplicationManifest {
for (String extension : uncompressedExtensions) {
additionalAaptOpts.add("-0").add(extension);
}
- if (!resourceConfigurationFilters.isEmpty()) {
- additionalAaptOpts.add("-c").add(resourceConfigurationFilters);
+ if (resourceFilter.hasConfigurationFilters() && !resourceFilter.isPrefiltering()) {
+ additionalAaptOpts.add("-c").add(resourceFilter.getConfigurationFilterString());
}
Artifact javaSourcesJar = null;
@@ -645,9 +640,11 @@ public final class ApplicationManifest {
javaSourcesJar, null, resourceContainer.getJavaPackage(), true);
}
- List<String> densities = ruleContext.getTokenizedStringListAttr("densities");
- aaptActionHelper.createGenerateApkAction(resourceApk,
- resourceContainer.getRenameManifestPackage(), additionalAaptOpts.build(), densities);
+ aaptActionHelper.createGenerateApkAction(
+ resourceApk,
+ resourceContainer.getRenameManifestPackage(),
+ additionalAaptOpts.build(),
+ resourceFilter.getDensities());
ResourceContainer updatedResources =
resourceContainer
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceConfigurationFilter.java b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceConfigurationFilter.java
deleted file mode 100644
index fb27cdd833..0000000000
--- a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceConfigurationFilter.java
+++ /dev/null
@@ -1,203 +0,0 @@
-// Copyright 2017 The Bazel Authors. All rights reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-package com.google.devtools.build.lib.rules.android;
-
-import com.android.ide.common.resources.configuration.FolderConfiguration;
-import com.android.ide.common.resources.configuration.VersionQualifier;
-import com.google.common.base.Joiner;
-import com.google.common.base.Preconditions;
-import com.google.common.collect.ImmutableList;
-import com.google.devtools.build.lib.actions.Artifact;
-import com.google.devtools.build.lib.analysis.RuleContext;
-import com.google.devtools.build.lib.collect.nestedset.NestedSet;
-import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
-
-/** Filters resources based on their qualifiers. */
-public class ResourceConfigurationFilter {
- public static final String ATTR_NAME = "resource_configuration_filters";
-
- /** The current {@link RuleContext}, used for reporting errors. */
- private final RuleContext ruleContext;
-
- /**
- * A list of filters that should be applied during the analysis phase. If resources should not be
- * filtered in analysis (e.g., if prefilter_resources = 0), this list should be empty.
- */
- private final ImmutableList<FolderConfiguration> filters;
-
- /**
- * The raw value of the resource_configuration_filters attribute, as a string of comma-separated
- * qualifier strings. This value is passed directly as input to resource processing utilities that
- * run in the execution phase, so it may represent different qualifiers than those in {@link
- * #filters} if resources should not be pre-filtered during analysis.
- */
- private final String filterString;
-
- /**
- * @param ruleContext the current {@link RuleContext}, used for reporting errors
- * @param filters a list of filters that should be applied during analysis time. If resources
- * should not be filtered in analysis (e.g., if prefilter_resources = 0), this list should be
- * empty.
- * @param filterString the raw value of the resource configuration filters, as a comma-seperated
- * string
- */
- private ResourceConfigurationFilter(
- RuleContext ruleContext, ImmutableList<FolderConfiguration> filters, String filterString) {
- this.ruleContext = ruleContext;
- this.filters = filters;
- this.filterString = filterString;
- }
-
- static boolean hasFilters(RuleContext ruleContext) {
- return ruleContext.attributes().isAttributeValueExplicitlySpecified(ATTR_NAME);
- }
-
- /**
- * Extracts the filters from the current RuleContext, as a string.
- *
- * <p>In BUILD files, filters can be represented as a list of strings, a single comma-seperated
- * string, or a combination of both. This method outputs a single comma-seperated string of
- * filters, which can then be passed directly to resource processing actions.
- *
- * @param ruleContext the current {@link RuleContext}
- * @return the resource configuration filters contained in the {@link RuleContext}, in a single
- * comma-separated string, or an empty string if no filters exist.
- */
- static String extractFilters(RuleContext ruleContext) {
- if (!hasFilters(ruleContext)) {
- return "";
- }
-
- return Joiner.on(',').join(ruleContext.getTokenizedStringListAttr(ATTR_NAME));
- }
-
- static ResourceConfigurationFilter fromRuleContext(RuleContext ruleContext) {
- Preconditions.checkNotNull(ruleContext);
-
- String resourceConfigurationFilters = extractFilters(ruleContext);
-
- if (resourceConfigurationFilters.isEmpty()) {
- return empty(ruleContext);
- }
-
- boolean shouldPrefilter =
- ruleContext.getFragment(AndroidConfiguration.class).useResourcePrefiltering();
-
- if (!shouldPrefilter) {
- return new ResourceConfigurationFilter(
- ruleContext, ImmutableList.<FolderConfiguration>of(), resourceConfigurationFilters);
- }
-
- ImmutableList.Builder<FolderConfiguration> builder = ImmutableList.builder();
-
- for (String filter : resourceConfigurationFilters.split(",")) {
- FolderConfiguration folderConfig = FolderConfiguration.getConfigForQualifierString(filter);
-
- if (folderConfig == null) {
- ruleContext.attributeError(
- ATTR_NAME, "String '" + filter + "' is not a valid resource configuration filter");
- } else {
- builder.add(folderConfig);
- }
- }
-
- return new ResourceConfigurationFilter(
- ruleContext, builder.build(), resourceConfigurationFilters);
- }
-
- static ResourceConfigurationFilter empty(RuleContext ruleContext) {
- return new ResourceConfigurationFilter(
- ruleContext, ImmutableList.<FolderConfiguration>of(), "");
- }
-
- NestedSet<ResourceContainer> filter(NestedSet<ResourceContainer> resources) {
- if (filters.isEmpty()) {
- /*
- * If the filter is empty or resource prefiltering is disabled, just return the original,
- * rather than make a copy.
- *
- * Resources should only be prefiltered in top-level android targets (such as android_binary).
- * The output of resource processing, which includes the input NestedSet<ResourceContainer>
- * returned by this method, is exposed to other actions via the AndroidResourcesProvider. If
- * this method did a no-op copy and collapse in those cases, rather than just return the
- * original NestedSet, we would lose all of the advantages around memory and time that
- * NestedSets provide: each android_library target would have to copy the resources provided
- * by its dependencies into a new NestedSet rather than just create a NestedSet pointing at
- * its dependencies's NestedSets.
- */
- return resources;
- }
-
- NestedSetBuilder<ResourceContainer> builder = new NestedSetBuilder<>(resources.getOrder());
-
- for (ResourceContainer resource : resources) {
- builder.add(resource.filter(this));
- }
-
- return builder.build();
- }
-
- ImmutableList<Artifact> filter(ImmutableList<Artifact> artifacts) {
- if (filters.isEmpty()) {
- return artifacts;
- }
-
- ImmutableList.Builder<Artifact> builder = ImmutableList.builder();
-
- for (Artifact artifact : artifacts) {
- String containingFolder = artifact.getPath().getParentDirectory().getBaseName();
- if (matches(containingFolder)) {
- builder.add(artifact);
- }
- }
-
- return builder.build();
- }
-
- private boolean matches(String containingFolder) {
- FolderConfiguration config = FolderConfiguration.getConfigForFolder(containingFolder);
-
- if (config == null) {
- ruleContext.ruleError(
- "Resource folder '" + containingFolder + "' has invalid resource qualifiers");
-
- return true;
- }
-
- // aapt explicitly ignores the version qualifier; duplicate this behavior here.
- config.setVersionQualifier(VersionQualifier.getQualifier(""));
-
- for (FolderConfiguration filter : filters) {
- if (config.isMatchFor(filter)) {
- return true;
- }
- }
-
- return false;
- }
-
- /**
- * Returns if this object represents an empty filter.
- *
- * <p>Note that non-empty filters are not guaranteed to filter resources during the analysis
- * phase.
- */
- boolean isEmpty() {
- return filterString.isEmpty();
- }
-
- String getFilterString() {
- return filterString;
- }
-}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceContainer.java b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceContainer.java
index c89de68856..a18daa0027 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceContainer.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceContainer.java
@@ -128,7 +128,7 @@ public abstract class ResourceContainer {
/**
* Returns a copy of this container with filtered resources. The original container is unchanged.
*/
- public ResourceContainer filter(ResourceConfigurationFilter filter) {
+ public ResourceContainer filter(ResourceFilter filter) {
return toBuilder().setResources(filter.filter(getResources())).build();
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceDependencies.java b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceDependencies.java
index 03f85b3556..72e76c8763 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceDependencies.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceDependencies.java
@@ -29,7 +29,7 @@ import com.google.devtools.build.lib.packages.BuildType;
* abstraction simplifies the process of managing and exporting the direct and transitive resource
* dependencies of an android rule, as well as providing type safety.
*
- * <p>The transitive and direct dependencies are not guaranteed to be disjoint. If a
+ * <p>The transitive and direct dependencies are not guaranteed to be disjoint. If a
* library is included in both the transitive and direct dependencies, it will appear twice. This
* requires consumers to manage duplicated resources gracefully.
*/
@@ -42,7 +42,7 @@ public final class ResourceDependencies {
private final NestedSet<ResourceContainer> transitiveResources;
/**
* Contains all the direct dependencies of the current target. Since a given direct dependency can
- * act as a "forwarding" library, collecting all the direct resource from it's dependencies
+ * act as a "forwarding" library, collecting all the direct resource from it's dependencies
* and providing them as "direct" dependencies to maintain merge order, this uses a NestedSet to
* properly maintain ordering and ease of merging.
*/
@@ -151,9 +151,9 @@ public final class ResourceDependencies {
this.transitiveResources = transitiveResources;
this.directResources = directResources;
}
-
+
/** Returns a copy of this instance with filtered resources. The original object is unchanged. */
- public ResourceDependencies filter(ResourceConfigurationFilter filter) {
+ public ResourceDependencies filter(ResourceFilter filter) {
return new ResourceDependencies(
neverlink, filter.filter(transitiveResources), filter.filter(directResources));
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceFilter.java b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceFilter.java
new file mode 100644
index 0000000000..e7652efbba
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceFilter.java
@@ -0,0 +1,502 @@
+// Copyright 2017 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.rules.android;
+
+import com.android.ide.common.resources.configuration.DensityQualifier;
+import com.android.ide.common.resources.configuration.FolderConfiguration;
+import com.android.ide.common.resources.configuration.VersionQualifier;
+import com.android.resources.Density;
+import com.android.resources.ResourceFolderType;
+import com.google.common.base.Joiner;
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.analysis.RuleContext;
+import com.google.devtools.build.lib.collect.nestedset.NestedSet;
+import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
+import com.google.devtools.build.lib.syntax.Type;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * Filters resources based on their qualifiers.
+ *
+ * <p>This includes filtering resources based on both the "resource_configuration_filters" and
+ * "densities" attributes.
+ */
+public class ResourceFilter {
+ public static final String RESOURCE_CONFIGURATION_FILTERS_NAME = "resource_configuration_filters";
+ public static final String DENSITIES_NAME = "densities";
+
+ /** The current {@link RuleContext}, used for reporting errors. */
+ private final RuleContext ruleContext;
+
+ /**
+ * A list of configuration filters that should be applied during the analysis phase. If resources
+ * should not be filtered in analysis (e.g., if android_binary.prefilter_resources is set to 0),
+ * this list should be empty.
+ */
+ private final ImmutableList<FolderConfiguration> configurationFilters;
+
+ /**
+ * The value of the {@link #RESOURCE_CONFIGURATION_FILTERS_NAME} attribute, as a list of qualifier
+ * strings. This value is passed directly as input to resource processing utilities that run in
+ * the execution phase, so it may represent different qualifiers than those in {@link
+ * #configurationFilters} if resources should not be pre-filtered during analysis.
+ */
+ private final ImmutableList<String> configurationFilterStrings;
+
+ /**
+ * A list of density filters that should be applied during the analysis phase. If resources should
+ * not be filtered in analysis (e.g., if prefilter_resources = 0), this list should be empty.
+ */
+ private final ImmutableList<Density> densities;
+
+ /**
+ * The value of the {@link #DENSITIES_NAME} attribute, as a list of qualifier strings. This value
+ * is passed directly as input to resource processing utilities that run in the execution phase,
+ * so it may represent different qualifiers than those in {@link #densities} if resources should
+ * not be pre-filtered during analysis.
+ */
+ private final ImmutableList<String> densityStrings;
+
+ private final ImmutableSet.Builder<String> filteredResources = ImmutableSet.builder();
+
+ /**
+ * Constructor.
+ *
+ * @param ruleContext the current {@link RuleContext}, used for reporting errors
+ * @param configurationFilters a list of configuration filters that should be applied during
+ * analysis time. If resources should not be filtered in analysis (e.g., if
+ * prefilter_resources = 0), this list should be empty.
+ * @param configurationFilterStrings the resource configuration filters, as a list of strings.
+ * @param densities a list of densities that should be applied to filter resources during analysis
+ * time. If resources should not be filtered in analysis (e.g., if prefilter_resources = 0),
+ * this list should be empty.
+ * @param densityStrings the densities, as a list of strings.
+ */
+ private ResourceFilter(
+ RuleContext ruleContext,
+ ImmutableList<FolderConfiguration> configurationFilters,
+ ImmutableList<String> configurationFilterStrings,
+ ImmutableList<Density> densities,
+ ImmutableList<String> densityStrings) {
+ this.ruleContext = ruleContext;
+ this.configurationFilters = configurationFilters;
+ this.configurationFilterStrings = configurationFilterStrings;
+ this.densities = densities;
+ this.densityStrings = densityStrings;
+ }
+
+ private static boolean hasAttr(RuleContext ruleContext, String attrName) {
+ return ruleContext.attributes().isAttributeValueExplicitlySpecified(attrName);
+ }
+
+ static boolean hasFilters(RuleContext ruleContext) {
+ return hasAttr(ruleContext, RESOURCE_CONFIGURATION_FILTERS_NAME)
+ || hasAttr(ruleContext, DENSITIES_NAME);
+ }
+
+ /**
+ * Extracts filters from the current RuleContext, as a list of strings.
+ *
+ * <p>In BUILD files, string lists can be represented as a list of strings, a single
+ * comma-separated string, or a combination of both. This method outputs a single list of
+ * individual string values, which can then be passed directly to resource processing actions.
+ *
+ * @return the values of this attribute contained in the {@link RuleContext}, as a list.
+ */
+ private static ImmutableList<String> extractFilters(RuleContext ruleContext, String attrName) {
+ if (!hasAttr(ruleContext, attrName)) {
+ return ImmutableList.<String>of();
+ }
+
+ /*
+ * To deal with edge cases involving placement of whitespace and multiple strings inside a
+ * single item of the given list, manually build the list here rather than call something like
+ * {@link RuleContext#getTokenizedStringListAttr}.
+ *
+ * Filter out all empty values, even those that were explicitly provided. Paying attention to
+ * empty values is never helpful: even if code handled them correctly (and not all of it does)
+ * empty filter values result in all resources matching the empty filter, meaning that filtering
+ * does nothing (even if non-empty filters were also provided).
+ */
+ List<String> rawValues = ruleContext.attributes().get(attrName, Type.STRING_LIST);
+ ImmutableList.Builder<String> builder = ImmutableList.builder();
+
+ for (String rawValue : rawValues) {
+ if (rawValue.contains(",")) {
+ for (String token : rawValue.split(",")) {
+ if (!token.trim().isEmpty()) {
+ builder.add(token.trim());
+ }
+ }
+ } else if (!rawValue.isEmpty()) {
+ builder.add(rawValue);
+ }
+ }
+
+ return builder.build();
+ }
+
+ static ResourceFilter fromRuleContext(RuleContext ruleContext) {
+ Preconditions.checkNotNull(ruleContext);
+
+ if (!hasFilters(ruleContext)) {
+ return empty(ruleContext);
+ }
+
+ ImmutableList<String> resourceConfigurationFilters =
+ extractFilters(ruleContext, RESOURCE_CONFIGURATION_FILTERS_NAME);
+ ImmutableList<String> densities = extractFilters(ruleContext, DENSITIES_NAME);
+
+ boolean usePrefiltering =
+ ruleContext.getFragment(AndroidConfiguration.class).useResourcePrefiltering();
+
+ ImmutableList.Builder<FolderConfiguration> filterBuilder = ImmutableList.builder();
+ if (usePrefiltering) {
+ for (String filter : resourceConfigurationFilters) {
+ addIfNotNull(
+ FolderConfiguration.getConfigForQualifierString(filter),
+ filter,
+ filterBuilder,
+ ruleContext,
+ RESOURCE_CONFIGURATION_FILTERS_NAME);
+ }
+ }
+
+ ImmutableList.Builder<Density> densityBuilder = ImmutableList.builder();
+ if (usePrefiltering) {
+ for (String density : densities) {
+ addIfNotNull(
+ Density.getEnum(density), density, densityBuilder, ruleContext, DENSITIES_NAME);
+ }
+ }
+
+ return new ResourceFilter(
+ ruleContext,
+ filterBuilder.build(),
+ resourceConfigurationFilters,
+ densityBuilder.build(),
+ densities);
+ }
+
+ /** Reports an attribute error if the given item is null, and otherwise adds it to the builder. */
+ private static <T> void addIfNotNull(
+ T item,
+ String itemString,
+ ImmutableList.Builder<T> builder,
+ RuleContext ruleContext,
+ String attrName) {
+ if (item == null) {
+ ruleContext.attributeError(
+ attrName, "String '" + itemString + "' is not a valid value for " + attrName);
+ } else {
+ builder.add(item);
+ }
+ }
+
+ static ResourceFilter empty(RuleContext ruleContext) {
+ return new ResourceFilter(
+ ruleContext,
+ ImmutableList.<FolderConfiguration>of(),
+ ImmutableList.<String>of(),
+ ImmutableList.<Density>of(),
+ ImmutableList.<String>of());
+ }
+
+ /**
+ * Filters a NestedSet of resource containers. This may be a no-op if this filter is empty or if
+ * resource prefiltering is disabled.
+ */
+ NestedSet<ResourceContainer> filter(NestedSet<ResourceContainer> resources) {
+ if (!isPrefiltering()) {
+ /*
+ * If the filter is empty or resource prefiltering is disabled, just return the original,
+ * rather than make a copy.
+ *
+ * Resources should only be prefiltered in top-level android targets (such as android_binary).
+ * The output of resource processing, which includes the input NestedSet<ResourceContainer>
+ * returned by this method, is exposed to other actions via the AndroidResourcesProvider. If
+ * this method did a no-op copy and collapse in those cases, rather than just return the
+ * original NestedSet, we would lose all of the advantages around memory and time that
+ * NestedSets provide: each android_library target would have to copy the resources provided
+ * by its dependencies into a new NestedSet rather than just create a NestedSet pointing at
+ * its dependencies's NestedSets.
+ */
+ return resources;
+ }
+
+ NestedSetBuilder<ResourceContainer> builder = new NestedSetBuilder<>(resources.getOrder());
+
+ for (ResourceContainer resource : resources) {
+ builder.add(resource.filter(this));
+ }
+
+ return builder.build();
+ }
+
+ ImmutableList<Artifact> filter(ImmutableList<Artifact> artifacts) {
+ if (!isPrefiltering()) {
+ return artifacts;
+ }
+
+ /*
+ * Build an ImmutableSet rather than an ImmutableList to remove duplicate Artifacts in the case
+ * where one Artifact is the best option for multiple densities.
+ */
+ ImmutableSet.Builder<Artifact> builder = ImmutableSet.builder();
+
+ List<BestArtifactsForDensity> bestArtifactsForAllDensities = new ArrayList<>();
+ for (Density density : densities) {
+ bestArtifactsForAllDensities.add(new BestArtifactsForDensity(density));
+ }
+
+ for (Artifact artifact : artifacts) {
+ FolderConfiguration configuration = getConfigForArtifact(artifact);
+ if (!matchesConfigurationFilters(configuration)) {
+ continue;
+ }
+
+ if (!shouldFilterByDensity(artifact)) {
+ builder.add(artifact);
+ continue;
+ }
+
+ for (BestArtifactsForDensity bestArtifactsForDensity : bestArtifactsForAllDensities) {
+ bestArtifactsForDensity.maybeAddArtifact(artifact);
+ }
+ }
+
+ for (BestArtifactsForDensity bestArtifactsForDensity : bestArtifactsForAllDensities) {
+ builder.addAll(bestArtifactsForDensity.get());
+ }
+
+ ImmutableSet<Artifact> keptArtifacts = builder.build();
+ for (Artifact artifact : artifacts) {
+ if (keptArtifacts.contains(artifact)) {
+ continue;
+ }
+
+ String parentDir = artifact.getPath().getParentDirectory().getBaseName();
+ filteredResources.add(parentDir + "/" + artifact.getFilename());
+ }
+
+ return keptArtifacts.asList();
+ }
+
+ /**
+ * Tracks the best artifact for a desired density for each combination of filename and non-density
+ * qualifiers.
+ */
+ private class BestArtifactsForDensity {
+ private final Density desiredDensity;
+
+ // Use a LinkedHashMap to preserve determinism.
+ private final Map<String, Artifact> nameAndConfigurationToBestArtifact = new LinkedHashMap<>();
+
+ public BestArtifactsForDensity(Density density) {
+ desiredDensity = density;
+ }
+
+ /**
+ * @param artifact if this artifact is a better match for this object's desired density than any
+ * other artifacts with the same name and non-density configuration, adds it to this object.
+ */
+ public void maybeAddArtifact(Artifact artifact) {
+ FolderConfiguration config = getConfigForArtifact(artifact);
+
+ // We want to find a single best artifact for each combination of non-density qualifiers and
+ // filename. Combine those two values to create a single unique key.
+ config.setDensityQualifier(null);
+ String nameAndConfiguration = config.getUniqueKey() + "/" + artifact.getFilename();
+
+ Artifact currentBest = nameAndConfigurationToBestArtifact.get(nameAndConfiguration);
+
+ if (currentBest == null || computeAffinity(artifact) < computeAffinity(currentBest)) {
+ nameAndConfigurationToBestArtifact.put(nameAndConfiguration, artifact);
+ }
+ }
+
+ /** @return the collection of best Artifacts for this density. */
+ public Collection<Artifact> get() {
+ return nameAndConfigurationToBestArtifact.values();
+ }
+
+ /**
+ * Compute how well this artifact matches the {@link #desiredDensity}.
+ *
+ * <p>Various different codebases have different and sometimes contradictory methods for which
+ * resources are better in different situations. All of them agree that an exact match is best,
+ * but:
+ *
+ * <p>The android common code (see {@link FolderConfiguration#getDensityQualifier()} treats
+ * larger densities as better than non-matching smaller densities.
+ *
+ * <p>aapt code to filter assets by density prefers the smallest density that is larger than or
+ * the same as the desired density, or, lacking that, the largest available density.
+ *
+ * <p>Other implementations of density filtering include Gradle (to filter which resources
+ * actually get built into apps) and Android code itself (for the device to decide which
+ * resource to use).
+ *
+ * <p>This particular implementation is based on {@link
+ * com.google.devtools.build.android.DensitySpecificResourceFilter}, which filters resources by
+ * density during execution. It prefers to use exact matches when possible, then tries to find
+ * resources with exactly double the desired density for particularly easy downsizing, and
+ * otherwise prefers resources that are closest to the desired density, relative to the smaller
+ * of the available and desired densities.
+ *
+ * <p>Once we always filter resources during analysis, we should be able to completely remove
+ * that code.
+ *
+ * @return a score for how well the artifact matches. Lower scores indicate better matches.
+ */
+ private double computeAffinity(Artifact artifact) {
+ DensityQualifier resourceQualifier = getConfigForArtifact(artifact).getDensityQualifier();
+ if (resourceQualifier == null) {
+ return Double.MAX_VALUE;
+ }
+
+ int resourceDensity = resourceQualifier.getValue().getDpiValue();
+ int density = desiredDensity.getDpiValue();
+
+ if (resourceDensity == density) {
+ // Exact match is the best.
+ return -2;
+ }
+
+ if (resourceDensity == 2 * density) {
+ // It's very efficient to downsample an image that's exactly twice the screen
+ // density, so we prefer that over other non-perfect matches.
+ return -1;
+ }
+
+ // Find the ratio between the larger and smaller of the available and desired densities.
+ double densityRatio =
+ Math.max(density, resourceDensity) / (double) Math.min(density, resourceDensity);
+
+ if (density < resourceDensity) {
+ return densityRatio;
+ }
+
+ // Apply a slight bias against resources that are smaller than those of the desired density.
+ // This becomes relevant only when we are considering multiple resources with the same ratio.
+ return densityRatio + 0.01;
+ }
+ }
+
+ private FolderConfiguration getConfigForArtifact(Artifact artifact) {
+ String containingFolder = getContainingFolder(artifact);
+ FolderConfiguration config = FolderConfiguration.getConfigForFolder(containingFolder);
+
+ if (config == null) {
+ ruleContext.ruleError(
+ "Resource folder '" + containingFolder + "' has invalid resource qualifiers");
+
+ return FolderConfiguration.getConfigForQualifierString("");
+ }
+
+ // aapt explicitly ignores the version qualifier; duplicate this behavior here.
+ config.setVersionQualifier(VersionQualifier.getQualifier(""));
+
+ return config;
+ }
+
+ /**
+ * Checks if we should filter this artifact by its density.
+ *
+ * <p>We filter by density if there are densities to filter by, the artifact is in a Drawable
+ * directory, and the artifact is not an XML file.
+ *
+ * <p>Similarly-named XML files may contain different resource definitions, so it's impossible to
+ * ensure that all required resources will be provided without that XML file unless we parse it.
+ */
+ private boolean shouldFilterByDensity(Artifact artifact) {
+ return !densities.isEmpty()
+ && !artifact.getExtension().equals("xml")
+ && ResourceFolderType.getFolderType(getContainingFolder(artifact))
+ == ResourceFolderType.DRAWABLE;
+ }
+
+ private static String getContainingFolder(Artifact artifact) {
+ return artifact.getPath().getParentDirectory().getBaseName();
+ }
+
+ private boolean matchesConfigurationFilters(FolderConfiguration config) {
+ for (FolderConfiguration filter : configurationFilters) {
+ if (config.isMatchFor(filter)) {
+ return true;
+ }
+ }
+
+ return configurationFilters.isEmpty();
+ }
+
+ /**
+ * Returns if this object contains a non-empty resource configuration filter.
+ *
+ * <p>Note that non-empty filters are not guaranteed to filter resources during the analysis
+ * phase.
+ */
+ boolean hasConfigurationFilters() {
+ return !configurationFilterStrings.isEmpty();
+ }
+
+ String getConfigurationFilterString() {
+ return Joiner.on(',').join(configurationFilterStrings);
+ }
+
+ /**
+ * Returns if this object contains a non-empty density filter.
+ *
+ * <p>Note that non-empty filters are not guaranteed to filter resources during the analysis
+ * phase.
+ */
+ boolean hasDensities() {
+ return !densityStrings.isEmpty();
+ }
+
+ String getDensityString() {
+ return Joiner.on(',').join(densityStrings);
+ }
+
+ List<String> getDensities() {
+ return densityStrings;
+ }
+
+ boolean isPrefiltering() {
+ return !densities.isEmpty() || !configurationFilters.isEmpty();
+ }
+
+ /**
+ * TODO: Stop tracking these once android_library targets also filter resources correctly.
+ *
+ * <p>Currently, android_library targets pass do no filtering, and all resources are built into
+ * their symbol files. The android_binary target filters out these resources in analysis. However,
+ * the filtered resources must be passed to resource processing at execution time so the code
+ * knows to ignore resources that were filtered out. Without this, resource processing code would
+ * see references to those resources in dependencies's symbol files, but then be unable to follow
+ * those references or know whether they were missing due to resource filtering or a bug.
+ *
+ * @return a list of resources that were filtered out by this filter
+ */
+ ImmutableList<String> getFilteredResources() {
+ return filteredResources.build().asList();
+ }
+}
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 517b124370..60d414ddc4 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
@@ -44,7 +44,7 @@ public class ResourceShrinkerActionBuilder {
private List<String> uncompressedExtensions = Collections.emptyList();
private List<String> assetsToIgnore = Collections.emptyList();
- private ResourceConfigurationFilter resourceConfigs;
+ private ResourceFilter resourceFilter;
/**
* @param ruleContext The RuleContext of the owning rule.
@@ -53,7 +53,7 @@ public class ResourceShrinkerActionBuilder {
this.ruleContext = ruleContext;
this.spawnActionBuilder = new SpawnAction.Builder();
this.sdk = AndroidSdkProvider.fromRuleContext(ruleContext);
- this.resourceConfigs = ResourceConfigurationFilter.empty(ruleContext);
+ this.resourceFilter = ResourceFilter.empty(ruleContext);
}
public ResourceShrinkerActionBuilder setUncompressedExtensions(
@@ -67,12 +67,9 @@ public class ResourceShrinkerActionBuilder {
return this;
}
- /**
- * @param resourceConfigs The configuration filters to apply to the resources.
- */
- public ResourceShrinkerActionBuilder setConfigurationFilters(
- ResourceConfigurationFilter resourceConfigs) {
- this.resourceConfigs = resourceConfigs;
+ /** @param resourceFilter The filters to apply to the resources. */
+ public ResourceShrinkerActionBuilder setResourceFilter(ResourceFilter resourceFilter) {
+ this.resourceFilter = resourceFilter;
return this;
}
@@ -149,7 +146,7 @@ public class ResourceShrinkerActionBuilder {
ImmutableList.Builder<Artifact> outputs = ImmutableList.builder();
CustomCommandLine.Builder commandLine = new CustomCommandLine.Builder();
-
+
// Set the busybox tool.
commandLine.add("--tool").add("SHRINK").add("--");
@@ -176,8 +173,8 @@ public class ResourceShrinkerActionBuilder {
if (ruleContext.getConfiguration().getCompilationMode() != CompilationMode.OPT) {
commandLine.add("--debug");
}
- if (!resourceConfigs.isEmpty()) {
- commandLine.add("--resourceConfigs").add(resourceConfigs.getFilterString());
+ if (resourceFilter.hasConfigurationFilters()) {
+ commandLine.add("--resourceConfigs").add(resourceFilter.getConfigurationFilterString());
}
checkNotNull(resourceFilesZip);
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 26b5eea844..62c30bc67f 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
@@ -1150,7 +1150,8 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase {
"r",
"android_binary(name = 'r',",
" manifest = 'AndroidManifest.xml',",
- " resource_configuration_filters = ['en'],",
+ " resource_configuration_filters = ['', 'en, es, '],",
+ " densities = ['hdpi, , ', 'xhdpi'],",
" resource_files = ['" + Joiner.on("', '").join(resources) + "'])");
ResourceContainer directResources = getResourceContainer(binary, /* transitive= */ false);
@@ -1159,6 +1160,11 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase {
// 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(resourceGeneratingAction(directResources).getArguments()).contains("en,es");
+ assertThat(resourceGeneratingAction(directResources).getArguments()).contains("hdpi,xhdpi");
}
@Test
@@ -1212,6 +1218,14 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase {
}
@Test
+ public void testFilteredResourcesMultipleFiltersWithWhitespace() throws Exception {
+ testDirectResourceFiltering(
+ " en , es ",
+ /* unexpectedQualifiers= */ ImmutableList.of("fr"),
+ /* expectedQualifiers= */ ImmutableList.of("en", "es"));
+ }
+
+ @Test
public void testFilteredResourcesMultipleFilterStrings() throws Exception {
testDirectResourceFiltering(
"en', 'es",
@@ -1313,20 +1327,165 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase {
/* expectedQualifiers= */ ImmutableList.of(filters));
}
+ @Test
+ public void testDensityFilteredResourcesSingleDensity() throws Exception {
+ testDensityResourceFiltering(
+ "hdpi", ImmutableList.of("ldpi", "mdpi", "xhdpi"), ImmutableList.of("hdpi"));
+ }
+
+ @Test
+ public void testDensityFilteredResourcesSingleClosestDensity() throws Exception {
+ testDensityResourceFiltering(
+ "hdpi", ImmutableList.of("ldpi", "mdpi"), ImmutableList.of("xhdpi"));
+ }
+
+ @Test
+ public void testDensityFilteredResourcesDifferingQualifiers() throws Exception {
+ testDensityResourceFiltering(
+ "hdpi",
+ ImmutableList.of("en-xhdpi", "fr"),
+ ImmutableList.of("en-hdpi", "fr-mdpi", "xhdpi"));
+ }
+
+ @Test
+ public void testDensityFilteredResourcesMultipleDensities() throws Exception {
+ testDensityResourceFiltering(
+ "ldpi,hdpi', 'xhdpi",
+ ImmutableList.of("mdpi", "xxhdpi"),
+ ImmutableList.of("ldpi", "hdpi", "xhdpi"));
+ }
+
+ @Test
+ public void testDensityFilteredResourcesDoubledDensity() throws Exception {
+ testDensityResourceFiltering(
+ "hdpi", ImmutableList.of("ldpi", "mdpi", "xhdpi"), ImmutableList.of("xxhdpi"));
+ }
+
+ @Test
+ public void testDensityFilteredResourcesDifferentRatiosSmallerCloser() throws Exception {
+ testDensityResourceFiltering("mdpi", ImmutableList.of("hdpi"), ImmutableList.of("ldpi"));
+ }
+
+ @Test
+ public void testDensityFilteredResourcesDifferentRatiosLargerCloser() throws Exception {
+ testDensityResourceFiltering("hdpi", ImmutableList.of("mdpi"), ImmutableList.of("xhdpi"));
+ }
+
+ @Test
+ public void testDensityFilteredResourcesSameRatioPreferLarger() throws Exception {
+ // xxhdpi is 480dpi and xxxhdpi is 640dpi.
+ // The ratios between 360dpi and 480dpi and between 480dpi and 640dpi are both 3:4.
+ testDensityResourceFiltering("xxhdpi", ImmutableList.of("360dpi"), ImmutableList.of("xxxhdpi"));
+ }
+
+ @Test
+ public void testDensityFilteredResourcesOptionsSmallerThanDesired() throws Exception {
+ testDensityResourceFiltering(
+ "xxxhdpi", ImmutableList.of("xhdpi", "mdpi", "ldpi"), ImmutableList.of("xxhdpi"));
+ }
+
+ @Test
+ public void testDensityFilteredResourcesOptionsLargerThanDesired() throws Exception {
+ testDensityResourceFiltering(
+ "ldpi", ImmutableList.of("xxxhdpi", "xxhdpi", "xhdpi"), ImmutableList.of("mdpi"));
+ }
+
+ @Test
+ public void testDensityFilteredResourcesSpecialValues() throws Exception {
+ testDensityResourceFiltering(
+ "xxxhdpi", ImmutableList.of("anydpi", "nodpi"), ImmutableList.of("ldpi"));
+ }
+
+ @Test
+ public void testDensityFilteredResourcesXml() throws Exception {
+ testDirectResourceFiltering(
+ /* resourceConfigurationFilters= */ "",
+ "hdpi",
+ ImmutableList.<String>of(),
+ ImmutableList.of("ldpi", "mdpi", "hdpi", "xhdpi"),
+ /* expectUnqualifiedResource= */ true,
+ "drawable",
+ "xml");
+ }
+
+ @Test
+ public void testDensityFilteredResourcesNotDrawable() throws Exception {
+ testDirectResourceFiltering(
+ /* resourceConfigurationFilters= */ "",
+ "hdpi",
+ ImmutableList.<String>of(),
+ ImmutableList.of("ldpi", "mdpi", "hdpi", "xhdpi"),
+ /* expectUnqualifiedResource= */ true,
+ "layout",
+ "png");
+ }
+
+ @Test
+ public void testQualifierAndDensityFilteredResources() throws Exception {
+ testDirectResourceFiltering(
+ "en,fr-mdpi",
+ "ldpi,hdpi",
+ ImmutableList.of("mdpi", "es-ldpi", "en-xxxhdpi", "fr-mdpi"),
+ ImmutableList.of("ldpi", "hdpi", "en-xhdpi", "fr-hdpi"),
+ /* expectUnqualifiedResource= */ false,
+ "drawable",
+ "png");
+ }
+
+ private void testDirectResourceFiltering(
+ String filters, List<String> unexpectedQualifiers, ImmutableList<String> expectedQualifiers)
+ throws Exception {
+ testDirectResourceFiltering(
+ filters,
+ /* densities= */ "",
+ unexpectedQualifiers,
+ expectedQualifiers,
+ /* expectUnqualifiedResource= */ true,
+ "drawable",
+ "png");
+ }
+
+ private void testDensityResourceFiltering(
+ String densities, List<String> unexpectedQualifiers, List<String> expectedQualifiers)
+ throws Exception {
+ testDirectResourceFiltering(
+ /* resourceConfigurationFilters= */ "",
+ densities,
+ unexpectedQualifiers,
+ expectedQualifiers,
+ /* expectUnqualifiedResource= */ false,
+ "drawable",
+ "png");
+ }
+
private void testDirectResourceFiltering(
String resourceConfigurationFilters,
+ String densities,
List<String> unexpectedQualifiers,
- List<String> expectedQualifiers)
+ List<String> expectedQualifiers,
+ boolean expectUnqualifiedResource,
+ String folderType,
+ String suffix)
throws Exception {
List<String> unexpectedResources = new ArrayList<>();
+ List<String> expectedFiltered = new ArrayList<>();
for (String qualifier : unexpectedQualifiers) {
- unexpectedResources.add("res/values-" + qualifier + "/foo.xml");
+ expectedFiltered.add(folderType + "-" + qualifier + "/foo." + suffix);
+ unexpectedResources.add("res/" + folderType + "-" + qualifier + "/foo." + suffix);
}
List<String> expectedResources = new ArrayList<>();
for (String qualifier : expectedQualifiers) {
- expectedResources.add("res/values-" + qualifier + "/foo.xml");
+ expectedResources.add("res/" + folderType + "-" + qualifier + "/foo." + suffix);
+ }
+
+ String unqualifiedResource = "res/" + folderType + "/foo." + suffix;
+ if (expectUnqualifiedResource) {
+ expectedResources.add(unqualifiedResource);
+ } else {
+ unexpectedResources.add(unqualifiedResource);
+ expectedFiltered.add(folderType + "/foo." + suffix);
}
// Default resources should never be filtered
@@ -1345,6 +1504,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase {
"android_binary(name = 'r',",
" manifest = 'AndroidManifest.xml',",
" resource_configuration_filters = ['" + resourceConfigurationFilters + "'],",
+ " densities = ['" + densities + "'],",
" resource_files = ['" + Joiner.on("', '").join(allResources) + "'])");
ResourceContainer directResources = getResourceContainer(binary, /* transitive= */ false);
@@ -1356,6 +1516,21 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase {
// Validate that the input to resource processing contains only the filtered values.
assertThat(resourceInputPaths(dir, directResources)).containsAllIn(expectedResources);
assertThat(resourceInputPaths(dir, directResources)).containsNoneIn(unexpectedResources);
+
+ if (expectedFiltered.isEmpty()) {
+ assertThat(resourceArguments(directResources)).doesNotContain("--prefilteredResources");
+ } else {
+ String[] flagValues =
+ flagValue("--prefilteredResources", resourceArguments(directResources)).split(",");
+ assertThat(flagValues).asList().containsAllIn(expectedFiltered);
+ assertThat(flagValues).hasLength(expectedFiltered.size());
+ }
+
+ // Validate resource filters are not passed to execution, since they were applied in analysis
+ assertThat(resourceGeneratingAction(directResources).getArguments())
+ .doesNotContain(ResourceFilter.RESOURCE_CONFIGURATION_FILTERS_NAME);
+ assertThat(resourceGeneratingAction(directResources).getArguments())
+ .doesNotContain(ResourceFilter.DENSITIES_NAME);
}
@Test