GNOME Bugzilla – Bug 656914
Load GIO_EXTRA_MODULES first, and ignore duplicates
Last modified: 2011-08-27 23:06:56 UTC
So that projects can load test in-tree modules properly, it should be possible to load modules from other directories, and have those override the installed modules from the same project. The GIO_EXTRA_MODULES environment variable (see #523039) already exists, but it loads after the built in modules, and then the installed and uninstalled modules conflict. As discussed IRC, a decent way to do this seems to be: a) look at GIO_EXTRA_MODULES directories first and b) don't load modules with the same basename
Created attachment 194233 [details] [review] giomodule: When loading GIO_EXTRA_MODULES skip duplicates * Load modules from paths listed in GIO_EXTRA_MODULES environment variable first. * Ignore duplicate modules based on module basename. * Document behavior.
see bug 656059 for an alternate approach
Aha. Yes, that seems like a good approach too. The g_io_module_test method Dan proposes seems slightly less likely to surprise someone. Either way is fine with me. Alex, which one should we merge?
I'm slightly in favour of the env var approach, since it can be used outside of tests too, and is more in line with what we do elsewhere (gdk-pixbuf loaders, immodules, etc)
Comment on attachment 194233 [details] [review] giomodule: When loading GIO_EXTRA_MODULES skip duplicates >* Load modules from paths listed in GIO_EXTRA_MODULES environment > variable first. > * Ignore duplicate modules based on module basename. fix indent on first line >+ * The GIO_EXTRA_MODULES environment variables can be used to specify <envar>GIO_EXTRA_MODULES</envar> (likewise with PATH later on) also, no "s" on "variable" >+is_duplicate_module_name (const gchar *basename, >+ GHashTable *module_names) align the "*"s >+ /* No duplicate checking */ >+ if (!module_names) >+ return FALSE; I was going to complain about the comment style (comments should say things that are true at the point where the comment appears, so that comment either needs an "if" in it, or else it should be inside the if body rather than before it) but then decided that you should probably just remove the comments, because they're obvious anyway. >+ * an extension point it implementes is used with e.g. fix the pre-existing typo while you're moving it
Yeah, i also favour the env var approach.
Review of attachment 194233 [details] [review]: It might be interesting to somehow expose the duplication-avoiding function in the API too, so other apps using gio modules can do the same thing. ::: gio/giomodule.c @@ +285,3 @@ + + /* Duplicate checking, but this is the first one */ + g_hash_table_insert (module_names, g_strdup (basename), "PRESENT"); "PRESENT" is kinda weird. I tend to use the same value as the key, GINT_TO_POINTER(1).
Created attachment 194565 [details] [review] giomodule: When loading GIO_EXTRA_MODULES skip duplicates * Load modules from paths listed in GIO_EXTRA_MODULES environment variable first. * Ignore duplicate modules based on module basename. * Add g_io_modules_scan_all_in_directory_full() function, which allows callers to specify flags to ignore duplicate base names when scanning. * Document behavior.
Updated for Dan's and Alex's comments. I hope this solves your use case Alex. It allows non-gio callers to get in on the 'no duplicate base name' action if desired. git bz, seems to do the screwy thing with removing whitespace when attaching commits to bugs... Will send Colin a patch.
Thats not right, duplicate name skipping should only be between each single instance of use of modules. For instance gio modules used to load plugins to some app should not be affected by basenames of core gio extensions.
Created attachment 194571 [details] [review] After discussing with Alex on IRC, exposed the duplicate blocking behavior and exposed it as a 'scope' optionally used when loading modules. giomodule: When loading GIO_EXTRA_MODULES skip duplicates * Load modules from paths listed in GIO_EXTRA_MODULES environment variable first. * Ignore duplicate modules based on module basename. * Add the concept of GIOModuleScope which allows other callers to skip duplicate loaded modules, or block specific modules based on basename. * Document behavior.
Checked with Alex on IRC. Added some missing Since:, and then merged.
*** Bug 656059 has been marked as a duplicate of this bug. ***