Bug 990544 - Review Request: mangler - VOIP client capable of connecting to Ventrilo 3.x servers
Review Request: mangler - VOIP client capable of connecting to Ventrilo 3.x s...
Status: ASSIGNED
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Christopher Meng
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2013-07-31 08:35 EDT by Palle Ravn
Modified: 2014-07-14 22:34 EDT (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed:
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
i: fedora‑review?


Attachments (Terms of Use)

  None (edit)
Description Palle Ravn 2013-07-31 08:35:23 EDT
Spec URL: http://zom.dk/mangler/mangler.spec
SRPM URL: http://zom.dk/mangler/mangler-1.2.5-1.fc19.src.rpm

Description: Mangler is a VOIP client capable of connecting to Ventrilo 3.x servers. It is capable of performing almost all standard user functionality found in the official Ventrilo client.

rpmlint: 2 packages and 1 specfiles checked; 0 errors, 0 warnings.

Koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=5683566

Fedora Account System Username: paller
Comment 1 Christopher Meng 2013-08-01 00:22:53 EDT
1. Can you tell me why removing

rm -f %{buildroot}%{_exec_prefix}/%{_lib}/libventrilo3.so

but not moving them to -devel?

2. Have you tried %configure --disable-static to avoid generating static libs?

3. You should use %{_libdir} instead of %{_exec_prefix}/%{_lib}

4. %{_datarootdir} can be %{_datadir} ;)

5. Package ships 2 license file, but you only include one.
Comment 2 Palle Ravn 2013-08-01 02:41:35 EDT
(In reply to Christopher Meng from comment #1)
> 1. Can you tell me why removing
> 
> rm -f %{buildroot}%{_exec_prefix}/%{_lib}/libventrilo3.so
> 
> but not moving them to -devel?

I was unaware of the devel part of packaging, as I never had the need. I have added a -devel package including .a .h and .so files.

> 2. Have you tried %configure --disable-static to avoid generating static
> libs?

Yes, and it did not change the static rpath.

> 3. You should use %{_libdir} instead of %{_exec_prefix}/%{_lib}

Fixed.

> 4. %{_datarootdir} can be %{_datadir} ;)

Works better with vim highlighting, thanks :D

> 5. Package ships 2 license file, but you only include one.

Both included.

All files are at www.zom.dk/mangler, note that I have not changed the revision number.

I will of course notify upstream about the wrong fsf address in one of the source files.
Comment 3 Christopher Meng 2013-08-01 04:26:22 EDT
Note if you split out devel package, don't include static libs.

1. Please remove them via some commands like this:

find %{buildroot} -name '*.*a' -delete

2. Don't adda BR of gcc-c++, please remove.

3. You'd better add isa macro to devel package Requires:

Requires:	   %{name}%{?_isa} = %{version}-%{release}

4. This package has bundled many libraries.

GPL (v2 or later) (with incorrect FSF address)
----------------------------------------------
/var/lib/mock/fedora-rawhide-i386/root/builddir/build/BUILD/mangler-1.2.5/libventrilo3/ventrilo3_handshake.c
/var/lib/mock/fedora-rawhide-i386/root/builddir/build/BUILD/mangler-1.2.5/libventrilo3/ventrilo_algo.h

This is from libventrilo, which hasn't been packaged yet in Fedora.

https://github.com/haxar/libventrilo3

5. This package has iphone and android files, I don't know if they are useful or not? Because licensecheck has found many license issues, but most are those files, so I have this question.
Comment 4 Michael Schwendt 2013-08-01 05:49:49 EDT
> This is from libventrilo, which hasn't been packaged yet in Fedora.
> 
> https://github.com/haxar/libventrilo3

It's part of the Mangler project, isn't it?
https://github.com/haxar?tab=repositories

Find out about the release cycle of this lib. Is it updated often and separately from Mangler? Else you could make the mangler.src.rpm build libventrilo3 and libventrilo3-devel packages, so if anything else wants to build with the lib, it would be available. There could still be a separate libventrilo3.src.rpm be added to the package collection at a later point (and depending on how upstream maintains these components).


