Skip to content

Commit

Permalink
[native/monodroid] Fix unmangling of satellite assembly names (#9533)
Browse files Browse the repository at this point in the history
Fixes: #9532

Context: #9410
Context: 86260ed
Context: c927026

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 (c927026), 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 (86260ed).

    This is the default behavior for Debug configuration builds when
    `$(EmbedAssembliesIntoApk)`=true.

Aside: #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.
  • Loading branch information
grendello authored and jonpryor committed Nov 25, 2024
1 parent 96b0a73 commit 4b20432
Show file tree
Hide file tree
Showing 6 changed files with 14 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ public void DotNetBuild (string runtimeIdentifiers, bool isRelease, bool aot, bo
new BuildItem ("EmbeddedResource", "Resource.es.resx") {
TextContent = () => InlineData.ResxWithContents ("<data name=\"CancelButton\"><value>Cancelar</value></data>")
},
new BuildItem ("EmbeddedResource", "Resource.de-DE.resx") {
TextContent = () => InlineData.ResxWithContents ("<data name=\"CancelButton\"><value>Abbrechen</value></data>")
},
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.
Expand Down Expand Up @@ -114,6 +117,7 @@ public void DotNetBuild (string runtimeIdentifiers, bool isRelease, bool aot, bo
.ToArray ();
var expectedFiles = new List<string> {
$"{proj.PackageName}-Signed.apk",
"de-DE",
"es",
$"{proj.ProjectName}.dll",
$"{proj.ProjectName}.pdb",
Expand Down Expand Up @@ -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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<string> ();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ public static string MakeZipArchivePath (string part1, ICollection<string>? 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 = "_";

/// <summary>
/// Mangles APK/AAB entry name for assembly and their associated pdb and config entries in the
Expand All @@ -208,7 +209,7 @@ public static string MakeZipArchivePath (string part1, ICollection<string>? 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}";
Expand Down
2 changes: 1 addition & 1 deletion src/native/monodroid/embedded-assemblies.hh
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
1 change: 1 addition & 0 deletions src/native/runtime-base/shared-constants.hh
Original file line number Diff line number Diff line change
Expand Up @@ -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" };
Expand Down
8 changes: 4 additions & 4 deletions tests/MSBuildDeviceIntegration/Tests/BundleToolTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}

Expand Down Expand Up @@ -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");
}

Expand Down

0 comments on commit 4b20432

Please sign in to comment.