-
Notifications
You must be signed in to change notification settings - Fork 564
[Mono.Android] Enumify API-36.1 #10515
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] Enumify API-36.1 #10515
Conversation
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
e3cdc48 to
de6bc19
Compare
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
b1824cc to
d74a6a3
Compare
Context: http://github.com/jpobst/BindingStudio Context: jpobst/BindingStudio#1 Use jpobst/BindingStudio to begin enumifying API-36.1. Note: current dotnet/java-interop emits an "extra" `,` on `map.csv` output, which would make for a "noisy" diff (every line changed!). The diff is reduced by removing trailing commas: sed 's/,$//' < src/Mono.Android/new-map.csv > src/Mono.Android/map.csv This keeps the diff to a reasonable size. Update `build-tools/manifest-attribute-codegen` so that: 1. It can be executed from topdir. 2. Warning message are actually emitted when using `dotnet build`. Previously, only the fact that the executed command failed would be printed, but none of the tool output. TODO: figure out if there are any other TODOs. TODO: Flush current BindingStudio output, with the extra commas.
When attempting to declare API-36.1 as the latest stable API,
my local (Debug) build promptly started *crashing*.
Which was rather unexpected.
The most useful lesson in this context: *do not* call
`Debug.Assert()` or throw exceptions from code which is called from
the Thread Pool. `Debug.Assert()` -- in Debug config builds -- will
immediately exit the process, which in turn means that MSBuild `binlog`
output is *incomplete*, as MSBuild never gets a chance to flush
everything (courtesy of the process aborting!).
Throwing an exception is very similar: unhandled exceptions on the
thread pool cause the process to abort. MSBuild never has a chance
to flush out binlog data.
Plumb logging through to `CodeGenDiff` so that the assertion failure
can instead become an "Internal Error" message. This fixes my crash.
Then there's the matter of the former assertion failure now error
message: why was `currentObject.InternalCounter` negative?
`currentObject.InternalCounter` was negative because of this line:
public static void AddEventHandler<TInterface, TImplementor>([System.Runtime.CompilerServices.NullableAttribute((byte)2)] ref System.WeakReference? implementor, System.Func<TImplementor> creator, System.Action<TInterface> setListener, System.Action<TImplementor> add) where TInterface : class where TImplementor : Java.Lang.Object, TInterface { }
The `where TInterface : class ` was matching this check:
content.IndexOf (" class ", StringComparison.Ordinal)
and thus the method was being interpreted as the introduction of
a new type declaration instead of something to add to the existing
`currentObject` scope. This meant that the new
`currentObject.InternalCounter` was 0, then we hit `}` to decrement
`.InternalCounter`, resulting in a negative value.
Fix this by checking for ` partial class `, as was done previously
with ` partial struct `, as `Microsoft.DotNet.GenAPI.dll` always
emits the `partial` modifier on everything except `enum`s.
This fixes the now internal error message former assertion.
60e50de to
862b483
Compare
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
jonathanpeppers
left a comment
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.
There are some NRT warnings (as errors) on CI:
(CoreCompile target) ->
/Users/builder/azdo/_work/3/s/android/build-tools/Xamarin.Android.Tools.BootstrapTasks/Xamarin.Android.Tools.BootstrapTasks/GenerateSupportedPlatforms.cs(50,7): error CS8602: Dereference of a possibly null reference. [/Users/builder/azdo/_work/3/s/android/build-tools/Xamarin.Android.Tools.BootstrapTasks/Xamarin.Android.Tools.BootstrapTasks.csproj]
/Users/builder/azdo/_work/3/s/android/build-tools/Xamarin.Android.Tools.BootstrapTasks/Xamarin.Android.Tools.BootstrapTasks/GenerateSupportedPlatforms.cs(23,23): error CS8618: Non-nullable property 'AndroidApiInfo' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the property as nullable. [/Users/builder/azdo/_work/3/s/android/build-tools/Xamarin.Android.Tools.BootstrapTasks/Xamarin.Android.Tools.BootstrapTasks.csproj]
/Users/builder/azdo/_work/3/s/android/build-tools/Xamarin.Android.Tools.BootstrapTasks/Xamarin.Android.Tools.BootstrapTasks/GenerateSupportedPlatforms.cs(29,17): error CS8618: Non-nullable property 'OutputFile' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the property as nullable. [/Users/builder/azdo/_work/3/s/android/build-tools/Xamarin.Android.Tools.BootstrapTasks/Xamarin.Android.Tools.BootstrapTasks.csproj]
/Users/builder/azdo/_work/3/s/android/build-tools/Xamarin.Android.Tools.BootstrapTasks/Xamarin.Android.Tools.BootstrapTasks/GenerateSupportedPlatforms.cs(35,17): error CS8618: Non-nullable property 'MinimumApiLevel' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the property as nullable. [/Users/builder/azdo/_work/3/s/android/build-tools/Xamarin.Android.Tools.BootstrapTasks/Xamarin.Android.Tools.BootstrapTasks.csproj]
/Users/builder/azdo/_work/3/s/android/build-tools/Xamarin.Android.Tools.BootstrapTasks/Xamarin.Android.Tools.BootstrapTasks/GenerateSupportedPlatforms.cs(40,17): error CS8618: Non-nullable property 'TargetApiLevel' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the property as nullable. [/Users/builder/azdo/_work/3/s/android/build-tools/Xamarin.Android.Tools.BootstrapTasks/Xamarin.Android.Tools.BootstrapTasks.csproj]
I think you can build with -p:_AndroidTreatWarningsAsErrors=true to see these locally.
Context: dotnet#10005 Normally when we declare a new API level as "stable", it is the *only* API that is built. For API-36.1, this is not the case: we want to continue building API-36, *and also* build API-36.1. Mirror dotnet#10005: * build both API levels by "repurposing" the `$(AndroidLatestUnstable*)` MSBuild properties as an "alternate set" of stable API levels. * What "really" indicates a `Mono.Android.dll` is unstable is the presence of `ANDROID_UNSTABLE` when building it. This is conditional on `$(IsUnstableVersion)`. Update `$(IsUnstableVersion)` so that it uses the [MSBuild version-comparison functions][0] to compare `$(AndroidApiLevel)` against `$(AndroidLatestStableApiLevel)` (previous behavior) *and also* against `$(AndroidLatestStableApiLevel2)` (new property). This allows API-36.1 to be built via the `*Unstable*` properties while not defining `ANDROID_UNSTABLE.` [0]: https://learn.microsoft.com/en-us/visualstudio/msbuild/property-functions?view=vs-2022#msbuild-version-comparison-functions
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@jonpryor did the API we're using as a test disappear? android/tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs Lines 969 to 973 in 6e91776
|
It shouldn't have: it's still listed: https://developer.android.com/reference/android/telecom/TelecomManager#ACTION_CALL_BACK It's also still present in my local build tree: public partial class TelecomManager : Java.Lang.Object {
// Metadata.xml XPath field reference: path="/api/package[@name='android.telecom']/class[@name='TelecomManager']/field[@name='ACTION_CALL_BACK']"
[Register ("ACTION_CALL_BACK", ApiSince = 36)]
[global::System.Runtime.Versioning.SupportedOSPlatformAttribute ("android36.1")]
public const string ActionCallBack = (string) "android.telecom.action.CALL_BACK";Is it possible that it's somehow building against the "wrong" |
Doh!
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
[Mono.Android] Enumify API-36.1, API-36.1 is "Stable"
Context: http://github.com/jpobst/BindingStudio
Context: https://github.com/dotnet/android/pull/10005
Use jpobst/BindingStudio to begin enumifying API-36.1.
Note: current dotnet/java-interop emits an "extra" `,` on `map.csv`
output, which would make for a "noisy" diff (every line changed!).
The diff size is reduced by removing trailing commas:
sed 's/,$//' < src/Mono.Android/new-map.csv > src/Mono.Android/map.csv
This keeps the diff to a reasonable size.
TODO: update `map.csv` to current BindingStudio output after this
is merged.
Update `build-tools/manifest-attribute-codegen` so that:
1. It can be executed from topdir.
2. Warning message are actually emitted when using `dotnet build`.
Previously, only the fact that the executed command failed would
be printed, but none of the tool output, which was needed!
~~ Stable API-36.1 ~~
Normally when we declare a new API level as "stable", it is the
*only* API that is built.
For API-36.1, this is not the case: we want to continue building
API-36, *and also* build API-36.1, *and also* using the the API-36.1
build artifacts doesn't require `$(EnablePreviewFeatures)`=true.
Mirror #10005:
* build both API levels by "repurposing" the
`$(AndroidLatestUnstable*)` MSBuild properties as an
"alternate set" of stable API levels.
* What "really" indicates a `Mono.Android.dll` is unstable is the
presence of `ANDROID_UNSTABLE` when building it.
This is conditional on `$(IsUnstableVersion)`.
Update `$(IsUnstableVersion)` so that it uses the
[MSBuild version-comparison functions][0] to compare
`$(AndroidApiLevel)` against `$(AndroidLatestStableApiLevel)`
(previous behavior) *and also* against
`$(AndroidLatestStableApiLevel2)` (new property).
This allows API-36.1 to be built via the `*Unstable*` properties
while not defining `ANDROID_UNSTABLE.`
~~ Fix assertion failure in <CheckApiCompatibility/> ~~
When attempting to declare API-36.1 as the latest stable API,
my local (Debug) build promptly started *crashing*.
Which was rather unexpected.
The most useful lesson in this context: *do not* call
`Debug.Assert()` or throw exceptions from code which is called from
the Thread Pool. `Debug.Assert()` -- in Debug config builds -- will
immediately exit the process, which in turn means that MSBuild `binlog`
output is *incomplete*, as MSBuild never gets a chance to flush
everything (courtesy of the process aborting!).
Throwing an exception is very similar: unhandled exceptions on the
thread pool cause the process to abort. MSBuild never has a chance
to write complete binlog data.
Plumb logging through to `CodeGenDiff` so that the assertion failure
can instead become an "Internal Error" message. This fixes the crash.
Then there's the matter of the former assertion failure now error
message: why was `currentObject.InternalCounter` negative?
`currentObject.InternalCounter` was negative because of this line
(newlines added for readability, but it's all one line):
public static void AddEventHandler<TInterface, TImplementor>(
[System.Runtime.CompilerServices.NullableAttribute((byte)2)] ref System.WeakReference? implementor,
System.Func<TImplementor> creator,
System.Action<TInterface> setListener,
System.Action<TImplementor> add)
where TInterface : class
where TImplementor : Java.Lang.Object, TInterface
{
}
The `where TInterface : class ` was matching this check:
content.IndexOf (" class ", StringComparison.Ordinal)
and thus the method was being interpreted as the introduction of
a new type declaration instead of something to add to the existing
`currentObject` scope. This meant that the new
`currentObject.InternalCounter` was 0, then we hit `}` to decrement
`.InternalCounter`, resulting in a negative value.
Fix this by checking for ` partial class `, as was done previously
with ` partial struct `, as `Microsoft.DotNet.GenAPI.dll` always
emits the `partial` modifier on everything except `enum`s.
This fixes the now internal error message former assertion.
[0]: https://learn.microsoft.com/en-us/visualstudio/msbuild/property-functions?iew=vs-2022#msbuild-version-comparison-functions |
Context: http://github.com/jpobst/BindingStudio Context: #10005 Use jpobst/BindingStudio to begin enumifying API-36.1. Note: current dotnet/java-interop emits an "extra" `,` on `map.csv` output, which would make for a "noisy" diff (every line changed!). The diff size is reduced by removing trailing commas: sed 's/,$//' < src/Mono.Android/new-map.csv > src/Mono.Android/map.csv This keeps the diff to a reasonable size. TODO: update `map.csv` to current BindingStudio output after this is merged. Update `build-tools/manifest-attribute-codegen` so that: 1. It can be executed from topdir. 2. Warning message are actually emitted when using `dotnet build`. Previously, only the fact that the executed command failed would be printed, but none of the tool output, which was needed! ~~ Stable API-36.1 ~~ Normally when we declare a new API level as "stable", it is the *only* API that is built. For API-36.1, this is not the case: we want to continue building API-36, *and also* build API-36.1, *and also* using the the API-36.1 build artifacts doesn't require `$(EnablePreviewFeatures)`=true. Mirror #10005: * build both API levels by "repurposing" the `$(AndroidLatestUnstable*)` MSBuild properties as an "alternate set" of stable API levels. * What "really" indicates a `Mono.Android.dll` is unstable is the presence of `ANDROID_UNSTABLE` when building it. This is conditional on `$(IsUnstableVersion)`. Update `$(IsUnstableVersion)` so that it uses the [MSBuild version-comparison functions][0] to compare `$(AndroidApiLevel)` against `$(AndroidLatestStableApiLevel)` (previous behavior) *and also* against `$(AndroidLatestStableApiLevel2)` (new property). This allows API-36.1 to be built via the `*Unstable*` properties while not defining `ANDROID_UNSTABLE.` ~~ Fix assertion failure in <CheckApiCompatibility/> ~~ When attempting to declare API-36.1 as the latest stable API, my local (Debug) build promptly started *crashing*. Which was rather unexpected. The most useful lesson in this context: *do not* call `Debug.Assert()` or throw exceptions from code which is called from the Thread Pool. `Debug.Assert()` -- in Debug config builds -- will immediately exit the process, which in turn means that MSBuild `binlog` output is *incomplete*, as MSBuild never gets a chance to flush everything (courtesy of the process aborting!). Throwing an exception is very similar: unhandled exceptions on the thread pool cause the process to abort. MSBuild never has a chance to write complete binlog data. Plumb logging through to `CodeGenDiff` so that the assertion failure can instead become an "Internal Error" message. This fixes the crash. Then there's the matter of the former assertion failure now error message: why was `currentObject.InternalCounter` negative? `currentObject.InternalCounter` was negative because of this line (newlines added for readability, but it's all one line): public static void AddEventHandler<TInterface, TImplementor>( [System.Runtime.CompilerServices.NullableAttribute((byte)2)] ref System.WeakReference? implementor, System.Func<TImplementor> creator, System.Action<TInterface> setListener, System.Action<TImplementor> add) where TInterface : class where TImplementor : Java.Lang.Object, TInterface { } The `where TInterface : class ` was matching this check: content.IndexOf (" class ", StringComparison.Ordinal) and thus the method was being interpreted as the introduction of a new type declaration instead of something to add to the existing `currentObject` scope. This meant that the new `currentObject.InternalCounter` was 0, then we hit `}` to decrement `.InternalCounter`, resulting in a negative value. Fix this by checking for ` partial class `, as was done previously with ` partial struct `, as `Microsoft.DotNet.GenAPI.dll` always emits the `partial` modifier on everything except `enum`s. This fixes the now internal error message former assertion. [0]: https://learn.microsoft.com/en-us/visualstudio/msbuild/property-functions?iew=vs-2022#msbuild-version-comparison-functions
Context: http://github.com/jpobst/BindingStudio
Context: jpobst/BindingStudio#1
Use jpobst/BindingStudio to begin enumifying API-36.1.
Note: current dotnet/java-interop emits an "extra"
,onmap.csvoutput, which would make for a "noisy" diff (every line changed!). The diff is reduced by removing trailing commas:This keeps the diff to a reasonable size.
TODO: Flush current BindingStudio output, with the extra commas.
TODO: methodmap.csv
TODO: Update attributes
TODO: figure out if there are any other TODOs.