Bug 225714 - Merge Review: e2fsprogs
Merge Review: e2fsprogs
Status: CLOSED CURRENTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Susi Lehtola
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-01-31 13:31 EST by Nobody's working on this, feel free to take it
Modified: 2010-01-04 12:46 EST (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2010-01-01 17:45:42 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
susi.lehtola: fedora‑review+
kevin: fedora‑cvs-


Attachments (Terms of Use)

  None (edit)
Description Nobody's working on this, feel free to take it 2007-01-31 13:31:09 EST
Fedora Merge Review: e2fsprogs

http://cvs.fedora.redhat.com/viewcvs/devel/e2fsprogs/
Initial Owner: twoerner@redhat.com
Comment 1 Karsten Hopp 2007-02-23 06:41:48 EST
e2fsprogs-1.39-11 prepared for review
Comment 2 Eric Sandeen 2007-06-20 13:17:05 EDT
Package Change Request
======================
Package Name: e2fsprogs
Updated Fedora Owners: sandeen@redhat.com,esandeen@redhat.com
Comment 3 Kevin Fenzi 2007-06-20 16:46:08 EDT
There isn't a sandeen@redhat.com address in the account system. 
Can you re-request what you want to do here? Just change owner? 
Comment 4 Robert Scheck 2009-01-13 17:24:38 EST
e2fsprogs.src: W: mixed-use-of-spaces-and-tabs (spaces: line 113, tab: line 1)
Comment 5 Susi Lehtola 2009-07-17 08:07:01 EDT
Taking over review.
Comment 6 Susi Lehtola 2009-07-17 08:27:23 EDT
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)
Comment 7 Susi Lehtola 2009-07-17 08:43:31 EDT
- %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
Comment 8 Eric Sandeen 2009-07-17 11:07:26 EDT
Thanks for the review, I know e2fsprogs needs some cleanup, it's been around for so long a lot of cruft has accumulated :)

-Eric
Comment 9 Susi Lehtola 2009-07-17 16:03:35 EDT
(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 :)
Comment 10 Eric Sandeen 2009-07-17 16:39:55 EDT
Ok, care to look at the results in:

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

Thanks,
-Eric
Comment 11 Susi Lehtola 2009-07-17 17:09:06 EDT
(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?
Comment 12 Eric Sandeen 2009-07-17 17:16:34 EDT
(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
Comment 13 Susi Lehtola 2009-08-05 07:24:45 EDT
ping?
Comment 14 Eric Sandeen 2009-08-05 10:21:31 EDT
Everything but the potential uuidd ownership change has been committed and I don't have more info on that at this point.
Comment 15 Susi Lehtola 2009-08-05 10:37:11 EDT
(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
Comment 16 Susi Lehtola 2010-01-01 17:45:42 EST
What's the status of the uuidd ownership?

Still, let's close this bug.
Comment 17 Eric Sandeen 2010-01-04 12:46:28 EST
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

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