Bug 180571 - Review Request: puppet
Review Request: puppet
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: John Mahowald
Fedora Package Reviews List
:
Depends On: 182064
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-02-08 21:11 EST by David Lutterkort
Modified: 2013-04-30 19:40 EDT (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-07-11 21:11:09 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description David Lutterkort 2006-02-08 21:11:55 EST
Spec Name or Url: http://people.redhat.com/dlutter/yum/spec/puppet.spec
                  http://people.redhat.com/dlutter/yum/spec/facter.spec
SRPM Name or Url: http://people.redhat.com/dlutter/yum/SRPMS/puppet-0.13.0-3.src.rpm
http://people.redhat.com/dlutter/yum/SRPMS/facter-1.1.1-3.src.rpm

Description [puppet]:
Puppet lets you centrally manage every important aspect of your system using a
cross-platform specification language that manages all the separate elements
normally aggregated in different files, like users, cron jobs, and hosts,
along with obviously discrete elements like packages, services, and files.

Description [facter]:
Facter is a module for collecting simple facts about a host Operating system.

Notes:
- I included facter, one of puppet's requirements in this request since it is miniscule

- This is my first Fedora Extras submission, and needs to be sponsored
Comment 1 Jason Tibbitts 2006-02-18 15:43:39 EST
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
Comment 2 David Lutterkort 2006-02-19 21:29:38 EST
- 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
Comment 3 Ralf Corsepius 2006-02-19 22:47:45 EST
> 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.
Comment 4 Jason Tibbitts 2006-02-20 10:23:18 EST
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?
Comment 5 Enrico Scholz 2006-03-01 03:43:56 EST
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.
Comment 6 David Lutterkort 2006-03-01 21:13:03 EST
- 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.
Comment 7 Kevin Fenzi 2006-03-05 16:13:38 EST
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. 
Comment 8 David Lutterkort 2006-03-06 15:27:54 EST
* 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
Comment 9 David Lutterkort 2006-03-13 16:39:41 EST
* 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
Comment 10 David Lutterkort 2006-03-22 16:30:54 EST
* 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
Comment 11 David Lutterkort 2006-03-30 15:07:00 EST
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
Comment 12 David Lutterkort 2006-04-07 20:53:53 EDT
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
Comment 13 David Lutterkort 2006-04-21 22:05:33 EDT
* 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
Comment 14 David Lutterkort 2006-05-03 20:08:17 EDT
* 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
Comment 15 John Mahowald 2006-05-06 13:48:20 EDT
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
Comment 16 David Lutterkort 2006-05-08 12:17:05 EDT
Did you rebuild facter after the problems you had on bug 182064 ? Can you paste
the output of 'rpm -ql facter' in this ticket ?
Comment 17 David Lutterkort 2006-06-20 19:38:17 EDT
* 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
Comment 18 Jason Tibbitts 2006-06-22 16:11:13 EDT
Unblocking FE-NEEDSPONSOR.
Comment 19 David Lutterkort 2006-07-06 13:59:22 EDT
* 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
Comment 20 John Mahowald 2006-07-08 00:03:16 EDT
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
Comment 21 David Lutterkort 2006-07-11 21:11:09 EDT
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

Note You need to log in before you can comment on or make changes to this bug.