Bug 180571
Summary: | Review Request: puppet | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | David Lutterkort <lutter> |
Component: | Package Review | Assignee: | John Mahowald <jpmahowald> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | hbrock, jlaska |
Target Milestone: | --- | ||
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2006-07-12 01:11:09 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: | |||
Bug Depends On: | 182064 | ||
Bug Blocks: | 163779 |
Description
David Lutterkort
2006-02-09 02:11:55 UTC
Just took a quick look. I can't sponsor so someone else will need to do the formal review. Firstly, you really should submit a separate review request for facter and make this one bug depend on it. Secondly, I don't think you can't just pick UID 317 like that. Extras has fedora-usermgmt for handling this; see http://fedoraproject.org/wiki/Packaging/UserCreation and http://fedoraproject.org/wiki/Packaging/UserRegistry - Thanks for taking the time to look at this. I filed a separate request for facter (bz 182064) and this request should only be applied to puppet. - puppet now uses fedora-usermgmt for creating the puppet user/group. - I used the relative uid/gid 24, which is the next free one on http://fedoraproject.org/wiki/Packaging/UserRegistry How do I get that added to that page ? Updated URL's: Spec URL: http://people.redhat.com/dlutter/yum/spec/puppet.spec SRPM URL: http://people.redhat.com/dlutter/yum/SRPMS/puppet-0.13.0-4.src.rpm > puppet now uses fedora-usermgmt for creating the puppet user/group.
Please elaborate in detail why the standard usedadd etc. aren't sufficient for
this package.
Uhh, the package has a new user. The wiki has instructions on how this is to be done in Extras. Are you saying that actually following the instructions in the Wiki needs to be justified? No; there does not exist an agreement/rule how to create users for Fedora Extras and there is no preference for or against 'fedora-usermgmt' or the plain 'useradd'. So, you do not need to justify the chosen technology and it is your decision whether you use fedora-usermgmt or not. - Updated to latest upstream release 0.13.5. Spec URL: http://people.redhat.com/dlutter/yum/spec/puppet.spec SRPM URL: http://people.redhat.com/dlutter/yum/SRPMS/puppet-0.13.5-1.src.rpm - I changed puppet back to using standard useradd/groupadd with dynamic id allocation; fedora-usermgmt makes it harder to use the rpm's on other distros and is not a FE requirement. Just a few comments I noticed looking at this: - Not sure it will currently build in the build system as ruby isn't in the base build env. Not sure what the answer is there. Add ruby to base? Hard code rubylibdir ? - There are some outstanding comments on facter (which this depends on). Probibly should look at fixing all those first. * Updated to latest puppet-0.14.0 Spec: http://people.redhat.com/dlutter/yum/spec/puppet.spec SRPM: http://people.redhat.com/dlutter/yum/SRPMS/puppet-0.14.0-1.src.rpm * The rubylibdir macro won't cause a problem, since it is only used during the build, at which time mock will have installed ruby, which is now a BuildRequires. * I addressed all the outstanding issues with facter * The comment https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=182064#c3 also applies to this package; removed BuildArch: noarch for now to address that * Updated to latest upstream release 0.15.0 Spec: http://people.redhat.com/dlutter/yum/spec/puppet.spec SRPM: http://people.redhat.com/dlutter/yum/SRPMS/puppet-0.15.0-1.src.rpm * Updated to latest upstream release 0.15.1 Spec: http://people.redhat.com/dlutter/yum/spec/puppet.spec SRPM: http://people.redhat.com/dlutter/yum/SRPMS/puppet-0.15.1-1.src.rpm To facilitate the review, here is the output I get from rpmlint, and why I think it's ok: * No output from the SRPM * For puppet-0.15.1-1.i386.rpm: E: puppet no-binary E: puppet only-non-binary-in-usr-lib Both should be ok; the package should be noarch, but because of bz184199 it's not possible to build ruby noarch packages right now. Once 181499 has been fixed, I'll switch to building this as noarch E: puppet non-standard-uid /var/log/puppet puppet E: puppet non-standard-gid /var/log/puppet puppet E: puppet non-standard-uid /var/run/puppet puppet E: puppet non-standard-gid /var/run/puppet puppet The puppet server wants to run as user 'puppet', and needs to write to these dirs E: puppet zero-length /usr/share/doc/puppet-0.15.1/examples/root/etc/otherfile E: puppet zero-length /usr/share/doc/puppet-0.15.1/examples/root/etc/configfile Two example files that are empty upstream. If this causes problems, should I exclude them ? E: puppet incoherent-subsys /etc/rc.d/init.d/puppet puppet} I believe rpmlint is getting confused by the init script * For puppet-server-0.15.1-1.i386.rpm W: puppet-server no-documentation There is no server-specific documentation upstream W: puppet-server incoherent-init-script-name puppetmaster I believe rpmlint is getting confused by the init script Updated to latest upstream version 0.15.3 Spec: http://people.redhat.com/dlutter/yum/spec/puppet.spec SRPM: http://people.redhat.com/dlutter/yum/SRPMS/puppet-0.15.3-1.src.rpm * Updated to latest upstream release 0.16.0 Spec: http://people.redhat.com/dlutter/yum/spec/puppet.spec SRPM: http://people.redhat.com/dlutter/yum/SRPMS/puppet-0.16.0-1.src.rpm * Updated to latest upstream release 0.16.5 Spec: http://people.redhat.com/dlutter/yum/spec/puppet.spec SRPM: http://people.redhat.com/dlutter/yum/SRPMS/puppet-0.16.5-1.src.rpm Not running on FC5, x86_64, ruby-1.8.4-3.2 $ puppet /usr/lib64/site_ruby/1.8/puppet/server/filebucket.rb:8:in `require': no such file to load -- facter (LoadError) from /usr/lib64/site_ruby/1.8/puppet/server/filebucket.rb:8 from /usr/lib64/site_ruby/1.8/puppet/server.rb:180 from /usr/lib64/site_ruby/1.8/puppet.rb:332 from /usr/bin/puppet:58 Did you rebuild facter after the problems you had on bug 182064 ? Can you paste the output of 'rpm -ql facter' in this ticket ? * Updated to latest upstream release 0.18.0 Spec: http://people.redhat.com/dlutter/yum/spec/puppet.spec SRPM: http://people.redhat.com/dlutter/yum/SRPMS/puppet-0.18.0-1.src.rpm Unblocking FE-NEEDSPONSOR. * Updated to latest upstream release 0.18.2 Spec: http://people.redhat.com/dlutter/yum/spec/puppet.spec SRPM: http://people.redhat.com/dlutter/yum/SRPMS/puppet-0.18.2-1.src.rpm Built against 1.3.3-1 facter. rpmlint is noisy. E: puppet zero-length /usr/share/doc/puppet-0.18.2/examples/root/etc/configfile E: puppet zero-length /usr/share/doc/puppet-0.18.2/examples/root/etc/otherfile Ignore. E: puppet non-standard-uid /var/lib/puppet puppet E: puppet non-standard-gid /var/lib/puppet puppet E: puppet non-standard-uid /var/log/puppet puppet E: puppet non-standard-gid /var/log/puppet puppet E: puppet non-standard-uid /var/run/puppet puppet E: puppet non-standard-gid /var/run/puppet puppet The user/group was added. Ignore. W: puppet doc-file-dependency /usr/share/doc/puppet-0.18.2/examples/root/bin/sleeper /usr/bin/ruby Required for puppet, ok. E: puppet incoherent-subsys /etc/rc.d/init.d/puppet puppet} W: puppet mixed-use-of-spaces-and-tabs Cosmetic. W: puppet-server no-documentation Fine. W: puppet-server incoherent-init-script-name puppetmaster Not puppet, ok. Defines %ruby_sitelibdir as per guidlines. - license (GPL) OK, text in %doc, matches source - spec file legible, in am. english - source matches upstream - package compiles on devel (x86) - no missing BR Do you need both of: Requires: ruby >= 1.8.1 and Requires: ruby(abi) = 1.8 ? Merely redundant if so. - no locales - not relocatable - owns all directories that it creates - no duplicate files - permissions ok - %clean ok - macro use consistent - code, not content - no need for -docs - nothing in %doc affects runtime - no need for .desktop file APPROVED The ruby(abi) is provided by ruby-libs, not ruby (!) Since the package contains scripts that want /usr/bin/ruby, I added the explicit R: ruby, analogous to what python packages do (e.g., yum requires both python and /usr/bin/python) If that's not the right thing to do, I'll fix it in the next release. Imported and built successfully as job 12479 |