https://bugzilla.redhat.com/show_bug.cgi?id=835275 Garrett Holmstrom <gholms@xxxxxxxxxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |gholms@xxxxxxxxxxxxxxxxx Flags| |fedora-review? --- Comment #2 from Garrett Holmstrom <gholms@xxxxxxxxxxxxxxxxx> --- Matt's review looks pretty close to me. Here are the issues I found: The sources don't indicate a LGPL version, but the source tree includes LGPL-2.1. This likely indicates LGPLv2+, but it might be worth double-checking with upstream to ensure it isn't LGPLv2. BuildRequires: util-linux is superfluous because it appears in the BuildRequires exception list [1]. The package doesn't own %{_datadir}/%{name}. The %doc files under the "examples" directory are executable, causing it to drag in /bin/sh unnecessarily. While that's a trifling issue, it's probably worth proactively fixing it to prevent future updates from accidentally pulling in other things. The test programs under the "src" directory aren't executed during %check. The packaging guidelines suggest using cp's -p option to preserve file timestamps. This one isn't required; just best practice. Just fix those and you should be good to go! An exhaustive review follows. [1] https://fedoraproject.org/wiki/Packaging:Guidelines#Exceptions_2 == Review of shflags-1.0.3-1 == Mandatory review guidelines: ok - rpmlint output: shflags.noarch: W: spelling-error %description -l en_US ksh -> ks, sh, ssh shflags.noarch: W: spelling-error %description -l en_US zsh -> sh, ssh, ash shflags.noarch: W: spelling-error %description -l en_US getopt -> get opt, get-opt, treetop shflags.noarch: W: spelling-error %description -l en_US google -> Google, goggle, googly shflags.noarch: W: spelling-error %description -l en_US gflags -> flags, gulags, g flags shflags.noarch: E: incorrect-fsf-address /usr/share/doc/shflags-1.0.3/doc/LICENSE.shunit2 shflags.noarch: E: incorrect-fsf-address /usr/share/doc/shflags-1.0.3/doc/LGPL-2.1 shflags.src: W: spelling-error %description -l en_US ksh -> ks, sh, ssh shflags.src: W: spelling-error %description -l en_US zsh -> sh, ssh, ash shflags.src: W: spelling-error %description -l en_US getopt -> get opt, get-opt, treetop shflags.src: W: spelling-error %description -l en_US google -> Google, goggle, googly shflags.src: W: spelling-error %description -l en_US gflags -> flags, gulags, g flags 2 packages and 0 specfiles checked; 2 errors, 10 warnings. You've already addressed the FSF address issue. The rest look harmless. ok - License is acceptable (LGPLv2) ?? - License field in spec is correct The sources don't indicate a LGPL version. Is upstream okay with LGPLv2+ or is it only LGPLv2? ok - License files included in package %docs if included in source package ok - 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 Upstream MD5: b4d7133696ec05b71b27d8df5e278f0f shflags-1.0.3.tgz Your MD5: b4d7133696ec05b71b27d8df5e278f0f shflags-1.0.3.tgz ok - Build succeeds on at least one primary arch ok - Build succeeds on all primary arches or has ExcludeArch + bugs filed NO - BuildRequires correct util-linux is part of the BuildRequires exception list. -- - Locales handled with %find_lang, not %_datadir/locale/* -- - %post, %postun call ldconfig if package contains shared .so files ok - No bundled libs -- - Relocatability is justified NO - Package owns all directories it creates Missing %{_datadir}/%{name} -- - Package requires others for directories it uses but does not own ok - No duplication in %files unless necessary for license files NO - File permissions are sane -rwxr-xr-x root root /usr/share/doc/shflags-1.0.3/examples/debug_output.sh -rwxr-xr-x root root /usr/share/doc/shflags-1.0.3/examples/hello_world.sh -rwxr-xr-x root root /usr/share/doc/shflags-1.0.3/examples/write_date.sh ok - Package contains permissible code or content -- - Large docs go in -doc subpackage ok - %doc files not required at runtime -- - Static libs go in -static package/virtual Provides -- - Development files go in -devel package -- - -devel packages Require base with fully-versioned dependency, %_isa ok - No .la files -- - GUI app uses .desktop file, installs it with desktop-file-install ok - File list does not conflict with other packages' without justification ok - File names are valid UTF-8 Optional review guidelines: -- - Query upstream about including license files no - Translations of description, summary ok - Builds in mock ok - Builds on all arches ok - Functions as described (e.g. no crashes) -- - Scriptlets are sane -- - Subpackages require base with fully-versioned dependency if sensible -- - .pc file subpackage placement is sensible ok - No file deps outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin -- - Include man pages if available Naming guidelines: ok - Package names use only a-zA-Z0-9-._+ subject to restrictions on -._+ ok - Package names are sane ok - No naming conflicts ok - Spec file name matches base package name ok - Version is sane ok - Version does not contain ~ ok - Release is sane ok - %dist tag ok - Case used only when necessary -- - Renaming handled correctly Packaging guidelines: ok - Useful without external bits ok - No kmods ok - Pre-built binaries, libs removed in %prep ok - Sources contain only redistributable code or content ok - Spec format is sane ok - Package obeys FHS, except libexecdir, /run, /usr/target ok - No files in /bin, /sbin, /lib* on >= F17 -- - Programs run before FS mounting use /run instead of /var/run -- - Binaries in /bin, /sbin do not depend on files in /usr on < F17 ok - No files under /srv, /opt, /usr/local ok - Changelog in prescribed format ok - No Packager, Vendor, Copyright, PreReq tags ok - Summary does not end in a period -- - Correct BuildRoot tag on < EL6 -- - Correct %clean section on < EL6 ok - Requires correct, justified where necessary ok - Summary, description do not use trademarks incorrectly ok - All relevant documentation is packaged, appropriately marked with %doc NO - Doc files do not drag in extra dependencies (e.g. due to +x) Executables in "examples" dir drag in /bin/sh. This is rather trifling, but chmod'ing them now can help prevent updates' causing issues in the future. -- - Code compilable with gcc is compiled with gcc -- - Build honors applicable compiler flags or justifies otherwise -- - PIE used for long-running/root daemons, setuid/filecap programs -- - Useful -debuginfo package or disabled and justified -- - Package with .pc files Requires pkgconfig on < EL6 ok - No static executables -- - Rpath absent or only used for internal libs -- - Config files marked with %config(noreplace) or justified %config ok - No config files under /usr -- - Third party package manager configs acceptable, in %_docdir -- - .desktop files are sane ok - Spec uses macros consistently ok - Spec uses macros instead of hard-coded names where appropriate ok - Spec uses macros for executables only when configurability is needed -- - %makeinstall used only when alternatives don't work -- - Macros in Summary, description are expandable at srpm build time -- - Spec uses %{SOURCE#} instead of $RPM_SOURCE_DIR and %sourcedir ok - No software collections (scl) ok - Build uses only python/perl/shell+coreutils/lua/BuildRequired langs -- - %global, not %define -- - Package translating with gettext BuildRequires it -- - Package translating with Linguist BuildRequires qt-devel no - File ops preserve timestamps Though optional, the packaging guidelines recommend cp's -p option. -- - Parallel make ok - No Requires(pre,post) notation -- - User, group creation handled correctly (See Packaging:UsersAndGroups) -- - Web apps go in /usr/share/%name, not /var/www -- - Conflicts are justified ok - One project per package ok - No bundled fonts -- - Patches have appropriate commentary NO - Available test suites executed in %check If running the tests isn't practical then please drop the util-linux build dep. -- - tmpfiles.d used for /run, /run/lock on >= F15 -- You are receiving this mail because: You are on the CC list for the bug. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review