Bug 1123044 - Review Request: python-pyrax - A Rackspace/Openstack client library
Summary: Review Request: python-pyrax - A Rackspace/Openstack client library
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Garrett Holmstrom
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2014-07-24 17:20 UTC by Ian McLeod
Modified: 2014-11-02 10:51 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2014-11-02 10:51:53 UTC
gholms: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Ian McLeod 2014-07-24 17:20:22 UTC
Spec URL: http://imcleod.fedorapeople.org/src/pyrax/pyrax.spec
SRPM URL: http://imcleod.fedorapeople.org/src/pyrax/pyrax-1.9.0-1.fc20.src.rpm
Description: pyrax is a client library for interacting with Rackspace and other OpenStack based clouds.  It includes Rackspace-specific features not found in the generic Openstack clients
Fedora Account System Username: imcleod

Comment 1 Ian McLeod 2014-07-24 19:55:00 UTC
The fork that I am using for RPM-specific changes only is here:

https://github.com/imcleod/pyrax

Comment 2 Ian McLeod 2014-07-24 21:55:56 UTC
Per feedback from gholms I have redone the release string to indicate that this is a git snapshot from my fork.  The new SRPM is:

https://imcleod.fedorapeople.org/src/pyrax/python-pyrax-1.9.0-1.20140724gitee6acc6.fc20.src.rpm

The SPEC and source files in this directory have been updated as well.

Comment 3 Garrett Holmstrom 2014-07-24 23:59:35 UTC
Most everything looks sane here.  The package just needs updating to address a few complaints from rpmlint and changes in packaging guidelines over the past few years.  I am also concerned about licensing for a bit of copypasted code that the software includes in utils.py, so perhaps we should bug legal about it to be sure that it is okay.

Please fix:
 - Non-executable scripts (I would remove the #! lines from the top of
   python modules.)
 - Licensing:  utils.py contains BSD code with no accompanying header, but
   the author appears to have not published said header.  We might need to
   check with the legal team on this.
 - Add commentary that explains how to construct the tarball for your git
   snapshot.
 - Add BuildRequires: python2-devel.
 - Prepend "0." to the Release field.
   http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Snapshot_packages
 - Use %global to set %python_sitelib, not %define.
 - Use python2 instead of %{__python}.
 - Use %{python2_sitelib} instead of %{python_sitelib}.

The wiki has boilerplate for the latter three here:
http://fedoraproject.org/wiki/Packaging:Python#Macros

I also suggest:
 - Add a version to Provides: pyrax

The nitty-gritty details follow.

Mandatory review guidelines:
NO - rpmlint output:
     python-pyrax.noarch: E: explicit-lib-dependency python-httplib2
     python-pyrax.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/pyrax/version.py 0644L /usr/bin/env
     python-pyrax.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/pyrax/queueing.py 0644L /usr/bin/env
     python-pyrax.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/pyrax/identity/keystone_identity.py 0644L /usr/bin/env
     python-pyrax.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/pyrax/base_identity.py 0644L /usr/bin/env
     python-pyrax.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/pyrax/cloudblockstorage.py 0644L /usr/bin/env
     python-pyrax.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/pyrax/utils.py 0644L /usr/bin/env
     python-pyrax.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/pyrax/exceptions.py 0644L /usr/bin/env
     python-pyrax.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/pyrax/cloudloadbalancers.py 0644L /usr/bin/env
     python-pyrax.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/pyrax/autoscale.py 0644L /usr/bin/env
     python-pyrax.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/pyrax/fakes.py 0644L /usr/bin/env
     python-pyrax.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/pyrax/object_storage.py 0644L /usr/bin/env
     python-pyrax.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/pyrax/clouddns.py 0644L /usr/bin/env
     python-pyrax.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/pyrax/identity/__init__.py 0644L /usr/bin/env
     python-pyrax.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/pyrax/cloudnetworks.py 0644L /usr/bin/env
     python-pyrax.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/pyrax/identity/rax_identity.py 0644L /usr/bin/env
     python-pyrax.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/pyrax/image.py 0644L /usr/bin/env
     python-pyrax.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/pyrax/client.py 0644L /usr/bin/env
     python-pyrax.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/pyrax/__init__.py 0644L /usr/bin/env
     python-pyrax.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/pyrax/clouddatabases.py 0644L /usr/bin/env
     python-pyrax.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/pyrax/cloudmonitoring.py 0644L /usr/bin/env
     python-pyrax.src: W: spelling-error %description -l en_US cloudfiles -> cloud files, cloud-files, cloudless
     python-pyrax.src:14: W: unversioned-explicit-provides pyrax
     python-pyrax.src: W: file-size-mismatch pyrax-1.9.0.tar.gz = 310594, http://imcleod.fedorapeople.org/src/pyrax/pyrax-1.9.0.tar.gz = 310590
     2 packages and 0 specfiles checked; 21 errors, 3 warnings.
