Bug 1193986

I'm taking care of the sponsorship process for Victoria.

@Victoria: when you'll have finished with your informal reviews (at least 2), please link them in this ticket. 
http://fedoraproject.org/PackageReviewStatus/NEW.html

This is an unofficial review only.


Package Review
==============
 
Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
 
Issues:
=======
- Package doesn't installs properly in my case.
- Package uses either %{buildroot} or $RPM_BUILD_ROOT
  Please be consistent with only one of them.
  See: http://fedoraproject.org/wiki/Packaging/Guidelines#macros
- Please use %license for your LICENSE file. This has changed rececntly.
  See: http://fedoraproject.org/wiki/Changes/Use_license_macro_in_RPMs_for_packages_in_Cloud_Image
- You are using too much asterisks in your description of the spec file.
  The description in your summary makes extensive use of asterisks. Please
  note that these are not translated to anything. So, IMHO, their use should
  be minimal in spec file.

===== MUST items =====
 
Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: License field in the package spec file matches the actual license.
     Note: Licenses found:
     "Apache (v2.0)", "Unknown or generated", "*No copyright* Apache (v2.0)".
     21 files have unknown license.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[-]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[!]: Package consistently uses macros (instead of hard-coded directory names).
     I am not sure about this. But your spec file makes considerable use of
     hard-coded directory names. Though, I see no advantage of using macros in
     this case, so this might be ok.
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[x]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Requires correct, justified where necessary.
[!]: Spec file is legible and written in American English.
     See Issues above.
[-]: Package contains systemd file(s) if in need.
[x]: Package is not known to require an ExcludeArch tag.
[-]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
[x]: Packages must not store files under /srv, /opt or /usr/local
[x]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least one
     supported primary architecture.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: 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 %license.
     See Issues above.
[x]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[x]: All build dependencies are listed in BuildRequires, except for any that
     are listed in the exceptions section of Packaging Guidelines.
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package use %makeinstall only when make install' ' DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package do not use a name that already exist
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as provided
     in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
 
Python:
[?]: Binary eggs must be removed in %prep
[x]: Python eggs must not download any dependencies during the build process.
[x]: A package which is used by another package via an egg interface should
     provide egg info.
[x]: Package meets the Packaging Guidelines::Python
[x]: Package contains BR: python2-devel or python3-devel
 
===== SHOULD items =====
 
Generic:
[-]: 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]: Final provides and requires are sane (see attachments).
[x]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[!]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
     See Issues above. Wrapping words with asterisks (*) won't mark them for translation.
[x]: Package should compile and build into binary rpms on all supported
     architectures.
[?]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed files.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: Dist tag is present (not strictly required in GL).
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.
 
===== EXTRA items =====
 
Generic:
[!]: Rpmlint is run on all installed packages.
     Mock build failed in my case.
[x]: Large data in /usr/share should live in a noarch subpackage if package is
     arched.
[x]: Spec file according to URL is the same as in SRPM.
 
 
Installation errors
-------------------
INFO: mock.py version 1.2.6 starting (python version = 2.7.8)...
Start: init plugins
INFO: selinux enabled
Finish: init plugins
Start: run
Start: chroot init
INFO: calling preinit hooks
INFO: enabled root cache
INFO: enabled yum cache
Start: cleaning yum metadata
Finish: cleaning yum metadata
INFO: enabled ccache
Mock Version: 1.2.6
INFO: Mock Version: 1.2.6
Finish: chroot init
INFO: installing package(s): /home/fedora/1193986-openstack-rally/results/openstack-rally-0.0.1-1.fc21.noarch.rpm
ERROR: Command failed. See logs for output.
 # /usr/bin/yum --installroot /var/lib/mock/fedora-21-x86_64/root/ --releasever 21 install /home/fedora/1193986-openstack-rally/results/openstack-rally-0.0.1-1.fc21.noarch.rpm --setopt=tsflags=nocontexts
 
 
Rpmlint
-------
Checking: openstack-rally-0.0.1-1.fc21.noarch.rpm
          openstack-rally-0.0.1-1.fc21.src.rpm
