Bug 225714

e2fsprogs-1.39-11 prepared for review

Package Change Request
======================
Package Name: e2fsprogs
Updated Fedora Owners: sandeen,esandeen

There isn't a sandeen address in the account system. 
Can you re-request what you want to do here? Just change owner? 

e2fsprogs.src: W: mixed-use-of-spaces-and-tabs (spaces: line 113, tab: line 1)

Taking over review.

rpmlint output:
e2fsprogs.src: W: strange-permission uuidd.init 0755
e2fsprogs.src:20: W: unversioned-explicit-obsoletes e4fsprogs
e2fsprogs.src:21: W: unversioned-explicit-provides e4fsprogs
e2fsprogs-libs.x86_64: W: no-documentation
5 packages and 0 specfiles checked; 0 errors, 4 warnings.

- I suggest enabling the e4fsprogs obsolete only when building on RHEL, i.e.
%if 0%{?rhel} > 0
Obsoletes: e4fsprogs < %{version}-%{release}
Provides: e4fsprogs = %{version}-%{release}
%endif

- I'd break the requires line in two due to the versioning.
 Requires: e2fsprogs-libs = %{version}-%{release}, device-mapper

- Is the
 Requires(post): /sbin/ldconfig
really necessary since ldconfig is part of glibc?

- Change
 Requires: /sbin/install-info
to
 Requires: info
as info provides install-info on all distributions, at least from RHEL 4 onwards.

- libuuid-devel needs to
 Provides: libuuid-static = %{version}-%{release}

- libdss-devel needs to
 Provides: libdss-static = %{version}-%{release}

- libcom_err-devel needs to
 Provides: libcom_err-static = %{version}-%{release}

- -devel needs to
 Provides: %{name}-static = %{version}-%{release}

- Summary of -libs is incorrect:
 Summary: Ext2/3/4 filesystem-specific shared libraries and headers
(no headers are present)

- %setup argument -n e2fsprogs-%{version} is not necessary.

- Actually, whole rpmlint output is:
e2fsprogs.src: W: strange-permission uuidd.init 0755
e2fsprogs.src:20: W: unversioned-explicit-obsoletes e4fsprogs
e2fsprogs.src:21: W: unversioned-explicit-provides e4fsprogs
e2fsprogs-libs.x86_64: W: no-documentation
libcom_err.x86_64: W: no-documentation
libss.x86_64: W: no-documentation
libuuid.x86_64: W: no-documentation
uuidd.x86_64: W: non-standard-uid /usr/sbin/uuidd uuidd
uuidd.x86_64: W: non-standard-gid /usr/sbin/uuidd uuidd
uuidd.x86_64: W: non-standard-uid /var/lib/libuuid uuidd
uuidd.x86_64: W: non-standard-gid /var/lib/libuuid uuidd
uuidd.x86_64: E: non-standard-dir-perm /var/lib/libuuid 02775
12 packages and 0 specfiles checked; 1 errors, 11 warnings.

You could add at least COPYING to the lib* and uuidd packages.

- You are explicitly referring to /etc/rc.d/init.d/uuidd in the %files of uuidd, I suggest using the %{_initrddir} macro.

**

MUST: The spec file for the package is legible and macros are used consistently. NEEDSWORK
- Mixing of %{buildroot} and $RPM_BUILD_ROOT which is not allowed.

MUST: The package must be named according to the Package Naming Guidelines. OK
MUST: The spec file name must match the base package %{name}. OK
MUST: The package must be licensed with a Fedora approved license and meet the  Licensing Guidelines. OK
MUST: The License field in the package spec file must match the actual license. OK
MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. OK
MUST: The package MUST successfully compile and build into binary rpms. OK
MUST: The spec file MUST handle locales properly. OK
MUST: Optflags are used and time stamps preserved. OK
MUST: Packages containing shared library files must call ldconfig. OK
MUST: A package must own all directories that it creates or require the package that owns the directory. OK
MUST: Files only listed once in %files listings. OK
MUST: Debuginfo package is complete. OK
MUST: Permissions on files must be set properly. OK
MUST: Clean section exists. OK
MUST: Large documentation files must go in a -doc subpackage. OK

MUST: All relevant items are included in %doc. Items in %doc do not affect runtime of application. NEEDSWORK
- Missing COPYING.
- I suggest placing 

MUST: Header files must be in a -devel package. OK

