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

Bump to xamarin/xamarin-android-tools/main@a5194e93 #6314

Merged
merged 1 commit into from
Sep 20, 2021

Conversation

jonpryor
Copy link
Member

Context: #6300
Context: dotnet/java-interop#883

Changes: http://github.com/xamarin/xamarin-android-tools/compare/9b658b29bd41157151f5515619d0d90dc062563d...a5194e93498e7f12225d87e2811415a45f742116

Note: while this xamarin-android-tools bump updates the Android SDK
Platform Tools version to 31.0.3, we are not updating
$(XAPlatformToolsVersion) to 31.0.3. As such, there will be a
mismatch between what we build xamarin-android against, vs. the
default suggested platform-tools package potentially installed
via the /t:InstallAndroidDependencies target.

The reason to not bump $(XAPlatformToolsVersion) is that
attempting to do so breaks the src/Mono.Android build:

  1. platform-tools r31.0.3 doesn't contain
    platform-tools/api/annotations.zip, which is used by
    generator to emit certain custom attributes such as
    RequiresPermission.

  2. While (1) can be worked around by instead using
    $(AndroidSdkDirectory)/platforms/android-*/data/annotations.zip,
    this introduces API changes reported by the
    _CheckApiCompatibility target, in particular changes due to
    custom attribute string changes. These string changes happen
    because Google ships invalid XML in data/annotations.zip for
    API-29+ (?!):

    <!-- annotations.zip!android/accounts/annotations.xml -->
    -  <item name="android.accounts.AccountManager android.accounts.AccountManagerFuture&lt;android.os.Bundle&gt; addAccount(java.lang.String, java.lang.String, java.lang.String[], android.os.Bundle, android.app.Activity, android.accounts.AccountManagerCallback&lt;android.os.Bundle&gt;, android.os.Handler)">
    +  <item name="android.accounts.AccountManager android.accounts.AccountManagerFuture&lt;android.os.Bundle&gt; addAccount(java.lang.String, java.lang.String, java.lang.String[], android.os.Bundle, android.app.Activity, android.accounts.AccountManagerCallback<android.os.Bundle>, android.os.Handler)">
    

    AccountManagerCallback<android.os.Bundle> is not a valid value
    within an XML attribute.

    This change doesn't break the build, but instead causes
    AndroidAnnotationSupport to hit a "fallback" codepath
    which uses HtmlAgilityPack, and this causes attribute values
    to "gain" quot;:

    -[global::Android.Runtime.RequiresPermission ("android.permission.MANAGE_ACCOUNTS")]
    +[global::Android.Runtime.RequiresPermission ("quot;android.permission.MANAGE_ACCOUNTS&quot")]
    

    See also: AndroidAnnotationSupport needs better invalid XML fixup java-interop#883

After manual review, no other file removals appear to be problematic,
so @jonpryor asserts that it should be acceptable for
$(XAPlatformToolsVersion) and $(AndroidSdkPlatformToolsVersion)
to be inconsistent with each other.

Context: dotnet#6300
Context: dotnet/java-interop#883

