Bug 464054 - Package Review Request for projectM-pulseaudio
Summary: Package Review Request for projectM-pulseaudio
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Orcan Ogetbil
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: libprojectM-qt
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-09-26 03:33 UTC by Jameson
Modified: 2016-08-14 16:27 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2008-11-14 12:46:47 UTC
Type: ---
Embargoed:
oget.fedora: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Jameson 2008-09-26 03:33:15 UTC
SPEC URL:  http://www.vtscrew.com/projectM-pulseaudio.spec
SRPM URL:  http://www.vtscrew.com/projectM-pulseaudio-1.2.0-1.fc9.src.rpm

Description:
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 06:49:37 UTC
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 22:04:57 UTC
* This package has licensing issues.
The COPYING file is GPLv2+. Also the following files claim GPLv2+:
   depcomp
   missing
   QPulseAudioDeviceModel.hpp

The file ConfigFile.h has a copy of MIT license, Modern Style with sublicense:
   http://fedoraproject.org/wiki/Licensing/MIT#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
   http://fedoraproject.org/wiki/Licensing#GPLCompatibilityMatrix
(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-07 01:24:14 UTC
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-07 02:35:23 UTC
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-07 03:00:47 UTC
Getting rid of the vendor tag gets rid of the extra desktop file.

Comment 6 Orcan Ogetbil 2008-11-07 03:34:15 UTC
"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-07 04:14:11 UTC
This should take care of all of the above.  I e-mailed upstream about the license.  The new SRPM is located:
http://www.vtscrew.com/projectM-pulseaudio-1.2.0-3.fc9.src.rpm

Comment 8 Orcan Ogetbil 2008-11-08 01:18:35 UTC
Great, one more done.

------------------------------------------------------
This package (projectM-pulseaudio) is approved by oget
------------------------------------------------------

Comment 9 Jameson 2008-11-12 01:10:30 UTC
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 18:30:30 UTC
cvs done.

Comment 11 Fedora Update System 2008-11-13 00:46:34 UTC
projectM-pulseaudio-1.2.0-3.fc9 has been submitted as an update for Fedora 9.
http://admin.fedoraproject.org/updates/projectM-pulseaudio-1.2.0-3.fc9

Comment 12 Fedora Update System 2008-11-14 12:46:43 UTC
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 05:44:06 UTC
projectM-pulseaudio-1.2.0-3.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/projectM-pulseaudio-1.2.0-3.fc10

Comment 14 Fedora Update System 2008-12-03 01:10:55 UTC
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 13:37:26 UTC
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 21:24:25 UTC
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.