Bug 856722 - Review Request: openstack-cinder - OpenStack Volume service
Summary: Review Request: openstack-cinder - OpenStack Volume service
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Matthias Runge
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2012-09-12 16:08 UTC by Pádraig Brady
Modified: 2012-10-10 15:05 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2012-10-10 15:05:24 UTC
Type: ---
Embargoed:
mrunge: fedora-review+


Attachments (Terms of Use)

Description Pádraig Brady 2012-09-12 16:08:03 UTC
Spec URL: http://fedorapeople.org/~pbrady/cinder/openstack-cinder.spec
SRPM URL: http://fedorapeople.org/~pbrady/cinder/openstack-cinder-2012.2-0.1.f3.fc18.src.rpm
Description: OpenStack Volume (codename Cinder) provides services to manage and
access block storage volumes for use by Virtual Machine instances.
Fedora Account System Username: pbrady

Comment 1 Matthias Runge 2012-09-13 05:18:31 UTC
I'll do the review

Comment 2 Matthias Runge 2012-09-13 07:46:25 UTC
Package Review
==============

Key:
- = N/A
x = Pass
! = Fail
? = Not evaluated



==== Generic ====
[x]: EXTRA Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: EXTRA Spec file according to URL is the same as in SRPM.
[x]: MUST Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: MUST Package successfully compiles and builds into binary rpms on at
     least one supported primary architecture.
[x]: MUST %build honors applicable compiler flags or justifies otherwise.
[x]: MUST All build dependencies are listed in BuildRequires, except for any
     that are listed in the exceptions section of Packaging Guidelines.
[x]: MUST Package contains no bundled libraries.
[x]: MUST Changelog in prescribed format.
[x]: MUST Sources contain only permissible code or content.
[x]: MUST %config files are marked noreplace or the reason is justified.
[!]: MUST Each %files section contains %defattr if rpm < 4.4
     Note: defattr(....) present in %files -n python-cinder section. This is
     OK if packaging for EPEL5. Otherwise not needed
[x]: MUST Macros in Summary, %description expandable at SRPM build time.
[-]: MUST Package contains desktop file if it is a GUI application.
[-]: MUST Development files must be in a -devel package
[x]: MUST Package requires other packages for directories it uses.
[x]: MUST Package uses nothing in %doc for runtime.
[x]: MUST Package is not known to require ExcludeArch.
[x]: MUST Permissions on files are set properly.
[x]: MUST Package does not contain duplicates in %files.
[x]: MUST Fully versioned dependency in subpackages, if present.
[x]: MUST Package complies to the Packaging Guidelines
[x]: MUST Spec file lacks Packager, Vendor, PreReq tags.
[x]: MUST Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
     Note: rm -rf would be needed if support for EPEL5 is required
[x]: MUST Large documentation files are in a -doc subpackage, if required.
[x]: MUST If (and only if) the source package includes the text of the
     license(s) in its own file, then that file, containing the text of the
     license(s) for the package is included in %doc.
[x]: MUST License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses found:
     "Apache (v2.0)", "Apache (v2.0) MIT/X11 (BSD like)", "GPL (v3 or later)"
     For detailed output of licensecheck see file: /home/mrunge/review/856722
     -openstack-cinder/licensecheck.txt
[x]: MUST License file installed when any subpackage combination is installed.
[x]: MUST Package consistently uses macro is (instead of hard-coded directory
     names).
[x]: MUST Package is named using only allowed ascii characters.
[x]: MUST Package is named according to the Package Naming Guidelines.
[x]: MUST No %config files under /usr.
[!]: MUST Package does not generate any conflict.
     Note: Package contains Conflicts: tag(s) needing fix or justification.
[x]: MUST Package obeys FHS, except libexecdir and /usr/target.
[x]: MUST If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: MUST Package must own all directories that it creates.
[x]: MUST Package does not own files or directories owned by other packages.
[x]: MUST Package installs properly.
[x]: MUST Package is not relocatable.
[x]: MUST Requires correct, justified where necessary.
[x]: MUST Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: MUST Sources used to build the package match the upstream source, as
     provided in the spec URL.
[x]: MUST Spec file is legible and written in American English.
[x]: MUST Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: MUST Package contains systemd file(s) if in need.
[x]: MUST File names are valid UTF-8.
[x]: SHOULD Reviewer should test that the package builds in mock.
[x]: SHOULD Buildroot is not present
     Note: Unless packager wants to package for EPEL5 this is fine
[x]: SHOULD Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
     Note: Clean would be needed if support for EPEL5 is required
[x]: SHOULD If the source package does not include license text(s) as a
     separate file from upstream, the packager SHOULD query upstream to
     include it.
[x]: SHOULD Dist tag is present.
[x]: SHOULD No file requires outside of /etc, /bin, /sbin, /usr/bin,
     /usr/sbin.
[x]: SHOULD Final provides and requires are sane (rpm -q --provides and rpm -q
     --requires).
[?]: SHOULD Package functions as described.
[x]: SHOULD Latest version is packaged.
[x]: SHOULD Package does not include license text files separate from
     upstream.
