From 4b20432d95ea8965a41cc73997e459d7fa561233 Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Fri, 22 Nov 2024 18:30:21 +0100 Subject: [PATCH] [native/monodroid] Fix unmangling of satellite assembly names (#9533) Fixes: https://github.com/dotnet/android/issues/9532 Context: https://github.com/dotnet/android/pull/9410 Context: 86260ed36dfe1a90c8ed6a2bb1cd0607d637f403 Context: c92702619f5fabcff0ed88e09160baf9edd70f41 In Debug configuration builds, `$(EmbedAssembliesIntoApk)`=false by default. This enables Fast Deployment in commercial/non-OSS builds. When `$(EmbedAssembliesIntoApk)`=true, there are two separate ways to embed assemblies into the `.apk`: * Assembly Stores (c9270261), which is a "single" (-ish) file that contains multiple assemblies, enabled by setting `$(AndroidUseAssemblyStore)`=true. This is the default behavior for Release configuration builds. * One file per assembly (86260ed3). This is the default behavior for Debug configuration builds when `$(EmbedAssembliesIntoApk)`=true. Aside: dotnet/android#9410 is an attempt to *remove* support for the "one file per assembly" packaging strategy, which will *not* be applied to release/9.0.1xx. When using the "one file per assembly" strategy, all the assemblies are wrapped in a valid ELF shared library image and placed in the `lib/{ABI}` directories inside the APK/AAB archive. Since those directories don't support subdirectories, we need to encode satellite assembly culture in a way that doesn't use the `/` directory separator char. This encoding, as originally implemented, unfortunately used the `-` character which made it ambiguous with culture names that consist of two parts, e.g. `de-DE`, since the unmangling process would look for the first occurrence of `-` to replace it with `/`, which would form invalid assembly names such as `de/DE-MyAssembly.resources.dll` instead of the correct `de-DE/MyAssembly.resources.dll`. This would, eventually, lead to a mismatch when looking for satellite assembly for that specific culture. Fix it by changing the encoding for `/` from `-` to `_`, so that the mangled assembly name looks like `lib_de-DE_MyAssembly.resources.dll.so` and we can unambiguously decode it to the correct `de-DE/MyAssembly.resources.dll` name. --- .../Tests/Xamarin.Android.Build.Tests/BuildTest.cs | 5 +++++ .../Utilities/ArchiveAssemblyHelper.cs | 2 +- .../Utilities/MonoAndroidHelper.Basic.cs | 3 ++- src/native/monodroid/embedded-assemblies.hh | 2 +- src/native/runtime-base/shared-constants.hh | 1 + tests/MSBuildDeviceIntegration/Tests/BundleToolTests.cs | 8 ++++---- 6 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest.cs index 47602ce7249..aedd58f29e2 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest.cs +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest.cs @@ -50,6 +50,9 @@ public void DotNetBuild (string runtimeIdentifiers, bool isRelease, bool aot, bo new BuildItem ("EmbeddedResource", "Resource.es.resx") { TextContent = () => InlineData.ResxWithContents ("Cancelar") }, + new BuildItem ("EmbeddedResource", "Resource.de-DE.resx") { + TextContent = () => InlineData.ResxWithContents ("Abbrechen") + }, new AndroidItem.TransformFile ("Transforms.xml") { // Remove two methods that introduced warnings: // Com.Balysv.Material.Drawable.Menu.MaterialMenuView.cs(214,30): warning CS0114: 'MaterialMenuView.OnRestoreInstanceState(IParcelable)' hides inherited member 'View.OnRestoreInstanceState(IParcelable?)'. To make the current member override that implementation, add the override keyword. Otherwise add the new keyword. @@ -114,6 +117,7 @@ public void DotNetBuild (string runtimeIdentifiers, bool isRelease, bool aot, bo .ToArray (); var expectedFiles = new List { $"{proj.PackageName}-Signed.apk", + "de-DE", "es", $"{proj.ProjectName}.dll", $"{proj.ProjectName}.pdb", @@ -163,6 +167,7 @@ public void DotNetBuild (string runtimeIdentifiers, bool isRelease, bool aot, bo helper.AssertContainsEntry ($"assemblies/{proj.ProjectName}.pdb", shouldContainEntry: !TestEnvironment.CommercialBuildAvailable && !isRelease); helper.AssertContainsEntry ($"assemblies/Mono.Android.dll", shouldContainEntry: expectEmbeddedAssembies); helper.AssertContainsEntry ($"assemblies/es/{proj.ProjectName}.resources.dll", shouldContainEntry: expectEmbeddedAssembies); + helper.AssertContainsEntry ($"assemblies/de-DE/{proj.ProjectName}.resources.dll", shouldContainEntry: expectEmbeddedAssembies); foreach (var abi in rids.Select (AndroidRidAbiHelper.RuntimeIdentifierToAbi)) { helper.AssertContainsEntry ($"lib/{abi}/libmonodroid.so"); helper.AssertContainsEntry ($"lib/{abi}/libmonosgen-2.0.so"); diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/ArchiveAssemblyHelper.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/ArchiveAssemblyHelper.cs index 73541abf637..59b2b4e6fca 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/ArchiveAssemblyHelper.cs +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/ArchiveAssemblyHelper.cs @@ -389,7 +389,7 @@ public int GetNumberOfAssemblies (bool forceRefresh = false, AndroidTargetArch a // Android doesn't allow us to put satellite assemblies in lib/{CULTURE}/assembly.dll.so, we must instead // mangle the name. fileTypeMarker = MonoAndroidHelper.MANGLED_ASSEMBLY_SATELLITE_ASSEMBLY_MARKER; - fileName = $"{culture}-{fileName}"; + fileName = $"{culture}{MonoAndroidHelper.SATELLITE_CULTURE_END_MARKER_CHAR}{fileName}"; } var ret = new List (); diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/MonoAndroidHelper.Basic.cs b/src/Xamarin.Android.Build.Tasks/Utilities/MonoAndroidHelper.Basic.cs index 7eaac59420c..e87e3b7a96b 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/MonoAndroidHelper.Basic.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/MonoAndroidHelper.Basic.cs @@ -199,6 +199,7 @@ public static string MakeZipArchivePath (string part1, ICollection? path public const string MANGLED_ASSEMBLY_NAME_EXT = ".so"; public const string MANGLED_ASSEMBLY_REGULAR_ASSEMBLY_MARKER = "lib_"; public const string MANGLED_ASSEMBLY_SATELLITE_ASSEMBLY_MARKER = "lib-"; + public const string SATELLITE_CULTURE_END_MARKER_CHAR = "_"; /// /// Mangles APK/AAB entry name for assembly and their associated pdb and config entries in the @@ -208,7 +209,7 @@ public static string MakeZipArchivePath (string part1, ICollection? path public static string MakeDiscreteAssembliesEntryName (string name, string? culture = null) { if (!String.IsNullOrEmpty (culture)) { - return $"{MANGLED_ASSEMBLY_SATELLITE_ASSEMBLY_MARKER}{culture}-{name}{MANGLED_ASSEMBLY_NAME_EXT}"; + return $"{MANGLED_ASSEMBLY_SATELLITE_ASSEMBLY_MARKER}{culture}_{name}{MANGLED_ASSEMBLY_NAME_EXT}"; } return $"{MANGLED_ASSEMBLY_REGULAR_ASSEMBLY_MARKER}{name}{MANGLED_ASSEMBLY_NAME_EXT}"; diff --git a/src/native/monodroid/embedded-assemblies.hh b/src/native/monodroid/embedded-assemblies.hh index 02c4375fd87..e475add696f 100644 --- a/src/native/monodroid/embedded-assemblies.hh +++ b/src/native/monodroid/embedded-assemblies.hh @@ -405,7 +405,7 @@ namespace xamarin::android::internal { if constexpr (IsSatelliteAssembly) { // Make sure assembly name is {CULTURE}/assembly.dll for (size_t idx = start_idx; idx < name.length (); idx++) { - if (name[idx] == SharedConstants::SATELLITE_ASSEMBLY_MARKER_CHAR) { + if (name[idx] == SharedConstants::SATELLITE_CULTURE_END_MARKER_CHAR) { name[idx] = '/'; break; } diff --git a/src/native/runtime-base/shared-constants.hh b/src/native/runtime-base/shared-constants.hh index d21c99fa79f..bb6f4c4fadd 100644 --- a/src/native/runtime-base/shared-constants.hh +++ b/src/native/runtime-base/shared-constants.hh @@ -27,6 +27,7 @@ namespace xamarin::android::internal static constexpr std::string_view MANGLED_ASSEMBLY_SATELLITE_ASSEMBLY_MARKER { "lib-" }; static constexpr size_t SATELLITE_ASSEMBLY_MARKER_INDEX = 3; // this ☝️ static constexpr char SATELLITE_ASSEMBLY_MARKER_CHAR = MANGLED_ASSEMBLY_SATELLITE_ASSEMBLY_MARKER[SATELLITE_ASSEMBLY_MARKER_INDEX]; + static constexpr char SATELLITE_CULTURE_END_MARKER_CHAR = '_'; static constexpr std::string_view MONO_ANDROID_RUNTIME_ASSEMBLY_NAME { "Mono.Android.Runtime" }; static constexpr std::string_view MONO_ANDROID_ASSEMBLY_NAME { "Mono.Android" }; diff --git a/tests/MSBuildDeviceIntegration/Tests/BundleToolTests.cs b/tests/MSBuildDeviceIntegration/Tests/BundleToolTests.cs index 7f5229a514f..6c61f9fbe70 100644 --- a/tests/MSBuildDeviceIntegration/Tests/BundleToolTests.cs +++ b/tests/MSBuildDeviceIntegration/Tests/BundleToolTests.cs @@ -166,13 +166,13 @@ public void BaseZip () expectedFiles.Add ($"{blobEntryPrefix}{abi}/lib_Java.Interop.dll.so"); expectedFiles.Add ($"{blobEntryPrefix}{abi}/lib_Mono.Android.dll.so"); expectedFiles.Add ($"{blobEntryPrefix}{abi}/lib_Localization.dll.so"); - expectedFiles.Add ($"{blobEntryPrefix}{abi}/lib-es-Localization.resources.dll.so"); + expectedFiles.Add ($"{blobEntryPrefix}{abi}/lib-es{MonoAndroidHelper.SATELLITE_CULTURE_END_MARKER_CHAR}Localization.resources.dll.so"); expectedFiles.Add ($"{blobEntryPrefix}{abi}/lib_UnnamedProject.dll.so"); } else { expectedFiles.Add ($"lib/{abi}/lib_Java.Interop.dll.so"); expectedFiles.Add ($"lib/{abi}/lib_Mono.Android.dll.so"); expectedFiles.Add ($"lib/{abi}/lib_Localization.dll.so"); - expectedFiles.Add ($"lib/{abi}/lib-es-Localization.resources.dll.so"); + expectedFiles.Add ($"lib/{abi}/lib-es{MonoAndroidHelper.SATELLITE_CULTURE_END_MARKER_CHAR}Localization.resources.dll.so"); expectedFiles.Add ($"lib/{abi}/lib_UnnamedProject.dll.so"); } @@ -226,13 +226,13 @@ public void AppBundle () expectedFiles.Add ($"{blobEntryPrefix}{abi}/lib_Java.Interop.dll.so"); expectedFiles.Add ($"{blobEntryPrefix}{abi}/lib_Mono.Android.dll.so"); expectedFiles.Add ($"{blobEntryPrefix}{abi}/lib_Localization.dll.so"); - expectedFiles.Add ($"{blobEntryPrefix}{abi}/lib-es-Localization.resources.dll.so"); + expectedFiles.Add ($"{blobEntryPrefix}{abi}/lib-es{MonoAndroidHelper.SATELLITE_CULTURE_END_MARKER_CHAR}Localization.resources.dll.so"); expectedFiles.Add ($"{blobEntryPrefix}{abi}/lib_UnnamedProject.dll.so"); } else { expectedFiles.Add ($"base/lib/{abi}/lib_Java.Interop.dll.so"); expectedFiles.Add ($"base/lib/{abi}/lib_Mono.Android.dll.so"); expectedFiles.Add ($"base/lib/{abi}/lib_Localization.dll.so"); - expectedFiles.Add ($"base/lib/{abi}/lib-es-Localization.resources.dll.so"); + expectedFiles.Add ($"base/lib/{abi}/lib-es{MonoAndroidHelper.SATELLITE_CULTURE_END_MARKER_CHAR}Localization.resources.dll.so"); expectedFiles.Add ($"base/lib/{abi}/lib_UnnamedProject.dll.so"); }