Spec URL: https://github.com/agroup/tripleo-rpm-spec-files/blob/master/ironic/openstack-ironic.spec SRPM URL: http://athomas.fedorapeople.org/tuskarclient/python-tuskarclient-0.1.0-2.fc20.src.rpm Description: Ironic provides an API for management and provisioning of physical machines Fedora Account System Username: athomas
I believe I am allowed to sponsor athomas for the openstack fedora group. If not, please let me know and I'll find someone who can. Overall, review looks good - please check the ! items below and tidy those up. Output from fedora-review: Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed, [j] = checked by me ===== MUST items ===== Generic: [j]: Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [j]: 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", "*No copyright* Apache (v2.0)". 2 files have unknown license. Detailed output of licensecheck in /home/jomara/dev/fedora-reviews/python-tuskarclient/licensecheck.txt Project is ASL 2.0 [j]: Package contains no bundled libraries without FPC exception. check [j]: Changelog in prescribed format. check [j]: Sources contain only permissible code or content. [j(NA)]: Package contains desktop file if it is a GUI application. [j]: Development files must be in a -devel package [j]: Package uses nothing in %doc for runtime. [j]: Package consistently uses macros (instead of hard-coded directory names). [j]: Package is named according to the Package Naming Guidelines. [j]: Package does not generate any conflict. [j]: Package obeys FHS, except libexecdir and /usr/target. [j]: If the package is a rename of another package, proper Obsoletes and Provides are present. [j]: Requires correct, justified where necessary. [j]: Spec file is legible and written in American English. [j]: Package contains systemd file(s) if in need. [j]: Package is not known to require an ExcludeArch tag. [j]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 20480 bytes in 2 files. [j]: 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 %doc. [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]: 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. [x]: Packages must not store files under /srv, /opt or /usr/local Python: [j]: Python eggs must not download any dependencies during the build process. [!]: A package which is used by another package via an egg interface should provide egg info. Egg Info is missing from this package, might be worth adding . I believe this IS required based on python packaging guidelines [!]: Package meets the Packaging Guidelines::Python - based on my read, you need to BuildRequires: python2-devel - egginfo [x]: Package contains BR: python2-devel or python3-devel [x]: Binary eggs must be removed in %prep ===== SHOULD items ===== Generic: [j]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [j]: Final provides and requires are sane (see attachments). [j]: Package functions as described. [j]: Latest version is packaged. [j]: 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. I believe, but am not positive, that translations for this package are surfaced in other packages [?]: Package should compile and build into binary rpms on all supported architectures. noarch [!j]: %check is present and all tests pass. I don't see a %check section, but I don't typically see one with packages like this. Also not in "MUST" section [?]: 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: [x]: Rpmlint is run on all installed packages. Great work!
Jordan, you can not sponsor Angus into the packager group. Angus, are you going to maintain this package?
Angus, I'll sponsor you. Please have a quick read: https://fedoraproject.org/wiki/Join_the_package_collection_maintainers Let me know when you have completed this step.
Angus, the link for the SRPM Url is for python-tuskarclient, not openstack-ironic :)
I suggest providing a raw download link to the spec if using github: Spec URL: https://raw.github.com/agroup/tripleo-rpm-spec-files/master/ironic/openstack-ironic.spec SRPM URL: http://repos.fedorapeople.org/repos/openstack-m/openstack-m/fedora-20/SRPMS/openstack-ironic-2014.1-b2.2.fc20.src.rpm
Hi, I'm doing an unofficial review. Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed Issues: ======= - Package contains BR: python2-devel or python3-devel Change the BR from python-devel to python2-devel - Inconsistent alignment in spec file - Looks like you got a request for more info on the fedora ticket request for the static uid/gid - Other issues in the comments below ===== 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", "*No copyright* Apache (v2.0)". 6 files have unknown license. Detailed output of licensecheck in /home/jslagle/rpmbuild/1069335-openstack-ironic/review-openstack- ironic/licensecheck.txt [x]: Package must own all directories that it creates. Note: Directories without known owners: /usr/lib/systemd/system, /usr/lib/systemd [x]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [!]: %config files are marked noreplace or the reason is justified. Note: No (noreplace) in %config %attr(-,root,ironic) /etc/ironic recommend to add noreplace to the above %config. We don't want local changes to these files overwritten by rpm do we? [-]: 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). Use %{_unitdir} for /usr/lib/systemd/system This requires adding BuildRequires: systemd per https://fedoraproject.org/wiki/Packaging:Systemd [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. [!]: Requires correct, justified where necessary. Why are there runtime requirements on the following? - swig - pyflakes These aren't in upstream ironic's requirements.txt, so they shouldn't be needed. Also need to remove the Requires on python-pbr. You'll have to provide a patch to remove the runtime requirement. See our other OpenStack packages for examples of this, (e.g. in glance there is a patch called 0002-Remove-runtime-dep-on-python-pbr.patch) [x]: Spec file is legible and written in American English. [x]: 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. Note: Documentation size is 20480 bytes in 2 files. [!]: Package complies to the Packaging Guidelines Outstanding issues mentioned elsewhere in this review. [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 %doc. [x]: Package requires other packages for directories it uses. [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]: 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]: No %config files under /usr. [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. [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. [!]: Package meets the Packaging Guidelines::Python See other outstanding issues [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. [!]: Final provides and requires are sane (see attachments). Why is there a Provides on ironic? This shouldn't be necessary should it? Other Outstanding issues here, please see other comments [ ]: 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. only tested on x86_64 [-]: %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: [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-ironic-2014.1-b2.2.fc21.noarch.rpm openstack-ironic-2014.1-b2.2.fc21.src.rpm openstack-ironic.noarch: W: conffile-without-noreplace-flag /etc/ironic/rootwrap.conf openstack-ironic.noarch: W: conffile-without-noreplace-flag /etc/ironic/policy.json openstack-ironic.noarch: W: conffile-without-noreplace-flag /etc/ironic/rootwrap.d/ironic-deploy-helper.filters openstack-ironic.noarch: W: conffile-without-noreplace-flag /etc/ironic/rootwrap.d/ironic-images.filters openstack-ironic.noarch: W: conffile-without-noreplace-flag /etc/ironic/rootwrap.d/ironic-manage-ipmi.filters openstack-ironic.noarch: W: conffile-without-noreplace-flag /etc/ironic/ironic.conf openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/cmd/dbsync.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/common/service.py 0644L /usr/bin/env openstack-ironic.noarch: W: non-standard-gid /etc/ironic/rootwrap.conf ironic openstack-ironic.noarch: E: non-readable /etc/ironic/rootwrap.conf 0640L openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/api/controllers/v1/link.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/cmd/conductor.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/openstack/common/rpc/zmq_receiver.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/drivers/modules/deploy_utils.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/api/controllers/v1/driver.py 0644L /usr/bin/env openstack-ironic.noarch: W: non-standard-gid /etc/ironic/policy.json ironic openstack-ironic.noarch: E: non-readable /etc/ironic/policy.json 0640L openstack-ironic.noarch: W: non-standard-gid /etc/ironic ironic openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/api/controllers/v1/state.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/tests/api/test_utils.py 0644L /usr/bin/env openstack-ironic.noarch: W: non-standard-gid /etc/ironic/rootwrap.d/ironic-deploy-helper.filters ironic openstack-ironic.noarch: E: non-readable /etc/ironic/rootwrap.d/ironic-deploy-helper.filters 0640L openstack-ironic.noarch: W: non-standard-uid /var/lib/ironic ironic openstack-ironic.noarch: W: non-standard-gid /var/lib/ironic ironic openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/api/controllers/v1/chassis.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/common/driver_factory.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/tests/api/test_root.py 0644L /usr/bin/env openstack-ironic.noarch: W: non-standard-gid /etc/ironic/rootwrap.d/ironic-images.filters ironic openstack-ironic.noarch: E: non-readable /etc/ironic/rootwrap.d/ironic-images.filters 0640L openstack-ironic.noarch: W: non-standard-gid /etc/ironic/rootwrap.d ironic openstack-ironic.noarch: W: non-standard-gid /etc/ironic/rootwrap.d/ironic-manage-ipmi.filters ironic openstack-ironic.noarch: E: non-readable /etc/ironic/rootwrap.d/ironic-manage-ipmi.filters 0640L openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/api/controllers/v1/utils.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/api/controllers/v1/collection.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/tests/api/test_drivers.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/cmd/api.py 0644L /usr/bin/env openstack-ironic.noarch: W: non-standard-gid /etc/ironic/ironic.conf ironic openstack-ironic.noarch: E: non-readable /etc/ironic/ironic.conf 0640L openstack-ironic.noarch: W: no-manual-page-for-binary ironic-rootwrap openstack-ironic.noarch: W: no-manual-page-for-binary ironic-dbsync openstack-ironic.noarch: W: no-manual-page-for-binary ironic-conductor openstack-ironic.noarch: W: no-manual-page-for-binary ironic-api openstack-ironic.src:16: W: unversioned-explicit-provides ironic openstack-ironic.src:83: E: hardcoded-library-path in %{buildroot}/usr/lib/systemd/system/ openstack-ironic.src:84: E: hardcoded-library-path in %{_prefix}/lib/systemd/system/ openstack-ironic.src:85: E: hardcoded-library-path in %{_prefix}/lib/systemd/system/ openstack-ironic.src:98: E: hardcoded-library-path in %{_prefix}/lib/systemd/system/* openstack-ironic.src:18: W: mixed-use-of-spaces-and-tabs (spaces: line 18, tab: line 8) 2 packages and 0 specfiles checked; 26 errors, 22 warnings. Besides the other feedback above, the other warnings and errors look ok here. Rpmlint (installed packages) ---------------------------- # rpmlint openstack-ironic openstack-ironic.noarch: W: conffile-without-noreplace-flag /etc/ironic/rootwrap.conf openstack-ironic.noarch: W: conffile-without-noreplace-flag /etc/ironic/policy.json openstack-ironic.noarch: W: conffile-without-noreplace-flag /etc/ironic/rootwrap.d/ironic-deploy-helper.filters openstack-ironic.noarch: W: conffile-without-noreplace-flag /etc/ironic/rootwrap.d/ironic-images.filters openstack-ironic.noarch: W: conffile-without-noreplace-flag /etc/ironic/rootwrap.d/ironic-manage-ipmi.filters openstack-ironic.noarch: W: conffile-without-noreplace-flag /etc/ironic/ironic.conf openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/cmd/dbsync.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/common/service.py 0644L /usr/bin/env openstack-ironic.noarch: W: non-standard-gid /etc/ironic/rootwrap.conf ironic openstack-ironic.noarch: E: non-readable /etc/ironic/rootwrap.conf 0640L openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/api/controllers/v1/link.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/cmd/conductor.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/openstack/common/rpc/zmq_receiver.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/drivers/modules/deploy_utils.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/api/controllers/v1/driver.py 0644L /usr/bin/env openstack-ironic.noarch: W: non-standard-gid /etc/ironic/policy.json ironic openstack-ironic.noarch: E: non-readable /etc/ironic/policy.json 0640L openstack-ironic.noarch: W: non-standard-gid /etc/ironic ironic openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/api/controllers/v1/state.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/tests/api/test_utils.py 0644L /usr/bin/env openstack-ironic.noarch: W: non-standard-gid /etc/ironic/rootwrap.d/ironic-deploy-helper.filters ironic openstack-ironic.noarch: E: non-readable /etc/ironic/rootwrap.d/ironic-deploy-helper.filters 0640L openstack-ironic.noarch: W: non-standard-uid /var/lib/ironic ironic openstack-ironic.noarch: W: non-standard-gid /var/lib/ironic ironic openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/api/controllers/v1/chassis.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/common/driver_factory.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/tests/api/test_root.py 0644L /usr/bin/env openstack-ironic.noarch: W: non-standard-gid /etc/ironic/rootwrap.d/ironic-images.filters ironic openstack-ironic.noarch: E: non-readable /etc/ironic/rootwrap.d/ironic-images.filters 0640L openstack-ironic.noarch: W: non-standard-gid /etc/ironic/rootwrap.d ironic openstack-ironic.noarch: W: non-standard-gid /etc/ironic/rootwrap.d/ironic-manage-ipmi.filters ironic openstack-ironic.noarch: E: non-readable /etc/ironic/rootwrap.d/ironic-manage-ipmi.filters 0640L openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/api/controllers/v1/utils.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/api/controllers/v1/collection.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/tests/api/test_drivers.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/cmd/api.py 0644L /usr/bin/env openstack-ironic.noarch: W: non-standard-gid /etc/ironic/ironic.conf ironic openstack-ironic.noarch: E: non-readable /etc/ironic/ironic.conf 0640L openstack-ironic.noarch: W: no-manual-page-for-binary ironic-rootwrap openstack-ironic.noarch: W: no-manual-page-for-binary ironic-dbsync openstack-ironic.noarch: W: no-manual-page-for-binary ironic-conductor openstack-ironic.noarch: W: no-manual-page-for-binary ironic-api 1 packages and 0 specfiles checked; 22 errors, 20 warnings. # echo 'rpmlint-done:' Requires -------- openstack-ironic (rpmlib, GLIBC filtered): /bin/sh /usr/bin/python config(openstack-ironic) pycrypto pyflakes python(abi) python-anyjson python-eventlet python-fixtures python-glanceclient python-iso8601 python-jinja2 python-jsonpatch python-keystoneclient python-kombu python-migrate python-mock python-netaddr python-neutronclient python-paramiko python-pbr python-pecan python-sqlalchemy python-stevedore python-wsme shadow-utils swig Provides -------- openstack-ironic: config(openstack-ironic) ironic openstack-ironic Source checksums ---------------- https://launchpad.net/ironic/icehouse/icehouse-2/+download/ironic-2014.1.b2.tar.gz : CHECKSUM(SHA256) this package : b3a3bdf04df2e15519058fbe66d63b5156bcde8fc35c6cda787db3c34f2c637e CHECKSUM(SHA256) upstream package : b3a3bdf04df2e15519058fbe66d63b5156bcde8fc35c6cda787db3c34f2c637e Generated by fedora-review 0.5.1 (bb9bf27) last change: 2013-12-13 Command line :/usr/bin/fedora-review -n openstack-ironic 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, EPEL5, BATCH, DISTTAG
James, Have a read of: https://fedoraproject.org/wiki/Packaging:ScriptletSnippets Does anything pop out to you as a suggestion related to systemd? Regards -steve
James, It is typically the packagers responsibility to diagnose and fix the rpmlint warnings, but as the package reviewer, you have an opportunity to provide guidance about what you would recommend changing to fix the rpmlint problems. Many of those errors and warnings cannot be ignored. If you as the package reviewer think they can be ignored, it is your responsibility to actually verify that is the case. It is the packagers responsibility to defend any decisions to ignore rpmlint warnings. One cool thing about being a packager is two brains working together > 2 brains working separately. Please have a more detailed look at the rpmlint warnings and errors and comment in detail. Regards, -steve
Angus, I'd recommend having a read through this review: https://bugzilla.redhat.com/show_bug.cgi?id=1066629 as well as: https://bugzilla.redhat.com/show_bug.cgi?id=1066633 then providing an unofficial review of: https://bugzilla.redhat.com/show_bug.cgi?id=1067002 followed by providing an unofficial review of the other two bugs mentioned in this comment. Then we should be all set :)
Angus, I just wanted to point out a couple pieces of documentation that you may or not be aware of. I would recommend blocking off about 1-2 hours and reading through them. The more experience you get reviewing, the less you will have to reference the documentation, but if its not documented, its probably not required. The package review process: https://fedoraproject.org/wiki/Package_Review_Process The packaging guidelines: https://fedoraproject.org/wiki/Packaging:ReviewGuidelines This contain 32 separate wiki pages. I would recommend reading through them so you know where to find information in future packaging efforts. The python-specific guidelines: https://fedoraproject.org/wiki/Packaging:Python Regards, -steve
(In reply to Steven Dake from comment #7) > James, > > Have a read of: > https://fedoraproject.org/wiki/Packaging:ScriptletSnippets > > Does anything pop out to you as a suggestion related to systemd? > > Regards > -steve Ah, Yes :) Per https://fedoraproject.org/wiki/Starting_services_by_default, openstack-ironic-api and openstack-ironic-conductor should not be started by default. And, need to add the following to the spec file: %post %systemd_post openstack-ironic-api.service %systemd_post openstack-ironic-conductor.service %preun %systemd_preun openstack-ironic-api.service %systemd_preun openstack-ironic-conductor.service %postun %systemd_postun_with_restart openstack-ironic-api.service %systemd_postun_with_restart openstack-ironic-conductor.service The other OpenStack packages use the scriptlets themselves and not the %systemd_* macros though. Is there a reason for that?
(In reply to Steven Dake from comment #8) > James, > > It is typically the packagers responsibility to diagnose and fix the rpmlint > warnings, but as the package reviewer, you have an opportunity to provide > guidance about what you would recommend changing to fix the rpmlint > problems. Many of those errors and warnings cannot be ignored. If you as > the package reviewer think they can be ignored, it is your responsibility to > actually verify that is the case. It is the packagers responsibility to > defend any decisions to ignore rpmlint warnings. One cool thing about being > a packager is two brains working together > 2 brains working separately. > Please have a more detailed look at the rpmlint warnings and errors and > comment in detail. > > Regards, > -steve I was using the other OpenStack packages already in rdo to determine what I thought was ok to allow here. Here's my deeper feedback on the SRPM lint: Rpmlint ------- Checking: openstack-ironic-2014.1-b2.2.fc21.noarch.rpm openstack-ironic-2014.1-b2.2.fc21.src.rpm openstack-ironic.noarch: W: conffile-without-noreplace-flag /etc/ironic/rootwrap.conf openstack-ironic.noarch: W: conffile-without-noreplace-flag /etc/ironic/policy.json openstack-ironic.noarch: W: conffile-without-noreplace-flag /etc/ironic/rootwrap.d/ironic-deploy-helper.filters openstack-ironic.noarch: W: conffile-without-noreplace-flag /etc/ironic/rootwrap.d/ironic-images.filters openstack-ironic.noarch: W: conffile-without-noreplace-flag /etc/ironic/rootwrap.d/ironic-manage-ipmi.filters openstack-ironic.noarch: W: conffile-without-noreplace-flag /etc/ironic/ironic.conf Suggest to add %noreplace for above files, see earlier feedback in review. openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/cmd/dbsync.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/common/service.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/api/controllers/v1/link.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/cmd/conductor.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/openstack/common/rpc/zmq_receiver.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/drivers/modules/deploy_utils.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/api/controllers/v1/driver.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/api/controllers/v1/state.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/tests/api/test_utils.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/api/controllers/v1/chassis.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/common/driver_factory.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/tests/api/test_root.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/api/controllers/v1/utils.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/api/controllers/v1/collection.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/tests/api/test_drivers.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/cmd/api.py 0644L /usr/bin/env Didn't see anything specific about what to do for these at https://fedoraproject.org/wiki/Packaging:Python rpmlint is complaining I assume b/c there's a shebang in the files, but they aren't actually meant to be scripts and aren't executable. Don't see anything about this at https://fedoraproject.org/wiki/Common_Rpmlint_issues The files themselves aren't +x, given the output above. What's the path forward here? It seems like the only thing that can be done in the spec itself is to make these files +x in %files. But, these things aren't actually scripts (from what I can tell), so a patch upstream to remove the #!/usr/bin/env lines from these files should probably be sumitted. Then we can add that patch to the rpm until it is merged. openstack-ironic.noarch: W: non-standard-gid /etc/ironic/rootwrap.conf ironic openstack-ironic.noarch: W: non-standard-gid /etc/ironic/policy.json ironic openstack-ironic.noarch: W: non-standard-gid /etc/ironic ironic openstack-ironic.noarch: W: non-standard-gid /etc/ironic/rootwrap.d/ironic-deploy-helper.filters ironic openstack-ironic.noarch: W: non-standard-uid /var/lib/ironic ironic openstack-ironic.noarch: W: non-standard-gid /var/lib/ironic ironic openstack-ironic.noarch: W: non-standard-gid /etc/ironic/rootwrap.d/ironic-images.filters ironic openstack-ironic.noarch: W: non-standard-gid /etc/ironic/rootwrap.d ironic openstack-ironic.noarch: W: non-standard-gid /etc/ironic/rootwrap.d/ironic-manage-ipmi.filters ironic openstack-ironic.noarch: W: non-standard-gid /etc/ironic/ironic.conf ironic These can be explained by the fact that ironic is a non standard group, so rpm doesn't know about it. I think that's ok. openstack-ironic.noarch: E: non-readable /etc/ironic/rootwrap.conf 0640L openstack-ironic.noarch: E: non-readable /etc/ironic/policy.json 0640L openstack-ironic.noarch: E: non-readable /etc/ironic/rootwrap.d/ironic-deploy-helper.filters 0640L openstack-ironic.noarch: E: non-readable /etc/ironic/rootwrap.d/ironic-images.filters 0640L openstack-ironic.noarch: E: non-readable /etc/ironic/rootwrap.d/ironic-manage-ipmi.filters 0640L openstack-ironic.noarch: E: non-readable /etc/ironic/ironic.conf 0640L These errors are b/c the files are not world readable. That is what is desired I believe. Do we need to flag them as exceptions to rpmlint somehow? openstack-ironic.noarch: W: no-manual-page-for-binary ironic-rootwrap openstack-ironic.noarch: W: no-manual-page-for-binary ironic-dbsync openstack-ironic.noarch: W: no-manual-page-for-binary ironic-conductor openstack-ironic.noarch: W: no-manual-page-for-binary ironic-api man pages don't exist for these commands. They don't for most OpenStack commands. Is that ok to ignore? openstack-ironic.src:16: W: unversioned-explicit-provides ironic This needs to be fixed. See earlier review comments. openstack-ironic.src:83: E: hardcoded-library-path in %{buildroot}/usr/lib/systemd/system/ openstack-ironic.src:84: E: hardcoded-library-path in %{_prefix}/lib/systemd/system/ openstack-ironic.src:85: E: hardcoded-library-path in %{_prefix}/lib/systemd/system/ openstack-ironic.src:98: E: hardcoded-library-path in %{_prefix}/lib/systemd/system/* This needs to be fixed. See earlier review comments. openstack-ironic.src:18: W: mixed-use-of-spaces-and-tabs (spaces: line 18, tab: line 8) This needs to be fixed. See earlier review comments. 2 packages and 0 specfiles checked; 26 errors, 22 warnings.
I don't see anything additional from the binary RPM lint that I haven't commented on above. openstack-ironic.noarch: W: conffile-without-noreplace-flag /etc/ironic/rootwrap.conf openstack-ironic.noarch: W: conffile-without-noreplace-flag /etc/ironic/policy.json openstack-ironic.noarch: W: conffile-without-noreplace-flag /etc/ironic/rootwrap.d/ironic-deploy-helper.filters openstack-ironic.noarch: W: conffile-without-noreplace-flag /etc/ironic/rootwrap.d/ironic-images.filters openstack-ironic.noarch: W: conffile-without-noreplace-flag /etc/ironic/rootwrap.d/ironic-manage-ipmi.filters openstack-ironic.noarch: W: conffile-without-noreplace-flag /etc/ironic/ironic.conf openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/cmd/dbsync.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/common/service.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/api/controllers/v1/link.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/cmd/conductor.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/openstack/common/rpc/zmq_receiver.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/drivers/modules/deploy_utils.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/api/controllers/v1/driver.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/api/controllers/v1/state.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/tests/api/test_utils.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/api/controllers/v1/chassis.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/common/driver_factory.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/tests/api/test_root.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/api/controllers/v1/utils.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/api/controllers/v1/collection.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/tests/api/test_drivers.py 0644L /usr/bin/env openstack-ironic.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/ironic/cmd/api.py 0644L /usr/bin/env openstack-ironic.noarch: W: non-standard-gid /etc/ironic/rootwrap.conf ironic openstack-ironic.noarch: W: non-standard-gid /etc/ironic/policy.json ironic openstack-ironic.noarch: W: non-standard-gid /etc/ironic ironic openstack-ironic.noarch: W: non-standard-gid /etc/ironic/rootwrap.d/ironic-deploy-helper.filters ironic openstack-ironic.noarch: W: non-standard-uid /var/lib/ironic ironic openstack-ironic.noarch: W: non-standard-gid /var/lib/ironic ironic openstack-ironic.noarch: W: non-standard-gid /etc/ironic/rootwrap.d/ironic-images.filters ironic openstack-ironic.noarch: W: non-standard-gid /etc/ironic/rootwrap.d ironic openstack-ironic.noarch: W: non-standard-gid /etc/ironic/rootwrap.d/ironic-manage-ipmi.filters ironic openstack-ironic.noarch: W: non-standard-gid /etc/ironic/ironic.conf ironic openstack-ironic.noarch: E: non-readable /etc/ironic/policy.json 0640L openstack-ironic.noarch: E: non-readable /etc/ironic/rootwrap.conf 0640L openstack-ironic.noarch: E: non-readable /etc/ironic/rootwrap.d/ironic-deploy-helper.filters 0640L openstack-ironic.noarch: E: non-readable /etc/ironic/rootwrap.d/ironic-images.filters 0640L openstack-ironic.noarch: E: non-readable /etc/ironic/rootwrap.d/ironic-manage-ipmi.filters 0640L openstack-ironic.noarch: E: non-readable /etc/ironic/ironic.conf 0640L openstack-ironic.noarch: W: no-manual-page-for-binary ironic-rootwrap openstack-ironic.noarch: W: no-manual-page-for-binary ironic-dbsync openstack-ironic.noarch: W: no-manual-page-for-binary ironic-conductor openstack-ironic.noarch: W: no-manual-page-for-binary ironic-api 1 packages and 0 specfiles checked; 22 errors, 20 warnings. # echo 'rpmlint-done:'
Regarding Comment #11, there is also some Requires (for systemd) required. I am pretty sure the rest of the openstack packages break out the api from the conductor. This is not something a reviewer would typically comment on (its the packager's responsibility to decide how they want to package something) but for consistency it might make sense to make them subpackages.
Re Comment #12: What's the path forward here? It seems like the only thing that can be done in the spec itself is to make these files +x in %files. But, these things aren't actually scripts (from what I can tell), so a patch upstream to remove the #!/usr/bin/env lines from these files should probably be sumitted. Then we can add that patch to the rpm until it is merged. Nice work. You identified the problem and what to do (submit a patch upstream). The review doesn't need to gate on this, but ideally upstream should fix this oversight. I don't think its necessary to carry a patch on top of the patch stream in the RPM to fix the rpmlint warnings as long as an upstream bug is filed by the submitter (Angus) and linked in the bugzilla for further reference. Re: openstack-ironic.noarch: E: non-readable /etc/ironic/rootwrap.conf 0640L openstack-ironic.noarch: E: non-readable /etc/ironic/policy.json 0640L openstack-ironic.noarch: E: non-readable /etc/ironic/rootwrap.d/ironic-deploy-helper.filters 0640L openstack-ironic.noarch: E: non-readable /etc/ironic/rootwrap.d/ironic-images.filters 0640L openstack-ironic.noarch: E: non-readable /etc/ironic/rootwrap.d/ironic-manage-ipmi.filters 0640L openstack-ironic.noarch: E: non-readable /etc/ironic/ironic.conf 0640L These errors are b/c the files are not world readable. That is what is desired I believe. Do we need to flag them as exceptions to rpmlint somehow? Unfortunately rpmlint doesn't have said flagging feature. It makes sense for the reviewer in this case to build the rpm and make sure /etc/ironic and /var/lib/ironic actually are installed with the uid/gid specified in the spec file before signing off on these specific rpmlint E messages. Generally if rpmlint gives an error, and you want to fedora-review+ the package, the reviewer is responsible for verifying the E is a false-negative. Re man pages, what I typically do is ask in the review for the submitter to request the upstream create manual pages for their binaries. In the case of OpenStack, the upstream doesn't seem to care, but it doesn't hurt to create said bugs anyway. Looking good James!
Many thanks for the detailed feedback. Based on all the recommended changes: Spec URL: https://raw.github.com/agroup/tripleo-rpm-spec-files/master/ironic/openstack-ironic.spec SRPM URL: http://athomas.fedorapeople.org/ironic/fedora20/openstack-ironic-2014.1-b2.3.fc20.src.rpm
Hi Angus, the latest iteration does not build in mock. Nothing gets installed because you also removed the BuildRequires on python-pbr. It's ok to have python-pbr as a BuildRequires (and is in fact needed), just not as a Requires.
Hi James. Sorry about that. It fooled me by building fine locally. Fixed now. Spec URL: https://raw.github.com/agroup/tripleo-rpm-spec-files/master/ironic/openstack-ironic.spec SRPM URL: http://athomas.fedorapeople.org/ironic/fedora20/openstack-ironic-2014.1-b2.4.fc20.src.rpm
(In reply to Angus Thomas from comment #18) > Hi James. > > Sorry about that. It fooled me by building fine locally. Fixed now. > Angus, You can always use mock to build locally but without all your installed packages which might not pick out problems. The fedora-review tool uses mock if you don't want to learn the details of mock, so that is always an option.
Angus, I would recommend splitting up the api and conductor into separate packages. All the OpenStack packaging behaves in this way. For an example of how it is done take a look at the nova or heat repos in fedpkg. (fedpkg clone openstack-heat). If you think this is too big a task to take on, we can approve as is, but I suspect someone will have to go in and rework that part of the packaging, so we might as well get it out of the way now. The reason for separate packages is the api may be installed separately from the engine.
Hi. New version which builds into -common, -api and -conductor packages Spec URL: https://raw.github.com/agroup/tripleo-rpm-spec-files/master/ironic/openstack-ironic.spec SRPM URL: http://athomas.fedorapeople.org/ironic/fedora20/openstack-ironic-2014.1-b2.5.fc20.src.rpm Angus
New version based on Icehouse RC1 Spec URL: https://raw.github.com/agroup/tripleo-rpm-spec-files/master/ironic/openstack-ironic.spec SRPM URL: http://athomas.fedorapeople.org/ironic/fedora20/SRPMS/openstack-ironic-2014.1-rc1.1.fc20.src.rpm
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)", "Apache (v2.0) MIT/X11 (BSD like)", "Unknown or generated", "*No copyright* Apache (v2.0)". 8 files have unknown license. Detailed output of licensecheck in /home/sdake/fedora-review/1069335 -openstack-ironic/licensecheck.txt [!]: License file installed when any subpackage combination is installed. [-]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [x]: 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. [x]: 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. Note: Documentation size is 20480 bytes in 2 files. [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 %doc. [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]: %config files are marked noreplace or the reason is justified. [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]: No %config files under /usr. [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. [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: [x]: 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]: Fully versioned dependency in subpackages if applicable. Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in openstack- ironic-common , openstack-ironic-api , openstack-ironic-conductor [x]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [-]: Patches link to upstream bugs/comments/lists or are otherwise justified. [x]: Scriptlets must be sane, if used. [x]: 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]: 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: [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-ironic-common-2014.1-rc1.1.fc20.noarch.rpm openstack-ironic-api-2014.1-rc1.1.fc20.noarch.rpm openstack-ironic-conductor-2014.1-rc1.1.fc20.noarch.rpm openstack-ironic-2014.1-rc1.1.fc20.src.rpm openstack-ironic-common.noarch: W: non-standard-gid /etc/ironic/rootwrap.conf ironic openstack-ironic-common.noarch: E: non-readable /etc/ironic/rootwrap.conf 0640L openstack-ironic-common.noarch: W: non-standard-gid /etc/ironic/policy.json ironic openstack-ironic-common.noarch: E: non-readable /etc/ironic/policy.json 0640L openstack-ironic-common.noarch: W: non-standard-gid /etc/ironic ironic openstack-ironic-common.noarch: W: non-standard-uid /var/lib/ironic ironic openstack-ironic-common.noarch: W: non-standard-gid /var/lib/ironic ironic openstack-ironic-common.noarch: W: non-standard-gid /etc/ironic/rootwrap.d/ironic-images.filters ironic openstack-ironic-common.noarch: E: non-readable /etc/ironic/rootwrap.d/ironic-images.filters 0640L openstack-ironic-common.noarch: W: non-standard-gid /etc/ironic/rootwrap.d ironic openstack-ironic-common.noarch: W: non-standard-gid /etc/ironic/rootwrap.d/ironic-manage-ipmi.filters ironic openstack-ironic-common.noarch: E: non-readable /etc/ironic/rootwrap.d/ironic-manage-ipmi.filters 0640L openstack-ironic-common.noarch: W: non-standard-gid /etc/ironic/rootwrap.d/ironic-utils.filters ironic openstack-ironic-common.noarch: E: non-readable /etc/ironic/rootwrap.d/ironic-utils.filters 0640L openstack-ironic-common.noarch: W: non-standard-gid /etc/ironic/ironic.conf ironic openstack-ironic-common.noarch: E: non-readable /etc/ironic/ironic.conf 0640L openstack-ironic-common.noarch: W: no-manual-page-for-binary ironic-rootwrap openstack-ironic-common.noarch: W: no-manual-page-for-binary ironic-dbsync openstack-ironic-api.noarch: W: only-non-binary-in-usr-lib openstack-ironic-api.noarch: W: no-documentation openstack-ironic-api.noarch: W: no-manual-page-for-binary ironic-api openstack-ironic-conductor.noarch: W: only-non-binary-in-usr-lib openstack-ironic-conductor.noarch: W: no-documentation openstack-ironic-conductor.noarch: W: no-manual-page-for-binary ironic-conductor openstack-ironic.src:20: W: macro-in-comment %{release_name} openstack-ironic.src:20: W: macro-in-comment %{release_name} openstack-ironic.src:20: W: macro-in-comment %{milestone} openstack-ironic.src:20: W: macro-in-comment %{full_release} openstack-ironic.src:194: W: mixed-use-of-spaces-and-tabs (spaces: line 194, tab: line 7) 4 packages and 0 specfiles checked; 6 errors, 23 warnings. Rpmlint (installed packages) ---------------------------- # rpmlint openstack-ironic-common openstack-ironic-api openstack ^M-ironic-conductor openstack-ironic-common.noarch: W: non-standard-gid /etc/ironic/rootwrap.conf ironic openstack-ironic-common.noarch: E: non-readable /etc/ironic/rootwrap.conf 0640L openstack-ironic-common.noarch: W: non-standard-gid /etc/ironic/policy.json ironic openstack-ironic-common.noarch: E: non-readable /etc/ironic/policy.json 0640L openstack-ironic-common.noarch: W: non-standard-gid /etc/ironic ironic openstack-ironic-common.noarch: W: non-standard-uid /var/lib/ironic ironic openstack-ironic-common.noarch: W: non-standard-gid /var/lib/ironic ironic openstack-ironic-common.noarch: W: non-standard-gid /etc/ironic/rootwrap.d/ironic-images.filters ironic openstack-ironic-common.noarch: E: non-readable /etc/ironic/rootwrap.d/ironic-images.filters 0640L openstack-ironic-common.noarch: W: non-standard-gid /etc/ironic/rootwrap.d ironic openstack-ironic-common.noarch: W: non-standard-gid /etc/ironic/rootwrap.d/ironic-manage-ipmi.filters ironic openstack-ironic-common.noarch: E: non-readable /etc/ironic/rootwrap.d/ironic-manage-ipmi.filters 0640L openstack-ironic-common.noarch: W: non-standard-gid /etc/ironic/rootwrap.d/ironic-utils.filters ironic openstack-ironic-common.noarch: E: non-readable /etc/ironic/rootwrap.d/ironic-utils.filters 0640L openstack-ironic-common.noarch: W: non-standard-gid /etc/ironic/ironic.conf ironic openstack-ironic-common.noarch: E: non-readable /etc/ironic/ironic.conf 0640L openstack-ironic-common.noarch: W: no-manual-page-for-binary ironic-rootwrap openstack-ironic-common.noarch: W: no-manual-page-for-binary ironic-dbsync openstack-ironic-api.noarch: W: only-non-binary-in-usr-lib openstack-ironic-api.noarch: W: no-documentation openstack-ironic-api.noarch: W: no-manual-page-for-binary ironic-api openstack-ironic-conductor.noarch: W: only-non-binary-in-usr-lib openstack-ironic-conductor.noarch: W: no-documentation openstack-ironic-conductor.noarch: W: no-manual-page-for-binary ironic-conductor 3 packages and 0 specfiles checked; 6 errors, 18 warnings. Requires -------- openstack-ironic-common (rpmlib, GLIBC filtered): /bin/sh /usr/bin/python2 config(openstack-ironic-common) pycrypto python(abi) python-alembic python-anyjson python-eventlet python-fixtures python-glanceclient python-iso8601 python-jinja2 python-jsonpatch python-keystoneclient python-kombu python-migrate python-mock python-netaddr python-neutronclient python-paramiko python-pecan python-pyghmi python-sqlalchemy python-stevedore python-wsme shadow-utils openstack-ironic-api (rpmlib, GLIBC filtered): /bin/sh /usr/bin/python2 openstack-ironic-common systemd openstack-ironic-conductor (rpmlib, GLIBC filtered): /bin/sh /usr/bin/python2 openstack-ironic-common systemd Provides -------- openstack-ironic-common: config(openstack-ironic-common) openstack-ironic-common openstack-ironic-api: openstack-ironic-api openstack-ironic-conductor: openstack-ironic-conductor Source checksums ---------------- https://launchpad.net/ironic/icehouse/icehouse-rc1/+download/ironic-2014.1.rc1.tar.gz : CHECKSUM(SHA256) this package : bd5981d3a9a870bfe3a853f87b6d6f636c586bd7ad1da31ebc87a13128ecfb35 CHECKSUM(SHA256) upstream package : bd5981d3a9a870bfe3a853f87b6d6f636c586bd7ad1da31ebc87a13128ecfb35
Angus, Strictly speaking this package meets the license guidelines by installing any package installs common which installs license and readme. I don't like to leave things open to interpretation re licensing however in package reviews, so I think it would make sense to add the license (and possibly the readme) to each of the subpackages as %doc sections. The rpmlint errors look fine, fedora-review gives a clean bill of health, and a manual review of the package since its complex shows it looks to be in good order. I'll be happy to approve once the %doc is added to each subpackage. Regards -steve
Thanks Steve. I've updated the package so that that license is present in the subpackages. Spec URL: https://raw.github.com/agroup/tripleo-rpm-spec-files/master/ironic/openstack-ironic.spec SRPM URL: http://athomas.fedorapeople.org/ironic/fedora20/SRPMS/openstack-ironic-2014.1-rc1.2.fc20.src.rpm Angus
Package in comment #25 APPROVED.
Please submit a fedora SCM request: http://fedoraproject.org/wiki/Package_SCM_admin_requests
New Package SCM Request ======================= Package Name: openstack-ironic Short Description: Ironic provides an API for management and provisioning of physical machines Owners: athomas Branches: f20 InitialCC:
Git done (by process-git-requests).
This is included in rawhide already.