Bug 233597
Summary: | Review Request: pigment - Media Center Toolkit | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Matthias Saou <matthias> |
Component: | Package Review | Assignee: | Jef Spaleta <jspaleta> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | 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: | --- | Target Upstream Version: | |
Embargoed: | |||
Bug Depends On: | |||
Bug Blocks: | 233598 |
Description
Matthias Saou
2007-03-23 12:32:55 UTC
Is this under active review along with elisa? If not i can start working on reviewing these now. -jef Status is not ASSIGNED yet, so I guess you could review :-) uhm lxtnow took assignment for this and the elisa ticket. 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 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 (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". 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 (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 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". 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. 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 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). New Package CVS Request ======================= Package Name: pigment Short Description: Media Center Toolkit Owners: matthias Branches: devel FC-6 EL-5 (only the most current branches as Elisa won't make much sense on old releases) InitialCC: 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 Oh, indeed, my bad! It's all done now, and rebuilt. Thanks! |