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

Fix an off-by-one error demangling satellite assembly names #9166

Merged
merged 1 commit into from
Aug 20, 2024

Conversation

grendello
Copy link
Contributor

When assemblies are packaged in the APK archive, their names are
"mangled" so that we don't create clashes with "real" shared libraries,
which are placed in the same directory. Additionally, since the lib/
directory in the APK archive supports only single level of directories,
we must encode satellite assembly culture in the file name, instead of
putting it in a subdirectory.

Unfortunately, when the code was committed, an off-by-one error crept
in, when converting satellite assembly names to their culture/name
for. As the result, the first character of the culture was omitted
which would cause the satellite assemblies not to be loaded properly.

Fix the issue by using the correct mangled name prefix length.

@grendello grendello force-pushed the dev/grendel/off-by-one-error-satellite-assemblies branch from ed1f9e3 to 6c0aa73 Compare August 19, 2024 08:57
When assemblies are packaged in the APK archive, their names are
"mangled" so that we don't create clashes with "real" shared libraries,
which are placed in the same directory.  Additionally, since the `lib/`
directory in the APK archive supports only single level of directories,
we must encode satellite assembly culture in the file name, instead of
putting it in a subdirectory.

Unfortunately, when the code was committed, an off-by-one error crept
in, when converting satellite assembly names to their `culture/name`
for.  As the result, the first character of the culture was omitted
which would cause the satellite assemblies not to be loaded properly.

Fix the issue by using the correct mangled name prefix length.
@grendello grendello force-pushed the dev/grendel/off-by-one-error-satellite-assemblies branch from 6c0aa73 to a99d4b8 Compare August 20, 2024 09:45
@grendello
Copy link
Contributor Author

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jonpryor jonpryor merged commit c978f35 into main Aug 20, 2024
58 checks passed
@jonpryor jonpryor deleted the dev/grendel/off-by-one-error-satellite-assemblies branch August 20, 2024 18:29
@jonpryor
Copy link
Member

@grendello: even though I've merged this, I think we should add or update a unit test for this scenario. (Surely we already have a unit test that has satellite assemblies? @dellis1972 ?)

grendello added a commit that referenced this pull request Aug 27, 2024
* main: (47 commits)
  Bump to dotnet/sdk@5642787dac 9.0.100-rc.2.24426.2 (#9247)
  LEGO: Merge pull request 9246
  Bump to 34.0.137 of the .NET 8 Android workload (#9243)
  Bump external/Java.Interop from `d30d554` to `51b784a` (#9241)
  Bump dotnet/android-tools@6575743 (#9235)
  Bump to mono/debugger-libs@d5664344 (#9238)
  [ci] Improve push_signed_nugets job condition (#9240)
  Bump to dotnet/android-tools@657574378a6 and xamarin/monodroid@8bd4bae7 (#9216)
  Bump to dotnet/java-interop@d30d554 (#9234)
  Localized file check-in by OneLocBuild Task (#9236)
  Bump to dotnet/sdk@e2b7b9d2b4 9.0.100-rc.2.24420.1 (#9228)
  $(AndroidPackVersionSuffix)=rc.2; net9 is 35.0.0-rc.2 (#9233)
  [Xamarin.Android.Build.Tasks] Scan for JCWs for each ABI in parallel (#9215)
  [Xamarin.Android.Build.Tasks] %(JavaArtifact) is a list (#9112)
  [native/monodroid] Fix error demangling satellite assembly names (#9166)
  [build] Update package metadata (#9230)
  [Xamarin.Android.Build.Tasks] Remove ILRepack (#9226)
  [Xamarin.Android.Build.Tasks] Fix an issue with incremental builds (#9183)
  [Xamarin.Android.Build.Tasks] remove `$XAMARIN_BUILD_ID` (#9223)
  [Mono.Android] Use .NET version of mdoc (#9225)
  ...
@github-actions github-actions bot locked and limited conversation to collaborators Sep 20, 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