Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.

Bug 233597

Summary: Review Request: pigment - Media Center Toolkit
Product: [Fedora] Fedora Reporter: Matthias Saou <matthias>
Component: Package ReviewAssignee: Jef Spaleta <jspaleta>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: jspaleta, lemenkov
Target Milestone: ---Flags: jspaleta: fedora-review+
wtogami: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2007-05-02 10:27:19 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
Bug Depends On:    
Bug Blocks: 233598    

Description Matthias Saou 2007-03-23 12:32:55 UTC
Spec URL: http://ftp.es6.freshrpms.net/tmp/extras/pigment/pigment.spec
SRPM URL: http://ftp.es6.freshrpms.net/tmp/extras/pigment/pigment-0.1.4-1.src.rpm
Description:
Pigment is a toolkit for writing Media Center software.

Comment 1 Jef Spaleta 2007-04-13 05:51:37 UTC
Is this under active review along with elisa?  If not i can start working on
reviewing these now.  

-jef

Comment 2 Matthias Saou 2007-04-13 09:18:35 UTC
Status is not ASSIGNED yet, so I guess you could review :-)

Comment 3 Jef Spaleta 2007-04-15 05:25:32 UTC
uhm lxtnow@gmail.com took assignment for this and the elisa ticket. 

Comment 4 Jef Spaleta 2007-04-15 05:43:41 UTC
Okay first thing to note the SOURCE0 is wrong
it now needs to be
http://elisa.fluendo.com/static/download/pigment/pigment-0.1.4.tar.gz

looks like they've re-organized.

-jef

Comment 5 Jef Spaleta 2007-04-15 06:01:11 UTC
I think we'll need to a legal review for pigment and elisa.
The actual license is GPL with an additional licensing exception for fluendo
non-gpl plugins. While I personally don't have a problem with that, it's still
not strictly GPL and I think we need a legal review of the additional licensing
terms.

Matthias, do you have any comments before I set the review failure and block on
FE-LEGAL?
 
-jef