openstack-rally.noarch: W: non-conffile-in-etc /etc/bash_completion.d/rally.bash_completion
openstack-rally.noarch: W: no-manual-page-for-binary rally-manage
openstack-rally.noarch: W: no-manual-page-for-binary rally
openstack-rally.noarch: W: no-manual-page-for-binary rally-api
openstack-rally.noarch: W: dangerous-command-in-%post chmod
2 packages and 0 specfiles checked; 0 errors, 5 warnings.
 
 
 
 
Requires
--------
openstack-rally (rpmlib, GLIBC filtered):
    /bin/sh
    /usr/bin/python2
    python(abi)
    python-babel
    python-decorator
    python-fixtures
    python-iso8601
    python-jinja2
    python-jsonschema
    python-netaddr
    python-openstack-ceilometerclient
    python-openstack-cinderclient
    python-openstack-designateclient
    python-openstack-glanceclient
    python-openstack-heatclient
    python-openstack-ironicclient
    python-openstack-keystoneclient
    python-openstack-neutronclient
    python-openstack-novaclient
    python-openstack-saharaclient
    python-openstack-troveclient
    python-openstack-zaqarclient
    python-oslo-config
    python-oslo-db
    python-oslo-i18n
    python-oslo-serialization
    python-oslo-utils
    python-paramiko
    python-pecan
    python-prettytable
    python-psycopg2
    python-pyyaml
    python-requests
    python-six
    python-sphinx
    python-sqlalchemy
    python-subunit
    python-wsme
 
 
 
Provides
--------
openstack-rally:
    openstack-rally
 
Looks OK.
 
Source checksums
----------------
http://tarballs.openstack.org/rally/rally-0.0.1.tar.gz :
  CHECKSUM(SHA256) this package     : a5eec9c78121d64c320e73036f353c2609e532698f35e1d54f4b0f1624d19e43
  CHECKSUM(SHA256) upstream package : a5eec9c78121d64c320e73036f353c2609e532698f35e1d54f4b0f1624d19e43

