Bug 226119 - Merge Review: MAKEDEV
Merge Review: MAKEDEV
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Todd Zullinger
Fedora Package Reviews List
:
Depends On:
Blocks: F9MergeReviewTarget
  Show dependency treegraph
 
Reported: 2007-01-31 14:36 EST by Nobody's working on this, feel free to take it
Modified: 2008-03-07 13:25 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-03-07 13:25:14 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
tmz: fedora‑review+


Attachments (Terms of Use)
Patch cleaning up most of the rpmlint complaints and other minor issues for merge review (3.03 KB, patch)
2008-03-02 16:51 EST, Todd Zullinger
no flags Details | Diff

  None (edit)
Description Nobody's working on this, feel free to take it 2007-01-31 14:36:49 EST
Fedora Merge Review: MAKEDEV

http://cvs.fedora.redhat.com/viewcvs/devel/MAKEDEV/
Initial Owner: nalin@redhat.com
Comment 1 Kevin Fenzi 2008-03-02 13:55:14 EST
Todd: Are you reviewing this package? 
Please set the fedora-review flag to ? if you are, and if not, you might want to
move it back to being assigned to nobody@fedoraproject.org. 
Comment 2 Todd Zullinger 2008-03-02 16:11:14 EST
Sorry for not getting to this one sooner Kevin.  I'll finish up a full review
shortly.
Comment 3 Todd Zullinger 2008-03-02 16:48:40 EST
Greetings Chris and Nalin,

Here be a merge review for MAKEDEV.

OK - Package meets naming and packaging guidelines
OK - Spec file matches base package name.
OK - Spec has consistant macro usage.
See below - Meets Packaging Guidelines.
See below - License = GPLv2+
See below - License field in spec matches
OK - License file included in package
OK - Spec in American English
OK - Spec is legible.
See below - Sources match upstream md5sum
OK - BuildRequires correct
OK - Package has %defattr and permissions on files is good.
OK - Package has a correct %clean section.
See below - Package has correct buildroot
OK - Package is code or permissible content.
OK - Packages %doc files don't affect runtime.

OK - Package compiles and builds on at least one arch. (see koji :)
OK - Package has no duplicate files in %files.
OK - Package doesn't own any directories other packages own.
OK - Package owns all the directories it creates.
See below - No rpmlint output.
See below - final provides and requires are sane:
    MAKEDEV-3.23-3.i386.rpm
    Provides:
    ---------
    config(MAKEDEV) = 3.23-3
    MAKEDEV = 3.23-3
    Requires:
    ---------
    /bin/sh  
    /usr/sbin/groupadd  
    /usr/sbin/useradd  
    config(MAKEDEV) = 3.23-3
    grep  
    libc.so.6  
    libselinux.so.1  
    mktemp  
    rtld(GNU_HASH)  
    
    MAKEDEV-3.23-3.src.rpm
    Provides:
    ---------
    (none)
    Requires:
    ---------
    libselinux-devel

SHOULD Items:

OK - Should build in mock. (again see koji for proof :)
OK - Should build on all supported archs
OK - Should function as described.
OK - Should have sane scriptlets.
OK - Should package latest version
See below - Outstanding bugs on package.

Issues:

1. http://fedoraproject.org/wiki/Packaging/SourceURL suggests adding a comment
above Source0 when we are upstream.  Would there be any value in setting up a
fedorahosted project for the sources to MAKEDEV?

2. The License tag needs to be updated.  However, the source package license
also needs adjusted to meet Red Hat policy.  The license should be GPLv2 only
(no "or any later version") according to CopyrightGuidelines on the Red Hat
intranet (so I am told).  MAKEDEV.c, mksock.c, and sel.h all include the "or any
later version" in the header.  This should be fixed and the License tag then
changed to GPLv2.

3. The buildroot tag must be one of the following, according to
http://fedoraproject.org/wiki/Packaging/Guidelines#line-181:
    %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)
    %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
    %{_tmppath}/%{name}-%{version}-%{release}-root

4. rpmlint output:

$ rpmlint MAKEDEV-3.23-3.src.rpm 
MAKEDEV.src:10: W: buildprereq-use libselinux-devel
MAKEDEV.src:12: W: prereq-use /usr/sbin/groupadd, /usr/sbin/useradd, grep, mktemp
MAKEDEV.src:465: W: macro-in-%changelog post
MAKEDEV.src:494: W: macro-in-%changelog _sysconfdir
MAKEDEV.src: W: summary-ended-with-dot A program used for creating device files
in /dev.
MAKEDEV.src: W: invalid-license GPL

