Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 225714 - Merge Review: e2fsprogs
Summary: Merge Review: e2fsprogs
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Susi Lehtola
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
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:
Clone Of:
Environment:
Last Closed: 2010-01-01 22:45:42 UTC
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.