[Mono.Android] introduce #if NATIVEAOT for monodroid_get_log_categories()
#9624
Conversation
…ories()` In a NativeAOT context, we don't have `libmonodroid.so` *at all*. This change introduces a new `#if NATIVEAOT` block in `Logger.cs` to avoid calling `monodroid_get_log_categories()` and using `Default | Assembly` by default. I also reworked the build, so that the `$(AndroidRuntime)` property is defined in `Configuration.props`. Other projects can eventually use it as-needed. In a future PR, we could consider p/invoke into `__system_property_get()` and parse values. But we might want to reconsider the *names* of some of the system properties as they have `debug.mono.*` prefixes. Hardcoding a default value is at least *better*, so it won't crash.
| <DefineConstants Condition=" '$(AndroidRuntime)' == 'Mono' ">$(DefineConstants);JAVA_INTEROP;MONO</DefineConstants> | ||
| <DefineConstants Condition=" '$(AndroidRuntime)' == 'NativeAOT' ">$(DefineConstants);JAVA_INTEROP;NATIVEAOT</DefineConstants> |
There was a problem hiding this comment.
I was debating the names here. Should it be MONO_RUNTIME and NATIVEAOT_RUNTIME?
There was a problem hiding this comment.
I'd prefer a RUNTIME_ prefix, vs. a _RUNTIME suffix: RUNTIME_MONOVM, RUNTIME_NATIVEAOT, etc.
I'd really prefer the need for any such changes to Mono.Android.dll, if possible…
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
|
||
| static Logger () | ||
| { | ||
| #if NATIVEAOT | ||
| // TODO: p/invoke into __system_property_get | ||
| Categories = LogCategories.Default | LogCategories.Assembly; |
There was a problem hiding this comment.
wrt my earlier comment:
I'd really prefer the need for any such changes to
Mono.Android.dll, if possible…
Could we instead have LogCategories (or an int variation) be a member of JnienvInitializeArgs, and have both NativeAOT and MonoVM use JniEnvInit.Initialize(JnienvInitializeArgs*)? This would allow removing the need for monodroid_get_log_categories() entirely, and aim for consistent initialization between the two environments.
There was a problem hiding this comment.
…especially when logCategories is already a field in JnnenvInitializeArgs (?!):
Why do we have this P/Invoke at all?!
There was a problem hiding this comment.
I’ll open a new PR to remove the p/invoke. I’ll see if we still need a define later.
There was a problem hiding this comment.
It seems like this will work:
But we'll want to merge #9622 first, or the pinvoke-table will have conflicts.
In a NativeAOT context, we don't have
libmonodroid.soat all.This change introduces a new
#if NATIVEAOTblock inLogger.csto avoid callingmonodroid_get_log_categories()and usingDefault | Assemblyby default.I also reworked the build, so that the
$(AndroidRuntime)property is defined inConfiguration.props. Other projects can eventually use it as-needed.In a future PR, we could consider p/invoke into
__system_property_get()and parse values. But we might want to reconsider the names of some of the system properties as they havedebug.mono.*prefixes.Hardcoding a default value is at least better, so it won't crash.