Skip to content

[Microsoft.Android.Ref] support for older targetSdkVersion #4873

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

Closed

Conversation

jonathanpeppers
Copy link
Member

Currently, if you build a .NET 5 project that has
targetSdkVersion="29" (older than 30):

error XA5207: Could not find android.jar for API level . This means the Android SDK platform for API level  is not installed. Either install it in the Android SDK Manager (Tools > Android > Android SDK Manager...), or change the Xamarin.Android project to target an API version that is installed. (C:\Users\jopepper\android-toolchain\sdk\platforms\android-\android.jar missing.)

A lot of string.Empty appears in the error messages...

This is because we only have one AndroidApiInfo.xml:

<AndroidApiInfo>
  <Id>30</Id>
  <Level>30</Level>
  <Name>R</Name>
  <Version>v11.0</Version>
  <Stable>True</Stable>
</AndroidApiInfo>

And so calls like MonoAndroidHelper.SupportedVersions.GetApiLevelFromId
won't work for API 29, because API 29 is unknown.

To fix this, we should ship all of the AndroidApiInfo.xml files in
the Microsoft.Android.Ref.nupkg, such as:

  • ref\net5.0\Java.Interop.dll
  • ref\net5.0\Mono.Android.dll
  • ref\net5.0\v10.0\AndroidApiInfo.xml
  • ref\net5.0\v11.0\AndroidApiInfo.xml

The code here will continue to work, because it searches recursively:

https://github.com/xamarin/xamarin-android-tools/blob/3974fc38c0f25f943b5d3bf0a4e174532a2a60ee/src/Xamarin.Android.Tools.AndroidSdk/AndroidVersions.cs#L45-L46

I added a test verifying app with targetSdkVersion="29" work under
.NET 5.

Currently, if you build a .NET 5 project that has
`targetSdkVersion="29"` (older than 30):

    error XA5207: Could not find android.jar for API level . This means the Android SDK platform for API level  is not installed. Either install it in the Android SDK Manager (Tools > Android > Android SDK Manager...), or change the Xamarin.Android project to target an API version that is installed. (C:\Users\jopepper\android-toolchain\sdk\platforms\android-\android.jar missing.)

A lot of `string.Empty` appears in the error messages...

This is because we only have one `AndroidApiInfo.xml`:

    <AndroidApiInfo>
      <Id>30</Id>
      <Level>30</Level>
      <Name>R</Name>
      <Version>v11.0</Version>
      <Stable>True</Stable>
    </AndroidApiInfo>

And so calls like `MonoAndroidHelper.SupportedVersions.GetApiLevelFromId`
won't work for API 29, because API 29 is unknown.

To fix this, we should ship *all* of the `AndroidApiInfo.xml` files in
the `Microsoft.Android.Ref.nupkg`, such as:

* `ref\net5.0\Java.Interop.dll`
* `ref\net5.0\Mono.Android.dll`
* `ref\net5.0\v10.0\AndroidApiInfo.xml`
* `ref\net5.0\v11.0\AndroidApiInfo.xml`

The code here will continue to work, because it searches recursively:

https://github.com/xamarin/xamarin-android-tools/blob/3974fc38c0f25f943b5d3bf0a4e174532a2a60ee/src/Xamarin.Android.Tools.AndroidSdk/AndroidVersions.cs#L45-L46

I added a test verifying app with `targetSdkVersion="29"` work under
.NET 5.
@jonathanpeppers jonathanpeppers marked this pull request as ready for review June 26, 2020 20:42
@pjcollins
Copy link
Member

pjcollins commented Jun 26, 2020

I think we should only support targeting a single Android binding version for a given pack? For instance, users who want to target API 29 would need to install and use 10.0.100 versioned Android packs. Is this the only way to improve the error messaging? I'm not too familiar with how we use these xml files to distinguish between "supported Android version" and "known Android version".

@jonathanpeppers
Copy link
Member Author

I think we should only support targeting a single Android binding version for a given pack?