[x]: SHOULD Patches link to upstream bugs/comments/lists or are otherwise
     justified.
[x]: SHOULD Scriptlets must be sane, if used.
[x]: SHOULD SourceX tarball generation or download is documented.
[!]: SHOULD SourceX / PatchY prefixed with %{name}.
     Note: Patch1 (0001-Ensure-we-don-t-access-the-net-when-building-
     docs.patch) Source2 (cinder.logrotate) Source0 (cinder-2012.2~f3.tar.gz)
     Source1 (cinder.conf) Source20 (cinder-sudoers)
[x]: SHOULD SourceX is a working URL.
[-]: SHOULD Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[x]: SHOULD Package should compile and build into binary rpms on all supported
     architectures.
[-]: SHOULD %check is present and all tests pass.
[x]: SHOULD Packages should try to preserve timestamps of original installed
     files.
[x]: SHOULD Spec use %global instead of %define.

Issues:
[!]: MUST Each %files section contains %defattr if rpm < 4.4
     Note: defattr(....) present in %files -n python-cinder section. This is
     OK if packaging for EPEL5. Otherwise not needed
See: http://fedoraproject.org/wiki/Packaging/Guidelines#FilePermissions
[!]: MUST Package does not generate any conflict.
     Note: Package contains Conflicts: tag(s) needing fix or justification.
See: http://fedoraproject.org/wiki/Packaging/Guidelines#Conflicts

Rpmlint
-------
Checking: openstack-cinder-2012.2-0.1.f3.fc19.src.rpm
          python-cinder-2012.2-0.1.f3.fc19.noarch.rpm
          openstack-cinder-doc-2012.2-0.1.f3.fc19.noarch.rpm
          openstack-cinder-2012.2-0.1.f3.fc19.noarch.rpm
openstack-cinder.src: W: spelling-error %description -l en_US codename -> code name, code-name, codependent
python-cinder.noarch: E: explicit-lib-dependency python-amqplib
python-cinder.noarch: W: spelling-error %description -l en_US codename -> code name, code-name, codependent
openstack-cinder-doc.noarch: W: spelling-error %description -l en_US codename -> code name, code-name, codependent
openstack-cinder.noarch: W: spelling-error %description -l en_US codename -> code name, code-name, codependent
openstack-cinder.noarch: W: only-non-binary-in-usr-lib
openstack-cinder.noarch: W: non-standard-gid /etc/cinder/policy.json cinder
openstack-cinder.noarch: E: non-readable /etc/cinder/policy.json 0640L
openstack-cinder.noarch: W: non-standard-uid /var/log/cinder cinder
openstack-cinder.noarch: E: non-readable /etc/sudoers.d/cinder 0440L
openstack-cinder.noarch: W: non-standard-uid /var/lib/cinder/tmp cinder
openstack-cinder.noarch: W: non-standard-gid /var/lib/cinder/tmp cinder
openstack-cinder.noarch: W: non-standard-uid /var/lib/cinder cinder
openstack-cinder.noarch: W: non-standard-gid /var/lib/cinder cinder
openstack-cinder.noarch: W: non-standard-gid /etc/cinder/rootwrap.conf cinder
openstack-cinder.noarch: E: non-readable /etc/cinder/rootwrap.conf 0640L
openstack-cinder.noarch: W: non-standard-gid /etc/cinder/cinder.conf cinder
openstack-cinder.noarch: E: non-readable /etc/cinder/cinder.conf 0640L
openstack-cinder.noarch: W: non-standard-uid /var/run/cinder cinder
openstack-cinder.noarch: W: non-standard-gid /etc/cinder/api-paste.ini cinder
openstack-cinder.noarch: E: non-readable /etc/cinder/api-paste.ini 0640L
openstack-cinder.noarch: W: no-manual-page-for-binary cinder-rootwrap
openstack-cinder.noarch: W: no-manual-page-for-binary cinder-clear-rabbit-queues
openstack-cinder.noarch: W: no-manual-page-for-binary cinder-api
openstack-cinder.noarch: W: no-manual-page-for-binary cinder-volume
openstack-cinder.noarch: W: no-manual-page-for-binary cinder-volume-usage-audit
openstack-cinder.noarch: W: no-manual-page-for-binary cinder-all
openstack-cinder.noarch: W: no-manual-page-for-binary cinder-scheduler
4 packages and 0 specfiles checked; 6 errors, 22 warnings.


Rpmlint (installed packages)
----------------------------
# rpmlint openstack-cinder-doc
openstack-cinder-doc.noarch: W: spelling-error %description -l en_US codename -> code name, code-name, codependent
1 packages and 0 specfiles checked; 0 errors, 1 warnings.
# echo 'rpmlint-done:'

Requires
--------
python-cinder-2012.2-0.1.f3.fc19.noarch.rpm (rpmlib, GLIBC filtered):
    
    MySQL-python  
    python(abi) = 2.7
    python-amqplib  
    python-anyjson  
    python-cheetah  
    python-daemon  
    python-eventlet  
    python-glanceclient >= 1:0
    python-greenlet  
    python-iso8601  
    python-kombu  
    python-lxml  
    python-migrate  
    python-netaddr  
    python-paramiko  
    python-paste-deploy  
    python-qpid  
    python-routes  
    python-sqlalchemy  
    python-webob  
    sudo  

