[tests] emit <uses-sdk/> & use Xamarin.Forms 4.7

[jonathanpeppers]

Context: dotnet/maui-samples@571e1c0

Xamarin.Forms 4.7.x is required to solve a crash on startup in .NET 5+.
We should bump to Xamarin.Forms 4.7 in our MSBuild tests. This requires
some additional AndroidX packages to be bumped.

However, after doing this I got build warnings such as:

warning XA1006: Android API level 30 is higher than the targetSdkVersion (28). Please increase the `android:targetSdkVersion` in the `AndroidManifest.xml` so that the API levels match.

The project has a AndroidManifest.xml with an empty <uses-sdk/>
element, but obj\Debug\android\AndroidManifest.xml has 28 listed.
The projects that the MSBuild tests generate have an empty
<uses-sdk/> by default.

So where does targetSdkVersion="28" come from?

The Xamarin.AndroidX.Media NuGet package brings in:

<manifest xmlns:android="http://schemas.android.com/apk/res/android"
    package="androidx.media" >
    <uses-sdk
        android:minSdkVersion="14"
        android:targetSdkVersion="28" />
...

This is coming directly from the .aar file via:

https://maven.google.com/androidx/media/media/1.1.0/media-1.1.0.aar

Why do our MSBuild tests even have an AndroidManifest.xml file with
an empty <uses-sdk/>? It seems like almost no developer would do
this--the templates have been defining minSdkVersion and
targetSdkVersion for a long time.

To address how the MSBuild tests work:

• Move Builder.GetMaxInstalledPlatform to AndroidSdkResolver, so
  it can be a static method. Add simple caching so we don't query
  the file system so much.
• Add TargetSdkVersion and MinSdkVersion properties to
  XamarinAndroidApplicationProject.
• The new properties impact <uses-sdk/> accordingly. You can set
  them to "" or null to omit them from AndroidManifest.xml.

Now our MSBuild tests will default to use:

<uses-sdk android:minSdkVersion="19" android:targetSdkVersion="30" />

...until we add another new API level.

The impacted MSBuild tests will now pass under a dotnet context.

[jonathanpeppers]

I think this ToggleFastDev test is broken by this change:

There should be no files in Fast Dev directories! Instead found: ls: /data/data/UnnamedProject.UnnamedProject/files/.override: Permission denied
Expected string length 0 but was 83. Strings differ at index 0.
Expected: <string.Empty>
But was:  "ls: /data/data/UnnamedProject.UnnamedProject/files/.__overrid..."

This command:

> adb shell ls /data/data/UnnamedProject.UnnamedProject/files/.override

It needs to use run-as, I don't actually see how it was working before?

> adb shell run-as UnnamedProject.UnnamedProject ls /data/data/UnnamedProject.UnnamedProject/files/.override

[dellis1972]

I think we need input from @jonpryor as to what the expected behaviour is in this instance.

[jonathanpeppers]

@jonpryor this is the main behavioral change here ^^

Before these changes, the absence of <uses-sdk/> worked differently than if you had an empty <uses-sdk/>.

[jonpryor]

@jonathanpeppers wrote in the PR message:

Reviewing the code in ManifestDocument, it seems like we should be
replacing the targetSdkVersion for empty elements?

Notably absent is why we "should" do that.

See also: https://github.com/xamarin/monodroid/commit/1c78098c5505a3e15ef2e59a9ca8f3e53b6da431

eno:  @jonp what behavior does Android actually change?
jonp: what doesn't it change? :-)
jonp: @eno seems that the "Android X APIs" page also lists semantic
      changes that depend on minSdkVersion, e.g.:
      http://developer.android.com/about/versions/android-4.2.html
      "Content providers... This change takes effect only if you set
      either android:targetSdkVersion or android:minSdkVersion to 17
      or higher."

In short, //uses-sdk/@android:minSdkVersion controls EVERYTHING.

targetSdkVersion is in a similar position: it's presence (or lack thereof!) and value will change the semantics of Android behavior.

For a more recent example consider Android 6.0's introduction of Runtime Permissions. The new behavior was only enabled when targetSdkVersion was >= 23, but for apps which had to update this value, it was a potentially significant amount of work to "fix" the app after that change.

In short, the lack of //uses-sdk/@android:minSdkVersion and/or //uses-sdk/@android:targetSdkVersion is semantically meaningful. At minimum, there needs to be a way to not have these attributes, if only for our own testing ("what happens to app behavior of we don't set these properties?").

"Differently" concerning to me is the underlying scenario: reference the Xamarin.AndroidX.Media, and your apps //uses-sdk attribute values are... what? Replaced by the values from Xamarin.AndroidX.Media? Used if present in Properties\AndroidManifest.xml, but otherwise come from "somewhere", including Xamarin.AndroidX.Media? Guess this means that the manifestmerger docs are now effectively required reading -- and has been since 2019? -- which isn't something that I had realized. Which is "differently joyful"?

• Attributes in the <uses-sdk> element always use the value from the higher-priority manifest, except in the following situations:

which seems to mean that //uses-sdk/@android:targetSdkVersion & co. are no longer fully controllable by the developer, meaning I can't write a one-off "personal" app that ignores the Runtime Permissions checks and also uses AndroidX, as the inclusion of some "random" AndroidX lib will force a targetSdkVersion value >= 23.

…which might be "fine", I guess? (I'm not necessarily thrilled, but this might be "fine"?)

[jonpryor]

That analysis out of the way, what should we do about it?

The ship has sailed on manifestmerger; it's been in since d16-5, so apparently this change hasn't "broken" anybody, and this is presumably more "in spirit" with what Java requires.

If Properties\AndroidManifest.xml does contain a //uses-sdk/@android:targetSdkVersion value, and it's being "ignored" (because manifestmerger is updating it based on dependencies), we should still emit an XA1006 warning. (This might be the case in this PR, but I'm not sure. This should be verified.) Why? Because the Properties\AndroidManifest.xml value is the "source of truth" as provided by the developer, and if this source of truth is being ignored -- which it is -- then they should be told so. The warning is useful, and can be disabled by updating the value within Properties\AnsroidManifest.xml.

This new behavior also needs to be adequately documented & described, particularly the relation to manifestmerger (which wasn't explicitly obvious).

[jonathanpeppers]

@jonpryor so the only issue here is an app has an empty <uses-sdk/> in their AndroidManifest.xml. I would guess that almost no one does this. The templates have defined minSdkVersion and targetSdkVersion for a long time--maybe always?

However, our MSBuild tests use <uses-sdk/> -- which is kind of odd. I think the fix might should just change these tests to use:

<uses-sdk android:minSdkVersion="{minSupportedApi}" android:targetSdkVersion="{latestStableApi}" />

And I won't change any behavior here.

[jonathanpeppers]

This was needed as discussed here: https://xamarinhq.slack.com/archives/C03CEGRUW/p1596468488148200