Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test fixlocalizationtests branch #8895

Merged
merged 3 commits into from
Apr 26, 2024
Merged

Test fixlocalizationtests branch #8895

merged 3 commits into from
Apr 26, 2024

Conversation

dellis1972
Copy link
Contributor

@dellis1972 dellis1972 commented Apr 23, 2024

Changes https://github.com/xamarin/monodroid/compare/a5742221b314864636f4356173a2535a539c7b2c...9ca6d9f64fce11f04d668ece50ada36de1d7efce.

Context: https://github.com/xamarin/xamarin-android/commit/86260ed36dfe1a90c8ed6a2bb1cd0607d637f403
Context: #8478
Context: #8895

Commit 86260ed3 altered packaging so that
all assemblies were considered to be ABI-specific, removing even
the idea of "CPU-agnostic assemblies".

This had unforeseen breakage: localization-specific .resx & related
files were no longer fast deployed to the correct location, breaking
all of our localization unit tests.

This wasn't found in #8478 because the
localization tests which broke were only run on nightly builds, and
not run as part of the normal PR process.

Oops.

The root cause of this is that the %(TargetPath) metadata on
Satellite assemblies is set to %(Culture)\%(Filename)%(Extension)
by default. The code we had in place to set %(TargetPath) first
checked to see if it was empty. In the case of XX.resource.dll
files, it was NOT empty, and as a result the XX.resource.dll files
would get deployed to files/.__override__/%(Culture) instead of
files/.__override__/%(Abi)/%(Culture).

Because our assembly loader now only looks in the %(Abi) directory
for its files, the resources were never found.

Update the _ComputeFastDevFiles target to check the value of
%(TargetPath) to see if it is the default value, and if not then
update it to use the %(DestinationSubDirectory) value.

Update the LocalizedAssemblies_ShouldBeFastDeployed unit test to include application based resource.dll files, so that the failure can be caught during smoke testing.

@dellis1972 dellis1972 marked this pull request as ready for review April 25, 2024 20:35
@dellis1972 dellis1972 requested a review from jonpryor as a code owner April 25, 2024 20:35
@dellis1972 dellis1972 force-pushed the fixlocalizationtests branch from 7f0f9b7 to 718bea2 Compare April 25, 2024 20:35
@jonpryor jonpryor merged commit 24fb6ab into main Apr 26, 2024
48 checks passed
@jonpryor jonpryor deleted the fixlocalizationtests branch April 26, 2024 14:49
jonathanpeppers pushed a commit that referenced this pull request Apr 29, 2024
Context: 86260ed
Context: #8478
Context: #8895

Changes xamarin/monodroid@a574222...9ca6d9f.

  * xamarin/monodroid@9ca6d9f64: [tools/msbuild] fix fastdev of Localization assemblies (xamarin/monodroid#1464)
  * xamarin/monodroid@418b1ece6: Remove "Xamarin" from messages (xamarin/monodroid#1468) (xamarin/monodroid#1469)
  * xamarin/monodroid@07803812f: Revert "Remove "Xamarin" from messages (#146xamarin/monodroid8)"
  * xamarin/monodroid@cf71c7e6e: Remove "Xamarin" from messages (xamarin/monodroid#1468)

Commit 86260ed altered packaging so that *all* assemblies were
considered to be ABI-specific, removing even the *idea* of
"CPU-agnostic assemblies".

This had unforeseen breakage: localization-specific `.resx` & related
files were no longer fast deployed to the correct location, breaking
*all* of our localization unit tests.

This wasn't found in PR #8478 because the localization tests which
broke were only run on *nightly* builds, and not run as part of the
normal PR process.

Oops.

The root cause of this is that the `%(TargetPath)` metadata on
Satellite assemblies is set to `%(Culture)\%(Filename)%(Extension)`
by default.  The code we had in place to set `%(TargetPath)` first
checked to see if it was empty.  In the case of `XX.resource.dll`
files, it was NOT empty, and as a result the `XX.resource.dll` files
would get deployed to `files/.__override__/%(Culture)` instead of
`files/.__override__/%(Abi)/%(Culture)`.

Because our assembly loader now *only* looks in the `%(Abi)` directory
for its files, the resources were never found.

xamarin/monodroid@9ca6d9f64 updates the `_ComputeFastDevFiles` target
to check the value of `%(TargetPath)` to see if it is the default
value, and if not then update it to use the `%(DestinationSubDirectory)`
value.

Update the `InstallTests.LocalizedAssemblies_ShouldBeFastDeployed()`
unit test to include application-based resource.dll files, so that
this failure can be caught during smoke testing.
grendello added a commit that referenced this pull request May 7, 2024
* main:
  Update README (#8913)
  Bumps to xamarin/Java.Interop/main@4e893bf (#8924)
  Bump to dotnet/installer@fa261b952d 9.0.100-preview.5.24253.16 (#8921)
  [Mono.Android] Dispose of the `RunnableImplementor` on error (#8907)
  Bump NDK to r26d (#8868)
  Bump to dotnet/installer@d301a122c4 9.0.100-preview.5.24229.2 (#8912)
  Localized file check-in by OneLocBuild Task (#8910)
  LEGO: Merge pull request 8909
  [api-merge] Add `removed-since` info (#8897)
  Bump to xamarin/monodroid@9ca6d9f6 (#8895)
  [Xamarin.Android.Build.Tasks] fix detection of "Android libraries" (#8904)
@github-actions github-actions bot locked and limited conversation to collaborators May 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants