Skip to content
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

[Mono.Android] Bind Android 12L Beta 1. #6601

Merged
merged 18 commits into from
Jan 14, 2022
Merged

[Mono.Android] Bind Android 12L Beta 1. #6601

merged 18 commits into from
Jan 14, 2022

Conversation

jpobst
Copy link
Contributor

@jpobst jpobst commented Jan 3, 2022

Context: https://developer.android.com/about/versions/12/12L
Context: https://android-developers.googleblog.com/2021/10/12L-preview-large-screens.html

Android 12L Developer Beta 1 has been released. The Android 12L
Developer Preview Program Overview Timeline and updates section
suggests the following timeline:

  • October: Developer Preview 1
  • December: Beta 1 (Final APIs)
  • January: Beta 2
  • February: Beta 3
  • Q1 2022: Final

Enumify API-32, set API-Sv2 as API-32, v12.1, and stable. Bump .NET 6 pack version to 32.0.100.

@jpobst jpobst force-pushed the api-32-beta1 branch 2 times, most recently from a8593f3 to f1f13df Compare January 4, 2022 20:44
You should still be able to target `net6.0-android31`, as I hardcoded
the version to what is released on NuGet.org:

https://www.nuget.org/packages/Microsoft.Android.Ref.31/31.0.101-preview.11.117

With these changes `net6.0-android31` resolves from:

    ~\.nuget\packages\microsoft.android.ref.31\31.0.101-preview.11.117\ref\net6.0\Mono.Android.dll

But with `net6.0-android` (defaults to 32 now), you get:

    dotnet\packs\Microsoft.Android.Ref.32\*\ref\net6.0\Mono.Android.dll

I also removed `%(DefaultRuntimeFrameworkVersion)` from
`@(KnownFrameworkReference)` as it is not needed.
Copy link
Member

@jonathanpeppers jonathanpeppers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any MSBuild test now fails with the warning:

Xamarin.Android.Aapt2.targets(157,3): warning APT2000: aapt2 W 01-06 00:34:15 13393 106914 LoadedArsc.cpp:657] Unknown chunk type '200'.

And so every test checking there are no warnings fails.

I'll look into bumping build-tools that we use during our build, that looks to be where we get aapt2 from. Hopefully that solves it?

Any app project build was hitting the warning:

    Xamarin.Android.Aapt2.targets(157,3): warning APT2000: aapt2 W 01-05 23:58:06  8352 43857 LoadedArsc.cpp:657] Unknown chunk type '200'.

And so any MSBuild test that asserts no warnings would fail.

I think we need to get a new version of `aapt2` to fix this.
@jonathanpeppers
Copy link
Member

@jonathanpeppers
Copy link
Member

As for this warning:

D8 : warning : An API level of 31 is not supported by this compiler. Please use an API level of 30 or earlier 

Hoping #6620 might fix?

@jonathanpeppers
Copy link
Member

This is now looking "greener".

So the issue now is build-tools 32.0.0 no longer ships with dx.jar:

  COMPILETODALVIK : error : Unable to access jarfile C:\Users\cloudtest\android-toolchain\sdk\build-tools\32.0.0\\lib\dx.jar [C:\a\_work\1\a\TestRelease\01-07_19.50.01\temp\CheckClassesDexIsIncludeddx\UnnamedProject.csproj]

Unsure what to do here if we want to support dx...

Apps with the `AndroidManifest.xml`:

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

Passes `--min-api 32` to d8/r8.

This emits the warning:

    D8 : warning : An API level of 32 is not supported by this compiler. Please use an API level of 31 or earlier

Let's remove this test case for now.
Otherwise, builds using `AndroidDexTool=dx` fail with:

    COMPILETODALVIK : error : Unable to access jarfile C:\Users\jopepper\android-toolchain\sdk\build-tools\32.0.0\\lib\dx.jar

