Bug 661272 - Review Request: lorax - tool for creating anaconda install images
Summary: Review Request: lorax - tool for creating anaconda install images
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Chris Lumens
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2010-12-08 11:49 UTC by Martin Gracik
Modified: 2013-07-04 12:53 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2010-12-22 09:23:08 UTC
Type: ---
Embargoed:
clumens: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Martin Gracik 2010-12-08 11:49:18 UTC
Spec URL: http://mgracik.fedorapeople.org/lorax/lorax.spec
SRPM URL: http://mgracik.fedorapeople.org/lorax/lorax-0.1-1.fc14.src.rpm

Description:

Lorax is a tool for creating anaconda install images. It's a replacement for the buildinstall scripts, that are part of anaconda.

Comment 1 Chris Lumens 2010-12-16 20:59:43 UTC
One quick note first:  It looks like you forgot to package the lorax/src/bin/lorax script.  This is likely because it's commented out in
lorax/setup.py, and then you'll also need to add a blurb to the .spec file too.

Mandatory review guidelines:
!! - rpmlint output
     lorax.src: W: invalid-url Source0: lorax-0.1.tar.bz2
     1 packages and 0 specfiles checked; 0 errors, 1 warnings.
ok - Package meets naming guidelines
ok - Spec file name matches base package name
ok - License is acceptable (GPLv2+)
ok - License field in spec is correct
!! - License files included in package %docs or not included in upstream source
     Please include the COPYING file (and preferably the other documentation-type files in the top level of the source) in the package.  You do this with "%doc COPYING AUTHORS ...".
na - License files installed when any subpackage combination is installed
ok - Spec written in American English
ok - Spec is legible
ok - Sources match upstream unless altered to fix permissibility issues
!! - Build succeeds on at least one supported platform
     The build succeeds, but this should be a noarch package (BuildArch: noarch).  That would also get rid of the useless -debuginfo package.
ok - Build succeeds on all supported platforms or has ExcludeArch + bugs filed
no - BuildRequires correct
     You need to BuildRequires: python{2,3}-devel as appropriate for the system.
ok - Package handles locales with %find_lang
     This is fine for now since lorax does not yet have translated strings.
na - %post, %postun call ldconfig if package contains shared .so files
na - No bundled system libs
na - Relocatability is justified
ok - Package owns all directories it creates
na - Package requires other packages for directories it uses but does not own
ok - No duplicate files in %files unless necessary for license files
ok - File permissions are sane
ok - Each %files section contains %defattr
ok - Consistent use of macros
ok - Sources contain only permissible code or content
na - Large documentation files go in -doc package
na - Missing %doc files do not affect runtime
na - Headers go in -devel package
na - Static libs go in -static package
na - Unversioned .so files go in -devel package
na - Devel packages require base with fully-versioned dependency
na - Package contains no .la files
na - GUI app installs .desktop file w/desktop-file-install or has justification
ok - Package's files and directories don't conflict with others' or justified
ok - File names are valid UTF-8

Optional review guidelines:
na - Query upstream about including license files
na - Translations of description, Summary
ok - Builds in mock
ok - Builds on all supported platforms
ok - Functions as described
na - Scriptlets are sane
na - Non-devel subpackage Requires are sane
na - .pc files go in -devel unless main package is a development tool
ok - No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin
no - Man pages included for all executables
     Once you have the /usr/sbin/lorax program in the package, it would be nice (but not required) to also have a man page.

Packaging guidelines:
ok - Has dist tag
ok - Useful without external bits
ok - Package obeys FHS, except libexecdir and /usr/target
ok - Changelog in prescribed format
ok - Spec file lacks Packager, Vendor, PreReq tags
no - Correct BuildRoot tag on < F10/EL6
     You don't need BuildRoot anymore.
no - Correct %clean section on < F13/EL6
     You don't need a %clean section anymore.
