After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 622071 - [importer] Allow native modules in gjs.so
[importer] Allow native modules in gjs.so
Status: RESOLVED FIXED
Product: gjs
Classification: Bindings
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gjs-maint
gjs-maint
Depends on:
Blocks: 571000 612226
 
 
Reported: 2010-06-19 13:03 UTC by Johan (not receiving bugmail) Dahlin
Modified: 2010-06-23 03:02 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
[importer] Allow native modules in gjs.so (4.30 KB, patch)
2010-06-19 13:03 UTC, Johan (not receiving bugmail) Dahlin
committed Details | Review

Description Johan (not receiving bugmail) Dahlin 2010-06-19 13:03:29 UTC
Allow native modules to be registered inside gjs.so.
This will be used by a few modules like sys (needs argv) and
byteArray (exports internal api to gi)
Comment 1 Johan (not receiving bugmail) Dahlin 2010-06-19 13:03:32 UTC
Created attachment 164075 [details] [review]
[importer] Allow native modules in gjs.so
Comment 2 Lucas Rocha 2010-06-22 17:25:50 UTC
Review of attachment 164075 [details] [review]:

I'm missing a bit of more context of what this patch actually does. Where's the code using this new feature?

::: gjs/importer.c
@@ +565,3 @@
         }
+        if (import_native_file(context, obj, name, NULL)) {
+        /* First try importing an internal module like byteArray */

What is this actually doing? How does that actually end up importing an internal module? For instance, what would be the 'name' arg value in that case?

::: gjs/native.c
@@ +96,3 @@
 {
                          GjsNativeFlags *flags_p)
+    GModule *gmodule = NULL;

How's this related to this patch? Fix this in a separate patch?
Comment 3 Johan (not receiving bugmail) Dahlin 2010-06-22 17:31:33 UTC
import_native_file() with a file name to NULL will assume that the type is already registered (present in internal import hash tables) instead of calling g_module_open on a shared library.

The GModule change is necessary, since when we pass in NULL there's no longer a GModule* around, we need to test for that below.

I just realized that we could call g_module_open on the fly on ourselves (eg NULL) instead of assuming that the module is registered, but that requires that we pass in -module and -rdynamic to the linker, I think.
Comment 4 Lucas Rocha 2010-06-22 17:45:26 UTC
Oh, ok, I missed the obvious part where byteArray is built into gjs in the patch in bug 571000. Ok, but where is importer used with filename null?
Comment 5 Lucas Rocha 2010-06-22 17:57:37 UTC
Ok, I got a bit confused initially because this patch adds a feature but adds no code using it. Managed to understand it when I read the patches on bug 571000. I'd like to see a more comment explicit comment on why/when filename might be null. Looks good otherwise.
Comment 6 Johan (not receiving bugmail) Dahlin 2010-06-23 03:02:33 UTC
Attachment 164075 [details] pushed as 7045f3f - [importer] Allow native modules in gjs.so