New version, new srpm. Spec is the same location

SRPM URL: http://fedora.roving-it.com/rawhide/gupnp-tools-0.6-1.fc9.src.rpm

New location

SPEC: http://pbrobinson.fedorapeople.org/gupnp-tools.spec
SRPM: http://pbrobinson.fedorapeople.org/gupnp-tools-0.6-1.fc9.src.rpm

Some initial issues.

- desktop files should be installed with desktop-file-install. See

https://fedoraproject.org/wiki/Packaging/Guidelines#Desktop_files

- all 3 utilities core dumps on startup. Looking closer, a simply patch is required. Look for the calls to g_thread_init() in each of the utility source directories, and remove the call. g_thread_init() can only be called once, and it's called automatically by gtk_init(), so no need to call it explicitly.

Hi Denis,

There is a new release of the package in testing to fix the g_thread_init issue. I just waiting for it to be released. I have also fixed up some issues in the spec file, and will also fixe the desktop file issue.

Thanks,
Peter

Hi Denis,

I've uploaded a new srpm and spec file based on the test release I have. It should have fixed all the issues.

SRPM: http://pbrobinson.fedorapeople.org/gupnp-tools-0.6.1-1.fc9.src.rpm

Hi Peter,

The 0.6.1 release is not available on the download site, which prevents me from verifying the tarball as part of the review process.

Also, the calls to g_thread_init() are still there and the tools still crash on startup. I would recommend to base the review on 0.6 for now and to write a patch to remove the 3 calls to g_thread_init().

Hi Denis,

Removing the calls to g_thread_init on 0.6 had the apps crash for me with the following error on universal-cp and av-cp, network-light segfaulted on rawhide (all 3 gave the error on F-9 but ran fine without the patch):

# gupnp-universal-cp 

GLib-ERROR **: The thread system is not yet initialized.
aborting...
Aborted

So it looks like there is other issues.

OK. There's a new 0.6.1 release out. I've tested this and it build and runs fine on the current rawhide.

I've done a scratch build in koji and tested the resulting binaries work on i386 rawhide
http://koji.fedoraproject.org/koji/taskinfo?taskID=849551

New SRPM.
http://pbrobinson.fedorapeople.org/gupnp-tools-0.6.1-1.fc9.src.rpm

Hi Denis,

Any chance to retest this?

Created attachment 320093 [details]
Patch to fix g_thread_init crash issue

Peter, do you have a F-10 system to test this ?

I've upgraded my system to F-10, and the GThread init crash still exist for all 3 utilities here. That's working on F-10 for you ? Maybe I'm seeing a different glib behavior because my system is dual-core, I don't know. I'm attaching the patch required for it to work on my system.

Yes, I've tested it on F-10 on both i386 (netbook) and x86_64 (VM) and wasn't seeing any crash when running on F-10. The VM is setup with 2 cores, and the netbook while not real dual core is hyperthreaded. I'll test it later today.

Matthias, I was wondering if you could shed some light on this. Peter and I are seeing a different runtime behavior of these gupnp tools. On my f10-system, the uv tools immediately abort because g_thread_init() is called twice. If I run the tool from the debugger, the gupnp tool explicitly calls gtk_init(), then g_thread_init() right below. On my system, gtk_init() also calls g_thread_init() (through atk_bridge_init(), bonobo_activation_init() and corba orb init()), hence the abort.

Any insights ?

From the API docs:
g_thread_init() might only be called once. On the second call it will abort with an error. If you want to make sure that the thread system is initialized, you can do this: 
if (!g_thread_supported ()) g_thread_init (NULL);

So either change your call to g_thread_init() to check whether threads are already supported, or run g_thread_init() before anything else (and if it still crashes, file a bug against whatever is not checking for g_thread_supported() before calling g_thread_init).

> So either change your call to g_thread_init() to check whether
> threads are already supported.

Yes I read the API doc as well, and that's exactly what the patch I attached does.

What I don't understand is why gtk_init() calls g_thread_init() on my system and not on Peter's system...

- License is GPLv2+
- rpmlint ok
- spec file is good
- source md5sum good
- packages build
- utilities function correctly , i386 and x86_64


2 minor things:

- according to guidelines, "--vendor=fedora" should be used for desktop-file-install

- the %files section could be written with just 3 lines:
  %{_datadir}/gupnp-tools
  %{_bindir}/gupnp*
  %{_datadir}/applications/*.desktop


If you're ok with the patch I submitted, consider the package APPROVED. I've tested the package on F-9 x86_64 and F10 i386.

> - according to guidelines, "--vendor=fedora" should be used for
> desktop-file-install

Actually according to the packaging guidelines the Vendor should only be set if there isn't one already in use. They already use 'gupnp'
https://fedoraproject.org/wiki/Packaging/Guidelines#Desktop_files
 
> - the %files section could be written with just 3 lines:
>   %{_datadir}/gupnp-tools
>   %{_bindir}/gupnp*
>   %{_datadir}/applications/*.desktop

Yes, I'm aware of that but the reason I tend to do it the other way is that you don't get unexpected files that may be introduced on a new release. Its one of those 6 to one, hlaf a dozen to the other things.

> Actually according to the packaging guidelines the Vendor should only be set if
> there isn't one already in use. They already use 'gupnp'

Sounds good.

So is that approved?

If you include the patch I provided, yes that's APPROVED.

I'll include the patch but as specified above it was causing me problems and if others report the issues I saw I will revert it.

New Package CVS Request
=======================
Package Name: gupnp-tools
Short Description: GUPnP-tools is a collection of dev tools utilising GUPnP and GTK+
Owners: pbrobinson
Branches: F-9

> I'll include the patch but as specified above it was causing me problems

This one won't, as it checks whether the init call is needed or not, as suggested by the API documentation. I didn't pursue further why it's needed on my F-10 system and not on my other F-9 server, but it's related to ATK and Gnome assistive setups.

cvs done.

Built and in rawhide