[Xamarin.Android.Build.Tasks] extractNativeLibs="true" by default (#5021

)

Fixes: #4986

The [`//application/@android:extractNativeLibs`][0] attribute within
`AndroidManifest.xml` has had a large-scale impact on `.apk`s:

When `extractNativeLibs` is `true`, then (1) native libraries within
the `.apk` are *compressed*, and (2) the native libraries within the
`.apk` are extracted during installation time.  This allows the `.apk`
to be smaller, while resulting in a *larger* installation footprint,
as two copies of the native libs -- one compressed, one uncompressed --
are present on the device.

When `extractNativeLibs` is `false`, then (1) native libraries within
the `.apk` are stored *uncompressed*, and (2) the native libraries
within the `.apk` are *not* extracted during installation time, but
*only* on API-23 and later devices.  This makes for a larger `.apk`,
but a *smaller* installation footprint.

When *omitted* this value defaults to `true`.

Commit 86737ca bumped `manifest-merger` from 26.5.0 to 27.0.0, which
[altered `manifest-merger.jar` behavior][1] so that it started setting
`//application/@android:extractNativeLibs`=false when `minSdkVersion`
is 23 or higher, "as if" `Properties\AndroidManifest.xml` contained:

	<application android:extractNativeLibs="false">

In the case of Issue #4986, which is an application which uses
Xamarin.Forms and sets `$(AotAssemblies)`=True, resulting in native
libraries being produced and included for *every assembly*, this
"minor" change to `extractNativeLibs` caused a 24MB `.apk` to balloon
in size to 64MB, which was unexpected.

This unexpected size increase was not caught by unit tests, as our
tests which check for `.apk` size regressions don't use Xamarin.Forms,
and it's AndroidX/Android Support -- used by Xamarin.Forms -- which
causes `manifest-merger.jar` to be used.  No `manifest-merger.jar`,
no unexpected size increase.

This `manifest-merger.jar` change is *also* the underlying cause of
bf657e8 and 8828fef.

To solve this, if not explicitly specified within apps'
`Properties\AndroidManifest.xml`, we will now insert
`android:extractNativeLibs="true"` by default.

This way Xamarin.Android applications will have consistent behavior.
Developers can set `extractNativeLibs="false"` if they prefer that
behavior.

This behavior also works fine on an API-19 emulator, since
`extractNativeLibs="true"` is already the default and seems to be
ignored.

[0]: https://developer.android.com/guide/topics/manifest/application-element#extractNativeLibs
[1]: https://android.googlesource.com/platform/tools/base/+/fd2ac00ab76dfdececce5a25cb7ad23b2b6f5d29%5E%21

Documentation/release-notes/extractNativeLibs.md

@@ -0,0 +1,35 @@
+#### Application and library build and deployment
+
+* [GitHub Issue 4986](https://github.com/xamarin/xamarin-android/issues/4986):
+  Updates to Android tooling (`manifest-merger`), caused
+  `//application/@android:extractNativeLibs` to be set to `false` by
+  default. This can cause an undesirable `.apk` file size increase
+  that is more noticeable for Xamarin.Android applications using AOT.
+  Xamarin.Android now sets `extractNativeLibs` to `true` by default.
+
+According to the [Android documentation][extractNativeLibs],
+`extractNativeLibs` affects `.apk` size and install size:
+
+> Whether or not the package installer extracts native libraries from
+> the APK to the filesystem. If set to false, then your native
+> libraries must be page aligned and stored uncompressed in the APK.
+> No code changes are required as the linker loads the libraries
+> directly from the APK at runtime. The default value is "true".
+
+This is a tradeoff that each developer should decide upon on a
+per-application basis. Is a smaller install size at the cost of a
+larger download size preferred?
+
+Since Xamarin.Android now emits `android:extractNativeLibs="true"` by
+default, you can get the opposite behavior with an
+`AndroidManifest.xml` such as:
+
+```xml
+<?xml version="1.0" encoding="utf-8"?>
+<manifest xmlns:android="http://schemas.android.com/apk/res/android" android:versionCode="1" android:versionName="1.0" package="com.companyname.hello">
+    <uses-sdk android:minSdkVersion="23" android:targetSdkVersion="30" />
+    <application android:label="Hello" android:extractNativeLibs="false" />
+</manifest>
+```
+
+[extractNativeLibs]: https://developer.android.com/guide/topics/manifest/application-element

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/PackagingTest.cs

@@ -165,6 +165,9 @@ public void EmbeddedDSOs ()
 			using (var b = CreateApkBuilder (Path.Combine ("temp", TestName))) {
 				Assert.IsTrue (b.Build (proj), "first build should have succeeded");
 
+				var manifest = Path.Combine (Root, b.ProjectDirectory, proj.IntermediateOutputPath, "android", "AndroidManifest.xml");
+				AssertExtractNativeLibs (manifest, extractNativeLibs: false);
+
 				var apk = Path.Combine (Root, b.ProjectDirectory,
 						proj.IntermediateOutputPath, "android", "bin", "UnnamedProject.UnnamedProject.apk");
 				AssertEmbeddedDSOs (apk);
@@ -893,5 +896,33 @@ public class Test
 				}
 			}
 		}
