Bug 464054 - Package Review Request for projectM-pulseaudio
Package Review Request for projectM-pulseaudio
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
All Linux
medium Severity medium
: ---
: ---
Assigned To: Orcan Ogetbil
Fedora Extras Quality Assurance
Depends On: libprojectM-qt
  Show dependency treegraph
Reported: 2008-09-25 23:33 EDT by Jameson
Modified: 2016-08-14 12:27 EDT (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2008-11-14 07:46:47 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
oget.fedora: fedora‑review+
kevin: fedora‑cvs+

Attachments (Terms of Use)

  None (edit)
Description Jameson 2008-09-25 23:33:15 EDT
SPEC URL:  http://www.vtscrew.com/projectM-pulseaudio.spec
SRPM URL:  http://www.vtscrew.com/projectM-pulseaudio-1.2.0-1.fc9.src.rpm

This package allows the use of the projectM visualization plugin through any
pulseaudio compatible applications.

No rpmlint errors or warnings.
Comment 1 Jameson 2008-11-06 01:49:37 EST
I have a new SPEC in original location, and a new SRPM:   http://www.vtscrew.com/projectM-pulseaudio-1.2.0-2.fc9.src.rpm
Comment 2 Orcan Ogetbil 2008-11-06 17:04:57 EST
* This package has licensing issues.
The COPYING file is GPLv2+. Also the following files claim GPLv2+:

The file ConfigFile.h has a copy of MIT license, Modern Style with sublicense:

The file qprojectM-pulseaudio.cpp claims LGPLv2+ which causes the problem. This file links to libprojectM-qt, which is licensed as GPLv2+. But according to
(the bottom matrix) you can't release a project under LGPLv2+ if you link to a library that is licensed under GPLv2+. This project has to be converted to GPLv2+.
According to the footnote 7 under the matrix, you are allowed to do that. Then the License tag in the SPEC should be
   License: GPLv2+ and MIT
But please confirm this with the upstream.

* According to the recent changes in the guidelines, for the new packages, you shouldn't have the --vendor tag in the desktop-file-install.

* Why do you have two .desktop files? They look pretty much the same to me. One should be removed.

* I'm not sure if you need
   Requires:	libprojectM = %{version}, pulseaudio
I will check this (you should check this too). Also I will finish the review sometime tonight or tomorrow.
Comment 3 Orcan Ogetbil 2008-11-06 20:24:14 EST
OK, here's the rest of the review:

* The files AUTHORS and NEWS are empty and useless. It's ok to not package them. But keep an eye on them whenever there is an update by upstream.

* Requires: libprojectM = %{version}
is definitely not needed. I'm not very sure about the 
  Requires: pulseaudio
rpmbuild picks up pulseaudio-libs as a dependency. This is probably enough. Is there a particular reason why you required pulseaudio?

* The line 
   find $RPM_BUILD_ROOT -name '*.la' -exec rm -f {} ';'
doesn't seem necessary since no .la files are built. Is this also true for libprojectM-qt? Can you check?

* Please make use of the %{name} macro whenever possible, e.g. in the %install and %files sections.

* It would be nice if you add a Generic Name to the .desktop file. Also "Application" is forbidden in the category field.
Comment 4 Jameson 2008-11-06 21:35:23 EST
Do I need both the fedora-projectM-pulseaudio.desktop and projectM-pulseaudio.desktop, or should I delete the latter?  Also, what kind of generic name should I use?  Something like Pulseaudio Music Visualization?  I'll talk to upstream about switching the LGPL'd file to GPL.  I think what happened was that they originally had everything licensed under the GPL, and changed the core package to license it to a commercial game developer.
Comment 5 Jameson 2008-11-06 22:00:47 EST
Getting rid of the vendor tag gets rid of the extra desktop file.
Comment 6 Orcan Ogetbil 2008-11-06 22:34:15 EST
"Pulseaudio Music Visualization" is not bad. Something like just "Pulseaudio Visualization" may work too. There is no strict rule for this. Some people just copy the contents of Name into Generic Name, but I don't like it that way.
It's really up to you.

So that's what the vendor tag does. I'm learning this with you :) 
Removal of the vendor tag was decided in the mailing list a while ago, just when I started packaging stuff. I see that it didn't make its way to the Guidelines yet.
Comment 7 Jameson 2008-11-06 23:14:11 EST
This should take care of all of the above.  I e-mailed upstream about the license.  The new SRPM is located:
Comment 8 Orcan Ogetbil 2008-11-07 20:18:35 EST
Great, one more done.

This package (projectM-pulseaudio) is approved by oget
Comment 9 Jameson 2008-11-11 20:10:30 EST
New Package CVS Request
Package Name: projectM-pulseaudio
Short Description: projectM visualization through pulseaudio
Owners: imntreal
Branches: F-8 F-9 F-10
InitialCC: imntreal
Comment 10 Kevin Fenzi 2008-11-12 13:30:30 EST
cvs done.
Comment 11 Fedora Update System 2008-11-12 19:46:34 EST
projectM-pulseaudio-1.2.0-3.fc9 has been submitted as an update for Fedora 9.
Comment 12 Fedora Update System 2008-11-14 07:46:43 EST
projectM-pulseaudio-1.2.0-3.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 13 Fedora Update System 2008-11-30 00:44:06 EST
projectM-pulseaudio-1.2.0-3.fc10 has been submitted as an update for Fedora 10.
Comment 14 Fedora Update System 2008-12-02 20:10:55 EST
projectM-pulseaudio-1.2.0-3.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 15 Ernesto Manríquez 2010-01-16 08:37:26 EST
projectM-pulseaudio needs to be updated in Fedora 12. The mix of libprojectM 2.0 and projectM-pulseaudio-1.2 leads to a black screen.
Comment 16 Jason Tibbitts 2010-01-20 16:24:25 EST
Commenting in the review ticket is not the proper way to report bugs against an existing package.  This ticket is closed; if you wish to report bugs in this package, you should open a ticket against it.

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