GNOME Bugzilla – Bug 694873
Statically link in native modules
Last modified: 2013-03-07 22:08:02 UTC
This is more of a proposal, but I wrote this while rewriting the import system so I could give unique prefixes to GType names so we wouldn't have extension conflicts in GType names anymore.
Created attachment 237604 [details] [review] Statically link in native modules Now that we have significantly less of them, there's no real reason to not link in the native implementations of this...
Created attachment 237605 [details] [review] native: Remove unused argument
Created attachment 237606 [details] [review] importer: Remove support for loading external native modules
Created attachment 237607 [details] [review] native: Pass the module name to the import call Rather than picking it up indirectly through meta properties.
Created attachment 237608 [details] [review] native: Remove support for nested native modules Now that all modules are guaranteed to be inline, this skips this step. With this gone, we can remove a large set of code that's been around simply for support for this.
Created attachment 237609 [details] [review] Don't use constructors to register static modules
Review of attachment 237604 [details] [review]: Hmm...true. Involves less I/O too.
Review of attachment 237605 [details] [review]: Hmmm...I'm kind of curious why this is unused, but OK.
Review of attachment 237606 [details] [review]: One minor comment. ::: gjs/native.c @@ -189,3 @@ - * If @filename is %NULL, do not dlopen, assume the library - * is already loaded in the modules hash table - */ Why entirely delete the docs when you could just change it to say that it loads a statically linked one?
Review of attachment 237607 [details] [review]: Makes sense.
Review of attachment 237608 [details] [review]: That is indeed a nice cleanup.
Review of attachment 237609 [details] [review]: One build comment, otherwise looks nice. ::: configure.ac @@ +111,3 @@ ]) AM_CONDITIONAL(ENABLE_CAIRO, test x$have_cairo = xyes) +AC_DEFINE([ENABLE_CAIRO],[$have_cairo],[Define if you want to build with cairo support]) This isn't quite right, because in modules.c you just use #ifdef, but it'll always be defined. You need to: AS_IF([test x$have_cairo = xyes], [ AC_DEFINE([ENABLE_CAIRO],[1],[Define if you want to build with cairo support]) ])
Attachment 237604 [details] pushed as 65257a3 - Statically link in native modules Attachment 237605 [details] pushed as ad376ca - native: Remove unused argument Attachment 237606 [details] pushed as aa9fb97 - importer: Remove support for loading external native modules Attachment 237607 [details] pushed as e01d8a1 - native: Pass the module name to the import call Attachment 237608 [details] pushed as 9e91b85 - native: Remove support for nested native modules
Attachment 237609 [details] pushed as 72afb88 - Don't use constructors to register static modules I think I got the autotools junk correct. Let's see...