-
-
Notifications
You must be signed in to change notification settings - Fork 21.4k
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
Fix loading GDExtension dependencies on Android #88381
Fix loading GDExtension dependencies on Android #88381
Conversation
2452c49
to
a24c871
Compare
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.
Heh, just yesterday, I was just explaining to someone that the dependencies aren't used at all when loading a GDExtension, only in exporting. I guess that won't be true anymore, at least for Android. :-)
At a high-level, this looks good to me! I haven't done any testing.
11cb180
to
0153d97
Compare
Discussed at the GDExtension meeting, and the (This unrelated GDExtension PR also adds a new argument: #87117) Perhaps we should consider having a more generic |
What about adding a gdextension specific argument which contains all the gdextension supplemental parameters and is |
I was imagining that the
But, yes, that could work too! Are you imagining like a |
Yeah somethink like a |
69b6284
to
102d62c
Compare
While it isn't required to solve this now (because this is a purely internal API and we can change it at any time), I'd personally prefer if we made a |
@dsnopek I've added the change into a second commit since the changes are orthogonal to each other, and to make it easier to review. Let me know if you want me to collapse the commits together. |
This is used to reduce the number of arguments to `OS::open_dynamic_library(...)`.
d553033
to
764de7f
Compare
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.
Thanks! I haven't tested it, but the code looks good to me :-)
I validated the fix in #87785 (comment) |
Thanks! |
Update the Android
gdextension
loading logic so that it copies any specified library dependencies alongside the gdextension shared library.Fixes #87785