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

[monodroid] remove monodroid_get_log_categories() #9625

Merged

Conversation

jonathanpeppers
Copy link
Member

In a NativeAOT context, we don't have libmonodroid.so at all.
Let's remove the monodroid_get_log_categories() function, as we
already pass in args->logCategories at startup. We can use a new
Logger.SetLogCategories() method to pass the value in.

I also removed JNIEnvInit.LogAssemblyCategory, as we can just use
the Logger class instead.

The startup sequence in C++ seems OK, so doesn't seem like this
changes any behavior:

  • Java_mono_android_Runtime_initInternal() sets up logging
    categories very early in startup.

  • create_and_initialize_domain() calls...

  • init_android_runtime() calls...

  • Calls managed code, JNIEnvInit.Initialize() using the correct
    logging categories.

In a NativeAOT context, we don't have `libmonodroid.so` *at all*.
Let's remove the `monodroid_get_log_categories()` function, as we
already pass in `args->logCategories` at startup. We can use a new
`Logger.SetLogCategories()` method to pass the value in.

I also removed `JNIEnvInit.LogAssemblyCategory`, as we can just use
the `Logger` class instead.

The startup sequence in C++ seems OK, so doesn't seem like this
changes any behavior:

* `Java_mono_android_Runtime_initInternal()` sets up logging
  categories very early in startup.

* `create_and_initialize_domain()` calls...

* `init_android_runtime()` calls...

* Calls managed code, `JNIEnvInit.Initialize()` using the correct
  logging categories.
@jonathanpeppers jonathanpeppers force-pushed the dev/peppers/remove/monodroid_get_log_categories branch from 81a97d6 to 72af30f Compare December 17, 2024 17:03
@jonathanpeppers
Copy link
Member Author

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@dotnet dotnet deleted a comment from azure-pipelines bot Dec 17, 2024
@jonathanpeppers jonathanpeppers marked this pull request as ready for review December 17, 2024 17:10
@samhouts samhouts requested a review from Copilot December 17, 2024 19:44

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 5 out of 9 changed files in this pull request and generated no comments.

Files not reviewed (4)
  • src/native/monodroid/internal-pinvokes.cc: Language not supported
  • src/native/pinvoke-override/generate-pinvoke-tables.cc: Language not supported
  • src/native/pinvoke-override/pinvoke-tables.include: Language not supported
  • src/native/runtime-base/internal-pinvokes.hh: Language not supported
Comments suppressed due to low confidence (4)

src/Mono.Android/Android.Runtime/JNINativeWrapper.cs:43

  • [nitpick] The name 'LogAssembly' could be more descriptive. Consider renaming it to something like 'IsAssemblyLoggingEnabled' for better clarity.
if (Logger.LogAssembly)

src/Mono.Android/Java.Interop/TypeManager.cs:227

  • The condition Logger.LogAssembly is used without ensuring that Logger is properly initialized. Ensure that Logger.SetLogCategories is called before this condition.
if (Logger.LogAssembly)

src/Mono.Android/Android.Runtime/JNIEnv.cs:518

  • The condition Logger.LogAssembly may not accurately reflect the logging category state if it is not properly initialized. Ensure that Logger.SetLogCategories is called before this check.
if (Logger.LogAssembly) {

src/Mono.Android/Android.Runtime/JNIEnvInit.cs:85

  • The cast to LogCategories should be verified to ensure that args->logCategories is a valid value. Consider adding a validation check or handling invalid values appropriately.
Logger.SetLogCategories ((LogCategories)args->logCategories);
@jonathanpeppers jonathanpeppers merged commit de04316 into main Dec 17, 2024
58 checks passed
@jonathanpeppers jonathanpeppers deleted the dev/peppers/remove/monodroid_get_log_categories branch December 17, 2024 20:00
grendello added a commit that referenced this pull request Jan 7, 2025
* main: (25 commits)
  [CI] Break "Linux Tests" into 2 parallel jobs. (#9642)
  Fix `WorkloadDependencies.proj` build. (#9648)
  [CI] Set "WearOS Tests" parallelization to 2 agents. (#9639)
  [CI] Break "Package Tests" into 2 parallel jobs. (#9638)
  Bump to DevDiv/android-platform-support@3b4e16f1 (#9632)
  [NativeAOT] improve build logic, part 2 (#9631)
  Bump to dotnet/java-interop@2c06b3c2 (#9633)
  [NativeAOT] improve build logic, part 1 (#9614)
  [build] Generate `WorkloadDependencies.json` (#9613)
  [monodroid] remove `monodroid_get_log_categories()` (#9625)
  [monodroid] remove `_monodroid_get_identity_hash_code` (#9622)
  Bump to dotnet/java-interop@f800ea52 (#9607)
  [XABT] Break BuildApk into individual tasks for each content type. (#9612)
  [Mono.Android] Bind Android API-Baklava DP1 (#9594)
  [Xamarin.Android.Build.Tasks] Extract `BuildArchive` from `BuildApk` (#9556)
  [NativeAOT] MSBuild-related logic to get projects to build (#9583)
  [build] remove remnants of `OpenTK-1.0.dll` (#9610)
  [build] remove `Xamarin.Android.CSharp.targets` (#9609)
  [build] runtime "flavors" part 2 (#9598)
  Bump com.android.tools.build:manifest-merger to 31.7.3 (#9600)
  ...
grendello added a commit that referenced this pull request Jan 7, 2025
* dev/grendel/use-libc++: (25 commits)
  [CI] Break "Linux Tests" into 2 parallel jobs. (#9642)
  Fix `WorkloadDependencies.proj` build. (#9648)
  [CI] Set "WearOS Tests" parallelization to 2 agents. (#9639)
  [CI] Break "Package Tests" into 2 parallel jobs. (#9638)
  Bump to DevDiv/android-platform-support@3b4e16f1 (#9632)
  [NativeAOT] improve build logic, part 2 (#9631)
  Bump to dotnet/java-interop@2c06b3c2 (#9633)
  [NativeAOT] improve build logic, part 1 (#9614)
  [build] Generate `WorkloadDependencies.json` (#9613)
  [monodroid] remove `monodroid_get_log_categories()` (#9625)
  [monodroid] remove `_monodroid_get_identity_hash_code` (#9622)
  Bump to dotnet/java-interop@f800ea52 (#9607)
  [XABT] Break BuildApk into individual tasks for each content type. (#9612)
  [Mono.Android] Bind Android API-Baklava DP1 (#9594)
  [Xamarin.Android.Build.Tasks] Extract `BuildArchive` from `BuildApk` (#9556)
  [NativeAOT] MSBuild-related logic to get projects to build (#9583)
  [build] remove remnants of `OpenTK-1.0.dll` (#9610)
  [build] remove `Xamarin.Android.CSharp.targets` (#9609)
  [build] runtime "flavors" part 2 (#9598)
  Bump com.android.tools.build:manifest-merger to 31.7.3 (#9600)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants