[Xamarin.Android.Build.Tools] Bump Android tools versions #4012
Conversation
brendanzagaeski
requested review from
dellis1972 and
jonathanpeppers
as code owners
jonathanpeppers
left a comment
jonathanpeppers
left a comment
There was a problem hiding this comment.
I think this should update what we install when building Xamarin.Android, too. I think it would be these properties:
|
Sounds good. I've added a commit for that. It looks like there are some tests that might also need to be updated to expect these versions. I'll take a look at those now. One consideration I'll keep in mind with those is that the template projects in Visual Studio are for the moment still defaulting to Android 9 Pie (API level 28), so it makes sense to have some tests using those versions. |
|
The latest commit failed to build because Android SDK Platform-Tools 29.0.5 contains changes to the
Excerpt of the diff of --- "a/platform-tools-29.0.1\\api\\api-versions.xml"
+++ "b/platform-tools-29.0.5\\api\\api-versions.xml"
@@ -10,0 +11 @@
+ <field name="ACCESS_BACKGROUND_LOCATION" since="29"/>
@@ -14,0 +16 @@
+ <field name="ACCESS_MEDIA_LOCATION" since="29"/>
@@ -20,0 +23 @@
+ <field name="ACTIVITY_RECOGNITION" since="29"/>QuestionsI think having these If so, I think I can resolve the build breakage by creating a new |
|
@jonpryor or @gugavaro is it fine for @brendanzagaeski to add this attributes here? |
|
Changes in the latest commit: For the sake of being ready to accept the To address the One subtlety with this change is that previously the Questions: I'm not sure if I've correctly understood how the It looks like the reason might be that the Maybe I've misunderstood when we want to update To do:
|
|
Changes in the latest commits: Revert the previous commit that changed |
|
Changes in the latest commit: Fix the test failures. It turned out the failing tests were for the automatic SDK download feature. Those tests have hardcoded strings that are compared against the default build-tools version, so those strings needed to be updated. That resolves the question I had earlier about making sure the updated tests would still cover the Android 9 Pie (API level 29) target framework version. Since the test fixes only touch the build-tools version, the coverage of target framework versions is unchanged. Remaining questionsDo we want to take the If so, is the current approach of updating The answers might need to wait until January to make sure any team members who are away for the end of the year can weigh in. |
|
@gugavaro, when you have a moment, I think a couple questions you'll be interested in are:
Thanks a bunch! |
Those are acceptable changes, we just need to update the acceptable files to allow those changes pass by. |
Where are you getting the Mono.Android.dll from? You should not use the one built locally, instead we use a released one. If there is no breakage, we can remove the acceptable-breakages-vReference.txt file. We can also remove the DebuggerStepThrough from our checks. |
@brendanzagaeski those are good questions: thanks! |
|
You can see in a build log what's happening. First we Then we Finally we |
|
...but that just raises a slightly different question: Consider
Why, then, are we binding it with no [Register ("ACCESS_BACKGROUND_LOCATION")]
public const string AccessBackgroundLocation = (string) "android.permission.ACCESS_BACKGROUND_LOCATION";
<field deprecated="not deprecated"
final="true"
name="ACCESS_BACKGROUND_LOCATION"
jni-signature="Ljava/lang/String;"
static="true"
transient="false"
type="java.lang.String"
type-generic-aware="java.lang.String"
value=""android.permission.ACCESS_BACKGROUND_LOCATION""
visibility="public"
volatile="false"
merge.SourceFile="../../bin/BuildDebug/api/api-29.xml.in"
/>(Newlines added for readability.) Note that https://github.com/xamarin/java.interop/blob/4565369743a58a1d43e5a79c406c27d46b81bb08/tools/generator/ApiVersionsSupport.cs#L12 ...yet the fact that we're not emitting |
|
It looks like we actually pull our ( Therefore I suspect that this file was updated in this Android SDK version. I have https://github.com/xamarin/java.interop/blob/master/tools/generator/CodeGenerator.cs#L158 |
|
@jpobst wrote:
That explains a lot. "Doubly" fun and interesting is that I have platform-tools 29.0.1 installed -- which matches the previous Apparently platform-tools 29.0.5 does add it and others, hence all the breakage in this PR. Which leads me to believe that How hard would it be to update |
|
Not very hard, though I would probably prefer to change |
A "funny" thing happened when we bumped `$(XAPlatformToolsVersion)` in PR #4012: ~all of the API compat checks started failing! AfterBuild: CheckApiCompatibility for ApiLevel: v10.0 CompatApi command: ../../packages/Microsoft.DotNet.ApiCompat.5.0.0-beta.19606.1/tools/net472/Microsoft.DotNet.ApiCompat.exe "../../tests/api-compatibility\reference\ApiCompatTemp" -i "F:\A\xamarin-v000006-1\_work\2\s\bin\Release\lib\xamarin.android\xbuild-frameworks\MonoAndroid\v10.0\ApiCompatTemp" --exclude-attributes ../../tests/api-compatibility\api-compat-exclude-attributes.txt Compat issues with assembly Mono.Android: CannotChangeAttribute : Attribute 'Android.Runtime.RegisterAttribute' on 'System.String Android.Manifest.Permission.AccessBackgroundLocation' changed from '[RegisterAttribute("ACCESS_BACKGROUND_LOCATION")]' in the contract to '[RegisterAttribute("ACCESS_BACKGROUND_LOCATION", ApiSince=29)]' in the implementation. ... The API compat checks are failing because of the *introduction* of `RegisterAttribute.ApiSince` property values for members added in API-29, and these errors didn't "make sense" to use because we had expected all these members to *already* have `ApiSince=29`! What Went Wrong™? For starters, our memory and expectations were...incomplete. `RegisterAttribute.ApiSince` is set based on the contents of the Android SDK file `platform-tools/api/api-versions.xml`, which is part of the "platform-tools" package. Turns Out™ that `api-versions.xml` was *not* updated by Google when the platform-tools package was updated to r29. Specifically, the `platform-tools/api/api-versions.xml` file within `platform-tools_r29.0.1-darwin.zip` doesn't contain *any* members from API-29, and `RegisterAttribute.ApiSince` is only emitted when the member is found within `api-versions.xml`. This explains why PR #4012 "caused" the API breakage: The platform-tools r29.0.5 package *does* contain an `api-versions.xml` which contains API-29 members; thus updating platform-tools "caused" the emission of the `RegisterAttribute.ApiSince` property values. Which is a long-winded way of saying that *we cannot trust* the `api-versions.xml` file within the platform-tools package. With that trust lost, what do we use instead? There is another set of `api-version.xml` files: starting with API-26, there are `platforms/android-*/data/api-versions.xml` files. Furthermore, the `api-versions.xml` file included with API-Q -- the preview for API-29 -- *did* contain the members added in API-29, and *did* indicate that those members were added in API-29. As such, we do consider this source of data to be trustworthy (this week, anyway). Update `Mono.Android.targets` to use `$(AndroidSdkDirectory)\platforms\android-$(AndroidApiLevel)\data\api-versions.xml` for `RegisterAttribute.ApiSince` information, if it exists. If it doesn't exist, we fallback and use the previously used platform-tools copy of `api-versions.xml`. Finally, two related changes: 1. Update to use `Microsoft.DotNet.GenAPI` and `Microsoft.DotNet.ApiCompat` package versions of 5.0.0-beta.20078.1, up from 5.0.0-beta.20078.1. This was done as it supposedly contains performance improvements. 2. Improve the mono crash detection from 5b1422c. With 5b1422c, sometimes there would be "odd behavior" when re-using the same `Process` object after the crash was detected. The `Process` usage recommendation is to use a new `Process` object for each process invocation; implement this recommendation.
|
With the merging of #4186, this PR (hopefully) will no longer trigger API compat breakage. Let's find out! |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
I think I can now I can remove the |
|
Ah ha. 4cd2060 contains identical changes to |
Context: dotnet@c523107 Context: https://dev.azure.com/devdiv/DevDiv/_workitems/edit/1032105 As described in [c523107][0], we should be using the latest versions of the Android tooling if possible, matching the major version with the max API level we support. Xamarin.Android now supports Android 10 (API level 29), so we should be using the 29.x.x versions of the tooling. Update `$(AndroidSdkBuildToolsVersion)` and `$(XABuildToolsVersion)` to 29.0.2. Update `$(AndroidSdkPlatformToolsVersion)` and `$(XAPlatformToolsVersion)` to 29.0.5. [0]: dotnet@c523107
brendanzagaeski
force-pushed
the
android-10-tools
branch
from
d3e481d to
37b868a
Compare
|
Changes in the latest force-push: Rebased on master and squashed the commits, removing the API compatibility adjustments along the way since those are now already in master. |
|
@brendanzagaeski how is this PR going? Looks like we might need to get it into d16-5? |
|
I think this PR is all set to merge to master. The CI test failures are unrelated to this PR specifically. So for the question of merging to master, I think this is ready to go. Thoughts on backporting: On the one hand, backporting this PR would address some oddness in Visual Studio 2019 version 16.5 Preview where the automatic SDK installation feature currently installs Android SDK Build-Tools version 29.0.2 even though it isn't used. And Build-Tools versions usually have good backwards compatibility, so using version 29.0.2 would in theory be fine for all of the current Android 9 Pie (API level 28) templates and users' existing projects. On the other hand, as long as users don't manually adjust the installed Build-Tools versions after accepting the auto SDK prompt, the experience is fairly smooth. And there could be some risk in changing the default Build-Tools at this point in the Xamarin.Android 10.2 schedule. If we do decide to backport, the Visual Studio Installer should probably also be adjusted to install Build-Tools 29.0.2 instead of 28.0.3. Separate from whatever we decide on this PR, for the next anticipated Build-Tools update (30.0), I think it might be worth updating the default Background info: In Visual Studio 2019 version 16.5 Preview 2 (and also the latest internal preview):
Overall, the current first-run user experience does work, but it's a little odd. In particular, it's odd that auto SDK asks to install Build-Tools version 29.0.2 when that version won't be used. I haven't studied the Visual Studio Tools for Xamarin (On a side note, with all of the various tools Xamarin.Android now has in its own installer, it's interesting how little of Android SDK Build-Tools Xamarin.Android uses for projects that are configured to use AAPT2, D8, and the Android App Bundle publishing format. I think it might only use |
Context: #4012 (comment) Context: #4012 (comment) Context: dotnet/java-interop@005e273 Context: a348617 Changes: dotnet/java-interop@377c4c7...186174c * dotnet/java-interop@186174c: [generator] Use //*/@api-since for RegisterAttribute.ApiSince (#644) * dotnet/java-interop@010161d: [generator] slnf file should use relative path. (#641) * dotnet/java-interop@942f787: [CI] Add a .NET Core Windows lane. (#640) When `generator --apiversions=FILE` is used, then the `RegisterAttribute.ApiSince` field will be set to contain the API version which introduced a method, e.g. when building `src/Mono.Android`: $ generator.exe --apiversions=$HOME/android-toolchain/sdk/platform-tools/api/api-versions.xml … and `$HOME/android-toolchain/sdk/platform-tools/api/api-versions.xml` contains: <class name="android/view/inspector/IntFlagMapping" since="29"> <extends name="java/lang/Object"/> then `generator` will emit: [Register ("android/view/inspector/IntFlagMapping", DoNotGenerateAcw=true, ApiSince=29)] partial class IntFlagMapping {} Unfortunately, we have found that Google's support of this file isn't guaranteed. Sometimes it is missing, other times it changes in unexpected ways; either can causes `ApiCompat` breakage when building `Mono.Android`, resulting in the removal or modification of the `RegisterAttribute.ApiSince` values. In short, we have lost faith in `api-versions.xml`. dotnet/java-interop@186174c updates `generator` so that *instead of* using an `api-versions.xml` file to provide `RegisterAttribute.ApiSince` values, those values can instead come from `//*/@api-since` attributes. The `//*/@api-since` attributes in turn can come from `src/Mono.Android/metadata`, based on the `//*/@merge.SourceFile` attribute values that `build-tools/api-merge` emits (a073d99). This data is computed directly from the `android.jar` files, and should always be updated and (reasonably) correct. ~~ Notable differences ~~ Our current method provides data for API levels that we do not use for `api-merge`, such as 2, 4, 11, and 14. However we feel it is ok to "lose" this data, as it isn't really useful to our users. That is, if the minimum Xamarin.Android can target is API-21 (a648981), it is not useful to know that an API was added in API-9, since there is no way to target an API level where the member is not available. As such, we are not collecting data for API levels 21 or below, and all members added in any of those levels are considered to "always" have been available, with no `RegisterAttribute.ApiSince` value. ~~ ApiCompat ~~ This change creates a significant amount of API "breakages" where `RegisterAttribute.ApiSince` is either getting added or removed. Rather than add an additional 25K lines to `acceptable-breakages`, we are updating the reference assembly. [Elsewhere][0] is a list of the `ApiSince` info that changed as a result, for reference. There are no non-`ApiSince` changes. * ~12.3K members previously had no `ApiSince` and now have `ApiSince=22-29`. * ~12.1K members previously had `ApiSince=1-21` and now have no `ApiSince` value. * ~200 related fields changed from `28` to `29`, this seems to be correct from looking at the source files. A snippet of the file: System.String Android.Service.Carrier.CarrierMessagingService.ServiceInterface - 0 => 22 Android.Service.Carrier.CarrierMessagingService..ctor() - 0 => 22 Android.Service.Carrier.CarrierMessagingService.OnBind(Android.Content.Intent) - 0 => 22 Android.Service.Carrier.CarrierMessagingService.OnDownloadMms(Android.Net.Uri, System.Int32, Android.Net.Uri, Android.Service.Carrier.CarrierMessagingService.IResultCallback) - 0 => 22 … Finally, ignore the attribute `T:System.Diagnostics.DebuggerStepThroughAttribute`, which seems to be generated in some versions of Roslyn when using an `async Task` method. My local compiler (VS2019 16.5) generates it but apparently the version on the Mac agents does not. [0]: https://github.com/xamarin/xamarin-android/files/4616691/api-compat-parsed.txt
Context: #4012 (comment) Context: #4012 (comment) Context: dotnet/java-interop@005e273 Context: a348617 Changes: dotnet/java-interop@e599781...d6024f1 * dotnet/java-interop@d6024f1: [generator] Use //*/@api-since for RegisterAttribute.ApiSince (#644) * dotnet/java-interop@3968236: [generator] slnf file should use relative path. (#641) * dotnet/java-interop@67b6f70: [CI] Add a .NET Core Windows lane. (#640) When `generator --apiversions=FILE` is used, then the `RegisterAttribute.ApiSince` field will be set to contain the API version which introduced a method, e.g. when building `src/Mono.Android`: $ generator.exe --apiversions=$HOME/android-toolchain/sdk/platform-tools/api/api-versions.xml … and `$HOME/android-toolchain/sdk/platform-tools/api/api-versions.xml` contains: <class name="android/view/inspector/IntFlagMapping" since="29"> <extends name="java/lang/Object"/> then `generator` will emit: [Register ("android/view/inspector/IntFlagMapping", DoNotGenerateAcw=true, ApiSince=29)] partial class IntFlagMapping {} Unfortunately, we have found that Google's support of this file isn't guaranteed. Sometimes it is missing, other times it changes in unexpected ways; either can causes `ApiCompat` breakage when building `Mono.Android`, resulting in the removal or modification of the `RegisterAttribute.ApiSince` values. In short, we have lost faith in `api-versions.xml`. dotnet/java-interop@186174c updates `generator` so that *instead of* using an `api-versions.xml` file to provide `RegisterAttribute.ApiSince` values, those values can instead come from `//*/@api-since` attributes. The `//*/@api-since` attributes in turn can come from `src/Mono.Android/metadata`, based on the `//*/@merge.SourceFile` attribute values that `build-tools/api-merge` emits (a073d99). This data is computed directly from the `android.jar` files, and should always be updated and (reasonably) correct. ~~ Notable differences ~~ Our current method provides data for API levels that we do not use for `api-merge`, such as 2, 4, 11, and 14. However we feel it is ok to "lose" this data, as it isn't really useful to our users. That is, if the minimum Xamarin.Android can target is API-21 (a648981), it is not useful to know that an API was added in API-9, since there is no way to target an API level where the member is not available. As such, we are not collecting data for API levels 21 or below, and all members added in any of those levels are considered to "always" have been available, with no `RegisterAttribute.ApiSince` value. ~~ ApiCompat ~~ This change creates a significant amount of API "breakages" where `RegisterAttribute.ApiSince` is either getting added or removed. Rather than add an additional 25K lines to `acceptable-breakages`, we are updating the reference assembly. [Elsewhere][0] is a list of the `ApiSince` info that changed as a result, for reference. There are no non-`ApiSince` changes. * ~12.3K members previously had no `ApiSince` and now have `ApiSince=22-29`. * ~12.1K members previously had `ApiSince=1-21` and now have no `ApiSince` value. * ~200 related fields changed from `28` to `29`, this seems to be correct from looking at the source files. A snippet of the file: System.String Android.Service.Carrier.CarrierMessagingService.ServiceInterface - 0 => 22 Android.Service.Carrier.CarrierMessagingService..ctor() - 0 => 22 Android.Service.Carrier.CarrierMessagingService.OnBind(Android.Content.Intent) - 0 => 22 Android.Service.Carrier.CarrierMessagingService.OnDownloadMms(Android.Net.Uri, System.Int32, Android.Net.Uri, Android.Service.Carrier.CarrierMessagingService.IResultCallback) - 0 => 22 … Finally, ignore the attribute `T:System.Diagnostics.DebuggerStepThroughAttribute`, which seems to be generated in some versions of Roslyn when using an `async Task` method. My local compiler (VS2019 16.5) generates it but apparently the version on the Mac agents does not. [0]: https://github.com/xamarin/xamarin-android/files/4616691/api-compat-parsed.txt
6 participants
Context: c523107
Context: https://dev.azure.com/devdiv/DevDiv/_workitems/edit/1032105
As described in c5231075, we should be using the latest versions of
the Android tooling if possible, matching the major version with the max
API level we support. Xamarin.Android now supports Android 10 (API
level 29), so we should be using the 29.x.x versions of the tooling.
Update
$(AndroidSdkBuildToolsVersion)to 29.0.2.Update
$(AndroidSdkPlatformToolsVersion)to 29.0.5.