Bug 813542

Summary: Review Request: python-pivy - Python binding for Coin
Product: [Fedora] Fedora Reporter: Richard Shaw <hobbes1069>
Component: Package ReviewAssignee: Kalev Lember <kalevlember>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: john, kalevlember, kwizart, notting, package-review
Target Milestone: ---Flags: kalevlember: fedora-review+
john: needinfo-
gwync: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: python-pivy-0.5.0-3.hg609.fc17 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2012-04-27 06:05:39 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:

Description Richard Shaw 2012-04-17 19:56:56 UTC
SPEC: http://hobbes1069.fedorapeople.org/Pivy/Pivy.spec
SRPM: http://hobbes1069.fedorapeople.org/Pivy/Pivy-0.5.0-1.hg609.fc16.src.rpm
Description:
Pivy is a Coin binding for Python. Coin is a high-level 3D graphics library with
a C++ Application Programming Interface. Coin uses scene-graph data structures
to render real-time graphics suitable for mostly all kinds of scientific and
engineering visualization applications.

rpmlint output:
Pivy.src:51: E: hardcoded-library-path in %{_prefix}/lib
Needed because it doesn't understand /usr/lib{,64}

Pivy.src: W: invalid-url Source0: Pivy-0.5.0-hg609.tar.gz
mecurial checkout, instructions for checkout are in the spec.

Comment 1 Richard Shaw 2012-04-17 20:06:23 UTC
*** Bug 458975 has been marked as a duplicate of this bug. ***

Comment 2 Kalev Lember 2012-04-18 01:53:51 UTC
Taking for review.

Instead of 'Pivy', I believe this package should be named 'python-pivy', as per python naming guidelines:

"Packages of python modules (thus they rely on python as a parent) use a slightly different naming scheme. They should take into account the upstream name of the python module. This makes a package name format of python-$NAME. When in doubt, use the name of the module that you type to import it in a script."

http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Addon_Packages_.28python_modules.29

Comment 3 Richard Shaw 2012-04-18 13:31:09 UTC
Ok, fixed. Is there anything else I need to fix before posting new links?

Thanks,
Richard

Comment 4 Kalev Lember 2012-04-18 14:18:45 UTC
No, please post new links. I haven't looked in it in detail yet, but the packaging seems nice and clean at least.

Comment 6 Kalev Lember 2012-04-18 15:52:11 UTC
Fedora review python-pivy-0.5.0-2.hg609.fc16.src.rpm 2012-04-18

+ OK
! needs attention

rpmlint output:
$ rpmlint python-pivy \
          python-pivy-debuginfo \
          python-pivy-0.5.0-2.hg609.fc16.src.rpm
python-pivy.src:52: E: hardcoded-library-path in %{_prefix}/lib
python-pivy.src: W: invalid-url Source0: Pivy-0.5.0-hg609.tar.gz
3 packages and 0 specfiles checked; 1 errors, 1 warnings.

+ Rpmlint warnings/errors are harmless and can be ignored
+ The package is named according to Fedora packaging guidelines
+ The spec file name matches the base package name.
+ The package meets the Packaging Guidelines
+ The package is licensed with a Fedora approved license and meets the
  Licensing Guidelines.

! The license field in the spec file doesn't match the actual license

The spec file currently states that it's BSD licensed. However, the license
headers in the source code and the LICENSE file appear to specify the ISC
license, which, according to the list of allowed licenses [1], should use
"License: ISC" tag in the spec file.

+ The package contains the license file (LICENSE)
+ Spec file is written in American English
+ Spec file is legible
+ Upstream hg checkout sources match sources in the srpm. 
+ The package builds in koji
n/a ExcludeArch bugs filed
+ BuildRequires look sane
n/a The spec file handles locales properly
n/a ldconfig in %post and %postun
+ Package does not bundle copies of system libraries
n/a Package isn't relocatable
+ Package owns all the directories it creates
+ No duplicate files in %files
+ Permissions are properly set
+ Consistent use of macros
+ The package must contain code or permissible content
n/a Large documentation files should go in -doc subpackage
+ Files marked %doc should not affect package
n/a Header files should be in -devel
n/a Static libraries should be in -static
n/a Library files that end in .so must go in a -devel package
n/a -devel must require the fully versioned base
+ Packages should not contain libtool .la files
n/a Proper .desktop file handling
+ Doesn't own files or directories already owned by other packages
+ Filenames are valid UTF-8