+
+		[Test]
+		public void ExtractNativeLibsTrue ()
+		{
+			var proj = new XamarinAndroidApplicationProject {
+				// This combination produces android:extractNativeLibs="false" by default
+				ManifestMerger = "manifestmerger.jar",
+			};
+			proj.AndroidManifest = proj.AndroidManifest.Replace ("<uses-sdk />", "<uses-sdk android:minSdkVersion=\"23\" />");
+			using (var b = CreateApkBuilder ()) {
+				Assert.IsTrue (b.Build (proj), "Build should have succeeded.");
+
+				// We should find extractNativeLibs="true"
+				var manifest = Path.Combine (Root, b.ProjectDirectory, proj.IntermediateOutputPath, "android", "AndroidManifest.xml");
+				AssertExtractNativeLibs (manifest, extractNativeLibs: true);
+
+				// All .so files should be compressed
+				var apk = Path.Combine (Root, b.ProjectDirectory,
+					proj.IntermediateOutputPath, "android", "bin", $"{proj.PackageName}.apk");
+				using (var zip = ZipHelper.OpenZip (apk)) {
+					foreach (var entry in zip) {
+						if (entry.FullName.EndsWith (".so")) {
+							AssertCompression (entry, compressed: true);
+						}
+					}
+				}
+			}
+		}
 	}
 }

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/BaseTest.cs

@@ -1,4 +1,4 @@
-using NUnit.Framework;
+using NUnit.Framework;
 using System;
 using System.Collections.Concurrent;
 using System.Collections.Generic;
@@ -7,6 +7,7 @@
 using System.Linq;
 using System.Text;
 using System.Threading;
+using System.Xml.Linq;
 using Xamarin.Android.Tasks;
 using Xamarin.ProjectTools;
 
@@ -460,6 +461,23 @@ protected string GetPathToAapt ()
 			return GetPathToLatestBuildTools (exe);
 		}
 
+		/// <summary>
+		/// Asserts that a AndroidManifest.xml file contains the expected //application/@android:extractNativeLibs value.
+		/// </summary>
+		protected void AssertExtractNativeLibs (string manifest, bool extractNativeLibs)
+		{
+			FileAssert.Exists (manifest);
+			using (var stream = File.OpenRead (manifest)) {
+				var doc = XDocument.Load (stream);
+				var element = doc.Root.Element ("application");
+				Assert.IsNotNull (element, $"application element not found in {manifest}");
+				var ns = (XNamespace) "http://schemas.android.com/apk/res/android";
+				var attribute = element.Attribute (ns + "extractNativeLibs");
+				Assert.IsNotNull (attribute, $"android:extractNativeLibs attribute not found in {manifest}");
+				Assert.AreEqual (extractNativeLibs ? "true" : "false", attribute.Value, $"Unexpected android:extractNativeLibs value found in {manifest}");
+			}
+		}
+
 		[SetUp]
 		public void TestSetup ()
 		{

src/Xamarin.Android.Build.Tasks/Utilities/ManifestDocument.cs

@@ -248,6 +248,8 @@ public IList<string> Merge (TaskLoggingHelper log, TypeDefinitionCache cache, Li
 
 			if (app.Attribute (androidNs + "label") == null && applicationName != null)
 				app.SetAttributeValue (androidNs + "label", applicationName);
+			if (app.Attribute (androidNs + "extractNativeLibs") == null)
+				app.SetAttributeValue (androidNs + "extractNativeLibs", "true");
 
 			var existingTypes = new HashSet<string> (
 				app.Descendants ().Select (a => (string) a.Attribute (attName)).Where (v => v != null));

tests/MSBuildDeviceIntegration/Tests/DebuggingTest.cs

@@ -46,8 +46,10 @@ public void ApplicationRunsWithoutDebugger ([Values (false, true)] bool isReleas
 			proj.SetDefaultTargetDevice ();
 			using (var b = CreateApkBuilder (Path.Combine ("temp", TestName))) {
 				SetTargetFrameworkAndManifest (proj, b);
-				proj.AndroidManifest = proj.AndroidManifest.Replace ("<application ", $"<application android:extractNativeLibs=\"{extractNativeLibs}\" ");
+				proj.AndroidManifest = proj.AndroidManifest.Replace ("<application ", $"<application android:extractNativeLibs=\"{extractNativeLibs.ToString ().ToLowerInvariant ()}\" ");
 				Assert.True (b.Install (proj), "Project should have installed.");
+				var manifest = Path.Combine (Root, b.ProjectDirectory, proj.IntermediateOutputPath, "android", "AndroidManifest.xml");
+				AssertExtractNativeLibs (manifest, extractNativeLibs);
 				ClearAdbLogcat ();
 				if (CommercialBuildAvailable)
 					Assert.True (b.RunTarget (proj, "_Run"), "Project should have run.");