Should you be able to put android:targetSdkVersion="29" in your manifest and use Microsoft.Android.Sdk 11.0.x? This is independent of the binding for API 30.

Right now if we bumped the samples here:

https://github.com/xamarin/net6-samples

We would have to update every manifest to say targetSdkVersion="30". It seems like you shouldn't have to do that? (even if the error message was better)

@dellis1972
Copy link
Contributor

The compileSdkVersion I believe should match the nuget we are referencing. Both min and target should in theory be independent of that I think. We probably need @jonpryor to chime in here.

@jonathanpeppers
Copy link
Member Author

So this appears to only be broken for specifically API 29, because it isn't listed here:

https://github.com/xamarin/xamarin-android-tools/blob/2d3690e428c8523b3779f84e5c804d1fd3c0d6fe/src/Xamarin.Android.Tools.AndroidSdk/AndroidVersions.cs#L139-L176

So the question is, do we ship all the AndroidApiInfo.xml or update this list?

@pjcollins
Copy link
Member

pjcollins commented Jun 29, 2020

I had a slight misunderstanding of the original issue at hand. I would agree that we should continue to emit a warning rather than an error in cases where the targetSdkVersion in your manifest set to a value lower than the binding version.

As for the fix, I think we want to avoid shipping multiple AndroidApiInfo.xml files with a single Mono.Android.dll if possible. I am worried about running into potential issues with AndroidVersions.InstalledBindingVersions containing versions that aren't available in a one .NET context. I think we should be able to tell the difference between installed and known versions, though maintaining two separate areas of Android version information is a bit tedious...

@jonpryor
Copy link
Contributor

jonpryor commented Jun 29, 2020

Obligatory:

I have no idea what I'm doing

Do we need the AndroidApiInfo.xml files at all? If so, how many do we truly need?

The purpose behind the AndroidApiInfo.xml files, added in 8942eca, is to have a mapping between the $(TargetFrameworkVersion), API level, and API ID (which may differ from the API level during previews, e.g. ID "R" was API-30).

In a .NET 5+ environment, $(TargetFrameworkVersion) is completely different; it'll "always" be e.g. net5.0-android or net6.0-android, plus a new property for the OS version (I forget the name), e.g. net5.0-android12.0 is a plausible TFV. Our historical mapping will not be relevant.

That "narrows" the scope to API-Level & ID, e.g. we'll need a mapping from:

  • net5.0-android11.0 to API-30
  • net5.0-android12.0 to API-31 (level) & API-S (Id)

AndroidApiInfo.xml may be useful for that, which "seconds" @pjcollins comment:

we want to avoid shipping multiple AndroidApiInfo.xml files with a single Mono.Android.dll if possible

A .net5.0-android12.0 binding would be a separate assembly, and presumably a separate "package" (noun?).

This suggests that we shouldn't ship e.g. ref\net5.0\v10.0\AndroidApiInfo.xml.

Thus we return to the original "bug":

Currently, if you build a .NET 5 project that has targetSdkVersion="29" (older than 30):

error XA5207: Could not find android.jar for API level .
This means the Android SDK platform for API level  is not installed.
Either install it in the Android SDK Manager (Tools > Android > Android SDK Manager...),
or change the Xamarin.Android project to target an API version that is installed.
(C:\Users\jopepper\android-toolchain\sdk\platforms\android-\android.jar missing.)

The source of the XA5207: https://github.com/xamarin/xamarin-android/blob/f8eb265c3b91e90fa959364368e2e7d98d7f49a7/src/Xamarin.Android.Build.Tasks/Utilities/MonoAndroidHelper.cs#L609-L614

Is because AndroidSdkInfo.TryGetPlatformDirectoryFromApiLevel() requires that there be a known mapping from idOrApiLevel to an ID, which comes from AndroidApiInfo.xml files. As there is no file providing information about ID "29", this returns null.

What if instead it allowed idOrApiLevel to be an ID, without requiring additional mapping information?