Issues
------
 1) The spec 'BSD' vs 'ISC' licensing issue outlined above
 2) Maybe it'd be better to use pre-release version numbering [2], in case
    upstream decides to release 0.5.0 final some day?
    0.5.0-0.2.hg609 instead of
    0.5.0-2.hg609

[1] http://fedoraproject.org/wiki/Licensing#Good_Licenses
[2] http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Pre-Release_packages

Comment 7 Kalev Lember 2012-04-18 15:55:36 UTC
Oone more suggestion I forgot -- might be better to compress the sources with xz instead of gz to save some space.

Comment 8 Richard Shaw 2012-04-18 16:04:12 UTC
(In reply to comment #6)
> Issues
> ------
>  1) The spec 'BSD' vs 'ISC' licensing issue outlined above

Fixed.


>  2) Maybe it'd be better to use pre-release version numbering [2], in case
>     upstream decides to release 0.5.0 final some day?
>     0.5.0-0.2.hg609 instead of
>     0.5.0-2.hg609

I thought this was a post-release checkout but I'll check.


(In reply to comment #7)
> Oone more suggestion I forgot -- might be better to compress the sources with
> xz instead of gz to save some space.

I would but mecurial doesn't seem to support it in their "hg archive" command.

New links to follow.

Comment 9 Richard Shaw 2012-04-18 17:48:27 UTC
Ok, both setup.py and NEWS says it's 0.5.0 so I assume it is a post-release snapshot.

SPEC: http://hobbes1069.fedorapeople.org/Pivy/python-pivy.spec
SRPM:
http://hobbes1069.fedorapeople.org/Pivy/python-pivy-0.5.0-3.hg609.fc16.src.rpm

Comment 10 Kalev Lember 2012-04-18 17:58:48 UTC
Looks good. Don't forget to update the bugzilla ticket's summary for Pivy -> python-pivy rename before filing the SCM request.

APPROVED

Comment 11 Richard Shaw 2012-04-18 18:22:05 UTC
Thanks for the review!

Comment 12 Richard Shaw 2012-04-18 18:22:55 UTC
New Package SCM Request
=======================
Package Name: python-pivy
Short Description: Python binding for Coin
Owners: hobbes1069
Branches: f16 f17
InitialCC:

Comment 13 Gwyn Ciesla 2012-04-18 18:27:47 UTC
Git done (by process-git-requests).

Comment 14 Fedora Update System 2012-04-18 22:33:23 UTC
python-pivy-0.5.0-3.hg609.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/python-pivy-0.5.0-3.hg609.fc16

Comment 15 Fedora Update System 2012-04-18 22:33:33 UTC
python-pivy-0.5.0-3.hg609.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/python-pivy-0.5.0-3.hg609.fc17

Comment 16 Fedora Update System 2012-04-19 05:23:22 UTC
python-pivy-0.5.0-3.hg609.fc16 has been pushed to the Fedora 16 testing repository.

Comment 17 Fedora Update System 2012-04-27 06:05:39 UTC
python-pivy-0.5.0-3.hg609.fc16 has been pushed to the Fedora 16 stable repository.

Comment 18 Fedora Update System 2012-04-29 00:56:35 UTC
python-pivy-0.5.0-3.hg609.fc17 has been pushed to the Fedora 17 stable repository.

Comment 19 John Morris 2012-10-17 20:53:53 UTC
Package Change Request
======================
Package Name: python-pivy
New Branches: el6
Owners: zultron hobbes1069
InitialCC: 

The owner of this package (hobbes1069) and I (zultron) are building this package for EPEL6.

Comment 20 Gwyn Ciesla 2012-10-17 21:26:12 UTC
Git done (by process-git-requests).

Comment 21 Richard Shaw 2014-05-28 18:30:39 UTC
Package Change Request
======================
Package Name: python-pivy
New Branches: epel7
Owners: zultron hobbes1069
InitialCC:

Comment 22 Gwyn Ciesla 2014-05-28 19:02:22 UTC
Git done (by process-git-requests).