Bug 180571

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