Bug 237380 (ruby-gettext-package)

Chitlesh Goorah has asked me to do an informal review of this package.
Upon installing all BuildRequires, the package builds cleanly in Fedora Core 6
with a single warning from rpmlint:
W: ruby-gettext-package invalid-license Ruby License/LGPL

This should be LGPL rather than Ruby License/LGPL

In examining your SPEC file, I see you can collapse some Requires and
BuildRequires using this format:
BuildRequires:  irb, ruby-devel, ruby(abi = %{rubyabi}
Requires:       irb, ruby(abi) = %{rubyabi}

rather than:
BuildRequires:  irb
BuildRequires:  ruby-devel
BuildRequires:  ruby(abi) = %{rubyabi}
Requires:       ruby(abi) = %{rubyabi}
Requires:       irb

I also notice in the SPEC, the %files section has:
%files -f %{name}.lang
You might want to look into using:
%{find_lang} 
instead.

See: rpmbuild --showrc | grep find_lang

(In reply to comment #1)
> with a single warning from rpmlint:
> W: ruby-gettext-package invalid-license Ruby License/LGPL
> 
> This should be LGPL rather than Ruby License/LGPL
See the real COPYING text. This is licensed under
LGPL or Ruby License. This type of dual license is very
common for ruby related modules

> 
> In examining your SPEC file, I see you can collapse some Requires and
> BuildRequires using this format:
> BuildRequires:  irb, ruby-devel, ruby(abi = %{rubyabi}
> Requires:       irb, ruby(abi) = %{rubyabi}
> 
> rather than:
> BuildRequires:  irb
> BuildRequires:  ruby-devel
> BuildRequires:  ruby(abi) = %{rubyabi}
> Requires:       ruby(abi) = %{rubyabi}
> Requires:       irb

I _strongly_ recommend the latter style (i.e. my style) and
also some other reviewers recommend the latter style
* This style makes it easy that what is really changed on
  taking diff when the dependency is changed.
* This style makes diff output smaller.
* For this package, the BuildRequires has only 3 packages so
  the difference is small.
  However, please consider the case in which one package has
  29 BuildRequires......

> I also notice in the SPEC, the %files section has:
> %files -f %{name}.lang
> You might want to look into using:
> %{find_lang} 
> instead.

Already I use %find_lang. Note that there is no 
%{name}.mo files
--------------------------------------------
%find_lang rails
%find_lang rgettext
%{__cat} *.lang >> %{name}.lang
---------------------------------------------

Any issues left on this package?

Same as ruby-amazon, the folders samples and test were omitted. Their contents 
should be added to one or more sub packages.

Well:

http://www.ioa.s.u-tokyo.ac.jp/~mtasaka/dist/extras/development/SPECS/ruby-gettext-package.spec
http://www.ioa.s.u-tokyo.ac.jp/~mtasaka/dist/extras/development/SRPMS/ruby-gettext-package-1.9.0-2.fc7.src.rpm
------------------------------------------------
* Mon May  7 2007 Mamoru Tasaka <mtasaka.u-tokyo.ac.jp> - 1.9.0-2
- Create -doc subpackage

For rpmlint:
------------------------------------------------
1 W: ruby-gettext-package invalid-license Ruby License/LGPL
1 W: ruby-gettext-package-debuginfo invalid-license Ruby License/LGPL
2 E: ruby-gettext-package-doc zero-length
/usr/share/doc/ruby-gettext-package-doc-1.9.0/samples/rails/public/favicon.ico
3 W: ruby-gettext-package-doc spurious-executable-perm
/usr/share/doc/ruby-gettext-package-doc-1.9.0/test/test.sh
1 W: ruby-gettext-package-doc invalid-license Ruby License/LGPL
------------------------------------------------
1: This type of dual license is very common for ruby related
   software
2: Well, I am not familiar with "ruby on rails" and I am not sure
   if I should (can) remove this file or not......
3: This is okay because this script is actually executable and
   only /bin/sh dependency is added to -doc subpackage due to
   this permission.

MUST Items:

- MUST: The package is named according to the Package Naming Guidelines.
- MUST: The spec file name matches the base package %{name}
- MUST: The package meets the Packaging Guidelines.
- MUST: The package is licensed with an open-source compatible license 
and meet other legal requirements as defined in the legal section of Packaging
Guidelines.
- MUST: The License field in the package spec file matches the actual license.
- MUST: the source package includes the text of the license(s) in its own 
file, then that file, containing the text of the license(s) for the package is
included in %doc.
- MUST: The spec file must be written in American English.
- MUST: The spec file for the package is be legible. 
- MUST: The sources used to build the package must matches the upstream 
source, as provided in the spec URL.
- MUST: The package successfully compiles and builds into binary rpms on at
least i386.
- MUST: All build dependencies is listed in BuildRequires.
- MUST: The spec file handles locales properly.
- MUST: If the package does not contain shared library files located in the
dynamic linker's default paths
- MUST: the package is not designed to be relocatable
- MUST: the package owns all directories that it creates.
- MUST: the package does not contain any duplicate files in the %files 
listing.
- MUST: Permissions on files are set properly.
- MUST: The package has a %clean section, which contains rm -rf %{buildroot} 
(or
$RPM_BUILD_ROOT).
- MUST: The package consistently uses macros, as described in the macros 
section of Packaging Guidelines.
- MUST: The package contains code, or permissable content. This is described 
in detail in the code vs. content section of Packaging Guidelines.
- MUST: There are no Large documentation files
- MUST: %doc does not affect the runtime of the application. To summarize: If 
it is in %doc, the program must run properly if it is not present.
- MUST: There are no Header files or static libraries 
- MUST: The package does not contain library files with a suffix 
- MUST: Package does NOT contain any .la libtool archives
- MUST: Package containing GUI applications includes a %{name}.desktop file, 
and that file must be properly installed with desktop-file-install in 
the %install section.
- MUST: Package does not own files or directories already owned by other 
packages. 

SHOULD Items:

 - SHOULD: The source package does include license text(s) as COPYING
 - SHOULD: mock builds successfully in i386.
 - SHOULD: The reviewer tested that the package functions as described. A
package should not segfault instead of running, for example.
 - SHOULD: No scriptlets were used, those scriptlets must be sane. 
 - SHOULD: No subpackages present.

APPROVED

Thank you!!

Request for CVS admin:
-----------------------------------------------
New Package CVS Request
=======================
Package Name:         ruby-gettext-package
Short Description:    Localization Library and Tools for Ruby
Owners:               mtasaka.u-tokyo.ac.jp
Branches:             devel FC-6 FC-5
InitialCC:            (nobody)
-----------------------------------------------

Rebuild done on all branches, closing

Thank you for review!!