Bug 717966 - Review Request: python-psphere - vSphere SDK for Python
Summary: Review Request: python-psphere - vSphere SDK for Python
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Chris Lalancette
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 721144
TreeView+ depends on / blocked
 
Reported: 2011-06-30 14:42 UTC by Chris Lalancette
Modified: 2013-08-29 14:02 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-07-27 09:34:46 UTC
markmc: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)
new spec (1.09 KB, text/plain)
2011-07-12 13:17 UTC, Mark McLoughlin
no flags Details

Description Chris Lalancette 2011-06-30 14:42:52 UTC
Spec URL: http://people.redhat.com/clalance/python-psphere/python-psphere.spec
SRPM URL: http://people.redhat.com/clalance/python-psphere/python-psphere-0.1-3.fc14.src.rpm

Description:
vSphere SDK for Python


[clalance@localhost SPECS]$ rpmlint python-psphere.spec
python-psphere.spec: W: invalid-url Source0: http://bitbucket.org/jkinred/psphere/get/python-psphere-0.1.tar.gz HTTP Error 500: Internal Server Error
0 packages and 1 specfiles checked; 0 errors, 1 warnings.
[clalance@localhost SPECS]$ rpmlint ../SRPMS/python-psphere-0.1-3.fc14.src.rpm
python-psphere.src: W: spelling-error Summary(en_US) vSphere -> v Sphere, sphere, spherule
python-psphere.src: W: summary-not-capitalized C vSphere SDK for Python
python-psphere.src: W: spelling-error %description -l en_US vSphere -> v Sphere, sphere, spherule
python-psphere.src: W: invalid-url Source0: http://bitbucket.org/jkinred/psphere/get/python-psphere-0.1.tar.gz HTTP Error 500: Internal Server Error
1 packages and 0 specfiles checked; 0 errors, 4 warnings.