> Requires:       gtkmm24 speex gsm espeak xosd

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


> %{_datadir}/applications/mangler.desktop

https://fedoraproject.org/wiki/Packaging:Guidelines#desktop-file-install_usage


> %{_libdir}/libventrilo3.la

Don't include libtool archives. It's explained in the static lib section:
https://fedoraproject.org/wiki/Packaging:Guidelines#Packaging_Static_Libraries
Comment 5 Palle Ravn 2013-08-01 06:57:08 EDT
(In reply to Christopher Meng from comment #3)
> Note if you split out devel package, don't include static libs.
> 
> 1. Please remove them via some commands like this:
> 
> find %{buildroot} -name '*.*a' -delete

All static libs should be removed now.

> 2. Don't adda BR of gcc-c++, please remove.

Done.

> 3. You'd better add isa macro to devel package Requires:
> 
> Requires:	   %{name}%{?_isa} = %{version}-%{release}

Done.

> 4. This package has bundled many libraries.
> 
> GPL (v2 or later) (with incorrect FSF address)
> ----------------------------------------------
> /var/lib/mock/fedora-rawhide-i386/root/builddir/build/BUILD/mangler-1.2.5/
> libventrilo3/ventrilo3_handshake.c
> /var/lib/mock/fedora-rawhide-i386/root/builddir/build/BUILD/mangler-1.2.5/
> libventrilo3/ventrilo_algo.h

Using rpmlint I only get an FSF address error from the debug rpm, only listing the file ventrilo3_handshake.c. However, the license should still be okay as it is GPLv3. If I notify upstream about the incorrect addresses this should not be a blocker.

> 5. This package has iphone and android files, I don't know if they are
> useful or not? Because licensecheck has found many license issues, but most
> are those files, so I have this question.

I can remove those, but it doesn't change anything in the final RPM. Likewise, the source RPM also includes the android and iphone files, as it contains the original tar file.

(In reply to Michael Schwendt from comment #4)
> > This is from libventrilo, which hasn't been packaged yet in Fedora.
> > 
> > https://github.com/haxar/libventrilo3
> 
> It's part of the Mangler project, isn't it?
> https://github.com/haxar?tab=repositories
> 
> Find out about the release cycle of this lib. Is it updated often and
> separately from Mangler? Else you could make the mangler.src.rpm build
> libventrilo3 and libventrilo3-devel packages, so if anything else wants to
> build with the lib, it would be available. There could still be a separate
> libventrilo3.src.rpm be added to the package collection at a later point
> (and depending on how upstream maintains these components).

It is part of Mangler and I can't find anyone else using this library. If it is not strictly required to have a separate library package, I would keep libventrilo3 and mangler in the same package, and split it up later if necessary.  

> > Requires:       gtkmm24 speex gsm espeak xosd
> 
> https://fedoraproject.org/wiki/Packaging:Guidelines#Explicit_Requires

Removed.

> > %{_datadir}/applications/mangler.desktop
> 
> https://fedoraproject.org/wiki/Packaging:Guidelines#desktop-file-
> install_usage

Now running desktop-file-validate as part of %install.
Comment 6 Christopher Meng 2013-09-15 23:58:59 EDT
(In reply to Palle Ravn from comment #5)

OK. Where is your new SPEC/SRPM?
Comment 7 Palle Ravn 2013-09-17 04:52:25 EDT
Just updated them, all files can be found at http://zom.dk/mangler/
Comment 8 Christopher Meng 2014-03-01 13:36:57 EST
404.
Comment 9 Palle Ravn 2014-03-26 16:36:19 EDT
I have lost the SPEC file. I hope to be able to remake it during the weekend.
Comment 10 Christopher Meng 2014-07-14 22:34:34 EDT
Lift needinfo on me if you are ready.

Note You need to log in before you can comment on or make changes to this bug.