[Bug 835275] New: Review Request: shflags - Simple handling of command-line flags in Bourne based Unix scripts
https://bugzilla.redhat.com/show_bug.cgi?id=835275
Bug ID: 835275 QA Contact: extras-qa@fedoraproject.org Severity: medium Version: rawhide Priority: medium CC: notting@redhat.com, package-review@lists.fedoraproject.org Assignee: nobody@fedoraproject.org Summary: Review Request: shflags - Simple handling of command-line flags in Bourne based Unix scripts Regression: --- Story Points: --- Classification: Fedora OS: Linux Reporter: rbean@redhat.com Type: --- Documentation: --- Hardware: All Mount Type: --- Status: NEW Component: Package Review Product: Fedora
Spec URL: http://threebean.org/rpm/shflags.spec SRPM URL: http://threebean.org/rpm/shflags-1.0.3-1.fc17.src.rpm Description: Shell Flags (shFlags) is a library written to greatly simplify the handling of command-line flags in Bourne based Unix shell scripts (bash, dash, ksh, sh, zsh) on many Unix OSes (Linux, Solaris, Mac OS X, etc.).
Most shell scripts use getopt for flags processing, but the different versions of getopt on various OSes make writing portable shell scripts difficult. shFlags instead provides an API that doesn't change across shell and OS versions so the script writer can be confident that the script will work.
shFlags is a port of the google-gflags C++/Python library.
Fedora Account System Username: ralph
rpmlint -------
--- ~/rpmbuild » rpmlint {SPECS,SRPMS}/shflags* 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 1 packages and 1 specfiles checked; 0 errors, 5 warnings. --- ~/rpmbuild » rpmlint /var/lib/mock/fedora-17-x86_64/result/*.rpm 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.
I'll file an issue upstream to update the FSF license shortly.
https://bugzilla.redhat.com/show_bug.cgi?id=835275
Ralph Bean rbean@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |835657
https://bugzilla.redhat.com/show_bug.cgi?id=835275
Matt Spaulding mspaulding06@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |mspaulding06@gmail.com
--- Comment #1 from Matt Spaulding mspaulding06@gmail.com --- This is an unofficial practice review.
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 1 packages and 0 specfiles checked; 2 errors, 5 warnings.
Package Review:
- package meets package naming guidelines - legible and in American English - license LGPLv2 is slightly ambiguous (see below) - license file included in %doc section - MD5 sum on tarball matches that of the upstream tarball - no missing BuildRequires - util-linux BuildRequires is unnecessary (see below) - no locales included - not a relocatable package - does not own all directories it creates (see below) - no duplicate files in %files section and all file names are valid utf-8 - needs to preserve timestamps on file copy (see below) - consistent macro use - might want to have a -docs subpackage, though not required - example scripts in %doc should not be executable (see below) - no bundled system libraries - no need for .desktop file - builds on x86 successfully in mock - verified that shflags works as expected using examples included in the tarball - included tests are not being run (see below)
Fixes:
- I believe util-linux is unnecessary in BuildRequires section and should be removed. The package exception guidelines mention util-linux-ng which is essentially the same package - contact upstream to fix incorrect fsf address (which you have commented you are doing) - contact upstream regardling license type. It's not clear if this is LGPLv2 or LGPLv2+ - no man pages, maybe work with upstream to get these added - make example scripts not executable with "chmod -x" - when copying files in %install section use "cp -p" to preserve timestamps - need to own /usr/share/doc/shflags directory in %files section - tests included in tarball should be run in the %check section
https://bugzilla.redhat.com/show_bug.cgi?id=835275
Garrett Holmstrom gholms@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |gholms@fedoraproject.org Flags| |fedora-review?
--- Comment #2 from Garrett Holmstrom gholms@fedoraproject.org --- 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
https://bugzilla.redhat.com/show_bug.cgi?id=835275
Garrett Holmstrom gholms@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Assignee|nobody@fedoraproject.org |gholms@fedoraproject.org
https://bugzilla.redhat.com/show_bug.cgi?id=835275
--- Comment #3 from Ralph Bean rbean@redhat.com --- Thanks guys. Here's a new release with:
- Updated license field to LGPLv2+ - Removed BuildRequires util-linux. Superfluous! - Ownership taken for %%{_datadir}/%%{name} - Copying with '-p' to preserve - Made examples non-exectuable. - Added %%check section.
Spec URL: http://threebean.org/rpm/shflags.spec SRPM URL: http://threebean.org/rpm/shflags-1.0.3-2.fc17.src.rpm
I just emailed upstream directly about the license ambiguity. I changed the license tag in the spec file to LGPLv2+ in the meantime at Garrett's suggestion.
https://bugzilla.redhat.com/show_bug.cgi?id=835275
Garrett Holmstrom gholms@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+
--- Comment #4 from Garrett Holmstrom gholms@fedoraproject.org --- Its tests don't seem to pass on F16; I hope that's okay. Everything else looks fine to me. Just don't forget to fix the License field if it turns out to be LGPLv2 after all.
Enjoy!
== Review of shflags-1.0.3-2 ==
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 shflags.src: W: invalid-url Source0: http://shflags.googlecode.com/files/shflags-1.0.3.tgz HTTP Error 404: Not Found 2 packages and 0 specfiles checked; 2 errors, 11 warnings. -- You've already addressed the FSF address issue. The 404 is due to Google's badly-behaved web servers. The rest are harmless. ok - License is acceptable (LGPLv2+) ok - License field in spec is correct Please make sure to fix the License field if upstream disagrees with the LGPLv2+ assessment. 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 ok - BuildRequires correct -- - 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 ok - Package owns all directories it creates -- - Package requires others for directories it uses but does not own ok - No duplication in %files unless necessary for license files ok - File permissions are sane 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 ok - Doc files do not drag in extra dependencies (e.g. due to +x) -- - 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 ok - File ops preserve timestamps -- - 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 ok - Available test suites executed in %check -- - tmpfiles.d used for /run, /run/lock on >= F15
https://bugzilla.redhat.com/show_bug.cgi?id=835275
--- Comment #5 from Ralph Bean rbean@redhat.com --- Got it, Garrett. And thanks again (to Matt, too)!
https://bugzilla.redhat.com/show_bug.cgi?id=835275
Ralph Bean rbean@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-cvs?
--- Comment #6 from Ralph Bean rbean@redhat.com --- New Package SCM Request ======================= Package Name: shflags Short Description: Simple handling of command-line flags in Bourne based Unix scripts Owners: ralph Branches: f17 el6 InitialCC:
https://bugzilla.redhat.com/show_bug.cgi?id=835275
--- Comment #7 from Jon Ciesla limburgher@gmail.com --- Git done (by process-git-requests).
https://bugzilla.redhat.com/show_bug.cgi?id=835275
--- Comment #8 from Ralph Bean rbean@redhat.com --- Updates -> https://admin.fedoraproject.org/updates/shflags
https://bugzilla.redhat.com/show_bug.cgi?id=835275
Ralph Bean rbean@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution|--- |ERRATA Last Closed| |2012-07-07 14:46:41
package-review@lists.fedoraproject.org
-
Red Hat Bugzilla