Spec URL: https://raw.github.com/agroup/tripleo-rpm-spec-files/master/tripleo-incubator/openstack-tripleo.spec SRPM URL: http://repos.fedorapeople.org/repos/openstack-m/openstack-m/fedora-20/SRPMS/openstack-tripleo-0.0.2-1.fc20.src.rpm Description: TripleO is a program aimed at installing, upgrading and operating OpenStack clouds using OpenStack's own cloud facilities as the foundations - building on nova, neutron and heat to automate fleet management at datacenter scale. Fedora Account System Username: slagle koji scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=6543886 This is my first package submission for Fedora, and I need a sponsor. I have a few more package review requests that I'll be submitting for the other OpenStack TripleO related projects. I'll look for some other reviews to review myself to help get up to speed on the process :).
James, I'll sponsor you. Please provide some links to other bugs where you have done package reviews in the past.
James, I haven't taken a look at the SRPM, but these problems pop out from the spec file. 1) Python packages need specific BuildRequires: https://fedoraproject.org/wiki/Packaging:Python#BuildRequires I'd add python-setuptools for good measure 2) The installation process is wrong. Please use the python installer to install packages. For an example, check out: https://github.com/sdake/fedora-reviews/blob/master/os-collect-config/os-collect-config.spec#L39 3) The git snapshotting used in the spec file is wrong and does not offer a seamless upgrade process for users. Please use a version of 0 and release of 0.1.snapshot.dist. For an example check out: com/sdake/fedora-reviews/blob/master/openstack-heat-templates/openstack-heat-templates.spec#L7 4) Alignment is likely a mix of tabs and spaces. Recommend sticking to tabs and aligning things on the nearest boundary that makes sense 5) the %files section is wrong. If something is commented out, that means the package doesn't work as you expect - recommend fixing. 6) Recommend %doc any LICENSE or README 7) did you run the spec file, SRPM, and RPM through rpmlint and fix the resulting problems? Regards -steve
Hi Steve, thanks for the initial feedback. I've made some updates: Spec URL: https://raw.github.com/agroup/tripleo-rpm-spec-files/master/tripleo-incubator/openstack-tripleo.spec SRPM URL: http://repos.fedorapeople.org/repos/openstack-m/openstack-m/fedora-20/SRPMS/openstack-tripleo-0.0.1-1.20140220git.fc20.src.rpm 1) & 2): This is not a python package per se. There are 1 or 2 python scripts underneath scripts/, but no python modules. The upstream setup.{py,cfg}, doesn't actually install anything. If we used them to install, it would just create an empty python module called tripleo-incubator. 3) updated for review 4) updated for review 5) updated for review 6) updated for review 7) yes, there are several Warnings, mostly for conffile-without-noreplace-flag (which I don't want), and no-manual-page-for-binary (no manpages exist). Are these Ok?
Steve, here are the bugzilla links for the reviews I've been doing: https://bugzilla.redhat.com/show_bug.cgi?id=1067445 (python-pyghmi) https://bugzilla.redhat.com/show_bug.cgi?id=1067200 (os-collect-config) https://bugzilla.redhat.com/show_bug.cgi?id=1067216 (os-refresh-config) https://bugzilla.redhat.com/show_bug.cgi?id=1065562 (python-scp)
James, Can you provide an unofficial review for: https://bugzilla.redhat.com/show_bug.cgi?id=1069335 Thanks -steve
just completed an unofficial review for openstack-ironic
James, The review isn't complete until the package is fedora-review+ :) But since you can't do that quite yet, please continue to drive the openstack-ironic review to conclusion. Regards -steve
Hi. I'll do an informal review Is it safe to put all those executables into /usr/bin, rather than in a a tripleo-specific directory. Some of the names look dangerously generalised e.g. /usr/bin/cleanup-env /usr/bin/set-os-type /usr/bin/user-config & /usr/bin/wait_for Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed ===== MUST items ===== Generic: [x]: Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [x]: License field in the package spec file matches the actual license. Note: Checking patched sources after %prep for licenses. Licenses found: "Apache (v2.0)", "Unknown or generated". 11 files have unknown license. Detailed output of licensecheck in /home/athomas/openstack- tripleo/licensecheck.txt [x]: License file installed when any subpackage combination is installed. [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 /etc/tripleo Is the absence of noreplace justified? [-]: Package contains desktop file if it is a GUI application. [-]: Development files must be in a -devel package [x]: Package uses nothing in %doc for runtime. [x]: Package consistently uses macros (instead of hard-coded directory names). [x]: Package is named according to the Package Naming Guidelines. [x]: Package does not generate any conflict. [x]: Package obeys FHS, except libexecdir and /usr/target. [-]: If the package is a rename of another package, proper Obsoletes and Provides are present. [x]: Requires correct, justified where necessary. [x]: Spec file is legible and written in American English. [-]: Package contains systemd file(s) if in need. [x]: Package is not known to require an ExcludeArch tag. [-]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 30720 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]: 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 ===== SHOULD items ===== Generic: [-]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [x]: Final provides and requires are sane (see attachments). [x]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [x]: Patches link to upstream bugs/comments/lists or are otherwise justified. [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]: Fully versioned dependency in subpackages if applicable. [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). Rpmlint ------- Checking: openstack-tripleo-0.0.1-1.20140220git.fc20.noarch.rpm openstack-tripleo-doc-0.0.1-1.20140220git.fc20.noarch.rpm openstack-tripleo-0.0.1-1.20140220git.fc20.src.rpm openstack-tripleo.noarch: W: spelling-error %description -l en_US datacenter -> data center, data-center, centerboard openstack-tripleo.noarch: W: conffile-without-noreplace-flag /etc/tripleo/undercloudrc openstack-tripleo.noarch: W: conffile-without-noreplace-flag /etc/tripleo/overcloudrc-user openstack-tripleo.noarch: W: conffile-without-noreplace-flag /etc/tripleo/overcloudrc openstack-tripleo.noarch: W: conffile-without-noreplace-flag /etc/tripleo/seedrc openstack-tripleo.noarch: W: conffile-without-noreplace-flag /etc/tripleo/cloudprompt openstack-tripleo.noarch: W: devel-file-in-non-devel-package /usr/bin/user-config openstack-tripleo.noarch: W: no-manual-page-for-binary get-vm-mac openstack-tripleo.noarch: W: no-manual-page-for-binary os-make-password openstack-tripleo.noarch: W: no-manual-page-for-binary setup-clienttools openstack-tripleo.noarch: W: no-manual-page-for-binary setup-baremetal openstack-tripleo.noarch: W: no-manual-page-for-binary cleanup-env openstack-tripleo.noarch: W: no-manual-page-for-binary pull-tools openstack-tripleo.noarch: W: no-manual-page-for-binary wait_for openstack-tripleo.noarch: W: no-manual-page-for-binary setup-endpoints openstack-tripleo.noarch: W: no-manual-page-for-binary takeovernode openstack-tripleo.noarch: W: no-manual-page-for-binary devtest_overcloud.sh openstack-tripleo.noarch: W: no-manual-page-for-binary devtest_end.sh openstack-tripleo.noarch: W: no-manual-page-for-binary devtest_seed.sh openstack-tripleo.noarch: W: no-manual-page-for-binary create-nodes openstack-tripleo.noarch: W: no-manual-page-for-binary register-nodes openstack-tripleo.noarch: W: no-manual-page-for-binary setup-undercloud-passwords openstack-tripleo.noarch: W: no-manual-page-for-binary assert-users openstack-tripleo.noarch: W: no-manual-page-for-binary load-image openstack-tripleo.noarch: W: no-manual-page-for-binary write-tripleorc openstack-tripleo.noarch: W: no-manual-page-for-binary setup-network openstack-tripleo.noarch: W: no-manual-page-for-binary boot-seed-vm openstack-tripleo.noarch: W: no-manual-page-for-binary assert-admin-users openstack-tripleo.noarch: W: no-manual-page-for-binary devtest_variables.sh openstack-tripleo.noarch: W: no-manual-page-for-binary setup-neutron openstack-tripleo.noarch: W: no-manual-page-for-binary devtest_undercloud.sh openstack-tripleo.noarch: W: no-manual-page-for-binary init-keystone openstack-tripleo.noarch: W: no-manual-page-for-binary user-config openstack-tripleo.noarch: W: no-manual-page-for-binary setup-env openstack-tripleo.noarch: W: no-manual-page-for-binary devtest.sh openstack-tripleo.noarch: W: no-manual-page-for-binary setup-overcloud-passwords openstack-tripleo.noarch: W: no-manual-page-for-binary assert-user openstack-tripleo.noarch: W: no-manual-page-for-binary configure-vm openstack-tripleo.noarch: W: no-manual-page-for-binary os-adduser openstack-tripleo.noarch: W: no-manual-page-for-binary install-dependencies openstack-tripleo.noarch: W: no-manual-page-for-binary setup-seed-vm openstack-tripleo.noarch: W: no-manual-page-for-binary refresh-env openstack-tripleo.noarch: W: no-manual-page-for-binary send-irc openstack-tripleo.noarch: W: no-manual-page-for-binary set-os-type openstack-tripleo.noarch: W: no-manual-page-for-binary devtest_ramdisk.sh openstack-tripleo.noarch: W: no-manual-page-for-binary stack-ready openstack-tripleo.noarch: W: no-manual-page-for-binary devtest_setup.sh openstack-tripleo.noarch: W: no-manual-page-for-binary register-endpoint openstack-tripleo.noarch: W: no-manual-page-for-binary devtest_testenv.sh openstack-tripleo-doc.noarch: W: spelling-error %description -l en_US datacenter -> data center, data-center, centerboard openstack-tripleo.src: W: spelling-error %description -l en_US datacenter -> data center, data-center, centerboard openstack-tripleo.src: W: no-%build-section 3 packages and 0 specfiles checked; 0 errors, 52 warnings. Rpmlint (installed packages) ---------------------------- # rpmlint openstack-tripleo-doc openstack-tripleo openstack-tripleo-doc.noarch: W: spelling-error %description -l en_US datacenter -> data center, data-center, centerboard openstack-tripleo.noarch: W: spelling-error %description -l en_US datacenter -> data center, data-center, centerboard openstack-tripleo.noarch: W: conffile-without-noreplace-flag /etc/tripleo/undercloudrc openstack-tripleo.noarch: W: conffile-without-noreplace-flag /etc/tripleo/overcloudrc-user openstack-tripleo.noarch: W: conffile-without-noreplace-flag /etc/tripleo/overcloudrc openstack-tripleo.noarch: W: conffile-without-noreplace-flag /etc/tripleo/seedrc openstack-tripleo.noarch: W: conffile-without-noreplace-flag /etc/tripleo/cloudprompt openstack-tripleo.noarch: W: devel-file-in-non-devel-package /usr/bin/user-config openstack-tripleo.noarch: W: no-manual-page-for-binary get-vm-mac openstack-tripleo.noarch: W: no-manual-page-for-binary os-make-password openstack-tripleo.noarch: W: no-manual-page-for-binary setup-clienttools openstack-tripleo.noarch: W: no-manual-page-for-binary setup-baremetal openstack-tripleo.noarch: W: no-manual-page-for-binary cleanup-env openstack-tripleo.noarch: W: no-manual-page-for-binary pull-tools openstack-tripleo.noarch: W: no-manual-page-for-binary wait_for openstack-tripleo.noarch: W: no-manual-page-for-binary setup-endpoints openstack-tripleo.noarch: W: no-manual-page-for-binary takeovernode openstack-tripleo.noarch: W: no-manual-page-for-binary devtest_overcloud.sh openstack-tripleo.noarch: W: no-manual-page-for-binary devtest_end.sh openstack-tripleo.noarch: W: no-manual-page-for-binary devtest_seed.sh openstack-tripleo.noarch: W: no-manual-page-for-binary create-nodes openstack-tripleo.noarch: W: no-manual-page-for-binary register-nodes openstack-tripleo.noarch: W: no-manual-page-for-binary setup-undercloud-passwords openstack-tripleo.noarch: W: no-manual-page-for-binary assert-users openstack-tripleo.noarch: W: no-manual-page-for-binary load-image openstack-tripleo.noarch: W: no-manual-page-for-binary write-tripleorc openstack-tripleo.noarch: W: no-manual-page-for-binary setup-network openstack-tripleo.noarch: W: no-manual-page-for-binary boot-seed-vm openstack-tripleo.noarch: W: no-manual-page-for-binary assert-admin-users openstack-tripleo.noarch: W: no-manual-page-for-binary devtest_variables.sh openstack-tripleo.noarch: W: no-manual-page-for-binary setup-neutron openstack-tripleo.noarch: W: no-manual-page-for-binary devtest_undercloud.sh openstack-tripleo.noarch: W: no-manual-page-for-binary init-keystone openstack-tripleo.noarch: W: no-manual-page-for-binary user-config openstack-tripleo.noarch: W: no-manual-page-for-binary setup-env openstack-tripleo.noarch: W: no-manual-page-for-binary devtest.sh openstack-tripleo.noarch: W: no-manual-page-for-binary setup-overcloud-passwords openstack-tripleo.noarch: W: no-manual-page-for-binary assert-user openstack-tripleo.noarch: W: no-manual-page-for-binary configure-vm openstack-tripleo.noarch: W: no-manual-page-for-binary os-adduser openstack-tripleo.noarch: W: no-manual-page-for-binary install-dependencies openstack-tripleo.noarch: W: no-manual-page-for-binary setup-seed-vm openstack-tripleo.noarch: W: no-manual-page-for-binary refresh-env openstack-tripleo.noarch: W: no-manual-page-for-binary send-irc openstack-tripleo.noarch: W: no-manual-page-for-binary set-os-type openstack-tripleo.noarch: W: no-manual-page-for-binary devtest_ramdisk.sh openstack-tripleo.noarch: W: no-manual-page-for-binary stack-ready openstack-tripleo.noarch: W: no-manual-page-for-binary devtest_setup.sh openstack-tripleo.noarch: W: no-manual-page-for-binary register-endpoint openstack-tripleo.noarch: W: no-manual-page-for-binary devtest_testenv.sh 2 packages and 0 specfiles checked; 0 errors, 50 warnings. # echo 'rpmlint-done:' Requires -------- openstack-tripleo-doc (rpmlib, GLIBC filtered): openstack-tripleo openstack-tripleo (rpmlib, GLIBC filtered): /bin/bash /bin/sh /usr/bin/env config(openstack-tripleo) Provides -------- openstack-tripleo-doc: openstack-tripleo-doc openstack-tripleo: config(openstack-tripleo) openstack-tripleo Source checksums ---------------- https://github.com/openstack/tripleo-incubator/archive/d305ad25f2538d829465092146de5cdfb4a803d8.tar.gz : CHECKSUM(SHA256) this package : fd5965b9ef811109e7d8be1754ba5b141df2e74700d5d005df6cc3c243f21186 CHECKSUM(SHA256) upstream package : fd5965b9ef811109e7d8be1754ba5b141df2e74700d5d005df6cc3c243f21186 Generated by fedora-review 0.5.1 (bb9bf27) last change: 2013-12-13 Command line :/usr/bin/fedora-review --rpm-spec -n /home/athomas/rpmbuild/SRPMS/openstack-tripleo-0.0.1-1.20140220git.fc20.src.rpm Buildroot used: fedora-20-x86_64 Active plugins: Generic, Shell-api Disabled plugins: Java, C/C++, Python, fonts, SugarActivity, Ocaml, Perl, Haskell, R, PHP, Ruby Disabled flags: EXARCH, EPEL5, BATCH, DISTTAG
Angus, There are a bit of steps beyond just providing the output of the fedora-review tool. The fedora-review tool makes life a whole bunch easier, but each step should be confirmed. Especially the rpmlint out should be analyzed to identify blockers from the reviewers perspective about what needs to be fixed versus which can be ignored.
James, The loads of binaries in /usr/bin needs to be fixed. You have a couple options. 1) Prefix every binary with tripleo or 2) If the binaries are only meant to be called by other tripleo tools and not the command line put them in /usr/libexec/openstacktripleo If neither of these will work, let me know. Regards -steve
(In reply to James Slagle from comment #3) > Hi Steve, thanks for the initial feedback. > > I've made some updates: > Spec URL: > https://raw.github.com/agroup/tripleo-rpm-spec-files/master/tripleo- > incubator/openstack-tripleo.spec > SRPM URL: > http://repos.fedorapeople.org/repos/openstack-m/openstack-m/fedora-20/SRPMS/ > openstack-tripleo-0.0.1-1.20140220git.fc20.src.rpm > > 1) & 2): > This is not a python package per se. There are 1 or 2 python scripts > underneath scripts/, but no python modules. The upstream setup.{py,cfg}, > doesn't actually install anything. If we used them to install, it would just > create an empty python module called tripleo-incubator. > > 3) updated for review > > 4) updated for review > > 5) updated for review > > 6) updated for review > > 7) yes, there are several Warnings, mostly for > conffile-without-noreplace-flag (which I don't want), and > no-manual-page-for-binary (no manpages exist). Are these Ok? If you want the conffile without noreplace, please give a rationale. Anything is fine, as long as it is recorded in the review. People may come along later and wonder "why" without understanding the packagers rationale. For manual pages, it is generally proper to request the packager to request the upstream to produce manual pages in their bug tracker. In the case of OpenStack, upstream doesn't care for manual pages, so there is little we can do about this point.
(In reply to Steven Dake from comment #10) > James, > > The loads of binaries in /usr/bin needs to be fixed. You have a couple > options. > > 1) Prefix every binary with tripleo > > or > > 2) If the binaries are only meant to be called by other tripleo tools and > not the command line put them in /usr/libexec/openstacktripleo openstack-tripleo - sorry keyboard miss :) > > If neither of these will work, let me know. > > Regards > -steve
(In reply to Steven Dake from comment #10) > James, > > The loads of binaries in /usr/bin needs to be fixed. You have a couple > options. > > 1) Prefix every binary with tripleo That wouldn't be ideal due to how several of the scripts call one another. > > or > > 2) If the binaries are only meant to be called by other tripleo tools and > not the command line put them in /usr/libexec/openstacktripleo They do need to be called from the cli. How about putting them under /usr/libexec/openstacktripleo, and then also packaging an rc file that can be sourced to manipulate the user's $PATH? > > If neither of these will work, let me know. > > Regards > -steve
(In reply to James Slagle from comment #13) > (In reply to Steven Dake from comment #10) > > James, > > > > The loads of binaries in /usr/bin needs to be fixed. You have a couple > > options. > > > > 1) Prefix every binary with tripleo > > That wouldn't be ideal due to how several of the scripts call one another. > > > > > or > > > > 2) If the binaries are only meant to be called by other tripleo tools and > > not the command line put them in /usr/libexec/openstacktripleo > > They do need to be called from the cli. How about putting them under > /usr/libexec/openstacktripleo, and then also packaging an rc file that can > be sourced to manipulate the user's $PATH? > What I mean here is do they need to be called directly from the shell as CLIs? If the answer is no, they can go directly in /usr/libexec/openstack-tripleo/* and the references to their calling can be done by changing the path. Another option is as you mentioned packaging an rc file possibly in profile.d. I'm not quite sure what the best way to proceed here is. I've added pbrady to the review CC and sent him an email for further comment. > > > > If neither of these will work, let me know. > > > > Regards > > -steve
I see all the binaries listed in comment #8 They really should be confined to /usr/libexec/openstack-tripleo If they are to be exposed to a standard user shell then they should be prefixed with something like tripleo-... However having so many CLI entry point commands available is very questionable, and I suspect some refactoring/wrapping from a single /usr/bin/tripleo command that calls out/includes these functions would be the way forward.
(In reply to Pádraig Brady from comment #15) > I see all the binaries listed in comment #8 > > They really should be confined to /usr/libexec/openstack-tripleo > > If they are to be exposed to a standard user shell then they > should be prefixed with something like tripleo-... > However having so many CLI entry point commands available > is very questionable, and I suspect some refactoring/wrapping > from a single /usr/bin/tripleo command that calls out/includes > these functions would be the way forward. These are all scripts that upstream tripleo has written to aid in doing OpenStack deployments. Longer term, the hope is they will be integrated back into the relevant OpenStack projects (e.g., init-keystone back into keystone as something else). Or, replaced entirely with calls to the tuskar cli. Many of them need to be called from the CLI. Given that, can I do a hybrid approach? I'm thinking moving them to get installed under /usr/libexec/openstack-tripleo, and then adding a /usr/bin/tripleo which is pretty much just: #!/bin/bash /usr/libexec/openstack-tripleo/$1 A large number of the scripts will need patches to add /usr/libexec/openstack-tripleo to $PATH though. I think that would be easier than prefixing all the calls with /usr/bin/tripleo. That would just be a maintenance burden every time we sync with upstream. Please let me know if this would be acceptable.
James that would be great, thanks. If the scripts were only ever initiated from that /usr/bin/tripleo script, then exporting an adjusted PATH there before calling the /usr/libexec script might suffice. If not then it might be possible to add a PATH adjustment line at the start of the each of each script (assuming all shell script), by doing something like this in the spec: sed -i '2iexport PATH="$PATH:/usr/libexec/openstack-tripleo"' %{buildroot}%{_bindir}/*
BTW the above is only illustrative and would be better to also have a condition inserted that at runtime checked whether our entry was in the path. That logic would be best centralized in say /usr/libexec/openstack-tripleo/settings and then each of the scripts could be adjusted to: sed -i '2i source /usr/libexec/openstack-tripleo/settings' %{buildroot}%{_bindir}/*
Spec URL: https://raw.github.com/agroup/tripleo-rpm-spec-files/master/tripleo-incubator/openstack-tripleo.spec SRPM URL: http://repos.fedorapeople.org/repos/openstack-m/openstack-m/fedora-20/SRPMS/openstack-tripleo-0.0.2-1.20140220git.fc20.src.rpm I've moved the binaries to be under /usr/libexec/openstack-tripleo, with just a single /usr/bin/tripleo wrapper script. For some reason the rpmlint output is saying: openstack-tripleo.src: W: strange-permission tripleo 0755L I'm not sure why that's the case. Those are the permissions I want for /usr/bin/tripleo. The rc files under /etc/tripleo should not be noreplace, they aren't meant to be user edited. Similar to /etc/bashrc, etc. I added a comment to the spec file as well. I believe I've addressed all the review feedback to date. Thanks!
Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed ===== MUST items ===== Generic: [x]: Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [x]: License field in the package spec file matches the actual license. Note: Checking patched sources after %prep for licenses. Licenses found: "Apache (v2.0)", "Unknown or generated". 11 files have unknown license. Detailed output of licensecheck in /home/sdake/fedora-review/1066629 -openstack-tripleo/licensecheck.txt [-]: License file installed when any subpackage combination is installed. [x]: Package requires other packages for directories it uses. Note: No known owner of /usr/libexec/openstack-tripleo [!]: Package must own all directories that it creates. Note: Directories without known owners: /usr/libexec/openstack-tripleo [x]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [x]: %config files are marked noreplace or the reason is justified. Note: No (noreplace) in %config /etc/tripleo These are justified in the spec comments [-]: Package contains desktop file if it is a GUI application. [-]: Development files must be in a -devel package [x]: Package uses nothing in %doc for runtime. [x]: Package consistently uses macros (instead of hard-coded directory names). [x]: Package is named according to the Package Naming Guidelines. [x]: Package does not generate any conflict. [x]: Package obeys FHS, except libexecdir and /usr/target. [-]: If the package is a rename of another package, proper Obsoletes and Provides are present. [x]: Requires correct, justified where necessary. [x]: Spec file is legible and written in American English. [-]: Package contains systemd file(s) if in need. [x]: Package is not known to require an ExcludeArch tag. [-]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 30720 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 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 ===== 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). [-]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [x]: Patches link to upstream bugs/comments/lists or are otherwise justified. [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]: Fully versioned dependency in subpackages if applicable. [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-tripleo-0.0.2-1.20140220git.fc20.noarch.rpm openstack-tripleo-doc-0.0.2-1.20140220git.fc20.noarch.rpm openstack-tripleo-0.0.2-1.20140220git.fc20.src.rpm openstack-tripleo.noarch: W: conffile-without-noreplace-flag /etc/tripleo/undercloudrc openstack-tripleo.noarch: W: conffile-without-noreplace-flag /etc/tripleo/overcloudrc-user openstack-tripleo.noarch: W: conffile-without-noreplace-flag /etc/tripleo/overcloudrc openstack-tripleo.noarch: W: conffile-without-noreplace-flag /etc/tripleo/seedrc openstack-tripleo.noarch: W: conffile-without-noreplace-flag /etc/tripleo/cloudprompt openstack-tripleo.noarch: W: no-manual-page-for-binary tripleo these are ok openstack-tripleo.src: W: strange-permission tripleo 0755L my speculation is because the directory has write permissions and datadir is meant for read only data. try removing the execute bit. If that doesn't fix it, we will just have to ignore this. Also recommend changing the macro to {%_datadir}. It isn't any different, but it could be in the future. Reference: https://fedoraproject.org/wiki/Packaging:RPMMacros openstack-tripleo.src: W: no-%build-section this is fine 3 packages and 0 specfiles checked; 0 errors, 8 warnings. Rpmlint (installed packages) ---------------------------- # rpmlint openstack-tripleo-doc openstack-tripleo openstack-tripleo-doc.noarch: W: spelling-error %description -l en_US datacenter -> data center, data-center, centerboard openstack-tripleo.noarch: W: spelling-error %description -l en_US datacenter -> data center, data-center, centerboard openstack-tripleo.noarch: W: conffile-without-noreplace-flag /etc/tripleo/undercloudrc openstack-tripleo.noarch: W: conffile-without-noreplace-flag /etc/tripleo/overcloudrc-user openstack-tripleo.noarch: W: conffile-without-noreplace-flag /etc/tripleo/overcloudrc openstack-tripleo.noarch: W: conffile-without-noreplace-flag /etc/tripleo/seedrc openstack-tripleo.noarch: W: conffile-without-noreplace-flag /etc/tripleo/cloudprompt openstack-tripleo.noarch: W: no-manual-page-for-binary tripleo 2 packages and 0 specfiles checked; 0 errors, 8 warnings. # echo 'rpmlint-done:' Requires -------- openstack-tripleo-doc (rpmlib, GLIBC filtered): openstack-tripleo openstack-tripleo (rpmlib, GLIBC filtered): /bin/bash /bin/sh /usr/bin/env config(openstack-tripleo) Provides -------- openstack-tripleo-doc: openstack-tripleo-doc openstack-tripleo: config(openstack-tripleo) openstack-tripleo Source checksums ---------------- https://github.com/openstack/tripleo-incubator/archive/d305ad25f2538d829465092146de5cdfb4a803d8.tar.gz : CHECKSUM(SHA256) this package : fd5965b9ef811109e7d8be1754ba5b141df2e74700d5d005df6cc3c243f21186 CHECKSUM(SHA256) upstream package : fd5965b9ef811109e7d8be1754ba5b141df2e74700d5d005df6cc3c243f21186 Generated by fedora-review 0.5.1 (bb9bf27) last change: 2013-12-13 Command line :/usr/bin/fedora-review -b 1066629 Buildroot used: fedora-20-x86_64 Active plugins: Generic, Shell-api Disabled plugins: Java, C/C++, Python, fonts, SugarActivity, Ocaml, Perl, Haskell, R, PHP, Ruby Disabled flags: EXARCH, EPEL5, BATCH, DISTTAG
James, The package is looking really good. Please fix the following things, and I'll approve the package for SCM. [!]: Package must own all directories that it creates. Note: Directories without known owners: /usr/libexec/openstack-tripleo penstack-tripleo.src: W: strange-permission tripleo 0755L my speculation is because the directory has write permissions and datadir is meant for read only data. try removing the execute bit. If that doesn't fix it, we will just have to ignore this. Also recommend changing the macro to {%_datadir}. It isn't any different, but it could be in the future. Reference: https://fedoraproject.org/wiki/Packaging:RPMMacros Regards, -steve
Spec URL: https://raw.github.com/agroup/tripleo-rpm-spec-files/master/tripleo-incubator/openstack-tripleo.spec SRPM URL: http://repos.fedorapeople.org/repos/openstack-m/openstack-m/fedora-20/SRPMS/openstack-tripleo-0.0.2-2.20140220git.fc20.src.rpm Package now owns /usr/libexec/openstack-tripleo. Switched to using %{_datadir} instead of %{_datarootdir} Still not sure what is going on with /usr/bin/tripleo. It doesn't live under datadir, and is supposed to be executable. Incidentally, I don't get the warning when running rpmlint locally against a built rpm. I only see it when using fedora-review. I'm fine to ignore it if you are :).
James, Apologies for not catching this earlier, but the -doc package should have a %doc LICENSE and README section. While not necessarily mandatory by the review standards, I think it helps reduce confusion about what package a license has. If you could address that with a 1 liner change, I'll go ahead and approve this package.
Spec URL: https://raw.github.com/agroup/tripleo-rpm-spec-files/master/tripleo-incubator/openstack-tripleo.spec SRPM URL: http://repos.fedorapeople.org/repos/openstack-m/openstack-m/fedora-20/SRPMS/openstack-tripleo-0.0.2-3.20140220git.fc20.src.rpm Added %doc LICENSE README.md to the -doc subpackage.
Comment #24 package is APPROVED.
New Package SCM Request ======================= Package Name: openstack-tripleo Short Description: OpenStack TripleO Owners: slagle Branches: f20 InitialCC:
Git done (by process-git-requests).