[Bug 1069335] Review Request: openstack-ironic - Management and provisioning of physical machines for Openstack
https://bugzilla.redhat.com/show_bug.cgi?id=1069335 Miroslav Suchý changed: What|Removed |Added Blocks|177841 (FE-NEEDSPONSOR) | Referenced Bugs: https://bugzilla.redhat.com/show_bug.cgi?id=177841 [Bug 177841] Tracker: Review requests from new Fedora packagers who need a sponsor -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1069335] Review Request: openstack-ironic - Management and provisioning of physical machines for Openstack
https://bugzilla.redhat.com/show_bug.cgi?id=1069335 Perry Myers changed: What|Removed |Added Assignee|pmy...@redhat.com |nob...@fedoraproject.org -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1069335] Review Request: openstack-ironic - Management and provisioning of physical machines for Openstack
https://bugzilla.redhat.com/show_bug.cgi?id=1069335 Matthias Runge changed: What|Removed |Added Status|ASSIGNED|CLOSED Resolution|--- |RAWHIDE Last Closed||2015-02-02 05:55:30 --- Comment #31 from Matthias Runge --- This is included in rawhide already. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1069335] Review Request: openstack-ironic - Management and provisioning of physical machines for Openstack
https://bugzilla.redhat.com/show_bug.cgi?id=1069335 John Skeoch changed: What|Removed |Added Assignee|sd...@redhat.com|pmy...@redhat.com -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1069335] Review Request: openstack-ironic - Management and provisioning of physical machines for Openstack
https://bugzilla.redhat.com/show_bug.cgi?id=1069335 Jon Ciesla changed: What|Removed |Added Flags|fedora-cvs? |fedora-cvs+ -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1069335] Review Request: openstack-ironic - Management and provisioning of physical machines for Openstack
https://bugzilla.redhat.com/show_bug.cgi?id=1069335 --- Comment #29 from Jon Ciesla --- Git done (by process-git-requests). -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1069335] Review Request: openstack-ironic - Management and provisioning of physical machines for Openstack
https://bugzilla.redhat.com/show_bug.cgi?id=1069335 Angus Thomas changed: What|Removed |Added Flags||fedora-cvs? --- Comment #28 from Angus Thomas --- 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: -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1069335] Review Request: openstack-ironic - Management and provisioning of physical machines for Openstack
https://bugzilla.redhat.com/show_bug.cgi?id=1069335 Steven Dake changed: What|Removed |Added Flags|fedora-review? |fedora-review+ --- Comment #26 from Steven Dake --- Package in comment #25 APPROVED. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1069335] Review Request: openstack-ironic - Management and provisioning of physical machines for Openstack
https://bugzilla.redhat.com/show_bug.cgi?id=1069335 --- Comment #27 from Steven Dake --- Please submit a fedora SCM request: http://fedoraproject.org/wiki/Package_SCM_admin_requests -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1069335] Review Request: openstack-ironic - Management and provisioning of physical machines for Openstack
https://bugzilla.redhat.com/show_bug.cgi?id=1069335 --- Comment #25 from Angus Thomas --- 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 -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1069335] Review Request: openstack-ironic - Management and provisioning of physical machines for Openstack
https://bugzilla.redhat.com/show_bug.cgi?id=1069335 --- Comment #24 from Steven Dake --- 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 -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1069335] Review Request: openstack-ironic - Management and provisioning of physical machines for Openstack
https://bugzilla.redhat.com/show_bug.cgi?id=1069335
--- Comment #23 from Steven Dake ---
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 supp
[Bug 1069335] Review Request: openstack-ironic - Management and provisioning of physical machines for Openstack
https://bugzilla.redhat.com/show_bug.cgi?id=1069335 --- Comment #22 from Angus Thomas --- 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 -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1069335] Review Request: openstack-ironic - Management and provisioning of physical machines for Openstack
https://bugzilla.redhat.com/show_bug.cgi?id=1069335 --- Comment #21 from Angus Thomas --- 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 -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1069335] Review Request: openstack-ironic - Management and provisioning of physical machines for Openstack
https://bugzilla.redhat.com/show_bug.cgi?id=1069335 --- Comment #20 from Steven Dake --- 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. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1069335] Review Request: openstack-ironic - Management and provisioning of physical machines for Openstack
https://bugzilla.redhat.com/show_bug.cgi?id=1069335 --- Comment #19 from Steven Dake --- (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. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1069335] Review Request: openstack-ironic - Management and provisioning of physical machines for Openstack
https://bugzilla.redhat.com/show_bug.cgi?id=1069335 Bug 1069335 depends on bug 1067445, which changed state. Bug 1067445 Summary: Review Request: python-pyghmi - Python General Hardware Management Initiative https://bugzilla.redhat.com/show_bug.cgi?id=1067445 What|Removed |Added Status|ON_QA |CLOSED Resolution|--- |ERRATA -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1069335] Review Request: openstack-ironic - Management and provisioning of physical machines for Openstack
https://bugzilla.redhat.com/show_bug.cgi?id=1069335 --- Comment #18 from Angus Thomas --- 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 -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1069335] Review Request: openstack-ironic - Management and provisioning of physical machines for Openstack
https://bugzilla.redhat.com/show_bug.cgi?id=1069335 --- Comment #17 from James Slagle --- 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. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1069335] Review Request: openstack-ironic - Management and provisioning of physical machines for Openstack
https://bugzilla.redhat.com/show_bug.cgi?id=1069335 --- Comment #16 from Angus Thomas --- 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 -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1069335] Review Request: openstack-ironic - Management and provisioning of physical machines for Openstack
https://bugzilla.redhat.com/show_bug.cgi?id=1069335 --- Comment #15 from Steven Dake --- 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! -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1069335] Review Request: openstack-ironic - Management and provisioning of physical machines for Openstack
https://bugzilla.redhat.com/show_bug.cgi?id=1069335 --- Comment #14 from Steven Dake --- 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. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1069335] Review Request: openstack-ironic - Management and provisioning of physical machines for Openstack
https://bugzilla.redhat.com/show_bug.cgi?id=1069335 --- Comment #13 from James Slagle --- 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:' -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1069335] Review Request: openstack-ironic - Management and provisioning of physical machines for Openstack
https://bugzilla.redhat.com/show_bug.cgi?id=1069335 --- Comment #12 from James Slagle --- (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-dep
[Bug 1069335] Review Request: openstack-ironic - Management and provisioning of physical machines for Openstack
https://bugzilla.redhat.com/show_bug.cgi?id=1069335 --- Comment #11 from James Slagle --- (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? -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1069335] Review Request: openstack-ironic - Management and provisioning of physical machines for Openstack
https://bugzilla.redhat.com/show_bug.cgi?id=1069335 --- Comment #10 from Steven Dake --- 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 -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1069335] Review Request: openstack-ironic - Management and provisioning of physical machines for Openstack
https://bugzilla.redhat.com/show_bug.cgi?id=1069335 --- Comment #9 from Steven Dake --- 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 :) -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1069335] Review Request: openstack-ironic - Management and provisioning of physical machines for Openstack
https://bugzilla.redhat.com/show_bug.cgi?id=1069335 --- Comment #8 from Steven Dake --- 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 -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1069335] Review Request: openstack-ironic - Management and provisioning of physical machines for Openstack
https://bugzilla.redhat.com/show_bug.cgi?id=1069335 --- Comment #7 from Steven Dake --- 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 -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1069335] Review Request: openstack-ironic - Management and provisioning of physical machines for Openstack
https://bugzilla.redhat.com/show_bug.cgi?id=1069335
--- Comment #6 from James Slagle ---
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 depend
[Bug 1069335] Review Request: openstack-ironic - Management and provisioning of physical machines for Openstack
https://bugzilla.redhat.com/show_bug.cgi?id=1069335 --- Comment #5 from James Slagle --- 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 -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1069335] Review Request: openstack-ironic - Management and provisioning of physical machines for Openstack
https://bugzilla.redhat.com/show_bug.cgi?id=1069335 James Slagle changed: What|Removed |Added CC||jsla...@redhat.com --- Comment #4 from James Slagle --- Angus, the link for the SRPM Url is for python-tuskarclient, not openstack-ironic :) -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1069335] Review Request: openstack-ironic - Management and provisioning of physical machines for Openstack
https://bugzilla.redhat.com/show_bug.cgi?id=1069335 --- Comment #3 from Steven Dake --- 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. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1069335] Review Request: openstack-ironic - Management and provisioning of physical machines for Openstack
https://bugzilla.redhat.com/show_bug.cgi?id=1069335 Steven Dake changed: What|Removed |Added Status|NEW |ASSIGNED Flags||fedora-review? -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1069335] Review Request: openstack-ironic - Management and provisioning of physical machines for Openstack
https://bugzilla.redhat.com/show_bug.cgi?id=1069335 Steven Dake changed: What|Removed |Added CC||sd...@redhat.com Assignee|jom...@redhat.com |sd...@redhat.com Flags|fedora-review? | -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1069335] Review Request: openstack-ironic - Management and provisioning of physical machines for Openstack
https://bugzilla.redhat.com/show_bug.cgi?id=1069335 Matthias Runge changed: What|Removed |Added CC||mru...@redhat.com Blocks||177841 (FE-NEEDSPONSOR) --- Comment #2 from Matthias Runge --- Jordan, you can not sponsor Angus into the packager group. Angus, are you going to maintain this package? Referenced Bugs: https://bugzilla.redhat.com/show_bug.cgi?id=177841 [Bug 177841] Tracker: Review requests from new Fedora packagers who need a sponsor -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1069335] Review Request: openstack-ironic - Management and provisioning of physical machines for Openstack
https://bugzilla.redhat.com/show_bug.cgi?id=1069335
--- Comment #1 from Jordan OMara ---
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
[?]: Pa
[Bug 1069335] Review Request: openstack-ironic - Management and provisioning of physical machines for Openstack
https://bugzilla.redhat.com/show_bug.cgi?id=1069335 Jordan OMara changed: What|Removed |Added CC||jom...@redhat.com Assignee|nob...@fedoraproject.org|jom...@redhat.com Flags||fedora-review? -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1069335] Review Request: openstack-ironic - Management and provisioning of physical machines for Openstack
https://bugzilla.redhat.com/show_bug.cgi?id=1069335 Angus Thomas changed: What|Removed |Added Depends On||1067445 Referenced Bugs: https://bugzilla.redhat.com/show_bug.cgi?id=1067445 [Bug 1067445] Review Request: pyghmi - Python General Hardware Management Initiative -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
