[#1816] Replace resourceConfigurations with androidResources.localeFilters - #2232
[#1816] Replace resourceConfigurations with androidResources.localeFilters#2232ChinmayGopal931 wants to merge 2 commits into
Conversation
…localeFilters Closes zodl-inc#1816 The old resourceConfigurations API is deprecated in AGP 8 and this swaps it for androidResources.localeFilters on the application module. The replacement is only available on ApplicationAndroidResources. Library and test variants don't expose localeFilters, so the library and test plugin configurations drop the call entirely rather than trying to keep parity. Library modules in this repo are not published as AARs and the com.android.test modules target :app, so neither needed its own locale filter to begin with.
@nesence-m Is there anything I can do to move this pr along? how can I help get this merged? |
|
@zodl-review |
🤖 Claude ReviewReviewCoverage — Full diff and file contents reviewed. Summary — Replaces the deprecated Risk Assessment — Low. Build-script-only change in a convention plugin; behavioral risk is limited to which locales survive in the final APK, which is governed solely by the application module's filter. Key Observations
Suggestions
Areas of disagreement
Automated review by |
localeFilters validates every entry and throws on anything its locale
parser rejects, which resourceConfigurations did not do. The two
pseudolocales carried over verbatim as "en_XA" and "ar_XB" fail that
check, so any build that links app resources dies with:
> Task :app:processZcashtestnetFossDebugResources FAILED
> The locale in localeFilters "en_XA" is invalid.
compileKotlin and help don't run resource linking, which is why this
wasn't caught earlier.
The entries are handed to aapt2's -c option, which takes Android
resource qualifiers and not BCP-47 tags, so "en-XA" and "ar-XB" are
rejected too ("invalid config 'en-XA' for -c option"). Only the "-r"
region form satisfies both the AGP validator and aapt2, so the
pseudolocales become "en-rXA" and "ar-rXB" and the existing
en-rUS/en-rGB/en-rAU entries stay as they are.
Verified by building :app:assembleZcashtestnetFossDebug both ways and
diffing aapt2 dump configurations on the two APKs: 43 configurations,
identical, so the migration is behavior preserving. The library block
comment is also corrected, since libraries don't inherit the filter;
the application module's filter applies to the resources merged in from
them at packaging time.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Hey @nesence-m, sorry about the dismissed approval. I pushed a follow-up commit because the version you approved doesn't actually build anymore (pr has been approved and open for a while) The two pseudolocale tags came straight across as I'd only run The fix is to spell them I did a bit of cleanup on the comments while I was in there. The one on the library block said libraries inherit the filter, which isn't really right. The app module's filter just applies to everything merged into the APK, libraries included. Mind taking another look when you get a chance? |
Closes #1816
resourceConfigurationsis deprecated in AGP 8. This replaces it withandroidResources.localeFilters.Why two of the tags changed spelling
The pseudolocales went from
en_XA/ar_XBtoen-rXA/ar-rXB. This isn't cosmetic:localeFiltersvalidates its entries andresourceConfigurationsdidn't, so the underscore form now fails the build outright.Warning
Please don't "modernize" these to BCP-47 (
en-US,en-XA). AGP hands the list toaapt2 -c, which only takes Android resource qualifiers and rejects BCP-47 withinvalid config 'en-US' for -c option. The-rregion form is the only spelling that satisfies both. That's also whyen-rUS/en-rGB/en-rAUare untouched.Why library and test modules just drop the call
localeFiltersonly exists onApplicationAndroidResources, so library and test variants can't set it. They don't need to: the app module's filter applies to everything merged into the APK, including resources from libraries. Library modules here aren't published as AARs and thecom.android.testmodules target:app, so nothing is lost.The
@Suppress("UnstableApiUsage")matches whatmanagedDevicesalready does in the same file.Verification
Built the APK twice from the same tree, once with the old API and once with the new one, then diffed
aapt2 dump configurationson both. Identical, 43 configurations each:en-rAU,en-rGB,esand theen-rXA/ar-rXBpseudolocales all kept, third party locales still stripped. So this is a like-for-like swap, not just something that compiles.:app:assembleZcashtestnetFossDebugpasses, as doesprocessZcashtestnetFossDebugResourceson:ui-integration-testand:ui-screenshot-test(the modules the call was removed from).