!! - Requires correct, justified where necessary
     Please audit the commands you call out to and verify that the packages containing those programs are listed as Requires.
ok - Summary, description do not use trademarks incorrectly
no - All relevant documentation is packaged, tagged appropriately
     See above.
na - %build honors applicable compiler flags or justifies otherwise
na - Package with .pc files Requires pkgconfig on < EL6
no - Useful -debuginfo package or disabled and justified
     See above.
na - No static executables
na - Rpath absent or only used for internal libs
no - Config files marked with %config
     %{_sysconfdir}/lorax/lorax.conf should be marked as a config file to avoid it being overwritten on upgrade.
no - %config files marked noreplace or justified
     See above.
!! - No %config files under /usr
     Should /usr/share/lorax/ramdisk.ltmpl be installed into /etc/lorax instead?  Is it expected to be modified by the user?
na  - SysV-style init script
ok - Spec uses macros instead of hard-coded directory names
ok - %makeinstall used only when ``make install DESTDIR=...'' doesn't work
na - Macros in Summary, %description expandable at SRPM build time
ok - Spec uses %{SOURCE#} instead of $RPM_SOURCE_DIR or %{sourcedir}
na - %global instead of %define where appropriate
na - Package containing translations BuildRequires gettext
na - File timestamps preserved by file ops
na - Parallel make
ok - Spec does not use Requires(pre,post) notation
na - User, group creation handled correctly (See Packaging:UsersAndGroups)
na - Web app files go in /usr/share/%{name}, not /var/www
na - Conflicts are justified
ok - No external kernel modules
ok - No files in /srv
ok - One project per package
na - Patches link to upstream bugs/comments/lists or are otherwise justified

Python Guidelines:
!! - Runtime Requires correct
     See above.
ok - Python macros declared on < F13/EL6
ok - All .py files packaged with .pyc, .pyo counterparts
ok - Includes .egg-info files/directories when generated
ok - Provides/Requires properly filtered
na - Code that invokes gtk.gdk.get_pixels_array() Requires numpy

Comment 2 Martin Gracik 2010-12-17 16:14:42 UTC
I fixed the things you pointed out, and updated the srpm and spec file on fedorahosted.

Comment 3 Chris Lumens 2010-12-17 16:38:31 UTC
Mandatory review guidelines:
ok - rpmlint output
     A comment in the .spec file excuses this.
ok - License files included in package %docs or not included in upstream source
ok - Build succeeds on at least one supported platform
ok - Build succeeds on all supported platforms or has ExcludeArch + bugs filed
ok - BuildRequires correct

Optional review guidelines:
no - Man pages included for all executables
     This is fine for now.

Packaging guidelines:
ok - Correct BuildRoot tag on < F10/EL6
ok - Correct %clean section on < F13/EL6
ok - Requires correct, justified where necessary
ok - All relevant documentation is packaged, tagged appropriately
     It would be nice to have some real documentation - either as docstrings or external.  This is a big, complex project and needs some explaining.
However, if having documentation were a requirement for being in Fedora, we'd have far less stuff.
ok - Useful -debuginfo package or disabled and justified
ok - Config files marked with %config
no - %config files marked noreplace or justified
     Please use %config(noreplace) - see https://fedoraproject.org/wiki/Packaging:Guidelines#Configuration_files
ok - No %config files under /usr

Python Guidelines:
ok - Runtime Requires correct

Comment 4 Martin Gracik 2010-12-17 16:47:41 UTC
I fixed the config(noreplace) part.
Will add the documentation part to my TODO.

Comment 5 Martin Gracik 2010-12-17 17:05:20 UTC
New Package SCM Request
=======================
Package Name: lorax
Short Description: Lorax is a tool for creating anaconda install images.
Owners: mgracik
Branches: 
InitialCC: anaconda-maint

Comment 6 Kevin Fenzi 2010-12-21 06:18:08 UTC
Git done (by process-git-requests).


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