-
Notifications
You must be signed in to change notification settings - Fork 63
[generator] Remove androidx.annotation from Annotation names #882
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
Merged
jonpryor
merged 1 commit into
dotnet:main
from
jonpryor:jonp-lax-annotation-name-matches
Sep 16, 2021
Merged
[generator] Remove androidx.annotation from Annotation names #882
jonpryor
merged 1 commit into
dotnet:main
from
jonpryor:jonp-lax-annotation-name-matches
Sep 16, 2021
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Context: dotnet/android#6300 A confluence of several "funny" things happened with dotnet/android#6300, which attempts to use the Android SDK platform-tools 31.0.3 package, up from 30.0.2: 1. The new platform-tools package *removes* the file `platform-tools/api/annotations.zip`. To work around that, we need to instead use the `data/annotations.zip` file from the Android platform directory, e.g. `$ANDROID_SDK_ROOT/platforms/android-30/data/annotations.zip`. 2. Between API-28 and API-29, `annotations.zip` changes the package name used for annotations, from e.g. `android.support.annotation.RequiresPermission` to `androidx.annotation.RequiresPermission`. In isolation, okay, but then we hit: 3. `AnnotationData` *removes* the "known" package-prefix of `android.support.annotation.`, and all use of `AnnotationData.Name` is via `string.operator==`, *not* `string.Contains()` or `string.IndexOf()`. The result of all these changes together is API breakage in `Mono.Android.dll` between API-28 and API-29: …/Mono.Android.targets(257,5): error : CannotRemoveAttribute : Attribute 'Android.Runtime.RequiresPermissionAttribute' exists on 'Android.Accounts.AccountManager.AddAccount(System.String, System.String, System.String[], Android.OS.Bundle, Android.App.Activity, Android.Accounts.IAccountManagerCallback, Android.OS.Handler)' in the contract but not the implementation. Or, in C# `diff -u` terms: --- obj/Debug/monoandroid10/android-28/mcw/Android.Accounts.AccountManager.cs 2021-09-16 09:00:53.000000000 -0400 +++ obj/Debug/monoandroid10/android-29/mcw/Android.Accounts.AccountManager.cs 2021-09-16 10:41:15.000000000 -0400 @@ -217,7 +217,6 @@ // Metadata.xml XPath method reference: path="/api/package[@name='android.accounts']/class[@name='AccountManager']/method[@name='addAccount' and count(parameter)=7 and parameter[1][@type='java.lang.String'] and parameter[2][@type='java.lang.String'] and parameter[3][@type='java.lang.String[]'] and parameter[4][@type='android.os.Bundle'] and parameter[5][@type='android.app.Activity'] and parameter[6][@type='android.accounts.AccountManagerCallback<android.os.Bundle>'] and parameter[7][@type='android.os.Handler']]" [Register ("addAccount", "(Ljava/lang/String;Ljava/lang/String;[Ljava/lang/String;Landroid/os/Bundle;Landroid/app/Activity;Landroid/accounts/AccountManagerCallback;Landroid/os/Handler;)Landroid/accounts/AccountManagerFuture;", "GetAddAccount_Ljava_lang_String_Ljava_lang_String_arrayLjava_lang_String_Landroid_os_Bundle_Landroid_app_Activity_Landroid_accounts_AccountManagerCallback_Landroid_os_Handler_Handler")] - [global::Android.Runtime.RequiresPermission ("android.permission.MANAGE_ACCOUNTS")] public virtual unsafe Android.Accounts.IAccountManagerFuture? AddAccount (string? accountType, string? authTokenType, string[]? requiredFeatures, Android.OS.Bundle? addAccountOptions, Android.App.Activity? activity, Android.Accounts.IAccountManagerCallback? @callback, Android.OS.Handler? handler) { const string __id = "addAccount.(Ljava/lang/String;Ljava/lang/String;[Ljava/lang/String;Landroid/os/Bundle;Landroid/app/Activity;Landroid/accounts/AccountManagerCallback;Landroid/os/Handler;)Landroid/accounts/AccountManagerFuture;"; Fix this breakage by updating `AnnotationData` to remove the package name for `androidx.annotation` as well. Additionally, add an `AnnotatedItem.ToString()` override to simplify future printf-style debugging.
jpobst
approved these changes
Sep 16, 2021
jonpryor
added a commit
to jonpryor/java.interop
that referenced
this pull request
Sep 23, 2021
…otnet#882)" This reverts commit 1e8f513. Context: dotnet#885 Testing?
jonpryor
added a commit
that referenced
this pull request
Sep 23, 2021
…882)" Partially revert commit 1e8f513. Context: #885 The problem with commit 1e8f513 is that the change to the `AnnotationData` constructor broke the xamarin-android build, introducing API Compatibility errors in `Mono.Android.dll`: …/src/Mono.Android/Mono.Android.targets(247,5): error : CompatApi command: $HOME/.nuget/packages/microsoft.dotnet.apicompat/5.0.0-beta.20181.7/tools/net472/Microsoft.DotNet.ApiCompat.exe "…/bin/Debug/lib/xamarin.android/xbuild-frameworks/MonoAndroid/v4.4.87/Mono.Android.dll" -i "…/bin/Debug/lib/xamarin.android/xbuild-frameworks/MonoAndroid/v5.0" --allow-default-interface-methods --baseline "…/tests/api-compatibility/acceptable-breakages-v5.0.txt" --validate-baseline --exclude-attributes "…/tests/api-compatibility/api-compat-exclude-attributes.txt" …/src/Mono.Android/Mono.Android.targets(247,5): error : CheckApiCompatibility found nonacceptable Api breakages for ApiLevel: v5.0. …/src/Mono.Android/Mono.Android.targets(247,5): error : Compat issues with assembly Mono.Android: …/src/Mono.Android/Mono.Android.targets(247,5): error : CannotChangeAttribute : Attribute 'Android.Runtime.RequiresPermissionAttribute' on 'Android.Net.ConnectivityManager.GetNetworkInfo(Android.Net.ConnectivityType)' changed from '[RequiresPermissionAttribute("quot;android.permission.ACCESS_NETWORK_STATE"")]' in the contract to '[RequiresPermissionAttribute("quot;android.permission.ACCESS_NETWORK_STATE"")]' in the implementation. …/src/Mono.Android/Mono.Android.targets(247,5): error : CannotChangeAttribute : Attribute 'Android.Runtime.RequiresPermissionAttribute' on 'Android.OS.Vibrator.Vibrate(System.Int64[], System.Int32)' changed from '[RequiresPermissionAttribute("quot;android.permission.VIBRATE"")]' in the contract to '[RequiresPermissionAttribute("quot;android.permission.VIBRATE"")]' in the implementation. …/src/Mono.Android/Mono.Android.targets(247,5): error : Total Issues: 2 See Issue #885 for more details and discussion. TL;DR: 1e8f513 allowed `androidx.annotation.RequiresPermission` to be surfaced as `[Android.Runtime.RequiresPermission]` custom attributes, and in doing so surfaced a pre-existing issue wherein the same `[RequiresPermission]` could be present multiple times with the same constructor values, which typically isn't desirable: namespace Android.OS { public abstract partial class Vibrator : Java.Lang.Object { [global::Android.Runtime.RequiresPermission ("quot;android.permission.VIBRATE"")] [global::Android.Runtime.RequiresPermission ("quot;android.permission.VIBRATE"")] public abstract void Vibrate (long milliseconds); } } How and why this is happening isn't currently understood. In the meantime, revert the `AnnotationData` changes in commit 1e8f513 so that we can bump Java.Interop within xamarin-android.
jpobst
pushed a commit
that referenced
this pull request
Sep 30, 2021
Context: dotnet/android#6300 A confluence of several "funny" things happened with dotnet/android#6300, which attempts to use the Android SDK platform-tools 31.0.3 package, up from 30.0.2: 1. The new platform-tools package *removes* the file `platform-tools/api/annotations.zip`. To work around that, we need to instead use the `data/annotations.zip` file from the Android platform directory, e.g. `$ANDROID_SDK_ROOT/platforms/android-30/data/annotations.zip`. 2. Between API-28 and API-29, `annotations.zip` changes the package name used for annotations, from e.g. `android.support.annotation.RequiresPermission` to `androidx.annotation.RequiresPermission`. In isolation, okay, but then we hit: 3. `AnnotationData` *removes* the "known" package-prefix of `android.support.annotation.`, and all use of `AnnotationData.Name` is via `string.operator==`, *not* `string.Contains()` or `string.IndexOf()`. The result of all these changes together is API breakage in `Mono.Android.dll` between API-28 and API-29: …/Mono.Android.targets(257,5): error : CannotRemoveAttribute : Attribute 'Android.Runtime.RequiresPermissionAttribute' exists on 'Android.Accounts.AccountManager.AddAccount(System.String, System.String, System.String[], Android.OS.Bundle, Android.App.Activity, Android.Accounts.IAccountManagerCallback, Android.OS.Handler)' in the contract but not the implementation. Or, in C# `diff -u` terms: --- obj/Debug/monoandroid10/android-28/mcw/Android.Accounts.AccountManager.cs 2021-09-16 09:00:53.000000000 -0400 +++ obj/Debug/monoandroid10/android-29/mcw/Android.Accounts.AccountManager.cs 2021-09-16 10:41:15.000000000 -0400 @@ -217,7 +217,6 @@ // Metadata.xml XPath method reference: path="/api/package[@name='android.accounts']/class[@name='AccountManager']/method[@name='addAccount' and count(parameter)=7 and parameter[1][@type='java.lang.String'] and parameter[2][@type='java.lang.String'] and parameter[3][@type='java.lang.String[]'] and parameter[4][@type='android.os.Bundle'] and parameter[5][@type='android.app.Activity'] and parameter[6][@type='android.accounts.AccountManagerCallback<android.os.Bundle>'] and parameter[7][@type='android.os.Handler']]" [Register ("addAccount", "(Ljava/lang/String;Ljava/lang/String;[Ljava/lang/String;Landroid/os/Bundle;Landroid/app/Activity;Landroid/accounts/AccountManagerCallback;Landroid/os/Handler;)Landroid/accounts/AccountManagerFuture;", "GetAddAccount_Ljava_lang_String_Ljava_lang_String_arrayLjava_lang_String_Landroid_os_Bundle_Landroid_app_Activity_Landroid_accounts_AccountManagerCallback_Landroid_os_Handler_Handler")] - [global::Android.Runtime.RequiresPermission ("android.permission.MANAGE_ACCOUNTS")] public virtual unsafe Android.Accounts.IAccountManagerFuture? AddAccount (string? accountType, string? authTokenType, string[]? requiredFeatures, Android.OS.Bundle? addAccountOptions, Android.App.Activity? activity, Android.Accounts.IAccountManagerCallback? @callback, Android.OS.Handler? handler) { const string __id = "addAccount.(Ljava/lang/String;Ljava/lang/String;[Ljava/lang/String;Landroid/os/Bundle;Landroid/app/Activity;Landroid/accounts/AccountManagerCallback;Landroid/os/Handler;)Landroid/accounts/AccountManagerFuture;"; Fix this breakage by updating `AnnotationData` to remove the package name for `androidx.annotation` as well. Additionally, add an `AnnotatedItem.ToString()` override to simplify future printf-style debugging.
jpobst
pushed a commit
that referenced
this pull request
Sep 30, 2021
…882)" Partially revert commit 1e8f513. Context: #885 The problem with commit 1e8f513 is that the change to the `AnnotationData` constructor broke the xamarin-android build, introducing API Compatibility errors in `Mono.Android.dll`: …/src/Mono.Android/Mono.Android.targets(247,5): error : CompatApi command: $HOME/.nuget/packages/microsoft.dotnet.apicompat/5.0.0-beta.20181.7/tools/net472/Microsoft.DotNet.ApiCompat.exe "…/bin/Debug/lib/xamarin.android/xbuild-frameworks/MonoAndroid/v4.4.87/Mono.Android.dll" -i "…/bin/Debug/lib/xamarin.android/xbuild-frameworks/MonoAndroid/v5.0" --allow-default-interface-methods --baseline "…/tests/api-compatibility/acceptable-breakages-v5.0.txt" --validate-baseline --exclude-attributes "…/tests/api-compatibility/api-compat-exclude-attributes.txt" …/src/Mono.Android/Mono.Android.targets(247,5): error : CheckApiCompatibility found nonacceptable Api breakages for ApiLevel: v5.0. …/src/Mono.Android/Mono.Android.targets(247,5): error : Compat issues with assembly Mono.Android: …/src/Mono.Android/Mono.Android.targets(247,5): error : CannotChangeAttribute : Attribute 'Android.Runtime.RequiresPermissionAttribute' on 'Android.Net.ConnectivityManager.GetNetworkInfo(Android.Net.ConnectivityType)' changed from '[RequiresPermissionAttribute("quot;android.permission.ACCESS_NETWORK_STATE"")]' in the contract to '[RequiresPermissionAttribute("quot;android.permission.ACCESS_NETWORK_STATE"")]' in the implementation. …/src/Mono.Android/Mono.Android.targets(247,5): error : CannotChangeAttribute : Attribute 'Android.Runtime.RequiresPermissionAttribute' on 'Android.OS.Vibrator.Vibrate(System.Int64[], System.Int32)' changed from '[RequiresPermissionAttribute("quot;android.permission.VIBRATE"")]' in the contract to '[RequiresPermissionAttribute("quot;android.permission.VIBRATE"")]' in the implementation. …/src/Mono.Android/Mono.Android.targets(247,5): error : Total Issues: 2 See Issue #885 for more details and discussion. TL;DR: 1e8f513 allowed `androidx.annotation.RequiresPermission` to be surfaced as `[Android.Runtime.RequiresPermission]` custom attributes, and in doing so surfaced a pre-existing issue wherein the same `[RequiresPermission]` could be present multiple times with the same constructor values, which typically isn't desirable: namespace Android.OS { public abstract partial class Vibrator : Java.Lang.Object { [global::Android.Runtime.RequiresPermission ("quot;android.permission.VIBRATE"")] [global::Android.Runtime.RequiresPermission ("quot;android.permission.VIBRATE"")] public abstract void Vibrate (long milliseconds); } } How and why this is happening isn't currently understood. In the meantime, revert the `AnnotationData` changes in commit 1e8f513 so that we can bump Java.Interop within xamarin-android.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Context: dotnet/android#6300
A confluence of several "funny" things happened with
dotnet/android#6300, which attempts to use the Android SDK
platform-tools 31.0.3 package, up from 30.0.2:
The new platform-tools package removes the file
platform-tools/api/annotations.zip
. To work around that, weneed to instead use the
data/annotations.zip
file from theAndroid platform directory, e.g.
$ANDROID_SDK_ROOT/platforms/android-30/data/annotations.zip
.Between API-28 and API-29,
annotations.zip
changes the packagename used for annotations, from e.g.
android.support.annotation.RequiresPermission
toandroidx.annotation.RequiresPermission
.In isolation, okay, but then we hit:
AnnotationData
removes the "known" package-prefix ofandroid.support.annotation.
, and all use ofAnnotationData.Name
is viastring.operator==
, notstring.Contains()
orstring.IndexOf()
.The result of all these changes together is API breakage in
Mono.Android.dll
between API-28 and API-29:Or, in C#
diff -u
terms:Fix this breakage by updating
AnnotationData
to remove the packagename for
androidx.annotation
as well.Additionally, add an
AnnotatedItem.ToString()
override to simplifyfuture printf-style debugging.