Bug 226198 - Merge Review: nfs-utils
Merge Review: nfs-utils
Status: CLOSED CURRENTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jason Tibbitts
Fedora Package Reviews List
: Reopened
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-01-31 15:16 EST by Nobody's working on this, feel free to take it
Modified: 2012-11-29 01:39 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2012-11-29 01:39:06 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
tibbs: fedora‑review?


Attachments (Terms of Use)

  None (edit)
Description Nobody's working on this, feel free to take it 2007-01-31 15:16:44 EST
Fedora Merge Review: nfs-utils

http://cvs.fedora.redhat.com/viewcvs/devel/nfs-utils/
Initial Owner: steved@redhat.com
Comment 1 Jason Tibbitts 2007-03-01 00:14:10 EST
Hmm, this fails to build for me:

checking dependency style of gcc... gcc3
./configure: line 3878: =yes: command not found
checking for libwrap... configure: error: *** libwrap missing
error: Bad exit status from /var/tmp/rpm-tmp.58495 (%build)
 
Also, I can't get the Source1 link.  I think sourceforge has undergone several
reorganizations since that file was last fetched.  In fact, all of the
information I can find about nfs.doc.tar.gz indicates that it was out of date as
of 2000.  Perhaps it shouldn't be shipped at all?
Comment 2 Jason Tibbitts 2007-03-09 16:25:50 EST
I saw a few updates come over CVS so I decided to try again.  It's building
fine for me now, so I can move forward with a review.

W: nfs-utils summary-ended-with-dot NFS utilities and supporting clients and
daemons for the kernel NFS server.
   Trivial to fix.

E: nfs-utils tag-not-utf8 %changelog
E: nfs-utils non-utf8-spec-file nfs-utils.spec
   Due to Trond's name way back in the changelog; a pass through iconv will
   fix it up.

W: nfs-utils macro-in-%changelog pre
   Just an errant unescaped percent sign.

W: nfs-utils no-url-tag
   Probably should point to nfs.sourceforge.net

W: nfs-utils strange-permission nfs.init 0755
W: nfs-utils strange-permission rpcgssd.init 0755
W: nfs-utils strange-permission nfslock.init 0755
W: nfs-utils strange-permission rpcsvcgssd.init 0755
W: nfs-utils strange-permission rpcidmapd.init 0755
   rpmlint complains about executable files in the SRPM; I don't think it's a
   big issue as long as the permissions are sane.

W: nfs-utils unversioned-explicit-obsoletes nfs-server
W: nfs-utils unversioned-explicit-obsoletes knfsd
W: nfs-utils unversioned-explicit-obsoletes knfsd-clients
W: nfs-utils unversioned-explicit-obsoletes nfs-server-clients
W: nfs-utils unversioned-explicit-obsoletes knfsd-lock
W: nfs-utils unversioned-explicit-provides nfs-server
W: nfs-utils unversioned-explicit-provides nfs-server-clients
W: nfs-utils unversioned-explicit-provides knfsd-lock
W: nfs-utils unversioned-explicit-provides knfsd-clients
W: nfs-utils unversioned-explicit-provides knfsd
   These are problematic.  They always need to be versioned, but there's also
   no reason to worry about providing an upgrade path for packages which
   haven't existed in the past two Fedora releases.  I don't even see those
   packages back in the RHL days, the obsoletes/provides bits were present in
   the initial CVS import of this spec, so I think it's quite safe to say that
   the obsoletes should just go.
   The provides should go if nothing depends on them, and as far as I can tell
   (by running repoquery --whatrequires) nothing does.

W: nfs-utils buildprereq-use nfs-utils-lib-devel libevent-devel libgssapi-devel
   These should either turn into BuildRequires or go away.
   It turns out they're already in BuildRequires;

W: nfs-utils prereq-use shadow-utils >= 4.0.3-25
W: nfs-utils prereq-use /sbin/chkconfig /sbin/nologin
W: nfs-utils prereq-use nfs-utils-lib >= 1.0.8-2 libevent libgssapi
   Prereq is pretty much meaningless and needs to be replaced with
   fine-grained dependencies like Requires(pre)

W: nfs-utils rpm-buildroot-usage %build --prefix=$RPM_BUILD_ROOT \
   I'll have to look deeper to see what's up here.

W: nfs-utils mixed-use-of-spaces-and-tabs (spaces: line 192, tab: line 139)
   Just rpmlint being picky.