(I believe all of the above warnings can be ignored; upstream doesn't do tarball releases)

Comment 1 Chris Lalancette 2011-07-05 18:47:26 UTC
We are still sorting out the license a bit with upstream.  The problem is that the upstream sources are GPLv3, but the program that wants to use them (imagefactory) is GPLv2.  We are discussing the best path forward with upstream, and I'll update the package once we have sorted it out.

Chris Lalancette

Comment 2 Chris Lalancette 2011-07-12 12:30:18 UTC
OK, we've sorted the license out for this package with the imagefactory.  This package will stay as GPLv3 for now, and the imagefactory as ASLv2, which is a valid combination.  So we should be good to go with the package review here.

Comment 3 Mark McLoughlin 2011-07-12 13:12:33 UTC
Okay, first up ... rpmlint output:

python-psphere.spec: W: invalid-url Source0: http://bitbucket.org/jkinred/psphere/get/python-psphere-0.1.tar.gz HTTP Error 500: Internal Server Error
0 packages and 1 specfiles checked; 0 errors, 1 warnings.

python-psphere.noarch: W: spelling-error Summary(en_US) vSphere -> v Sphere, sphere, spherule
python-psphere.noarch: W: summary-not-capitalized C vSphere SDK for Python
python-psphere.noarch: W: spelling-error %description -l en_US vSphere -> v Sphere, sphere, spherule
python-psphere.noarch: W: no-documentation
python-psphere.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/psphere/scripting.py 0644L /usr/bin/env
1 packages and 0 specfiles checked; 1 errors, 4 warnings.

What I think needs fixing:

  - There is no 0.1 release of psphere yet, so we should either wait for a
    release or (uggh) get into:

      http://fedoraproject.org/wiki/Packaging:NamingGuidelines#NonNumericRelease

  - We need either a URL for the tarball created by upstream, or no
    URL at all and instructions on how to regenerate the tarball from
    a tag. See:

      http://fedoraproject.org/wiki/Packaging:SourceURL#Referencing_Source

  - The non-executable-script error can be fixed using a variant of:

      http://fedoraproject.org/wiki/Packaging_tricks#Remove_shebang_from_Python_libraries

What can be ignored:

  - Spelling warnings about vSphere, it's correct

  - no-documentation warning - none is included in the tarball

  - The licensing change; no need to wait for that, GPLv3 is perfectly
    fine for inclusion into Fedora

Comment 4 Mark McLoughlin 2011-07-12 13:15:01 UTC
Okay, some more comments:

 - no need for the BuildRoot tag anymore
     http://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag

 - no need for %clean section anymore
     http://fedoraproject.org/wiki/Packaging/Guidelines#.25clean

 - the description should be longer and end with a period. Can just take
   the "Welcome" text from the website

 - the -n argument to %setup is redundant; you're using the default value

 - no need for Prefix tag

 - we should own the /usr/lib/python2.7/site-packages/psphere/ directory

 - recommendation is to avoid using INSTALLED_FILES
     http://fedoraproject.org/wiki/Packaging:Python#Byte_compiling
     http://fedoraproject.org/wiki/Packaging:Python_Eggs

 - use the %{__python} macro

 - URL: instead of Url:

 - should pass --skip-build argument to setuptools install

 - I'd avoid the optimize argument to setuptools install, no reason to
   override the default

Comment 5 Mark McLoughlin 2011-07-12 13:17:41 UTC
Created attachment 512426 [details]
new spec

Here's a new spec with all the comments above resolved if you want to use it

Comment 6 Mark McLoughlin 2011-07-12 13:19:04 UTC
If you use the new spec, the only thing that you still need is an upstream release - either a released tarball, or a tag in hg

Comment 7 Chris Lalancette 2011-07-12 13:54:03 UTC
Thanks for the review, Mark.

(In reply to comment #3)
> Okay, first up ... rpmlint output:
> 
> python-psphere.spec: W: invalid-url Source0:
> http://bitbucket.org/jkinred/psphere/get/python-psphere-0.1.tar.gz HTTP Error
> 500: Internal Server Error
> 0 packages and 1 specfiles checked; 0 errors, 1 warnings.
> 
> python-psphere.noarch: W: spelling-error Summary(en_US) vSphere -> v Sphere,
> sphere, spherule
> python-psphere.noarch: W: summary-not-capitalized C vSphere SDK for Python
> python-psphere.noarch: W: spelling-error %description -l en_US vSphere -> v
> Sphere, sphere, spherule
> python-psphere.noarch: W: no-documentation
> python-psphere.noarch: E: non-executable-script
> /usr/lib/python2.7/site-packages/psphere/scripting.py 0644L /usr/bin/env
> 1 packages and 0 specfiles checked; 1 errors, 4 warnings.
> 
> What I think needs fixing:
> 
>   - There is no 0.1 release of psphere yet, so we should either wait for a
>     release or (uggh) get into:
> 
>      
> http://fedoraproject.org/wiki/Packaging:NamingGuidelines#NonNumericRelease

This is the nastiest one here.  When you run "python setup.py sdist" on the hg sources, you get a tarball out that is named "psphere-0.1.tar.gz".  Given the "Referencing_Source" link below, I feel like we should stick with that version number, even though there is no official upstream release.

> 
>   - We need either a URL for the tarball created by upstream, or no
>     URL at all and instructions on how to regenerate the tarball from
>     a tag. See:
> 
>       http://fedoraproject.org/wiki/Packaging:SourceURL#Referencing_Source

Right, fixed this up according to the link above.

> 
>   - The non-executable-script error can be fixed using a variant of:
> 
>      
> http://fedoraproject.org/wiki/Packaging_tricks#Remove_shebang_from_Python_libraries

Stolen from your example SPEC, and fixed.

> 
> What can be ignored:
> 
>   - Spelling warnings about vSphere, it's correct
> 
>   - no-documentation warning - none is included in the tarball
> 
>   - The licensing change; no need to wait for that, GPLv3 is perfectly
>     fine for inclusion into Fedora

(In reply to comment #4)
> Okay, some more comments:
> 
>  - no need for the BuildRoot tag anymore
>      http://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag

Removed.

> 
>  - no need for %clean section anymore
>      http://fedoraproject.org/wiki/Packaging/Guidelines#.25clean

Removed.

> 
>  - the description should be longer and end with a period. Can just take
>    the "Welcome" text from the website

Fixed.

> 
>  - the -n argument to %setup is redundant; you're using the default value

Fixed.

> 
>  - no need for Prefix tag

Removed.

> 
>  - we should own the /usr/lib/python2.7/site-packages/psphere/ directory

Fixed.

> 
>  - recommendation is to avoid using INSTALLED_FILES
>      http://fedoraproject.org/wiki/Packaging:Python#Byte_compiling
>      http://fedoraproject.org/wiki/Packaging:Python_Eggs

Fixed.

> 
>  - use the %{__python} macro

Fixed.

> 
>  - URL: instead of Url:

Fixed.

> 
>  - should pass --skip-build argument to setuptools install

Fixed.

> 
>  - I'd avoid the optimize argument to setuptools install, no reason to
>    override the default

Fixed.

I've uploaded a new SPEC: http://people.redhat.com/clalance/python-psphere/python-psphere.spec and new SRPM: http://people.redhat.com/clalance/python-psphere/python-psphere-0.1-4.fc14.src.rpm

Comment 8 Mark McLoughlin 2011-07-12 16:48:01 UTC
> %{!?python_sitelib: %global python_sitelib %(python -c "from distutils.sysconfig import get_python_lib; print get_python_lib()")}

Not needed anymore:

http://fedoraproject.org/wiki/Packaging:Python#Macros

> %dir %attr(0755, root, root) %{python_sitelib}/psphere
> %{python_sitelib}/psphere/*

A simple:

  %{python_sitelib}/psphere/

does the exact same. See:

http://fedoraproject.org/wiki/Packaging:UnownedDirectories#Wildcarding_Files_inside_a_Created_Directory

Comment 9 Mark McLoughlin 2011-07-12 17:03:07 UTC
(In reply to comment #7)
> (In reply to comment #3)
> >   - There is no 0.1 release of psphere yet, so we should either wait for a
> >     release or (uggh) get into:
> > 
> >      
> > http://fedoraproject.org/wiki/Packaging:NamingGuidelines#NonNumericRelease
> 
> This is the nastiest one here.  When you run "python setup.py sdist" on the hg
> sources, you get a tarball out that is named "psphere-0.1.tar.gz".  Given the
> "Referencing_Source" link below, I feel like we should stick with that version
> number, even though there is no official upstream release.

Nope, that doesn't cut it - upstream could release 0.1 some time in the future with newer code

0.1 as a version number is meaningless right now - it doesn't refer to a specific version of the code

It sounds like you're in good contact with upstream - how about suggesting a release? That might be the simplest way of resolving it?

Otherwise, it should be python-vsphere-0.1-0.1.42accc3ffe67hg

Comment 10 Chris Lalancette 2011-07-12 17:24:02 UTC
(In reply to comment #8)
> > %{!?python_sitelib: %global python_sitelib %(python -c "from distutils.sysconfig import get_python_lib; print get_python_lib()")}
> 
> Not needed anymore:
> 
> http://fedoraproject.org/wiki/Packaging:Python#Macros

Ah, I had no idea about that one.  Fixed.

> 
> > %dir %attr(0755, root, root) %{python_sitelib}/psphere
> > %{python_sitelib}/psphere/*
> 
> A simple:
> 
>   %{python_sitelib}/psphere/

And fixed.

(In reply to comment #9)
> (In reply to comment #7)
> > (In reply to comment #3)
> > >   - There is no 0.1 release of psphere yet, so we should either wait for a
> > >     release or (uggh) get into:
> > > 
> > >      
> > > http://fedoraproject.org/wiki/Packaging:NamingGuidelines#NonNumericRelease
> > 
> > This is the nastiest one here.  When you run "python setup.py sdist" on the hg
> > sources, you get a tarball out that is named "psphere-0.1.tar.gz".  Given the
> > "Referencing_Source" link below, I feel like we should stick with that version
> > number, even though there is no official upstream release.
> 
> Nope, that doesn't cut it - upstream could release 0.1 some time in the future
> with newer code
> 
> 0.1 as a version number is meaningless right now - it doesn't refer to a
> specific version of the code
> 
> It sounds like you're in good contact with upstream - how about suggesting a
> release? That might be the simplest way of resolving it?
> 
> Otherwise, it should be python-vsphere-0.1-0.1.42accc3ffe67hg

I've written to the upstream maintainer, I'll wait on his reply.

Comment 11 Chris Lalancette 2011-07-14 15:14:34 UTC
OK, the upstream maintainer responded.  He:

a)  Changed the license to ASL 2.0, which I've updated in the SPEC and
b)  Done a formal release of psphere 0.1.4 upstream.

Based on this, I took the tarball from his release and used that to generate a new package.  It is available here:

SPEC: http://people.redhat.com/clalance/python-psphere/python-psphere.spec
SRPM: http://people.redhat.com/clalance/python-psphere/python-psphere-0.1.4-1.fc14.src.rpm

Comment 12 Mark McLoughlin 2011-07-14 16:56:36 UTC
Awesome, looks perfect now!

Comment 13 Chris Lalancette 2011-07-14 17:08:31 UTC
Thanks Mark.

Comment 14 Chris Lalancette 2011-07-14 17:09:15 UTC
New Package SCM Request
=======================
Package Name: python-psphere
Short Description: vSphere SDK for Python
Owners: clalance

Comment 15 Gwyn Ciesla 2011-07-14 23:38:44 UTC
Git done (by process-git-requests).

Comment 16 Chris Lalancette 2011-09-21 18:18:47 UTC
Package Change Request
======================
Package Name: python-psphere
New Branches: f15
Owners: clalance
InitialCC:

Comment 17 Gwyn Ciesla 2011-09-24 15:28:15 UTC
Git done (by process-git-requests).

Comment 18 Ian McLeod 2013-08-29 13:42:41 UTC
Package Change Request
======================
Package Name: python-psphere
New Branches: el6
Owners: imcleod

Comment 19 Gwyn Ciesla 2013-08-29 14:02:05 UTC
Git done (by process-git-requests).


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