Changes: http://github.com/xamarin/xamarin-android-tools/compare/9b658b29bd41157151f5515619d0d90dc062563d...a5194e93498e7f12225d87e2811415a45f742116

  * dotnet/android-tools@a5194e9: [Xamarin.Android.Tools.AndroidSdk] Downgrade build-tools to API-30
  * dotnet/android-tools@440e6be: [Xamarin.Android.Tools.AndroidSdk] Update SDK component for API-31 (dotnet#134)

Note: while this xamarin-android-tools bump updates the Android SDK
[Platform Tools][0] version to 31.0.3, we are *not* updating
`$(XAPlatformToolsVersion)` to 31.0.3.  As such, there will be a
mismatch between what we build xamarin-android against, vs. the
default suggested platform-tools package potentially installed
via the `/t:InstallAndroidDependencies` target.

The reason to *not* bump `$(XAPlatformToolsVersion)` is that
attempting to do so breaks the `src/Mono.Android` build:

 1. platform-tools r31.0.3 doesn't contain
    `platform-tools/api/annotations.zip`, which is used by
    `generator` to emit certain custom attributes such as
    `RequiresPermission`.

 2. While (1) can be worked around by instead using
    `$(AndroidSdkDirectory)/platforms/android-*/data/annotations.zip`,
    this introduces API changes reported by the
    `_CheckApiCompatibility` target, in particular changes due to
    custom attribute string changes.  These string changes happen
    because Google ships *invalid XML* in `data/annotations.zip` for
    API-29+ (?!):

        <!-- annotations.zip!android/accounts/annotations.xml -->
        -  <item name="android.accounts.AccountManager android.accounts.AccountManagerFuture&lt;android.os.Bundle&gt; addAccount(java.lang.String, java.lang.String, java.lang.String[], android.os.Bundle, android.app.Activity, android.accounts.AccountManagerCallback&lt;android.os.Bundle&gt;, android.os.Handler)">
        +  <item name="android.accounts.AccountManager android.accounts.AccountManagerFuture&lt;android.os.Bundle&gt; addAccount(java.lang.String, java.lang.String, java.lang.String[], android.os.Bundle, android.app.Activity, android.accounts.AccountManagerCallback<android.os.Bundle>, android.os.Handler)">

    `AccountManagerCallback<android.os.Bundle>` is not a valid value
    within an XML attribute.

    This change doesn't break the build, but instead causes
    [`AndroidAnnotationSupport`][1] to hit a "fallback" codepath
    which uses HtmlAgilityPack, and this causes attribute values
    to "gain" `quot;`:

        -[global::Android.Runtime.RequiresPermission ("android.permission.MANAGE_ACCOUNTS")]
        +[global::Android.Runtime.RequiresPermission ("quot;android.permission.MANAGE_ACCOUNTS&quot")]

    See also: dotnet/java-interop#883

After manual review, no other file removals appear to be problematic,
so @jonpryor asserts that it should be acceptable for
`$(XAPlatformToolsVersion)` and `$(AndroidSdkPlatformToolsVersion)`
to be inconsistent with each other.

[0]: https://developer.android.com/studio/releases/platform-tools
[1]: https://github.com/xamarin/java.interop/blob/1e8f5137345db3160c99265ff3a56c43a132194f/src/Xamarin.Android.Tools.AnnotationSupport/AndroidAnnotationsSupport.cs#L58-L87
@jonpryor jonpryor merged commit f6011d9 into dotnet:main Sep 20, 2021
jonathanpeppers pushed a commit that referenced this pull request Sep 21, 2021
Context: #6300
Context: dotnet/java-interop#883

Changes: http://github.com/xamarin/xamarin-android-tools/compare/9b658b29bd41157151f5515619d0d90dc062563d...a5194e93498e7f12225d87e2811415a45f742116

  * dotnet/android-tools@a5194e9: [Xamarin.Android.Tools.AndroidSdk] Downgrade build-tools to API-30
  * dotnet/android-tools@440e6be: [Xamarin.Android.Tools.AndroidSdk] Update SDK component for API-31 (#134)

Note: while this xamarin-android-tools bump updates the Android SDK
[Platform Tools][0] version to 31.0.3, we are *not* updating
`$(XAPlatformToolsVersion)` to 31.0.3.  As such, there will be a
mismatch between what we build xamarin-android against, vs. the
default suggested platform-tools package potentially installed
via the `/t:InstallAndroidDependencies` target.

The reason to *not* bump `$(XAPlatformToolsVersion)` is that
attempting to do so breaks the `src/Mono.Android` build:

 1. platform-tools r31.0.3 doesn't contain
    `platform-tools/api/annotations.zip`, which is used by
    `generator` to emit certain custom attributes such as
    `RequiresPermission`.

 2. While (1) can be worked around by instead using
    `$(AndroidSdkDirectory)/platforms/android-*/data/annotations.zip`,
    this introduces API changes reported by the
    `_CheckApiCompatibility` target, in particular changes due to
    custom attribute string changes.  These string changes happen
    because Google ships *invalid XML* in `data/annotations.zip` for
    API-29+ (?!):

        <!-- annotations.zip!android/accounts/annotations.xml -->
        -  <item name="android.accounts.AccountManager android.accounts.AccountManagerFuture&lt;android.os.Bundle&gt; addAccount(java.lang.String, java.lang.String, java.lang.String[], android.os.Bundle, android.app.Activity, android.accounts.AccountManagerCallback&lt;android.os.Bundle&gt;, android.os.Handler)">
        +  <item name="android.accounts.AccountManager android.accounts.AccountManagerFuture&lt;android.os.Bundle&gt; addAccount(java.lang.String, java.lang.String, java.lang.String[], android.os.Bundle, android.app.Activity, android.accounts.AccountManagerCallback<android.os.Bundle>, android.os.Handler)">

    `AccountManagerCallback<android.os.Bundle>` is not a valid value
    within an XML attribute.

    This change doesn't break the build, but instead causes
    [`AndroidAnnotationSupport`][1] to hit a "fallback" codepath
    which uses HtmlAgilityPack, and this causes attribute values
    to "gain" `quot;`:

        -[global::Android.Runtime.RequiresPermission ("android.permission.MANAGE_ACCOUNTS")]
        +[global::Android.Runtime.RequiresPermission ("quot;android.permission.MANAGE_ACCOUNTS&quot")]

    See also: dotnet/java-interop#883

After manual review, no other file removals appear to be problematic,
so @jonpryor asserts that it should be acceptable for
`$(XAPlatformToolsVersion)` and `$(AndroidSdkPlatformToolsVersion)`
to be inconsistent with each other.

[0]: https://developer.android.com/studio/releases/platform-tools
[1]: https://github.com/xamarin/java.interop/blob/1e8f5137345db3160c99265ff3a56c43a132194f/src/Xamarin.Android.Tools.AnnotationSupport/AndroidAnnotationsSupport.cs#L58-L87
@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.

1 participant