ok - License is acceptable (ASL 2.0)
NO - License field in spec is correct
     utils.py includes third party BSD code without its corresponding notice.
     Original source contains no such notice; need to check with legal as to
     acceptability.
ok - License files included in package %docs if included in source package
     Note that a guideline change to use the %license macro is pending:
     https://fedorahosted.org/fpc/ticket/411
ok - License files installed when any subpackage combination is installed
ok - Spec written in American English
ok - Spec is legible
NO - Sources match upstream unless altered to fix permissibility issues
     This is a git snapshot.  Please include commentary that instructs one on
     how to construct the source tarball.
ok - Build succeeds on at least one primary arch
ok - Build succeeds on all primary arches or has ExcludeArch + justification
NO - BuildRequires correct, justified where necessary
     Need to add BuildRequires: python2-devel
-- - Locales handled with %find_lang, not %_datadir/locale/*
-- - %post, %postun call ldconfig if package contains shared .so files
ok - No bundled libs
-- - Relocatability is justified
ok - Package owns all directories it creates
ok - Package requires others for directories it uses but does not own
ok - No duplication in %files unless necessary for license files
ok - File permissions are sane
ok - Package contains permissible code or content
-- - Large docs go in -doc subpackage
ok - %doc files not required at runtime
-- - Static libs go in -static package/virtual Provides
-- - Development files go in -devel package
-- - -devel packages Require base with fully-versioned dependency, %_isa
-- - No .la files
-- - GUI app uses .desktop file, installs it with desktop-file-install
ok - File list does not conflict with other packages' without justification
ok - File names are valid UTF-8

Optional review guidelines:
-- - Query upstream about including license files
no - Translations of description, summary
ok - Builds in mock
ok - Builds on all arches
-- - Scriptlets are sane
-- - Subpackages require base with fully-versioned dependency if sensible
-- - .pc file subpackage placement is sensible
ok - No file deps outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin
-- - Include man pages if available

Naming guidelines:
ok - Package names use only a-zA-Z0-9-._+ subject to restrictions on -._+
ok - Package names are sane
ok - No naming conflicts
ok - Spec file name matches base package name
     "python" prefix mandated by python guidelines
ok - Version is sane
ok - Version does not contain ~
NO - Release is sane
     Pre-release snapshots must begin with "0."
ok - %dist tag
ok - Case used only when necessary
ok - Package names follow applicable language/addon rules

Packaging guidelines:
ok - Useful without external bits
ok - No kmods
ok - Pre-built binaries, libs removed in %prep
ok - Sources contain only redistributable code or content
ok - Spec format is sane
-- - noarch package with unported deps has correct ExclusiveArch
-- - Arch-specific sources/patches are applied, not included, conditionally
ok - Package obeys FHS, except libexecdir, /run, /usr/target
-- - %{_prefix}/lib only used for multilib-exempt packages
-- - Programs run before FS mounting use /run instead of /var/run
ok - No files under /srv, /opt, /usr/local
ok - Changelog in prescribed format
ok - No Packager, Vendor, Copyright, PreReq tags
ok - Summary does not end in a period
ok - Requires correct, justified where necessary
ok - BuildRequires lack %{_isa}
ok - Summary, description do not use trademarks incorrectly
ok - All relevant documentation is packaged, appropriately marked with %doc
ok - Doc files do not drag in extra dependencies (e.g. due to +x)
-- - Code compilable with gcc is compiled with gcc
-- - Build honors applicable compiler flags or justifies otherwise
-- - PIE used for long-running/root daemons, setuid/filecap programs
-- - Useful -debuginfo package or disabled and justified
-- - No static executables (except OCaml)
-- - Rpath absent or only used for internal libs
-- - Config files marked with %config(noreplace) or justified %config
ok - No config files under /usr
-- - Third party package manager configs acceptable, only in %_docdir
-- - .desktop files are sane
-- - desktop-file-install/validate run on .desktop files, as appropriate
-- - No desktop-file-install --vendor on >= F19
ok - Spec uses macros consistently
ok - Spec uses macros instead of hard-coded names where appropriate
NO - Spec uses macros for executables only when configurability is needed
     %{__python} is deprecated; use python2 or %{__python2} instead
-- - %makeinstall used only when alternatives don't work
-- - Macros in Summary, description are expandable at srpm build time
-- - Spec uses %{SOURCE#} instead of $RPM_SOURCE_DIR and %sourcedir
ok - No software collections (scl)
-- - Macro files named /etc/rpm/macros.%name or /usr/lib/rpm/macros.%name
-- - Macro files not marked with %config
ok - Build uses only python/perl/shell+coreutils/lua/BuildRequired langs
NO - %global, not %define
     Use %global to set %python_sitelib, not %define
-- - Package translating with gettext BuildRequires it
-- - Package translating with Linguist BuildRequires qt-devel
-- - File ops preserve timestamps
-- - Parallel make
-- - User, group creation handled correctly (See Packaging:UsersAndGroups)
-- - Web apps go in /usr/share/%name, not /var/www
-- - Conflicts are justified
ok - One project per package
ok - No bundled fonts
-- - Patches have appropriate commentary
no - Available test suites executed in %check
-- - tmpfiles.d used for /run, /run/lock on >= F15
-- - Package renaming/replacement handled correctly
ok - Package builds without network access

Python guidelines:
ok - Runtime Requires correct
NO - BuildRequires: python2-devel and/or python3-devel
     python2-devel is required
NO - Spec uses versioned macros
     Use python2 or %{__python2} instead of %{__python}
     Use %{python2_sitelib} instead of %{python_sitelib}
     For boilerplate see: http://fedoraproject.org/wiki/Packaging:Python#Macros
ok - All .py files packaged with .pyc, .pyo counterparts
ok - INSTALLED_FILES not used for %files list
ok - Includes .egg-info files/directories when generated
ok - .py not under site-libs byte-compiled against correct runtimes
-- - Python 3 built as upstream instructs, if at all
-- - Patches are not specific to python 2 or 3 when sources are combined
-- - Non-parallel-installable scripts only installed for default runtime
ok - Eggs built from source
ok - Eggs do not download deps during build
-- - Compat packages use easy_install -m to avoid conflicts
ok - At least one version of each module must be importable w/o version
ok - Provides/Requires properly filtered
-- - Code that invokes gtk.gdk.get_pixels_array() Requires numpy

Comment 4 Ian McLeod 2014-07-28 18:39:54 UTC
I believe all the above concerns are addressed in:

https://imcleod.fedorapeople.org/src/pyrax/python-pyrax-1.9.0-1.20140728git2c89bfc.fc20.src.rpm

With one exception.  Because this is based off of the official upstream 1.9.0 release I have given it a release string of "1." plus the date and git tags.

The tarball and SPEC in the /src/pyrax directory are updated as well.

Comment 6 Ian McLeod 2014-07-28 20:38:29 UTC
OK.  Based on further discussion, here is a cleanly named incremental
change:

https://imcleod.fedorapeople.org/src/pyrax/python-pyrax-1.9.0-2.fc20.src.rpm

Details remain at the same upstream git location.

I have committed to maintaining an accurate RPM changelog as long as
I am responsible for package builds.

Comment 7 Garrett Holmstrom 2014-07-30 21:13:58 UTC
That looks good to me.  Thanks!

I would also like to note that although Ian intends to maintain this package's spec file in the upstream source tree rather than Fedora's, he has committed to merging changes from Fedora's packaging repository and not simply overwriting them.

Just don't forget to be vigilant about that, Ian.  8^)

I also suggest:
 - Provide a pyrax release as well (Provides: pyrax = %{version}-%{release})
 - Use %license for the COPYING file instead of %doc

Mandatory review guidelines:
ok - rpmlint output:
     python-pyrax.src: W: spelling-error %description -l en_US cloudfiles -> cloud files, cloud-files, cloudless
     2 packages and 0 specfiles checked; 0 errors, 1 warnings.
ok - License is acceptable (ASL 2.0)
ok - License field in spec is correct
ok - License files included in package %docs if included in source package
     Note that a guideline change to use the %license macro is pending:
     https://fedorahosted.org/fpc/ticket/411
ok - License files installed when any subpackage combination is installed
ok - Spec written in American English
ok - Spec is legible
ok - Sources match upstream unless altered to fix permissibility issues
ok - Build succeeds on at least one primary arch
ok - Build succeeds on all primary arches or has ExcludeArch + justification
ok - BuildRequires correct, justified where necessary
-- - Locales handled with %find_lang, not %_datadir/locale/*
-- - %post, %postun call ldconfig if package contains shared .so files
ok - No bundled libs
-- - Relocatability is justified
ok - Package owns all directories it creates
ok - Package requires others for directories it uses but does not own
ok - No duplication in %files unless necessary for license files
ok - File permissions are sane
ok - Package contains permissible code or content
-- - Large docs go in -doc subpackage
ok - %doc files not required at runtime
-- - Static libs go in -static package/virtual Provides
-- - Development files go in -devel package
-- - -devel packages Require base with fully-versioned dependency, %_isa
-- - No .la files
-- - GUI app uses .desktop file, installs it with desktop-file-install
ok - File list does not conflict with other packages' without justification
ok - File names are valid UTF-8

Optional review guidelines:
-- - Query upstream about including license files
no - Translations of description, summary
ok - Builds in mock
ok - Builds on all arches
-- - Scriptlets are sane
-- - Subpackages require base with fully-versioned dependency if sensible
-- - .pc file subpackage placement is sensible
ok - No file deps outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin
-- - Include man pages if available

Naming guidelines:
ok - Package names use only a-zA-Z0-9-._+ subject to restrictions on -._+
ok - Package names are sane
ok - No naming conflicts
ok - Spec file name matches base package name
     "python" prefix mandated by python guidelines
ok - Version is sane
ok - Version does not contain ~
ok - Release is sane
ok - %dist tag
ok - Case used only when necessary
ok - Package names follow applicable language/addon rules

Packaging guidelines:
ok - Useful without external bits
ok - No kmods
ok - Pre-built binaries, libs removed in %prep
ok - Sources contain only redistributable code or content
ok - Spec format is sane
-- - noarch package with unported deps has correct ExclusiveArch
-- - Arch-specific sources/patches are applied, not included, conditionally
ok - Package obeys FHS, except libexecdir, /run, /usr/target
-- - %{_prefix}/lib only used for multilib-exempt packages
-- - Programs run before FS mounting use /run instead of /var/run
ok - No files under /srv, /opt, /usr/local
ok - Changelog in prescribed format
ok - No Packager, Vendor, Copyright, PreReq tags
ok - Summary does not end in a period
ok - Requires correct, justified where necessary
ok - BuildRequires lack %{_isa}
ok - Summary, description do not use trademarks incorrectly
ok - All relevant documentation is packaged, appropriately marked with %doc
ok - Doc files do not drag in extra dependencies (e.g. due to +x)
-- - Code compilable with gcc is compiled with gcc
-- - Build honors applicable compiler flags or justifies otherwise
-- - PIE used for long-running/root daemons, setuid/filecap programs
-- - Useful -debuginfo package or disabled and justified
-- - No static executables (except OCaml)
-- - Rpath absent or only used for internal libs
-- - Config files marked with %config(noreplace) or justified %config
ok - No config files under /usr
-- - Third party package manager configs acceptable, only in %_docdir
-- - .desktop files are sane
-- - desktop-file-install/validate run on .desktop files, as appropriate
-- - No desktop-file-install --vendor on >= F19
ok - Spec uses macros consistently
ok - Spec uses macros instead of hard-coded names where appropriate
ok - Spec uses macros for executables only when configurability is needed
-- - %makeinstall used only when alternatives don't work
-- - Macros in Summary, description are expandable at srpm build time
-- - Spec uses %{SOURCE#} instead of $RPM_SOURCE_DIR and %sourcedir
ok - No software collections (scl)
-- - Macro files named /etc/rpm/macros.%name or /usr/lib/rpm/macros.%name
-- - Macro files not marked with %config
ok - Build uses only python/perl/shell+coreutils/lua/BuildRequired langs
-- - %global, not %define
-- - Package translating with gettext BuildRequires it
-- - Package translating with Linguist BuildRequires qt-devel
-- - File ops preserve timestamps
-- - Parallel make
-- - User, group creation handled correctly (See Packaging:UsersAndGroups)
-- - Web apps go in /usr/share/%name, not /var/www
-- - Conflicts are justified
ok - One project per package
ok - No bundled fonts
-- - Patches have appropriate commentary
no - Available test suites executed in %check
-- - tmpfiles.d used for /run, /run/lock on >= F15
-- - Package renaming/replacement handled correctly
ok - Package builds without network access

Python guidelines:
ok - Runtime Requires correct
ok - BuildRequires: python2-devel and/or python3-devel
ok - Spec uses versioned macros
ok - All .py files packaged with .pyc, .pyo counterparts
ok - INSTALLED_FILES not used for %files list
ok - Includes .egg-info files/directories when generated
ok - .py not under site-libs byte-compiled against correct runtimes
-- - Python 3 built as upstream instructs, if at all
-- - Patches are not specific to python 2 or 3 when sources are combined
-- - Non-parallel-installable scripts only installed for default runtime
ok - Eggs built from source
ok - Eggs do not download deps during build
-- - Compat packages use easy_install -m to avoid conflicts
ok - At least one version of each module must be importable w/o version
ok - Provides/Requires properly filtered
-- - Code that invokes gtk.gdk.get_pixels_array() Requires numpy

Comment 8 Ian McLeod 2014-07-30 22:17:25 UTC
New Package SCM Request
=======================
Package Name: python-pyrax
Short Description: Python bindings for Rackspace and OpenStack clouds
Upstream URL: https://github.com/rackspace/pyrax
Owners: imcleod
Branches: f19 f20 f21 el6 epel7
InitialCC:

Comment 9 Gwyn Ciesla 2014-07-31 12:24:49 UTC
WARNING: Requested package name python-pyrax doesn't match bug summary pyrax

Comment 10 Ian McLeod 2014-07-31 13:52:16 UTC
Jon,

pyrax is the name of the upstream project

During the review process I updated the RPM name from just "pyrax" to "python-pyrax" per feedback from gholms (to meet Fedora package naming standards).

I have changed the bug summary just now to reflect this and am re-setting fedora-cvs to ?

If this is not acceptable please let me know what I need to do.  The package, as named, has been approved and I'd really like to start executing some builds.

Comment 11 Gwyn Ciesla 2014-07-31 15:49:38 UTC
Git done (by process-git-requests).

Comment 12 Ian McLeod 2014-11-02 10:51:53 UTC
Closing to, one hopes, complete the new process submission process.


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