(In reply to Pranav Kant from comment #2)
> This is an unofficial review only.
> 
> 
> Package Review
> ==============
>  
> Legend:
> [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
>  
> Issues:
> =======
> - Package doesn't installs properly in my case.
> - Package uses either %{buildroot} or $RPM_BUILD_ROOT
>   Please be consistent with only one of them.
>   See: http://fedoraproject.org/wiki/Packaging/Guidelines#macros
> - Please use %license for your LICENSE file. This has changed rececntly.
>   See:
> http://fedoraproject.org/wiki/Changes/
> Use_license_macro_in_RPMs_for_packages_in_Cloud_Image
> - You are using too much asterisks in your description of the spec file.
>   The description in your summary makes extensive use of asterisks. Please
>   note that these are not translated to anything. So, IMHO, their use should
>   be minimal in spec file.
> 
> ===== MUST items =====
>  
> Generic:
> [x]: Package is licensed with an open-source compatible license and meets
>      other legal requirements as defined in the legal section of Packaging
>      Guidelines.
> [x]: License field in the package spec file matches the actual license.
>      Note: Licenses found:
>      "Apache (v2.0)", "Unknown or generated", "*No copyright* Apache (v2.0)".
>      21 files have unknown license.
> [x]: Package contains no bundled libraries without FPC exception.
> [x]: Changelog in prescribed format.
> [x]: Sources contain only permissible code or content.
> [-]: Package contains desktop file if it is a GUI application.
> [-]: Development files must be in a -devel package
> [x]: Package uses nothing in %doc for runtime.
> [!]: Package consistently uses macros (instead of hard-coded directory
> names).
>      I am not sure about this. But your spec file makes considerable use of
>      hard-coded directory names. Though, I see no advantage of using macros
> in
>      this case, so this might be ok.
> [x]: Package is named according to the Package Naming Guidelines.
> [x]: Package does not generate any conflict.
> [x]: Package obeys FHS, except libexecdir and /usr/target.
> [x]: If the package is a rename of another package, proper Obsoletes and
>      Provides are present.
> [x]: Requires correct, justified where necessary.
> [!]: Spec file is legible and written in American English.
>      See Issues above.
> [-]: Package contains systemd file(s) if in need.
> [x]: Package is not known to require an ExcludeArch tag.
> [-]: Large documentation must go in a -doc subpackage. Large could be size
>      (~1MB) or number of files.
> [x]: Packages must not store files under /srv, /opt or /usr/local
> [x]: Package complies to the Packaging Guidelines
> [x]: Package successfully compiles and builds into binary rpms on at least
> one
>      supported primary architecture.
> [x]: Rpmlint is run on all rpms the build produces.
>      Note: There are rpmlint messages (see attachment).
> [x]: 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 %license.
>      See Issues above.
> [x]: Package requires other packages for directories it uses.
> [x]: Package must own all directories that it creates.
> [x]: Package does not own files or directories owned by other packages.
> [x]: All build dependencies are listed in BuildRequires, except for any that
>      are listed in the exceptions section of Packaging Guidelines.
> [x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
>      beginning of %install.
> [x]: Macros in Summary, %description expandable at SRPM build time.
> [x]: Package does not contain duplicates in %files.
> [x]: Permissions on files are set properly.
> [x]: Package use %makeinstall only when make install' ' DESTDIR=... doesn't
>      work.
> [x]: Package is named using only allowed ASCII characters.
> [x]: Package do not use a name that already exist
> [x]: Package is not relocatable.
> [x]: Sources used to build the package match the upstream source, as provided
>      in the spec URL.
> [x]: Spec file name must match the spec package %{name}, in the format
>      %{name}.spec.
> [x]: File names are valid UTF-8.
>  
> Python:
> [?]: Binary eggs must be removed in %prep
> [x]: Python eggs must not download any dependencies during the build process.
> [x]: A package which is used by another package via an egg interface should
>      provide egg info.
> [x]: Package meets the Packaging Guidelines::Python
> [x]: Package contains BR: python2-devel or python3-devel
>  
> ===== SHOULD items =====
>  
> Generic:
> [-]: 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]: Final provides and requires are sane (see attachments).
> [x]: Package functions as described.
> [x]: Latest version is packaged.
> [x]: Package does not include license text files separate from upstream.
> [!]: Description and summary sections in the package spec file contains
>      translations for supported Non-English languages, if available.
>      See Issues above. Wrapping words with asterisks (*) won't mark them for
> translation.
> [x]: Package should compile and build into binary rpms on all supported
>      architectures.
> [?]: %check is present and all tests pass.
> [x]: Packages should try to preserve timestamps of original installed files.
> [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
> [x]: Sources can be downloaded from URI in Source: tag
> [x]: Reviewer should test that the package builds in mock.
> [x]: Buildroot is not present
> [x]: Package has no %clean section with rm -rf %{buildroot} (or
>      $RPM_BUILD_ROOT)
> [x]: Dist tag is present (not strictly required in GL).
> [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
> [x]: SourceX is a working URL.
> [x]: Spec use %global instead of %define unless justified.
>  
> ===== EXTRA items =====
>  
> Generic:
> [!]: Rpmlint is run on all installed packages.
>      Mock build failed in my case.
> [x]: Large data in /usr/share should live in a noarch subpackage if package
> is
>      arched.
> [x]: Spec file according to URL is the same as in SRPM.
>  
>  
> Installation errors
> -------------------
> INFO: mock.py version 1.2.6 starting (python version = 2.7.8)...
> Start: init plugins
> INFO: selinux enabled
> Finish: init plugins
> Start: run
> Start: chroot init
> INFO: calling preinit hooks
> INFO: enabled root cache
> INFO: enabled yum cache
> Start: cleaning yum metadata
> Finish: cleaning yum metadata
> INFO: enabled ccache
> Mock Version: 1.2.6
> INFO: Mock Version: 1.2.6
> Finish: chroot init
> INFO: installing package(s):
> /home/fedora/1193986-openstack-rally/results/openstack-rally-0.0.1-1.fc21.
> noarch.rpm
> ERROR: Command failed. See logs for output.
>  # /usr/bin/yum --installroot /var/lib/mock/fedora-21-x86_64/root/
> --releasever 21 install
> /home/fedora/1193986-openstack-rally/results/openstack-rally-0.0.1-1.fc21.
> noarch.rpm --setopt=tsflags=nocontexts
>  
>  
> Rpmlint
> -------
> Checking: openstack-rally-0.0.1-1.fc21.noarch.rpm
>           openstack-rally-0.0.1-1.fc21.src.rpm
> openstack-rally.noarch: W: non-conffile-in-etc
> /etc/bash_completion.d/rally.bash_completion
> openstack-rally.noarch: W: no-manual-page-for-binary rally-manage
> openstack-rally.noarch: W: no-manual-page-for-binary rally
> openstack-rally.noarch: W: no-manual-page-for-binary rally-api
> openstack-rally.noarch: W: dangerous-command-in-%post chmod
> 2 packages and 0 specfiles checked; 0 errors, 5 warnings.
>  
>  
>  
>  
> Requires
> --------
> openstack-rally (rpmlib, GLIBC filtered):
>     /bin/sh
>     /usr/bin/python2
>     python(abi)
>     python-babel
>     python-decorator
>     python-fixtures
>     python-iso8601
>     python-jinja2
>     python-jsonschema
>     python-netaddr
>     python-openstack-ceilometerclient
>     python-openstack-cinderclient
>     python-openstack-designateclient
>     python-openstack-glanceclient
>     python-openstack-heatclient
>     python-openstack-ironicclient
>     python-openstack-keystoneclient
>     python-openstack-neutronclient
>     python-openstack-novaclient
>     python-openstack-saharaclient
>     python-openstack-troveclient
>     python-openstack-zaqarclient
>     python-oslo-config
>     python-oslo-db
>     python-oslo-i18n
>     python-oslo-serialization
>     python-oslo-utils
>     python-paramiko
>     python-pecan
>     python-prettytable
>     python-psycopg2
>     python-pyyaml
>     python-requests
>     python-six
>     python-sphinx
>     python-sqlalchemy
>     python-subunit
>     python-wsme
>  
>  
>  
> Provides
> --------
> openstack-rally:
>     openstack-rally
>  
> Looks OK.
>  
> Source checksums
> ----------------
> http://tarballs.openstack.org/rally/rally-0.0.1.tar.gz :
>   CHECKSUM(SHA256) this package     :
> a5eec9c78121d64c320e73036f353c2609e532698f35e1d54f4b0f1624d19e43
>   CHECKSUM(SHA256) upstream package :
> a5eec9c78121d64c320e73036f353c2609e532698f35e1d54f4b0f1624d19e43

Thanks for the review! I fixed the issues you pointed out. Running mock -r fedora-rawhide-x86_64 openstack-rally-0.0.1-1.fc21.src.rpm and mock -r fedora-21-x86_64 openstack-rally-0.0.1-1.fc21.src.rpm successfully. How did you run mock?

(In reply to Haïkel Guémar from comment #1)
> I'm taking care of the sponsorship process for Victoria.
> 
> @Victoria: when you'll have finished with your informal reviews (at least
> 2), please link them in this ticket. 
> http://fedoraproject.org/PackageReviewStatus/NEW.html

Thanks Haïkel!

https://bugzilla.redhat.com/show_bug.cgi?id=1195835
https://bugzilla.redhat.com/show_bug.cgi?id=1195573

I didn't run mock separately. It was done under fedora-review tool

(In reply to Pranav Kant from comment #5)
> I didn't run mock separately. It was done under fedora-review tool

You are right... apparently there are some missing requirements. Precisely, python-designateclient. I'll take a look, thanks!

python-designateclient is now in Rawhide, is there anything else missing?

No, I'll submit the build for openstack-rally soon.

Spec URL: https://vkmc.fedorapeople.org/openstack_rally-0.0.1/openstack-rally.spec
SRPM URL: https://vkmc.fedorapeople.org/openstack_rally-0.0.1/openstack-rally-0.0.1-1.fc21.src.rpm
Description: Benchmark as a service for OpenStack
Fedora Account System Username: vkmc

(In reply to Victoria Martinez de la Cruz from comment #9)
> Spec URL:
> https://vkmc.fedorapeople.org/openstack_rally-0.0.1/openstack-rally.spec
> SRPM URL:
> https://vkmc.fedorapeople.org/openstack_rally-0.0.1/openstack-rally-0.0.1-1.
> fc21.src.rpm
> Description: Benchmark as a service for OpenStack
> Fedora Account System Username: vkmc

Any objection to updating this to use 0.0.4, latest rally release?

I published a 0.0.4 tarball here https://slinabery.fedorapeople.org/rally-0.0.4.tar.gz in the hopes of getting this into delorean asap.

(In reply to Steve Linabery from comment #10)
> (In reply to Victoria Martinez de la Cruz from comment #9)
> > Spec URL:
> > https://vkmc.fedorapeople.org/openstack_rally-0.0.1/openstack-rally.spec
> > SRPM URL:
> > https://vkmc.fedorapeople.org/openstack_rally-0.0.1/openstack-rally-0.0.1-1.
> > fc21.src.rpm
> > Description: Benchmark as a service for OpenStack
> > Fedora Account System Username: vkmc
> 
> Any objection to updating this to use 0.0.4, latest rally release?
> 
> I published a 0.0.4 tarball here
> https://slinabery.fedorapeople.org/rally-0.0.4.tar.gz in the hopes of
> getting this into delorean asap.

sorry, for clarity: I meant 'RDO' not 'delorean'.

I see there's no reason to create a separate tarball; the Source0 tarball already has the PKG-INFO (when you grab it from github from the 0.0.4 tag, that's missing).

I edited the spec and successfully built it with mock. In case this helps here it is:
https://slinabery.fedorapeople.org/openstack-rally.spec

Thanks Steve!

I could build the SRPM using your SPEC. Works as expected!

SPEC: https://vkmc.fedorapeople.org/openstack_rally-0.0.4/openstack-rally.spec
SRPM: https://vkmc.fedorapeople.org/openstack_rally-0.0.4/openstack-rally-0.0.4-1.fc22.src.rpm

Missing Requires:
* bash-completion (you install a file in directory owned by this package)
* python-boto

Missing min versions (known as necessary):
* python-six
* python-requires
* python-oslo-* preferably
Others are not necessary, and most of these deps works fine even if slightly older than specified in requirements,txt

rdopkg reqcheck could help you :)
https://www.rdoproject.org/packaging/rdopkg/rdopkg-adv-requirements.7.html

* Source0: use preferably pypi

* Avoid %{version} in %files to reduce gap with delorean packages


needinfo me when it's ready.

(In reply to Haïkel Guémar from comment #14)
> Missing Requires:
> * bash-completion (you install a file in directory owned by this package)

bash-completion is an example, where this is not needed, see the following snippet from https://fedoraproject.org/wiki/Packaging:Guidelines#File_and_Directory_Ownership

<snip>
Another example:

bash-completion owns the /etc/bash_completion.d directory and uses the files placed there to configure itself.
git places files into /etc/bash_completion.d
bzr places files into /etc/bash_completion.d

Solution: Both the git and bzr packages should own the /etc/bash_completion.d directory as bash-completion is optional functionality and the installation of git or bzr should not force the installation of bash-completion. 
</snip>

I updated my versions of the spec and srpm to help this effort along:
https://slinabery.fedorapeople.org/openstack-rally.spec
https://slinabery.fedorapeople.org/openstack-rally-0.0.4-1.fc20.src.rpm

Haikel, I think you meant python-requests. Correct me if I'm wrong.

Victoria, I think these are the changes recommended in #14, if you agree please update the canonical spec/srpm for this review. Thanks!

@Jens You're right about it, forgot about this guideline :)

@Steve: yes, I must have been tired when I typed, Before you import the package, I request you to do these changes:
* dropping the requirements on bash-completion as explained in #15
* sed -i 's/pyyaml/PyYAML/g' on spec as there's no pyyaml package and rally installs/runs fine after this change.
* move %{_prefix}%{_sysconfdir}/bash_completion.d/rally.bash_completion to %{_sysconfdir}/bash_completion.d/rally.bash_completion

These are the last mile before approval :)

(In reply to Haïkel Guémar from comment #17)
> @Jens You're right about it, forgot about this guideline :)
> 
> @Steve: yes, I must have been tired when I typed, Before you import the
> package, I request you to do these changes:
> * dropping the requirements on bash-completion as explained in #15
> * sed -i 's/pyyaml/PyYAML/g' on spec as there's no pyyaml package and rally
> installs/runs fine after this change.
> * move %{_prefix}%{_sysconfdir}/bash_completion.d/rally.bash_completion to
> %{_sysconfdir}/bash_completion.d/rally.bash_completion
> 
> These are the last mile before approval :)

that %{_prefix}%{_sysconfdir}/bash_completion.d/rally.bash_completion is in there because that's where setup.py drops that file. from build log:

copying etc/rally.bash_completion -> /builddir/build/BUILDROOT/openstack-rally-0.0.4-1.fc24.x86_64/usr/etc/bash_completion.d

/me shrugs.

I can add a build step to move it to the suggested location. Please advise.

In the interim I made the other two changes here:
https://slinabery.fedorapeople.org/openstack-rally.spec
https://slinabery.fedorapeople.org/openstack-rally-0.0.4-1.fc20.src.rpm

I'm inclined to leave that rally.bash_completion file in /usr/etc/bash_completion.d

rpmlint complains about it a little when I put it in /etc/bash_completion.d, I think possibly because it has a setbang line:
openstack-rally.noarch: W: non-conffile-in-etc /etc/bash_completion.d/rally.bash_completion

Updated to move the file to /etc/bash_completion.d

Please see updated spec/srpm:
https://slinabery.fedorapeople.org/openstack-rally.spec
https://slinabery.fedorapeople.org/openstack-rally-0.0.4-1.fc20.src.rpm

> rpmlint complains about it a little when I put it in /etc/bash_completion.d,
> I think possibly because it has a setbang line:
> openstack-rally.noarch: W: non-conffile-in-etc
> /etc/bash_completion.d/rally.bash_completion

It's b/c it's not %config in %file but we'll just ignore that rpmlint warning.

Updated the canonical spec/srpm for this review

Spec URL: http://vmartinezdelacruz.com/rpms/openstack-rally/openstack-rally.spec
SRPM URL: http://vmartinezdelacruz.com/rpms/openstack-rally/openstack-rally-0.0.4-1.fc22.src.rpm

Please add the following requires that are missing: python-oslo-i18n and python-oslog-log.

I tested with delorean repo, and rally ran fine. we just need to update some clients in rawhide.

Next step is publishing revised spec + srpm, and it should done.

Added python-oslo-log require. python-oslo-i18n was already in the spec.

Spec URL: http://vmartinezdelacruz.com/rpms/openstack-rally/openstack-rally.spec
SRPM URL: http://vmartinezdelacruz.com/rpms/openstack-rally/openstack-rally-0.0.4-1.fc22.src.rpm

Thanks Victoria, this is approved :)

As we're still refining the process, I'll take care of importing this package.

How often is this package rebased with upstream? I have added a patch so rally will be able to run Tempest (cmd: "rally verify start") without requiring virtual environment.

This patch is essential for running rally verify command without pip.

patch: https://review.openstack.org/#/c/220414/

@Rohan Rally is branchless, so probably we won't be rebasing it frequently. Although, if you consider that the patch is critical then we should add it.

(Posting formal review report)

I hereby approve this package since it complies with RDO packaging guidelines,

Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed



===== MUST items =====

Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "Apache (v2.0)", "Unknown or generated". 6 files have unknown
     license. Detailed output of licensecheck in /home/haikel/1193986
     -openstack-rally/licensecheck.txt
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[-]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[x]: Package consistently uses macros (instead of hard-coded directory
     names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: Package is not known to require an ExcludeArch tag.
[x]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: 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 %license.
[x]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[x]: All build dependencies are listed in BuildRequires, except for any
     that are listed in the exceptions section of Packaging Guidelines.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Dist tag is present.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package use %makeinstall only when make install DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package does not use a name that already exists.
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as
     provided in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 0 bytes in 0 files.
[x]: Packages must not store files under /srv, /opt or /usr/local

Python:
[x]: Python eggs must not download any dependencies during the build
     process.
[x]: A package which is used by another package via an egg interface should
     provide egg info.
[x]: Package meets the Packaging Guidelines::Python
[x]: Package contains BR: python2-devel or python3-devel
[x]: Binary eggs must be removed in %prep

===== SHOULD items =====

Generic:
[-]: 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]: Final provides and requires are sane (see attachments).
[x]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[-]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[x]: Package should compile and build into binary rpms on all supported
     architectures.
[-]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed
     files.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: openstack-rally-0.0.4-1.fc24.noarch.rpm
          openstack-rally-0.0.4-1.fc24.src.rpm
openstack-rally.noarch: W: spelling-error Summary(en_US) Benchmarking -> Bench marking, Bench-marking, Benchmark
openstack-rally.noarch: W: spelling-error %description -l en_US benchmarking -> bench marking, bench-marking, benchmark
openstack-rally.noarch: W: no-documentation
openstack-rally.noarch: W: non-conffile-in-etc /etc/bash_completion.d/rally.bash_completion
openstack-rally.noarch: W: no-manual-page-for-binary rally-manage
openstack-rally.noarch: W: no-manual-page-for-binary rally
openstack-rally.noarch: W: dangerous-command-in-%post chmod
openstack-rally.src: W: spelling-error Summary(en_US) Benchmarking -> Bench marking, Bench-marking, Benchmark
openstack-rally.src: W: spelling-error %description -l en_US benchmarking -> bench marking, bench-marking, benchmark
2 packages and 0 specfiles checked; 0 errors, 9 warnings.




Rpmlint (installed packages)
----------------------------
openstack-rally.noarch: W: no-documentation
openstack-rally.noarch: W: non-conffile-in-etc /etc/bash_completion.d/rally.bash_completion
openstack-rally.noarch: W: no-manual-page-for-binary rally-manage
openstack-rally.noarch: W: no-manual-page-for-binary rally
openstack-rally.noarch: W: dangerous-command-in-%post chmod
1 packages and 0 specfiles checked; 0 errors, 5 warnings.



Requires
--------
openstack-rally (rpmlib, GLIBC filtered):
    /bin/sh
    /usr/bin/python2
    PyYAML
    python(abi)
    python-babel
    python-boto
    python-ceilometerclient
    python-cinderclient
    python-decorator
    python-designateclient
    python-fixtures
    python-glanceclient
    python-heatclient
    python-ironicclient
    python-iso8601
    python-jinja2
    python-jsonschema
    python-keystoneclient
    python-netaddr
    python-neutronclient
    python-novaclient
    python-oslo-config
    python-oslo-db
    python-oslo-i18n
    python-oslo-serialization
    python-oslo-utils
    python-paramiko
    python-pecan
    python-prettytable
    python-psycopg2
    python-requests
    python-saharaclient
    python-six
    python-sphinx
    python-sqlalchemy
    python-subunit
    python-troveclient
    python-wsme
    python-zaqarclient



Provides
--------
openstack-rally:
    openstack-rally



Source checksums
----------------
https://pypi.python.org/packages/source/r/rally/rally-0.0.4.tar.gz :
  CHECKSUM(SHA256) this package     : 24302f9cfbd7dc2fea9a1ec84fafcc34cd061dbd21a34c0e403a953b99792d43
  CHECKSUM(SHA256) upstream package : 24302f9cfbd7dc2fea9a1ec84fafcc34cd061dbd21a34c0e403a953b99792d43


Generated by fedora-review 0.5.3 (bcf15e3) last change: 2015-05-04
Command line :/usr/bin/fedora-review -b 1193986 -m fedora-rawhide-x86_64
Buildroot used: fedora-rawhide-x86_64
Active plugins: Python, Generic, Shell-api
Disabled plugins: Java, C/C++, fonts, SugarActivity, Ocaml, Perl, Haskell, R, PHP, Ruby
Disabled flags: EXARCH, DISTTAG, EPEL5, BATCH, EPEL6

@Rohan : this will be added in delorean, so every commit in upstream master branch will be built into packages. So Delorean package of Rally will be what you want to use.

We do not ship patches in RDO stable packages unless there's a good reason (it needs a ticket and been reviewed by the packaging team).

@Victoria, Please help me get this patch in RDO "openstack-rally" package. The patch is very essential as explained in comment 26.

patch: https://review.openstack.org/#/c/220414/

Victoria,

Any updates on comment 30?

Rohan, as Haikel explained, openstack-rally is built continuously in Delorean,

latest build is http://trunk.rdoproject.org/centos7-liberty/4b/3f/4b3fa606e2bca7d7f0326ef3d0cfa67e93cbc18b_403a03bf/openstack-rally-0.1.2-dev22.el7.centos.noarch.rpm

from latest packaging spec commit https://github.com/openstack-packages/rally/commits/rpm-master
and upstream git hash https://github.com/openstack/rally/commit/4b3fa606e2bca7d7f0326ef3d0cfa67e93cbc18b
which includes your commit https://github.com/openstack/rally/commit/44466034668e75af0294e4b89b192eaccc3a3375