It looks like we have existing logic falling back to older `build-tools` when you install them side-by-side.
* The default `$(TargetPlatformVersion)` is 32.0
* Keep the `Microsoft.Android.Ref.31` packs in our workload, hardcoded to the previous release on NuGet.org.
* Import `Microsoft.Android.Sdk.SupportedPlatforms.targets` *before* `Microsoft.Android.Sdk.BundledVersions.targets`
We get an XA1008 when targetSdkVersion="32" and $(TargetPlatformVersion)=31.0 otherwise. This causes various MSBuild tests to fail that assert no warnings.
@jonathanpeppers jonathanpeppers marked this pull request as ready for review January 12, 2022 03:45
@jonathanpeppers
Copy link
Member

Oh, this might not actually be green, there is a merge conflict, lol...

@jonathanpeppers
Copy link
Member

Ok this is now only failure 1 set of tests:

platforms/android-32 should be a dependency.

I think I need to make them check for 31 under .NET 6, and then hopefully this will be green.

},
"Microsoft.Android.Runtime.31.android-x64": {
"kind": "framework",
"version": "@WORKLOAD_VERSION@"
"version": "31.0.101-preview.11.117"
Copy link
Member

@pjcollins pjcollins Jan 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems the choice to keep API 31 as the .NET default is introducing some new complexity... It looks like we're locking the API 31 ref/runtime pack versions to the latest version available on nuget.org. I think this makes sense as we're now only building the net6.0 version of Mono.Android.dll for API 32.

On the .NET MSBuild testing side we may have a new gap in coverage though. I think the default test project will continue to target net6.0-android31, which will be running against ref/runtime packs that have already been published and not the ones produced by the build. The positive side of this is that we will ensure that any changes to the SDK continue to work with our previously produced API 31 packs. A potential downside is that we may now miss an issue caused by a change in Mono.Android.csproj. I'm not sure what the best solution for this is at the moment and I don't think it necessarily needs to be addressed in this PR, but it's worth further discussion.

On that note we have a few new hardcoded API 31 instances in the tests which might be easy to forget to update in the future. Maybe we should make this a constant somewhere, or add a note for this in some API update checklist or something like that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, if we end up with 32 as the default in .NET 6, we wouldn't have to put the 31 packs in the workload.

This logic here, allows them to come from NuGet or the workload:

https://github.com/xamarin/xamarin-android/blob/5f66c36fc10ae2f159885fafe15b16b871378bd1/build-tools/create-packs/Microsoft.Android.Sdk.proj#L122-L142

I think we should wait and see what we end up doing.

<_AndroidRuntimePackId Condition=" '%24(TargetPlatformVersion)' != '$(AndroidLatestUnstableApiLevel).0' ">$(AndroidLatestStableApiLevel)</_AndroidRuntimePackId>
<_AndroidRuntimePackId Condition=" '%24(TargetPlatformVersion)' == '$(AndroidLatestUnstableApiLevel).0' ">$(AndroidLatestUnstableApiLevel)</_AndroidRuntimePackId>
</PropertyGroup>
<PropertyGroup Condition=" '%24(TargetPlatformVersion)' == '31.0' ">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be using $(AndroidTargetDotNetApiLevel) (and possibly other new properties)? Isn't that what $(AndroidTargetDotNetApiLevel) is for?

Related: instead of $(AndroidTargetDotNetApiLevel), should we call it $(AndroidDefaultTargetDotnetApiLevel)? This "emphasizes" the "default" nature of this value.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In some of the other tests here, I'll try to use $(AndroidDefaultTargetDotnetApiLevel) so we don't have to change them again later.

In this case, however, the number 31 needs to remain, it will need to eternally pull this version from NuGet (or our workload):

https://github.com/xamarin/xamarin-android/blob/bedecc25f93a82ecaa8df5cb4e92424aa669125d/build-tools/create-packs/Microsoft.Android.Sdk.proj#L122-L125

I guess that over time, we'll have numbers listed for 32, 33, and so on. We can also update those packs at some point if ever needed (hopefully not ever?), we'd just list a different version number here.

* Renamed `$(AndroidTargetDotNetApiLevel)` -> `$(AndroidDefaultTargetDotnetApiLevel)`
* Put this value in `XABuildConfig`, so we can use it in tests
* Updated several places to use `XABuildConfig` instead of 31 hardcoded inline
@jonpryor
Copy link
Member

@jonathanpeppers : in commit 38fddb7 you wrote:

But with net6.0-android (defaults to 32 now), you get:

dotnet\packs\Microsoft.Android.Ref.32\*\ref\net6.0\Mono.Android.dll

is this still the case? I believe we don't want that to be the case; net6.0-android should remain as net6.0-android31.0.

@jonpryor
Copy link
Member

Proposed commit message:

[Mono.Android] API-32 stabilization

Context: https://developer.android.com/about/versions/12/12L
Context: https://android-developers.googleblog.com/2021/12/beta-1-update-for-12l-feature-drop.html

Android 12L Developer Beta 1 has been released.

  * [API diff vs. API-31][0]
  * [API diff vs. Sv2 DP1][1]

The Android 12L Developer Preview Program Overview
[Timeline and updates][2] section suggests the following timeline
(which differs from ef011213):

  * October:    Developer Preview 1
  * December:   Beta 1; Final APIs (previously expected in Beta 2)
  * January:    Beta 2; Incremental beta update
  * February:   Beta 3; Incremental beta update
  * Q1 2022:    Final release

In particular, Beta 1 has "Final APIs".  We now have API-32!

Enumify API-32, and declare it stable.

As a change from "upstream Google", Classic Xamarin.Android uses
`$(TargetFrameworkVersion)`=v12.1 for API-32, as there is no way to
express "12L" in a `System.Version`.  We have decreed 12.1 as 12L.

.NET SDK for Android uses `$(TargetFramework)`=net6.0-android32.0.

However, we don't want to change the default API level for .NET 6
projects; the default will remain `net6.0-android31.0` (API-31),
using the [31.0.101-preview.11.117 binaries on NuGet.org][3].
This requires various changes to the unit test infrastructure and
.NET SDK workflow package creation.

Additionally, `d8` doesn't appear to support API-32 yet (?!);
building an app with an `AndroidManifest.xml` containing:

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

would use `d8 --min-api 32`, which results in a warning:

	D8 : warning : An API level of 32 is not supported by this compiler.
	Please use an API level of 31 or earlier

Update the `XASdkTests.SupportedOSPlatformVersion()` test so that it
only uses API-31 and not API-32.  This avoids the above warning.

Finally, we had to update xaprepare to install the new
Android SDK Build-tools version 32.0.0, as `aapt` from previous
Build-tools versions would emit a warning when building for API-32:

	Xamarin.Android.Aapt2.targets(157,3): warning APT2000: aapt2 W 01-05 23:58:06  8352 43857 LoadedArsc.cpp:657] Unknown chunk type '200'.