I've run out of time at the moment; more rpmlint complaints later.  I'll also be
submitting a patch to fix most of these up.
Comment 3 Steve Dickson 2007-03-12 11:55:00 EDT
> W: nfs-utils summary-ended-with-dot
fixed.

> E: nfs-utils tag-not-utf8 %changelog
> E: nfs-utils non-utf8
fixed.

> W: nfs-utils macro-in-%changelog pre
found the the uncovered '%'

> W: nfs-utils no-url-tag
Fixed.

> W: nfs-utils strange-permission 
Not sure I can fix this one without deleting
and then re-adding the scripts back... and I
believe CVS has issues with this type of activity
so I would rather not... 

> W: nfs-utils unversioned-explicit-obsoletes
Removed all obsoletes tags since they really 
didn't make sense... at least not to me... 

> W: nfs-utils unversioned-explicit-provides
Reworked all the Provides: tags to show all the
binaries that will be installed.

> W: nfs-utils buildprereq-use 
fixed.

> W: nfs-utils prereq-use
fixed.

> W: nfs-utils rpm-buildroot-usage
removed the --prefix=$RPM_BUILD_ROOT since it was not
needed..

> W: nfs-utils mixed-use-of-spaces-and-tabs
fixed.
Comment 4 Jason Tibbitts 2007-05-05 22:20:40 EDT
Wow, looks like I had planned to add more to this ticket and then promptly ran out of time for a couple of months.  Here's where we're at.

Some strange-permission warnings in the srpm, which should be OK.

Several rpmlint issues with the built RPM:

E: nfs-utils executable-marked-as-config-file /etc/rc.d/init.d/nfs
E: nfs-utils executable-marked-as-config-file /etc/rc.d/init.d/nfslock
E: nfs-utils executable-marked-as-config-file /etc/rc.d/init.d/rpcgssd
E: nfs-utils executable-marked-as-config-file /etc/rc.d/init.d/rpcidmapd
E: nfs-utils executable-marked-as-config-file /etc/rc.d/init.d/rpcsvcgssd
W: nfs-utils conffile-without-noreplace-flag /etc/rc.d/init.d/nfs
W: nfs-utils conffile-without-noreplace-flag /etc/rc.d/init.d/nfslock
W: nfs-utils conffile-without-noreplace-flag /etc/rc.d/init.d/rpcgssd
W: nfs-utils conffile-without-noreplace-flag /etc/rc.d/init.d/rpcidmapd
W: nfs-utils conffile-without-noreplace-flag /etc/rc.d/init.d/rpcsvcgssd
  The current packaging guidelines indicate that init files should be
  executable and should not be marked %config.

E: nfs-utils zero-length /var/lib/nfs/rmtab
E: nfs-utils zero-length /var/lib/nfs/etab
E: nfs-utils zero-length /var/lib/nfs/state
E: nfs-utils zero-length /var/lib/nfs/xtab
  Generally we don't ship zero-length files, but I guess something has to be
  installed there and I'm not sure if the files can contain comments.

E: nfs-utils setuid-binary /sbin/mount.nfs root 04755
E: nfs-utils setuid-binary /sbin/umount.nfs root 04755
E: nfs-utils setuid-binary /sbin/mount.nfs4 root 04755
E: nfs-utils setuid-binary /sbin/umount.nfs4 root 04755
E: nfs-utils non-standard-executable-perm /sbin/mount.nfs 04755
E: nfs-utils non-standard-executable-perm /sbin/umount.nfs 04755
E: nfs-utils non-standard-executable-perm /sbin/mount.nfs4 04755
E: nfs-utils non-standard-executable-perm /sbin/umount.nfs4 04755
E: nfs-utils non-readable /var/lib/nfs/state 0600
E: nfs-utils non-standard-uid /var/lib/nfs/statd rpcuser
E: nfs-utils non-standard-gid /var/lib/nfs/statd rpcuser
E: nfs-utils non-standard-dir-perm /var/lib/nfs/statd 0700
   These are OK.

E: nfs-utils explicit-lib-dependency libevent
E: nfs-utils explicit-lib-dependency libgssapi
  I'm not sure I understand these.  What's the point of putting just a library
  dependency in Requires(pre)?  If something in %pre needs those libraries,
  won't it have its own dependencies?

W: nfs-utils incoherent-version-in-changelog 1.0.12-4 1:1.0.12-4.fc7
  rpmlint is complaining about not seeing the epoch inthe changelog entry 
  I don't think this it's a big deal.