$ rpmlint MAKEDEV-3.23-3.i386.rpm
MAKEDEV.i386: W: summary-ended-with-dot A program used for creating device files
in /dev.
MAKEDEV.i386: W: invalid-license GPL
MAKEDEV.i386: W: conffile-without-noreplace-flag /etc/makedev.d/00macros
MAKEDEV.i386: W: conffile-without-noreplace-flag /etc/makedev.d/01alsa
MAKEDEV.i386: W: conffile-without-noreplace-flag /etc/makedev.d/01cciss
MAKEDEV.i386: W: conffile-without-noreplace-flag /etc/makedev.d/01cdrom
MAKEDEV.i386: W: conffile-without-noreplace-flag /etc/makedev.d/01console
MAKEDEV.i386: W: conffile-without-noreplace-flag /etc/makedev.d/01dac960
MAKEDEV.i386: W: conffile-without-noreplace-flag /etc/makedev.d/01ftape
MAKEDEV.i386: W: conffile-without-noreplace-flag /etc/makedev.d/01generic
MAKEDEV.i386: W: conffile-without-noreplace-flag /etc/makedev.d/01ia64
MAKEDEV.i386: W: conffile-without-noreplace-flag /etc/makedev.d/01ibcs
MAKEDEV.i386: W: conffile-without-noreplace-flag /etc/makedev.d/01ida
MAKEDEV.i386: W: conffile-without-noreplace-flag /etc/makedev.d/01ide
MAKEDEV.i386: W: conffile-without-noreplace-flag /etc/makedev.d/01ipfilter
MAKEDEV.i386: W: conffile-without-noreplace-flag /etc/makedev.d/01isdn
MAKEDEV.i386: W: conffile-without-noreplace-flag /etc/makedev.d/01linux-2.6.x
MAKEDEV.i386: W: conffile-without-noreplace-flag /etc/makedev.d/01linux1394
MAKEDEV.i386: W: conffile-without-noreplace-flag /etc/makedev.d/01mouse
MAKEDEV.i386: W: conffile-without-noreplace-flag /etc/makedev.d/01qic
MAKEDEV.i386: W: conffile-without-noreplace-flag /etc/makedev.d/01raid
MAKEDEV.i386: W: conffile-without-noreplace-flag /etc/makedev.d/01redhat
MAKEDEV.i386: W: conffile-without-noreplace-flag /etc/makedev.d/01s390
MAKEDEV.i386: W: conffile-without-noreplace-flag /etc/makedev.d/01sound
MAKEDEV.i386: W: conffile-without-noreplace-flag /etc/makedev.d/01std
MAKEDEV.i386: W: conffile-without-noreplace-flag /etc/makedev.d/01undocumented
MAKEDEV.i386: W: conffile-without-noreplace-flag /etc/makedev.d/01v4l

I think the conffile-without-noreplace-flag be safely ignored, if it needs to
be.  But is it required for MAKEDEV to install the files in /etc/makedev.d from
the package, overwriting a users changes?  If /etc/makedev.d was marked as
noreplace, would things be highly likely to break if a user made some change to
one of the scripts and then MAKEDEV was updated?

Other than the config warning and the license tag, I've corrected the other
warnings and will attach a patch for your perusal.  I changed the user and group
creation slightly to match the guidelines in
http://fedoraproject.org/wiki/Packaging/UsersAndGroups

5. Are the requires on grep and mktemp still needed?  The only place I find
reference to them in the tarball is in the spec file.

6. There are two outstanding bugs:
    
Bug #196042: Permissions for /dev/net/tun are too restrictive.

    This looks like it could be closed without even adding the Conflicts
    proposed, since the kernel package it would conflict is not in any supported
    release.

Bug #425832: "MAKEDEV: mkdir: File exists" if SELinux context wrong on /dev

    A full fix for this might include making sure start_udev calls restorecon if
    it mounts /dev.  It does seem like it'd be nice to try and catch this and
    give a more useful error.  But it's certainly not a hugely pressing bug.
Comment 4 Todd Zullinger 2008-03-02 16:51:20 EST
Created attachment 296525 [details]
Patch cleaning up most of the rpmlint complaints and other minor issues for merge review

Feel free to use some/all/none of this, with or without attribution.  I seek
neither fame nor fortune in the MAKEDEV changelog. :)
Comment 5 Chris Lumens 2008-03-03 11:38:20 EST
Okay, I took all your changes and also changed the license and added the
noreplace bit.  Thanks for the review and the patch.  This is all in build 3.23-4.
Comment 6 Todd Zullinger 2008-03-07 13:25:14 EST
Thanks.  All looks good now.  One less merge review in the queue.

APPROVED

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