[Bug 225714] New: Merge Review: e2fsprogs

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225714

Summary: Merge Review: e2fsprogs
           Product: Fedora Extras
           Version: devel
          Platform: All
        OS/Version: Linux
            Status: NEW
          Severity: normal
          Priority: normal
         Component: Package Review
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]
         QAContact: [e-mail]
                CC: [e-mail]

Fedora Merge Review: e2fsprogs

http://cvs.fedora.redhat.com/viewcvs/devel/e2fsprogs/
Initial Owner: [e-mail]

[Bug 225714] Merge Review: e2fsprogs

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Merge Review: e2fsprogs

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225714

------- Additional Comments From [e-mail]  2007-02-23 06:41 EST -------
e2fsprogs-1.39-11 prepared for review

[Bug 225714] Merge Review: e2fsprogs

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Merge Review: e2fsprogs

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225714

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |medium
           Priority|normal                      |medium
            Product|Fedora Extras               |Fedora

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|                            |fedora-cvs?

------- Additional Comments From [e-mail]  2007-06-20 13:17 EST -------
Package Change Request
======================
Package Name: e2fsprogs
Updated Fedora Owners: [e-mail],[e-mail]

[Bug 225714] Merge Review: e2fsprogs

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Merge Review: e2fsprogs

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225714

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]
               Flag|fedora-cvs?                 |fedora-cvs-

------- Additional Comments From [e-mail]  2007-06-20 16:46 EST -------
There isn't a [e-mail] address in the account system. 
Can you re-request what you want to do here? Just change owner?

[Bug 225714] Merge Review: e2fsprogs

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=225714

--- Comment #4 from Robert Scheck   2009-01-13 17:24:38 EDT ---
e2fsprogs.src: W: mixed-use-of-spaces-and-tabs (spaces: line 113, tab: line 1)

[Bug 225714] Merge Review: e2fsprogs

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=225714

Jussi Lehtola  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |[e-mail]
         AssignedTo|[e-mail]    |[e-mail]
               Flag|                            |fedora-review?

--- Comment #5 from Jussi Lehtola   2009-07-17 08:07:01 EDT ---
Taking over review.

[Bug 225714] Merge Review: e2fsprogs

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=225714

--- Comment #6 from Jussi 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)

[Bug 225714] Merge Review: e2fsprogs

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=225714

--- Comment #7 from Jussi 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

[Bug 225714] Merge Review: e2fsprogs

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=225714

--- Comment #8 from 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

[Bug 225714] Merge Review: e2fsprogs

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=225714

--- Comment #9 from Jussi 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 :)

[Bug 225714] Merge Review: e2fsprogs

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=225714

--- Comment #10 from 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

[Bug 225714] Merge Review: e2fsprogs

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=225714

--- Comment #11 from Jussi 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?

[Bug 225714] Merge Review: e2fsprogs

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=225714

--- Comment #12 from 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

[Bug 225714] Merge Review: e2fsprogs

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=225714

--- Comment #13 from Jussi Lehtola   2009-08-05 07:24:45 EDT ---
ping?

[Bug 225714] Merge Review: e2fsprogs

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=225714

--- Comment #14 from 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.

[Bug 225714] Merge Review: e2fsprogs

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=225714

Jussi Lehtola  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|fedora-review?              |fedora-review+

--- Comment #15 from Jussi 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

[Bug 225714] Merge Review: e2fsprogs

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=225714

Jussi Lehtola  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |CLOSED
         Resolution|                            |CURRENTRELEASE

--- Comment #16 from Jussi Lehtola   2010-01-01 17:45:42 EDT ---
What's the status of the uuidd ownership?

Still, let's close this bug.

[Bug 225714] Merge Review: e2fsprogs

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=225714

--- Comment #17 from Eric Sandeen   2010-01-04 12:46:28 EDT ---
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