fix(android): replace non-compiling JNI constructor with [DynamicDependency] - #322
Merged
winnerspiros merged 2 commits intoMay 9, 2026
Conversation
…dency attribute Agent-Logs-Url: https://github.com/winnerspiros/osu/sessions/0a6c506c-c593-40ea-8768-bf1f2f842808 Co-authored-by: winnerspiros <1675249+winnerspiros@users.noreply.github.com>
Copilot created this pull request from a session on behalf of
winnerspiros
May 9, 2026 07:47
View session
|
|
…id on class declarations) Agent-Logs-Url: https://github.com/winnerspiros/osu/sessions/171d847e-6009-4492-b2b0-9c391c67af5d Co-authored-by: winnerspiros <1675249+winnerspiros@users.noreply.github.com>
winnerspiros
approved these changes
May 9, 2026
There was a problem hiding this comment.
Pull request overview
Fixes an Android build/CI failure by relocating the [DynamicDependency] attribute to a valid target, while keeping the linker-trimming protection for JNI activation paths in place.
Changes:
- Add
System.Diagnostics.CodeAnalysisimport and apply[DynamicDependency]toOsuGameActivity()(constructor) instead of the class. - Remove the previously-added explicit JNI activation constructor that didn’t compile.
- Update
Linker.xmldocumentation to describe the new “additional defence” approach.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| osu.Android/OsuGameActivity.cs | Moves [DynamicDependency] onto the parameterless constructor and removes the non-compiling JNI constructor attempt. |
| osu.Android/Linker.xml | Updates the explanatory comment describing the trimming/linker defence strategy for JNI constructors. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+92
to
+96
| // Preserve AndroidGameActivity's (IntPtr, JniHandleOwnership) JNI-activation constructor. | ||
| // The [assembly: preserve="all"] descriptor for osu.Framework.Android in Linker.xml is deferred | ||
| // (IL2007) because the framework assembly is not yet in ILLink's search path at descriptor time. | ||
| // Placing [DynamicDependency] on this constructor (which lives in osu.Android — always resolvable) | ||
| // roots the AndroidGameActivity constructor in ILLink's walk before it can be trimmed away. |
| // Placing [DynamicDependency] on this constructor (which lives in osu.Android — always resolvable) | ||
| // roots the AndroidGameActivity constructor in ILLink's walk before it can be trimmed away. | ||
| [DynamicDependency(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors, | ||
| "osu.Framework.Android.AndroidGameActivity", "osu.Framework.Android")] |
Comment on lines
+108
to
+116
| attribute that explicitly roots AndroidGameActivity's constructors from the | ||
| osu.Android assembly. Because osu.Android is unambiguously preserve="all" and is | ||
| always resolvable at descriptor-processing time, ILLink processes this root | ||
| reference before it can trim the JNI-activation constructor, regardless of whether | ||
| the IL2007-deferred rule for osu.Framework.Android is honoured. This supersedes | ||
| the v2026.508.227 approach (explicit OsuGameActivity(IntPtr, JniHandleOwnership) | ||
| constructor) which did not compile because AndroidGameActivity does not expose an | ||
| explicit (IntPtr, JniHandleOwnership) constructor — only its SDLActivity grandparent | ||
| does, and C# base() invocations resolve only against the direct parent class. --> |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
[DynamicDependency]not valid on class declarations, only on constructor/method/field[DynamicDependency]attribute from class declaration to the parameterless constructor