Comment 6 Ralf Corsepius 2007-04-15 06:18:02 UTC
(In reply to comment #5)
> I think we'll need to a legal review for pigment and elisa.
> The actual license is GPL with an additional licensing exception for fluendo
> non-gpl plugins. While I personally don't have a problem with that, it's still
> not strictly GPL and I think we need a legal review of the additional licensing
> terms.

IMO, this is just a case of "GPL with exceptions".


Comment 7 Jef Spaleta 2007-04-15 06:39:58 UTC
Okay here's the run down of the rest of the technical review blockers.

- pigment-devel needs to require gtk-doc for directory ownership of
/usr/share/gtk-doc/html/

- need to exclude /usr/lib/pigment-0.1/gstreamer/libpgmrendersink.la
- need to exclude /usr/lib/pigment-0.1/libpgmrendergl1.la

- fix the SOURCE and URL tags to use 
elisa.fluendo.com instead of www.fluendo.com

Additional non-blocking suggestion:
 remove glib2-devel BuildRequires as its pulled in by gtk2-devel

rpmlint returns clean on my locally rpmbuild --rebuild built development binaries

One question:
does pigment count as a python addon, and if so does the python naming scheme
require python-pigment for the package name?


-jef

Comment 8 Jef Spaleta 2007-04-15 06:44:02 UTC
(In reply to comment #6)
> IMO, this is just a case of "GPL with exceptions".

Doesn't each unique exceptional clause demand a review? While I'm personally
fine with the fluendo plugin exception, I'd like to avoid a problem later on
after this is published.

-jef


Comment 9 Matthias Saou 2007-04-16 09:27:36 UTC
Updated packages here, which fix all problems reported :
Spec URL: http://ftp.es6.freshrpms.net/tmp/extras/pigment/pigment.spec
SRPM URL: http://ftp.es6.freshrpms.net/tmp/extras/pigment/pigment-0.1.4-2.src.rpm
(not the glib2-devel BR, since I prefer keeping it in this particular case)

About the license, I also think it's fine, but feel free to ask on the
mailing-list or even get FE-LEGAL to confirm if you think it's required.

About the name... indeed, pigment should maybe be considered like a python
package. I didn't really think about it, and since it's not just a python
module, but also a system library (libpgmrender.so) and a gstreamer plugin
(libpgmrendersink.so), it might be best to not rename it "python-pigment".

Comment 10 Matthias Saou 2007-04-16 10:46:21 UTC
Spec URL: http://ftp.es6.freshrpms.net/tmp/extras/pigment/pigment.spec
SRPM URL: http://ftp.es6.freshrpms.net/tmp/extras/pigment/pigment-0.1.5-1.src.rpm

* Mon Apr 16 2007 Matthias Saou <http://freshrpms.net/> 0.1.5-1
- Update to 0.1.5.
- Get rid of the /usr/lib64 RPATH on 64bit.

Comment 11 Jef Spaleta 2007-04-17 04:34:08 UTC
Good catch on the rpath

Things look pretty good, just looking over the build log of 0.1.5-1 on x86 and i
still see multiple references to -rpath. Are these a problem as well?

-rpath /usr/lib
-rpath /usr/lib/pigment-0.1/gstreamer
-rpath /usr/lib/python2.5/site-packages

-jef

Comment 12 Jef Spaleta 2007-04-18 06:54:16 UTC
okay those -rpath calls are false alarms, running a local build on x86 with
check-rpaths doesn't abort.

0.1.5 APPROVED
Looks good. I'm going to take assignment of this as the reviewer and flag it as
approved.  From the discussion on maintainers-list its pretty clear that this
GPL exception falls into normal allowed license practices. 

+ All build dependencies listed in BuildRequires, 
+ Packages do not contain any .la libtool archives.
+ The package named according to the Package Naming Guidelines.
  contains shared library as well as python bindings... 
  not strictly a python addon.
+ MUST: The package must meet the Packaging Guidelines.
+ The package is licensed with GPL with special exception
+ License field in the package spec is GPL.
+ The sources used to build the package match the upstream source, 
   http://elisa.fluendo.com/static/download/pigment/pigment-0.1.5.tar.gz
   md5sum   d39000c031e35d5a5835343161ce4bf8
   matches included source
+ rpmlint appears to return cleanly
+ The spec file name must match the base package %{name}
+ includes the text of the license(s) in %doc.
+ The spec file is in english-ese.
+ The spec file for the package is legible. 
+ The package must successfully compiles and builds into binary rpms 
   on x86 developement using mock
+ no locales to worry about
+ ldconfig in %post and %postun. 
+ package not relocatable
+ owns all directories that it creates. 
+ no duplicate files in the %files listing.
+ Permissions on files set properly. 
+ %clean section, which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT).
+ consistent use of macros
+ The package contains code, or permissable content. 
+ no doc subpackage. gtk-docs are placed in the -devel package 
+ %doc items do not affect runtime      
+ Header files are in a -devel package.
+ no statics
+ -devel package correctly 'Requires: pkgconfig'
+ library files that end in .so (without suffix) are in a -devel package.
+ devel packages requires the base package using a fully versioned dependency
+ No GUI apps
+ Doesn't duplicate directory or file ownership afaict
+ beginning of %install has rm -rf %{buildroot} (or $RPM_BUILD_ROOT).


Comment 13 Matthias Saou 2007-04-18 09:44:53 UTC
New Package CVS Request
=======================
Package Name: pigment
Short Description: Media Center Toolkit
Owners: matthias@rpmforge.net
Branches: devel FC-6 EL-5
(only the most current branches as Elisa won't make much sense on old releases)
InitialCC: 

Comment 14 Jef Spaleta 2007-04-29 04:56:25 UTC
ping!
Matthias, you forgot to set the fedora-cvs flag.  I'm setting it now.

-jef"wondering why i hadn't seen a pigment build notice, cuz I'm really keen on
doing the review for elisa after pigments in the tree"spaleta


Comment 15 Matthias Saou 2007-05-02 10:27:19 UTC
Oh, indeed, my bad! It's all done now, and rebuilt. Thanks!