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 673472 - Removes the calls to g_thread_init() and g_thread_supported
Removes the calls to g_thread_init() and g_thread_supported
Status: RESOLVED FIXED
Product: gupnp-dlna
Classification: Other
Component: General
unspecified
Other Linux
: Normal trivial
: ---
Assigned To: GUPnP Maintainers
GUPnP Maintainers
Depends on:
Blocks:
 
 
Reported: 2012-04-04 04:06 UTC by Riko Yamada
Modified: 2019-02-22 06:01 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
It is the patch I made. Hopefully it works (2.61 KB, patch)
2012-04-04 04:06 UTC, Riko Yamada
none Details | Review
Second try to remove the deprecated g_thread_init() calls by adding glib version checking in configure.ac (3.21 KB, patch)
2012-05-24 04:00 UTC, Riko Yamada
accepted-commit_now Details | Review
A third attempt. See details in comment (3.28 KB, patch)
2012-06-05 03:19 UTC, Riko Yamada
reviewed Details | Review
Remove explicit calls to g_thread_init() (2.96 KB, patch)
2012-06-23 14:18 UTC, Zeeshan Ali
committed Details | Review

Description Riko Yamada 2012-04-04 04:06:00 UTC
These two functions have been deprecated since glib 2.31/2.32. It doesn't hurt to remove them.
Comment 1 Riko Yamada 2012-04-04 04:06:44 UTC
Created attachment 211255 [details] [review]
It is the patch I made. Hopefully it works
Comment 2 Andreas Henriksson 2012-04-11 17:21:56 UTC
Do gupnp-dlna actually require glib 2.31/2.32 yet? Otherwise just removing them might be a bad idea.
Comment 3 Zeeshan Ali 2012-04-11 18:30:16 UTC
(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?
Comment 4 Jens Georg 2012-04-11 20:10:55 UTC
2.24 is enough if we replace that with calls to g_type_init (which gst_init presumably does)
Comment 5 Andreas Henriksson 2012-04-12 06:36:38 UTC
(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).
Comment 6 Zeeshan Ali 2012-04-15 01:31:05 UTC
(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.
Comment 7 Riko Yamada 2012-05-24 04:00:11 UTC
Created attachment 214833 [details] [review]
Second try to remove the deprecated g_thread_init() calls by adding glib version checking in configure.ac
Comment 8 Zeeshan Ali 2012-05-31 23:10:07 UTC
Review of attachment 214833 [details] [review]:

ACK. Let me know if you don't have commit access to git.gnome.
Comment 9 Riko Yamada 2012-06-01 04:49:30 UTC
I don't have the commit permission to git.gnome. May I have one?
Comment 10 Zeeshan Ali 2012-06-01 15:45:38 UTC
(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..
Comment 11 Riko Yamada 2012-06-05 03:19:16 UTC
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.
Comment 12 Zeeshan Ali 2012-06-07 00:21:56 UTC
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 :)
Comment 13 Zeeshan Ali 2012-06-23 14:18:16 UTC
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.
Comment 14 Zeeshan Ali 2012-06-23 14:18:57 UTC
Attachment 217075 [details] pushed as bdd1a01 - Remove explicit calls to g_thread_init()