MUST: Static libraries must be in a -static package. NEEDSWORK
- See comment #6.

MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'. NEEDSWORK
- Devel needs to Requires: pkgconfig.

MUST: If a package contains library files with a suffix then library files ending in .so must go in a -devel package. OK
MUST: In the vast majority of cases, devel packages must require the base package using a fully versioned dependency. OK
MUST: Packages does not contain any .la libtool archives. N/A
MUST: Desktop files are installed properly. N/A
MUST: No file conflicts with other packages and no general names. OK
MUST: Buildroot cleaned before install. OK
SHOULD: %{?dist} tag is used in release. OK
SHOULD: If the package does not include license text(s) as separate files from upstream, the packager should query upstream to include it. OK
SHOULD: The package builds in mock. OK

Thanks for the review, I know e2fsprogs needs some cleanup, it's been around for so long a lot of cruft has accumulated :)

-Eric

(In reply to comment #8)
> Thanks for the review, I know e2fsprogs needs some cleanup, it's been around
> for so long a lot of cruft has accumulated :)

Oh but this was from the cleanest end :)

Ok, care to look at the results in:

http://kojipkgs.fedoraproject.org/scratch/sandeen/task_1482920/ ?

Thanks,
-Eric

(In reply to comment #10)
> Ok, care to look at the results in:
> 
> http://kojipkgs.fedoraproject.org/scratch/sandeen/task_1482920/ ?

You don't need to include COPYING in -devel, since -devel requires e2fsprogs-libs which contains it.

Otherwise it seems that all my comments have been taken into account.

rpmlint gives now
uuidd.x86_64: W: non-standard-uid /usr/sbin/uuidd uuidd
uuidd.x86_64: W: non-standard-gid /usr/sbin/uuidd uuidd
uuidd.x86_64: W: non-standard-uid /var/lib/libuuid uuidd
uuidd.x86_64: W: non-standard-gid /var/lib/libuuid uuidd
uuidd.x86_64: E: non-standard-dir-perm /var/lib/libuuid 02775
12 packages and 0 specfiles checked; 1 errors, 4 warnings.

Out of these the owner of the binary is a bit odd, shouldnt it be root:root?

(In reply to comment #11)
> (In reply to comment #10)
> > Ok, care to look at the results in:
> > 
> > http://kojipkgs.fedoraproject.org/scratch/sandeen/task_1482920/ ?
> 
> You don't need to include COPYING in -devel, since -devel requires
> e2fsprogs-libs which contains it.

Oops that was an oversight ... will fix.

> Otherwise it seems that all my comments have been taken into account.
> 
> rpmlint gives now
> uuidd.x86_64: W: non-standard-uid /usr/sbin/uuidd uuidd
> uuidd.x86_64: W: non-standard-gid /usr/sbin/uuidd uuidd
> uuidd.x86_64: W: non-standard-uid /var/lib/libuuid uuidd
> uuidd.x86_64: W: non-standard-gid /var/lib/libuuid uuidd
> uuidd.x86_64: E: non-standard-dir-perm /var/lib/libuuid 02775
> 12 packages and 0 specfiles checked; 1 errors, 4 warnings.
> 
> Out of these the owner of the binary is a bit odd, shouldnt it be root:root?  

Hm, there was a reason for it, I need to remember :)  I'll check in as it is now (w/ the COPYING fix) and look into that last bit, I need to refresh my memory.

Thanks,
-Eric

ping?

Everything but the potential uuidd ownership change has been committed and I don't have more info on that at this point.

(In reply to comment #14)
> Everything but the potential uuidd ownership change has been committed and I
> don't have more info on that at this point.  

Right. Actually rpmlint output is now clean since uuid has been moved to util-linux-ng.

I don't have any more comments.

APPROVED

What's the status of the uuidd ownership?

Still, let's close this bug.

uuidd is clearly in util-linux-ng now:

# rpm -qi uuidd
Name        : uuidd                        Relocations: (not relocatable)
Version     : 2.17                              Vendor: Fedora Project
Release     : 0.1.git5e51568.fc13           Build Date: Mon 19 Oct 2009 07:56:41 AM CDT
Install Date: Wed 28 Oct 2009 12:59:16 PM CDT      Build Host: x86-5.fedora.phx.redhat.com
Group       : System Environment/Daemons    Source RPM: util-linux-ng-2.17-0.1.git5e51568.fc13.src.rpm