-
Notifications
You must be signed in to change notification settings - Fork 64
[generator] add [IntDefinition (JniField = ...)] everywhere. #127
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
[generator] add [IntDefinition (JniField = ...)] everywhere. #127
Conversation
|
Unit test failure. You likely need to add a |
de7fadb to
6aebca5
Compare
| namespace Android.Runtime | ||
| { | ||
| [AttributeUsage (AttributeTargets.Field)] | ||
| public class IntDefinitionAttribute : Attribute |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems odd; why do you need this type in both expected and expected.cp? The version in SupportFiles should be enough, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought adding those fixed the tests? Maybe it was fixed rather by cleaning up files. Anyways they are removed now.
6aebca5 to
282472e
Compare
| @@ -1,5 +1,6 @@ | |||
| namespace Android.Text { | |||
| public enum SpanTypes { | |||
| [global::Android.Runtime.IntDefinition (null, JniField = "I:android/text/Spanned.SPAN_COMPOSING")] | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm still not clear on why some of these values have a type prefix -- I: -- and some don't.
Perhaps the type should be separated from the member name, for simplicity?
[IntDefinition (JniSourceFieldType="I", JniSourceDeclaringType="android/text/Spanned", JniSourceFieldMember="SPAN_COMPOSING")]
Perhaps the type+member can be combined with a . separator, but I'm not sure we should have <jni-type>: as an optional prefix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WTF. Those "I:" are actually from map.csv to indicate that the type is an interface (not a class), which should not have been required from the beginning. I made a change that kills those "I:" prefixes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yay, I found a bug! :-D
It will be helpful when people read binding dlls and understand the enum conversion, as well as tooling.
282472e to
b12575a
Compare
Context: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1314263 Context: c936d09 Changes: dotnet/android-tools@d92fc3e...34e98e2 * dotnet/android-tools@34e98e2: [build] Allow Assembly "vendorization" (#136) * dotnet/android-tools@061bcc2: [build] Import "parent directory.override.targets" (#135) * 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) * dotnet/android-tools@9b658b2: Merge pull request #133 from xamarin/ndk-r23 * dotnet/android-tools@ff73f92: [build] Use GitInfo to generate $(Version) (#131) * dotnet/android-tools@4c2e36c: [Xamarin.Android.Tools.AndroidSdk] Eclipse Adoptium support (#132) * dotnet/android-tools@eaec4e3: [Xamarin.Android.Tools.AndroidSdk] More Microsoft Dist JDK Support (#130) * dotnet/android-tools@f9c1b0d: [BaseTasks] improve Task settings in AsyncTaskExtensions (#129) * dotnet/android-tools@efc9b67: Merge pull request #128 from dellis1972/bumpitybump * dotnet/android-tools@40adec0: Bump LibZipSharp and Mono.Unix to latest stable versions, * dotnet/android-tools@87acd6b: [Microsoft.Android.Build.BaseTasks] add StrongName (#127) * dotnet/android-tools@02f7ae7: [NDK] Properly detect 64-bit NDK * dotnet/android-tools@623332d: Bump LibZipSharp to 2.0.0-alpha7 (#126) * dotnet/android-tools@c055fa8: [Microsoft.Android.Build] Bump to MSBuild 16.10.0 (#125) * dotnet/android-tools@49936d6: Merge pull request #124 from xamarin/update-libzipsharp * dotnet/android-tools@ef78dfc: Bump LibZipSharp to 2.0.0-alpha6 * dotnet/android-tools@e3d708c: [BaseTasks] fix `\`-delimited paths on macOS (#122) * dotnet/android-tools@bdcf899: Reference the new Mono.Unix nuget (#123) * dotnet/android-tools@90d7621: [BaseTasks] add ABI detection for RIDs (#121) * dotnet/android-tools@79e3b97: [JdkInfo] handle invalid XML from /usr/libexec/java_home (#120) * dotnet/android-tools@81519fe: Add SECURITY.md (#119) * dotnet/android-tools@683f375: [Microsoft.Android.Build] Use MSBuild NuGets (#118) * dotnet/android-tools@2241489: [Xamarin.Android.Tools.AndroidSdk] Support Microsoft Dist JDK (#117) * dotnet/android-tools@52ef989: [Xamarin.Android.Tools.AndroidSdk] Fix CS0168 warning (#116) Moving the Roslyn Analyzers initialization code from `Directory.Build.props` to `Directory.Build.targets` (c936d09) caused the Roslyn Analyzers to be applied to code imported from `external/xamarin-android-tools`. Previously the xamarin-android-tools code was unaffected because it had its own `Directory.Build.props` file, so it did not traverse the file system upwards any further to find the Java.Interop files. Fixes for Roslyn issues has not been applied to `xamarin-android-tools` code, so it adds ~150 warnings to the Java.Interop build. dotnet/android-tools@ff73f925 added a `Directory.Build.targets`, so updating `external/xamarin-android-tools` to the latest xamarin-android-tools commit will prevent Java.Interop's `Directory.Build.targets` file from being used, which will exclude xamarin-android-tools from the Roslyn Analyzers.
DO NOT MERGE THIS WITHOUT MERGING dotnet/android#444, it depends on Mono.Android.dll updates.
It will be helpful when people read binding dlls and understand the enum
conversion, as well as tooling.