W: nfs-utils dangerous-command-in-%pre mv
  I wonder if we still support upgrading from anything old enough to need that
  bit in %pre.  It looks like FC4 still used "rpc.*" while FC5 uses "rpc*", so
  we probably still need it.

W: nfs-utils dangerous-command-in-%preun userdel
  The guidelines for this aren't finished yet, but general sentiment is that
  we shouldn't delete service users.  It's obvious if uninstalling the package
  would leave unowned files

W: nfs-utils service-default-enabled /etc/rc.d/init.d/nfslock
W: nfs-utils service-default-enabled /etc/rc.d/init.d/rpcidmapd
W: nfs-utils service-default-enabled /etc/rc.d/init.d/rpcgssd
  Generally it's bad if merely installing a package results in a running
  service, but this is the nfs client and perhaps Red Hat has some other policy
  here.

W: nfs-utils no-reload-entry /etc/rc.d/init.d/nfslock
  I've always wondered why that script is missing reload.

Honestly, besides those rpmlint bits and the above question about the nfs.doc
tarball, I think this package is fine.
Comment 5 Jason Tibbitts 2007-07-13 14:15:54 EDT
Any response?
Comment 6 Steve Dickson 2007-07-16 12:08:10 EDT
> I'm not sure I understand these.  What's the point of putting just a library
> dependency in Requires(pre)?  If something in %pre needs those libraries,
> won't it have its own dependencies?
These libraries are need for some of the daemons in nfs-utils to
function. Should the just be Requires;?
Comment 7 Jason Tibbitts 2007-07-26 23:41:47 EDT
> These libraries are need for some of the daemons in nfs-utils to
> function. Should the just be Requires;

Well, if the dependencies are needed at runtime, they should be Requires:, not
Requires(pre):.  But generally RPM will happily figure out any library
dependencies automatically.

I was going to do a build and continue moving forward with this review, but then
I noticed that the devel branch seems to be older than the F-7 branch.  Should I
wait for devel to catch up?
Comment 8 Steve Dickson 2007-07-27 09:13:58 EDT
Yes... Due to some travel I have not update the devel branch...
I will asap...  
Comment 9 Jeff Layton 2009-02-21 13:14:32 EST
Looks like this hasn't been updated in a long time and since nfs-utils is part of fedora and has been for a long time. I'll go ahead and close this. Please reopen if anything more needs to be done with it...
Comment 10 Jason Tibbitts 2009-02-21 14:40:27 EST
Please do not close merge review tickets unless you're doing the reviews.  This package is still under review and has yet to be approved.  It doesn't matter that it has been in Fedora for a long time; all of the packages which came in from core need to be reviewed.
Comment 11 Jason Tibbitts 2012-05-09 18:08:07 EDT
Well, this is ancient but still open, so I took a look at the current package.  Really I only have a few notes:

You can remove some cruft on modern Fedora releases: BuildRoot:, %clean, the first line of %install, %defattr in the %files section.

There seem to be several bits to handle upgrades from versions that should be older than anything Fedora supports.  The first bit of %pre, for example, and the creation of the nfsnobody user (which has been default in the setup package for probably forever).  The %pre script also goes through some odd mechanics to make sure the users don't already exist, but the recommended scriptlets are much simpler.  (And I've no idea why setup has the nfsnobody user but not the group.)

The %preun script deletes users and groups, which... isn't something that we would normally ever do.

The versioned dependencies can almost certainly be dropped, as no Fedora release has such old versions of krb5-libs, autoconf, openldap-devel or shadow-utils.

The systemd truggerun stuff is not what's recommended these days, but then again I suppose it's too late to change it now.  It can go away in another couple of releases anyway.
Comment 12 Fedora Update System 2012-11-08 13:06:31 EST
nfs-utils-1.2.6-14.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/nfs-utils-1.2.6-14.fc18
Comment 13 Fedora Update System 2012-11-08 22:19:39 EST
Package nfs-utils-1.2.6-14.fc18:
* should fix your issue,
* was pushed to the Fedora 18 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing nfs-utils-1.2.6-14.fc18'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2012-17810/nfs-utils-1.2.6-14.fc18
then log in and leave karma (feedback).
Comment 14 Fedora Update System 2012-11-29 01:39:10 EST
nfs-utils-1.2.6-14.fc18 has been pushed to the Fedora 18 stable repository.  If problems still persist, please make note of it in this bug report.

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