Unfortunately, installing Build-tools 32.0.0 promptly broke unit
tests using `$(AndroidDexTool)`=dx, with:

	COMPILETODALVIK : error : Unable to access jarfile C:\Users\jopepper\android-toolchain\sdk\build-tools\32.0.0\\lib\dx.jar

as Build-tools 31 and later no longer contains `lib/dx.jar`.  Update
xaprepare to install *both* Build-tools 30.0.3 *and* 32.0.0, so that
tests which require `dx.jar` continue to run, and we can get *some*
test coverage against Build-tools 32.

Co-authored-by: Jonathan Peppers <jonathan.peppers@microsoft.com>

[0]: https://developer.android.com/sdk/api_diff/32/changes
[1]: https://developer.android.com/sdk/api_diff/32-incr/changes
[2]: https://web.archive.org/web/20220113142518/https://developer.android.com/about/versions/12/12L/overview
[3]: https://www.nuget.org/packages/Microsoft.Android.Ref.31/31.0.101-preview.11.117

@jonathanpeppers
Copy link
Member

@jonpryor that old commit was true at the time, but should now be:

But with net6.0-android (defaults to 31 now), you get:

dotnet\packs\Microsoft.Android.Ref.31\*\ref\net6.0\Mono.Android.dll

@jonpryor jonpryor merged commit 6eb11f1 into main Jan 14, 2022
@jonpryor jonpryor deleted the api-32-beta1 branch January 14, 2022 12:52
@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.

6 participants