diff --git a/src/Xamarin.Android.Tools.AndroidSdk/AndroidSdkInfo.cs b/src/Xamarin.Android.Tools.AndroidSdk/AndroidSdkInfo.cs
index 1297e51..d2d3b5f 100644
--- a/src/Xamarin.Android.Tools.AndroidSdk/AndroidSdkInfo.cs
+++ b/src/Xamarin.Android.Tools.AndroidSdk/AndroidSdkInfo.cs
@@ -98,11 +98,15 @@ namespace Xamarin.Android.Tools
 
 		public string? TryGetPlatformDirectoryFromApiLevel (string idOrApiLevel, AndroidVersions versions)
 		{
+			string? dir = GetPlatformDirectoryFromId (idOrApiLevel);
+			if (Directory.Exists (dir))
+				return dir;
+
 			var id  = versions.GetIdFromApiLevel (idOrApiLevel);
 			if (id == null)
 				return null;
 
-			string? dir = GetPlatformDirectoryFromId (id);
+			dir = GetPlatformDirectoryFromId (id);
 
 			if (Directory.Exists (dir))
 				return dir;

Other AndroidApiInfo methods may need to be likewise updated, but this should reduce the number of instances in which the AndroidApiInfo.xml files are required, which hopefully will in turn mean we don't need to ship "extra" such files.

@jonathanpeppers jonathanpeppers marked this pull request as draft June 29, 2020 20:10
jonathanpeppers added a commit to jonathanpeppers/xamarin-android-tools that referenced this pull request Jun 29, 2020
Context: dotnet/android#4873

Although this is related to xamarin-android#4873, it is not the full fix. We probably should list the missing versions here anyway, though.
@jonathanpeppers
Copy link
Member Author

It looks like we should fix this here instead: dotnet/android-tools#90

jonpryor pushed a commit to dotnet/android-tools that referenced this pull request Jun 30, 2020
Context: dotnet/android#4873
Context: dotnet/android#4873 (comment)

When xamarin-android is built to support .NET 5, the .NET 5 install
directory contains a single `AndroidApiInfo.xml` file:

	<AndroidApiInfo>
	  <Id>30</Id>
	  <Level>30</Level>
	  <Name>R</Name>
	  <Version>v11.0</Version>
	  <Stable>True</Stable>
	</AndroidApiInfo>

`AndroidVersions`, meanwhile, is setup to read a *set* of
`AndroidApiInfo.xml` files (aad97f1) to "dynamically" compute mappings
between possible `$(TargetFrameworkVersion)` values, API-levels, and
IDs for those API levels.

When there is only one such file, if you call:

	int? apiLevel = androidVersions.GetApiLevelFromId (29);

then `apiLevel` will always be `null`, because (at present)
`AndroidVersions.KnownVersions` doesn't know about API-29 or API-30.

We *could* update `AndroidVersions.KnownVersions` to contain entries
for API-29 & API-30, but doing so means that we reintroduce the
scenario that `AndroidVersions` was added to help defend against: a
need/requirement to update `AndroidVersions.KnownVersions` *every time*
a new API level was released, lest ~everything break.

We *don't* want to require `AndroidVersions.KnownVersions` updates.

To allow a .NET 5-like environment to work *without* updating
`KnownVersions`, update the various `GetApiLevelFromId()` methods to
return the incoming API level as a fallback.  If `"29"` comes in, then
`29` can be returned and assumed to be a valid API level.
jonathanpeppers added a commit to jonathanpeppers/xamarin-android-tools that referenced this pull request Jul 8, 2020
Context: dotnet/android#4873

Although this is related to xamarin-android#4873, it is not the full fix. We probably should list the missing versions here anyway, though.
jonathanpeppers added a commit to jonathanpeppers/xamarin-android-tools that referenced this pull request Jul 8, 2020
Context: dotnet/android#4873

Although this is related to xamarin-android#4873, it is not the full fix. We probably should list the missing versions here anyway, though.
@github-actions github-actions bot locked and limited conversation to collaborators Jan 24, 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.

4 participants