aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
Diffstat
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/android/ResourceFilterFactory.java35
-rw-r--r--src/test/java/com/google/devtools/build/lib/rules/android/ResourceFilterFactoryTest.java92
2 files changed, 29 insertions, 98 deletions
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 4a9d2bf5c7..7f2b6ae570 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
@@ -254,7 +254,7 @@ public class ResourceFilterFactory {
ImmutableList.Builder<FolderConfiguration> filterBuilder = ImmutableList.builder();
for (String filter : configFilters) {
addIfNotNull(
- getFolderConfiguration(ruleErrorConsumer, filter),
+ getFolderConfiguration(filter),
filter,
filterBuilder,
ruleErrorConsumer,
@@ -264,13 +264,12 @@ public class ResourceFilterFactory {
return filterBuilder.build();
}
- private FolderConfiguration getFolderConfiguration(
- RuleErrorConsumer ruleErrorConsumer, String filter) {
+ private FolderConfiguration getFolderConfiguration(String filter) {
// Clean up deprecated representations of resource qualifiers that FolderConfiguration can't
// handle.
for (DeprecatedQualifierHandler handler : deprecatedQualifierHandlers) {
- filter = handler.fixAttributeIfNeeded(ruleErrorConsumer, filter);
+ filter = handler.fixAttributeIfNeeded(filter);
}
return FolderConfiguration.getConfigForQualifierString(filter);
@@ -281,7 +280,6 @@ public class ResourceFilterFactory {
private final String replacement;
private final String description;
- private boolean warnedForAttribute = false;
private boolean warnedForResources = false;
private DeprecatedQualifierHandler(String pattern, String replacement, String description) {
@@ -290,26 +288,14 @@ public class ResourceFilterFactory {
this.description = description;
}
- private String fixAttributeIfNeeded(RuleErrorConsumer ruleErrorConsumer, String qualifier) {
+ private String fixAttributeIfNeeded(String qualifier) {
Matcher matcher = pattern.matcher(qualifier);
if (!matcher.matches()) {
return qualifier;
}
- String fixed = matcher.replaceFirst(replacement);
- // We don't want to spam users. Only warn about this kind of issue once per target.
- // TODO(asteinb): Will this cause problems when settings are propagated via dynamic
- // configuration?
- if (!warnedForAttribute) {
- ruleErrorConsumer.attributeWarning(
- RESOURCE_CONFIGURATION_FILTERS_NAME,
- String.format(
- "When referring to %s, use of qualifier '%s' is deprecated. Use '%s' instead.",
- description, matcher.group(), fixed));
- warnedForAttribute = true;
- }
- return fixed;
+ return matcher.replaceFirst(replacement);
}
private String fixResourceIfNeeded(
@@ -339,7 +325,16 @@ public class ResourceFilterFactory {
}
}
- /** List of deprecated qualifiers that should currently by handled with a warning */
+ /**
+ * List of deprecated qualifiers that are not supported by {@link FolderConfiguration}.
+ *
+ * For resources, we should warn if these qualifiers are encountered, since aapt supports the
+ * fixed version (and aapt2 only supports that version).
+ *
+ * For resource filters, however, aapt only supports this old version. Convert the qualifiers so
+ * that they can be parsed by FolderConfiguration, but do not warn (since they are, actually, what
+ * aapt expects) and save the original qualifier strings to be passed to aapt.
+ */
private final List<DeprecatedQualifierHandler> deprecatedQualifierHandlers =
ImmutableList.of(
/*
diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/ResourceFilterFactoryTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/ResourceFilterFactoryTest.java
index aabcf0435e..7564c5d376 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/android/ResourceFilterFactoryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/android/ResourceFilterFactoryTest.java
@@ -137,10 +137,8 @@ public class ResourceFilterFactoryTest extends ResourceTestBase {
ImmutableList.of("values-en/foo.xml", "values-en-rUS/foo.xml"),
ImmutableList.of("values-fr/foo.xml", "values-en-rCA/foo.xml"));
- assertThat(
- errorConsumer.getAndClearAttributeWarnings(
- ResourceFilterFactory.RESOURCE_CONFIGURATION_FILTERS_NAME))
- .hasSize(1);
+ errorConsumer.assertNoAttributeWarnings(
+ ResourceFilterFactory.RESOURCE_CONFIGURATION_FILTERS_NAME);
}
@Test
@@ -152,10 +150,8 @@ public class ResourceFilterFactoryTest extends ResourceTestBase {
ImmutableList.of("values-en/foo.xml", "values-en-rUS/foo.xml"),
ImmutableList.of("values-fr/foo.xml", "values-en-rCA/foo.xml"));
- assertThat(
- errorConsumer.getAndClearAttributeWarnings(
- ResourceFilterFactory.RESOURCE_CONFIGURATION_FILTERS_NAME))
- .hasSize(1);
+ errorConsumer.assertNoAttributeWarnings(
+ ResourceFilterFactory.RESOURCE_CONFIGURATION_FILTERS_NAME);
}
@Test
@@ -167,10 +163,8 @@ public class ResourceFilterFactoryTest extends ResourceTestBase {
ImmutableList.of("values-en/foo.xml", "values-en-rUS/foo.xml"),
ImmutableList.of("values-fr/foo.xml", "values-en-rCA/foo.xml"));
- assertThat(
- errorConsumer.getAndClearAttributeWarnings(
- ResourceFilterFactory.RESOURCE_CONFIGURATION_FILTERS_NAME))
- .hasSize(1);
+ errorConsumer.assertNoAttributeWarnings(
+ ResourceFilterFactory.RESOURCE_CONFIGURATION_FILTERS_NAME);
}
@Test
@@ -182,10 +176,8 @@ public class ResourceFilterFactoryTest extends ResourceTestBase {
ImmutableList.of("values-en/foo.xml", "values-en-rUS/foo.xml"),
ImmutableList.of("values-fr/foo.xml", "values-en-rCA/foo.xml"));
- assertThat(
- errorConsumer.getAndClearAttributeWarnings(
- ResourceFilterFactory.RESOURCE_CONFIGURATION_FILTERS_NAME))
- .hasSize(1);
+ errorConsumer.assertNoAttributeWarnings(
+ ResourceFilterFactory.RESOURCE_CONFIGURATION_FILTERS_NAME);
}
@Test
@@ -202,10 +194,8 @@ public class ResourceFilterFactoryTest extends ResourceTestBase {
// Serbian in explicitly Cyrillic characters should be filtered out.
ImmutableList.of("values-b+sr+Cyrl/foo.xml"));
- assertThat(
- errorConsumer.getAndClearAttributeWarnings(
- ResourceFilterFactory.RESOURCE_CONFIGURATION_FILTERS_NAME))
- .hasSize(1);
+ errorConsumer.assertNoAttributeWarnings(
+ ResourceFilterFactory.RESOURCE_CONFIGURATION_FILTERS_NAME);
}
@Test
@@ -235,10 +225,8 @@ public class ResourceFilterFactoryTest extends ResourceTestBase {
// Spanish with another region specified should be filtered out.
ImmutableList.of("values-es-rUS/foo.xml"));
- assertThat(
- errorConsumer.getAndClearAttributeWarnings(
- ResourceFilterFactory.RESOURCE_CONFIGURATION_FILTERS_NAME))
- .hasSize(1);
+ errorConsumer.assertNoAttributeWarnings(
+ ResourceFilterFactory.RESOURCE_CONFIGURATION_FILTERS_NAME);
}
@Test
@@ -257,54 +245,6 @@ public class ResourceFilterFactoryTest extends ResourceTestBase {
ResourceFilterFactory.RESOURCE_CONFIGURATION_FILTERS_NAME);
}
- /**
- * Tests that filtering with deprecated resource qualifiers only warns once for each type of
- * problem in each resource filter object.
- */
- @Test
- public void testFilterDeprecatedQualifierFormatsAttributeWarnings() throws RuleErrorException {
- ImmutableList<String> badQualifiers =
- ImmutableList.of("en_US", "sr-Latn", "es-419", "mcc310-en_US", "sr_rLatn", "es_419");
-
- ImmutableList<String> expectedWarnings =
- ImmutableList.of(
- "When referring to Serbian in Latin characters, use of qualifier 'sr-Latn' is"
- + " deprecated. Use 'b+sr+Latn' instead.",
- "When referring to Spanish for Latin America and the Caribbean, use of qualifier"
- + " 'es-419' is deprecated. Use 'b+es+419' instead.",
- "When referring to locale qualifiers with regions, use of qualifier 'en_US' is"
- + " deprecated. Use 'en-rUS' instead.");
-
- ResourceFilterFactory filter =
- new ResourceFilterFactory(
- badQualifiers, ImmutableList.of(), FilterBehavior.FILTER_IN_ANALYSIS);
-
- doFilter(filter, ImmutableList.of());
- assertThat(
- errorConsumer.getAndClearAttributeWarnings(
- ResourceFilterFactory.RESOURCE_CONFIGURATION_FILTERS_NAME))
- .containsExactlyElementsIn(expectedWarnings);
- errorConsumer.assertNoRuleWarnings();
-
- // Filtering again with this filter should not produce additional warnings
- doFilter(filter, ImmutableList.of());
- errorConsumer.assertNoAttributeWarnings(
- ResourceFilterFactory.RESOURCE_CONFIGURATION_FILTERS_NAME);
- errorConsumer.assertNoRuleWarnings();
-
- // Filtering with a new filter should produce warnings again, since it is working on a different
- // target
- filter =
- new ResourceFilterFactory(
- badQualifiers, ImmutableList.of(), FilterBehavior.FILTER_IN_ANALYSIS);
- doFilter(filter, ImmutableList.of());
- assertThat(
- errorConsumer.getAndClearAttributeWarnings(
- ResourceFilterFactory.RESOURCE_CONFIGURATION_FILTERS_NAME))
- .containsExactlyElementsIn(expectedWarnings);
- errorConsumer.assertNoRuleWarnings();
- }
-
@Test
public void testFilterDeprecatedQualifierFormatsRuleWarnings() throws RuleErrorException {
ImmutableList<Artifact> badResources =
@@ -494,14 +434,10 @@ public class ResourceFilterFactoryTest extends ResourceTestBase {
assertThat(transitiveToDiscard.getResourcesRoots()).isEmpty();
ResourceContainer transitiveToKeep = transitiveContainers.get(1);
- assertThat(transitiveToKeep.getResources())
- .containsExactly(transitiveResourceToKeep);
+ assertThat(transitiveToKeep.getResources()).containsExactly(transitiveResourceToKeep);
assertThat(transitiveToKeep.getResourcesRoots())
.containsExactly(
- transitiveResourceToKeep
- .getExecPath()
- .getParentDirectory()
- .getParentDirectory());
+ transitiveResourceToKeep.getExecPath().getParentDirectory().getParentDirectory());
// We tell the resource processing actions to ignore references to filtered resources from
// dependencies.
generated by cgit v1.2.3 (git 2.39.1) at 2024-07-09 08:59:23 +0000