openstack-cinder-doc-2012.2-0.1.f3.fc19.noarch.rpm (rpmlib, GLIBC filtered):
    
    openstack-cinder = 2012.2-0.1.f3.fc19

openstack-cinder-2012.2-0.1.f3.fc19.noarch.rpm (rpmlib, GLIBC filtered):
    
    /bin/sh  
    /usr/bin/python  
    config(openstack-cinder) = 2012.2-0.1.f3.fc19
    lvm2  
    openstack-utils  
    python-cinder = 2012.2-0.1.f3.fc19
    scsi-target-utils  
    shadow-utils  
    systemd-units  

Provides
--------
python-cinder-2012.2-0.1.f3.fc19.noarch.rpm:
    
    python-cinder = 2012.2-0.1.f3.fc19

openstack-cinder-doc-2012.2-0.1.f3.fc19.noarch.rpm:
    
    openstack-cinder-doc = 2012.2-0.1.f3.fc19

openstack-cinder-2012.2-0.1.f3.fc19.noarch.rpm:
    
    config(openstack-cinder) = 2012.2-0.1.f3.fc19
    openstack-cinder = 2012.2-0.1.f3.fc19

MD5-sum check
-------------
https://launchpad.net/cinder/folsom/folsom-3/+download/cinder-2012.2~f3.tar.gz :
  CHECKSUM(SHA256) this package     : 1c7054d4d4b52e5cd2b857cb869a261dfe9ccf6c6f99055deb4df4c05f40fbd9
  CHECKSUM(SHA256) upstream package : 1c7054d4d4b52e5cd2b857cb869a261dfe9ccf6c6f99055deb4df4c05f40fbd9


Generated by fedora-review 0.2.2 (9f8c0e5) last change: 2012-08-09
Command line :/usr/bin/fedora-review -b 856722


just a few notes:
- you could at least drop the defattr-line from %files -n python-cinder.
Also, I'm unsure, how desirable python-cinder is without openstack-cinder.

- Summary for openstack-cinder-doc should read:
Documentation for Openstack Volume (codename Cinder)
- Since the -doc subpackage requires the main package, there's no need to include the license twice.

- I'd prefer to replace the conflicts by something like 
Obsoletes: openstack-nova-volume

Comment 3 Pádraig Brady 2012-09-13 13:04:53 UTC
I changed as per all your suggestions above.
However I didn't add the reverse dep between python-cinder and the main package,
as we don't do that elsewhere in the openstack packages.
I'll consider updating all packages like that in future.
Also related to this, is the doc package should probably be installable
independently of the main package. Again all openstack packages
would need to be updated.

thanks!

Spec URL: http://fedorapeople.org/~pbrady/cinder/openstack-cinder.spec
SRPM URL: http://fedorapeople.org/~pbrady/cinder/openstack-cinder-2012.2-0.2.f3.fc18.src.rpm
Description: OpenStack Volume (codename Cinder) provides services to manage and
access block storage volumes for use by Virtual Machine instances.
Fedora Account System Username: pbrady

Comment 4 Matthias Runge 2012-09-13 15:00:28 UTC
alright, 

Package APPROVED

Comment 5 Pádraig Brady 2012-09-13 16:00:51 UTC
New Package SCM Request
=======================
Package Name: openstack-cinder
Short Description: OpenStack Volume service
Owners: pbrady eharney apevec
Branches: f18 el6
InitialCC:

Comment 6 Gwyn Ciesla 2012-09-13 16:21:17 UTC
Git done (by process-git-requests).

Comment 7 Fedora Update System 2012-09-14 15:32:29 UTC
openstack-cinder-2012.2-0.2.f3.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/openstack-cinder-2012.2-0.2.f3.fc18

Comment 8 Fedora Update System 2012-09-16 17:12:20 UTC
Package openstack-cinder-2012.2-0.2.f3.fc18:
* should fix your issue,
* was pushed to the Fedora 18 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing openstack-cinder-2012.2-0.2.f3.fc18'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2012-13971/openstack-cinder-2012.2-0.2.f3.fc18
then log in and leave karma (feedback).

Comment 9 Pádraig Brady 2012-09-21 10:40:21 UTC
Package Change Request
======================
Package Name: openstack-cinder

Please delete the master-patches git branch
which I created by mistake. thanks

Comment 10 Gwyn Ciesla 2012-09-21 12:34:53 UTC
That can't be done AFAIK, just follow the retirement procedure for it.  It
shouldn't harm anything.

Comment 11 Fedora Update System 2012-09-21 16:50:32 UTC
openstack-cinder-2012.2-0.5.rc1.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/openstack-cinder-2012.2-0.5.rc1.fc18

Comment 12 Fedora Update System 2012-09-27 17:42:53 UTC
openstack-cinder-2012.2-1.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/openstack-cinder-2012.2-1.fc18


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