GNOME Bugzilla – Bug 673472
Removes the calls to g_thread_init() and g_thread_supported
Last modified: 2019-02-22 06:01:03 UTC
These two functions have been deprecated since glib 2.31/2.32. It doesn't hurt to remove them.
Created attachment 211255 [details] [review] It is the patch I made. Hopefully it works
Do gupnp-dlna actually require glib 2.31/2.32 yet? Otherwise just removing them might be a bad idea.
(In reply to comment #2) > Do gupnp-dlna actually require glib 2.31/2.32 yet? Otherwise just removing them > might be a bad idea. Latest gupnp needs to be compatible with latest deps so if that implies bumping the deps, why not? If someone wants to use latest gupnp, why wouldn't that person want to use latest (stable) glib as well?
2.24 is enough if we replace that with calls to g_type_init (which gst_init presumably does)
(In reply to comment #3) [...] > If someone wants to use latest gupnp, why wouldn't that > person want to use latest (stable) glib as well? My point was that the patch should then also add this dependency explicitly to configure.ac perhaps? (What happens if the library is built and ran without initializing threads against an older version of glib? I guess that's a big no-no...) To answer your question, needlessly bumping dependencies is not very good because sometimes problematic transitions needs to happen in distributions. An upgrade can thus take some time (sometimes several months in the case of Debian atleast). Would you want people to continue running an old version of your software simply because you bumped the requirements of the dependencies... Keeping versions as low as possible is a good thing and should be desired when it doesn't take to much effort (where too much might be a pretty low barrier). Now a new glib dependency might not be very problematic, but as a general idea..... and it also makes backporting harder. Meaning if you want to run a newer rygel on an older (stable) distribution you've raised the bar so much that many people won't figure out how to do it. Basically, you need to consider bumping dependencies kind of a tradeoff.... Bumping them means it will be harder for people to upgrade and you'll have users running older (known buggy?) versions of your software for longer time (and complaining when something doesn't work).
(In reply to comment #5) > (In reply to comment #3) > [...] > > If someone wants to use latest gupnp, why wouldn't that > > person want to use latest (stable) glib as well? > > My point was that the patch should then also add this dependency explicitly to > configure.ac perhaps? Sure thing! > (What happens if the library is built and ran without > initializing threads against an older version of glib? I guess that's a big > no-no...) I won't be against any alternative patch(es) that won't require bumping the dep. > To answer your question, needlessly bumping dependencies is not very good I would never support bumping of dep just for the fun of it. > because sometimes problematic transitions needs to happen in distributions. An > upgrade can thus take some time (sometimes several months in the case of Debian > atleast). Thats more a fault of current linux distribution model, we really need to move to bundles model as described here: http://blogs.gnome.org/alexl/2011/09/30/rethinking-the-linux-distibution/ > Would you want people to continue running an old version of your > software simply because you bumped the requirements of the dependencies... > Keeping versions as low as possible is a good thing and should be desired when > it doesn't take to much effort (where too much might be a pretty low barrier). 'Efforts' is the keyword here. Easiest solution is to bump the dep and port the code and there is already a patch for that.
Created attachment 214833 [details] [review] Second try to remove the deprecated g_thread_init() calls by adding glib version checking in configure.ac
Review of attachment 214833 [details] [review]: ACK. Let me know if you don't have commit access to git.gnome.
I don't have the commit permission to git.gnome. May I have one?
(In reply to comment #9) > I don't have the commit permission to git.gnome. May I have one? Its not that simple. Commit access is global on git.gnome so you'll have to contribute a lot more to gain this big trust. In the meantime, me and jens can push your patches..
Created attachment 215598 [details] [review] A third attempt. See details in comment gupnp-dlna depends on gstreamer which depends on glib-2.24 therefore we can replace g_thread_init() with g_type_init() because since glib-2.24 g_type_init() will initialize the thread system.
Review of attachment 215598 [details] [review]: Apart from that, I'll just write a simple commit log like this: Drop usage of deprecated glib API ::: tests/dlna-encoding.c @@ -200,3 +200,3 @@ gchar *inputuri; - if (!g_thread_supported ()) + g_type_init (); /* Since GLib 2.24 this also initializes the thread system */ Don't see the need for this comment. ::: tests/dlna-profile-parser.c @@ -104,3 +104,3 @@ GOptionContext *ctx; - - if (!g_thread_supported ()) + + g_type_init (); /* Since GLib 2.24 this also initializes the thread system */ same here ::: tools/gupnp-dlna-info.c @@ -453,3 +453,3 @@ GOptionContext *ctx; - if (!g_thread_supported ()) + g_type_init (); /* Since GLib 2.24 this also initializes the thread system */ and here ::: tools/gupnp-dlna-ls-profiles.c @@ -104,3 +104,3 @@ GOptionContext *ctx; - if (!g_thread_supported ()) + g_type_init (); /* Since GLib 2.24 this also initializes the thread system */ and here :)
Created attachment 217075 [details] [review] Remove explicit calls to g_thread_init() Modified the patch according to my last review, corrected the commit log to follow the conventions and removed trailing whitespace.
Attachment 217075 [details] pushed as bdd1a01 - Remove explicit calls to g_thread_init()