Bug 225714 - Merge Review: e2fsprogs
Summary: Merge Review: e2fsprogs
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review   
(Show other bugs)
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Susi Lehtola
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Keywords:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-01-31 18:31 UTC by Nobody's working on this, feel free to take it
Modified: 2010-01-04 17:46 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2010-01-01 22:45:42 UTC
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)

Description Nobody's working on this, feel free to take it 2007-01-31 18:31:09 UTC
Fedora Merge Review: e2fsprogs

http://cvs.fedora.redhat.com/viewcvs/devel/e2fsprogs/
Initial Owner: twoerner@redhat.com

Comment 1 Karsten Hopp 2007-02-23 11:41:48 UTC
e2fsprogs-1.39-11 prepared for review

Comment 2 Eric Sandeen 2007-06-20 17:17:05 UTC
Package Change Request
======================
Package Name: e2fsprogs
Updated Fedora Owners: sandeen@redhat.com,esandeen@redhat.com

Comment 3 Kevin Fenzi 2007-06-20 20:46:08 UTC
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 22:24:38 UTC
e2fsprogs.src: W: mixed-use-of-spaces-and-tabs (spaces: line 113, tab: line 1)

Comment 5 Susi Lehtola 2009-07-17 12:07:01 UTC
Taking over review.

Comment 6 Susi Lehtola 2009-07-17 12:27:23 UTC
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 12:43:31 UTC
- %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 15:07:26 UTC
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 20:03:35 UTC
(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 20:39:55 UTC
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 21:09:06 UTC
(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 21:16:34 UTC
(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 11:24:45 UTC
ping?

Comment 14 Eric Sandeen 2009-08-05 14:21:31 UTC
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 14:37:11 UTC
(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 22:45:42 UTC
What's the status of the uuidd ownership?

Still, let's close this bug.

Comment 17 Eric Sandeen 2010-01-04 17:46:28 UTC
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.