Resource filtering should preserve all matching artifacts, despite shared names
Before this change, density-based resource filtering tracked resources by
qualifiers and name. Resources with density qualifiers specified would go into
this code, but only one resource would be chosen from each each (qualifier,
name) pair.
Instead, track the resource using its entire path, this tracking resources with
the same name seperately.
Also, in case multiple resource are passed to the resource processing action,
resource filtering only ignores a file if its name was in the list of resources
to ignore *and* it does not exist. Otherwise, legitimate resources with the
same name as a filtered resource might be ignored.
RELNOTES: none
PiperOrigin-RevId: 163235681
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
index 52f155a..1462428 100644
--- 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
@@ -33,6 +33,7 @@
import com.google.devtools.build.lib.packages.AttributeMap;
import com.google.devtools.build.lib.packages.RuleErrorConsumer;
import com.google.devtools.build.lib.syntax.Type;
+import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.common.options.EnumConverter;
import com.google.devtools.common.options.OptionsParsingException;
import java.util.ArrayList;
@@ -501,8 +502,15 @@
// 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.
+ // We also need to include the path to the resource, otherwise resource conflicts (multiple
+ // resources with the same name but different locations) might accidentally get resolved here
+ // (possibly incorrectly). Resource conflicts should be resolve during merging in execution
+ // instead.
config.setDensityQualifier(null);
- String nameAndConfiguration = config.getUniqueKey() + "/" + artifact.getFilename();
+ Path qualifierDir = artifact.getPath().getParentDirectory();
+ String resourceDir = qualifierDir.getParentDirectory().toString();
+ String nameAndConfiguration =
+ Joiner.on('/').join(resourceDir, config.getUniqueKey(), artifact.getFilename());
Artifact currentBest = nameAndConfigurationToBestArtifact.get(nameAndConfiguration);
diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/ResourceFilterTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/ResourceFilterTest.java
index 65f6d2e..e2d7b9f 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/android/ResourceFilterTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/android/ResourceFilterTest.java
@@ -350,6 +350,17 @@
}
@Test
+ public void testFilterResourceConflict() throws Exception {
+ testNoopFilter(
+ "en",
+ "hdpi",
+ FilterBehavior.FILTER_IN_ANALYSIS,
+ ImmutableList.of(
+ "first-subdir/res/drawable-en-hdpi/foo.png",
+ "second-subdir/res/drawable-en-hdpi/foo.png"));
+ }
+
+ @Test
public void testFilterWithDynamicConfiguration() throws Exception {
testFilter(
"en",
diff --git a/src/tools/android/java/com/google/devtools/build/android/AndroidDataDeserializer.java b/src/tools/android/java/com/google/devtools/build/android/AndroidDataDeserializer.java
index 94a35dd..f48a796 100644
--- a/src/tools/android/java/com/google/devtools/build/android/AndroidDataDeserializer.java
+++ b/src/tools/android/java/com/google/devtools/build/android/AndroidDataDeserializer.java
@@ -153,7 +153,7 @@
// Using Path.subpath would return a backslash-using path on Windows.
String shortPath =
source.getPath().getParent().getFileName() + "/" + source.getPath().getFileName();
- if (filteredResources.contains(shortPath)) {
+ if (filteredResources.contains(shortPath) && !Files.exists(source.getPath())) {
// Skip files that were filtered out during analysis.
// TODO(asteinb): Properly filter out these files from android_library symbol files during
// analysis instead, and remove this list.
