Bug 465740 - (rubygem-gettext) Review Request: rubygem-gettext - RubyGem of Localization Library and Tools for Ruby
Review Request: rubygem-gettext - RubyGem of Localization Library and Tools f...
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Orcan Ogetbil
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-10-06 04:01 EDT by Mamoru TASAKA
Modified: 2010-12-27 19:48 EST (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-10-08 07:26:50 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
oget.fedora: fedora‑review+
tibbs: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Mamoru TASAKA 2008-10-06 04:01:30 EDT
Spec URL: http://mtasaka.fedorapeople.org/Review_request/rubygem-gettext/rubygem-gettext.spec
SRPM URL: http://mtasaka.fedorapeople.org/Review_request/rubygem-gettext/rubygem-gettext.spec

Koji build:
- F-10: http://koji.fedoraproject.org/koji/taskinfo?taskID=863051
- F-9:  http://koji.fedoraproject.org/koji/taskinfo?taskID=863049

Description:
Ruby-GetText-Package is a GNU GetText-like program for Ruby. 
The catalog file(po-file) is same format with GNU GetText. 
So you can use GNU GetText tools for maintaining.

This package provides gem for Ruby-Gettext-Package.

Note:
There are so many rpmlint messages (2 errors, 490 warnings)
* dangling-relative-symlink
  - ruby-gettext-package package Requires rubygem-gettext and installing
    both correctly resolves dangling relative symlinks, so these rpmlint
    messages can be ignored.
* file-not-in-%lang
  - RubyGem wants all files to be installed under %geminstdir, so
    these messages cannot be suppressed.
  - Also some others are under %_docdir and rpmlint messages for those
    files are safe to ignore.
* file-not-utf8
  - These are gettext po files and converting character codes need not
    changing
* zero-length
  - While I am not familiar with rails, I guess these files are
    actually needed.
Comment 2 Orcan Ogetbil 2008-10-06 15:21:58 EDT
The package looks very good. Here are my notes
-------------------------------------------------------------------------
* The spec file MUST handle locales properly. This is done by using the %find_lang macro. Using %{_datadir}/locale/* is strictly forbidden.
   http://fedoraproject.org/wiki/Packaging/Guidelines#Handling_Locale_Files
Could you BuildRequire gettext and use the %find_lang macro as depicted in the Guidelines? Will this cause a problem?
-------------------------------------------------------------------------
* The package does not use the macro's consistently. e.g. the lines
   %{__rm} -rf %{buildroot}%{gemdir}/bin/
   ...
   rm -rf %{buildroot}
cause some inconsistency. 
Also you use the "%{__command}" notation for some commands and "command" notation for the others. One example: "mv" should be "%{__mv}" if you want to be consistent.
-------------------------------------------------------------------------
* From http://fedoraproject.org/wiki/Packaging/Ruby  :
The package must have a Requires and a BuildRequires on rubygems
You only have
   BuildRequires:  ruby(rubygems)  
   Requires:  ruby(rubygems)
-------------------------------------------------------------------------
*  Requires:       ruby(rubygems)
   Requires:       irb
Don't you need to require a specific version (or above)?  (just asking)
-------------------------------------------------------------------------
* If the same Ruby library is to be packaged for use as a Gem and as a straight Ruby library without Gem support, it must be packaged as a Gem first. To make it available to code that does not use Ruby Gems, a subpackage called ruby-%{gemname} must be created in the rubygem-%{gemname} package such that

But your straight library is named ruby-%{gemname}-package. Can you explain why?
Comment 3 Mamoru TASAKA 2008-10-07 00:34:24 EDT
Hello, thank you for your comments.

(In reply to comment #2)
> -------------------------------------------------------------------------
> * The spec file MUST handle locales properly. This is done by using the
> %find_lang macro. Using %{_datadir}/locale/* is strictly forbidden.
>    http://fedoraproject.org/wiki/Packaging/Guidelines#Handling_Locale_Files
> Could you BuildRequire gettext and use the %find_lang macro as depicted in the
> Guidelines? Will this cause a problem?

 - For this package "BR: gettext" is not needed because gettext .mo files
   are already shipped in gem.
   Also %find_lang is actually used (for symlinked mo files under %_datadir/locale)
   as:
---------------------------------------------------------------------------
%find_lang rails
%find_lang rgettext
%{__cat} *.lang >> %{name}.lang

%files -n	ruby-gettext-package	-f %{name}.lang
---------------------------------------------------------------------------
   The problem is that %find_lang cannot handle files under %gemdir

> -------------------------------------------------------------------------
> * The package does not use the macro's consistently. e.g. the lines
>    %{__rm} -rf %{buildroot}%{gemdir}/bin/
>    ...
>    rm -rf %{buildroot}
> cause some inconsistency. 
> Also you use the "%{__command}" notation for some commands and "command"
> notation for the others. One example: "mv" should be "%{__mv}" if you want to
> be consistent.

  - Will fix them

> -------------------------------------------------------------------------
> * From http://fedoraproject.org/wiki/Packaging/Ruby  :
> The package must have a Requires and a BuildRequires on rubygems
> You only have
>    BuildRequires:  ruby(rubygems)  
>    Requires:  ruby(rubygems)

  - ruby(rubygems) pulls rubygems. And using virtual Provides for
    (Build)Requires is preferred (as well as perl) if possible.

> -------------------------------------------------------------------------
> *  Requires:       ruby(rubygems)
>    Requires:       irb
> Don't you need to require a specific version (or above)?  (just asking)
> -------------------------------------------------------------------------

   - This package already has "(Build)Requires: ruby(abi) = %rubyabi".

> * If the same Ruby library is to be packaged for use as a Gem and as a straight
> Ruby library without Gem support, it must be packaged as a Gem first. To make
> it available to code that does not use Ruby Gems, a subpackage called
> ruby-%{gemname} must be created in the rubygem-%{gemname} package such that
> 
> But your straight library is named ruby-%{gemname}-package. Can you explain
> why?

   - Because
     * non-gem version is actually called as ruby-gettext-package (on
       the upstream URL)
     * Also there is already a rpm named "ruby-gettext-package" in Fedora
       (which I currently maintain). This new rpm is to supersede
      current ruby-gettext-package after switching source from native
      tarball to gem.
Comment 4 Orcan Ogetbil 2008-10-07 02:02:47 EDT
You're welcome. So everything seems fine but I am a little concerned about the locale files:

(In reply to comment #3)
> Hello, thank you for your comments.
> 
> (In reply to comment #2)
> > -------------------------------------------------------------------------
> > * The spec file MUST handle locales properly. This is done by using the
> > %find_lang macro. Using %{_datadir}/locale/* is strictly forbidden.
> >    http://fedoraproject.org/wiki/Packaging/Guidelines#Handling_Locale_Files
> > Could you BuildRequire gettext and use the %find_lang macro as depicted in the
> > Guidelines? Will this cause a problem?
> 
>  - For this package "BR: gettext" is not needed because gettext .mo files
>    are already shipped in gem.

But the .mo files are binary. I think you need to remove them and rebuild them from the .po files.
Comment 5 Mamoru TASAKA 2008-10-07 03:27:04 EDT
Hello.

(In reply to comment #4)
> But the .mo files are binary. I think you need to remove them and rebuild them
> from the .po files.

Okay. This time I recreated gettext .mo files.

Note that this package still does not have "BuildRequires: gettext".
As the name of this package shows this package should provide 
the ruby inplementation of gettext and if msgfmt or so is needed
this package should do it by itself.
Actually this package uses calls ruby "iconv" method, which calls
C gettext related functions.

http://mtasaka.fedorapeople.org/Review_request/rubygem-gettext/rubygem-gettext.spec
http://mtasaka.fedorapeople.org/Review_request/rubygem-gettext/rubygem-gettext-1.93.0-4.tmp.src.rpm
* Tue Oct  7 2008 Mamoru Tasaka <mtasaka@ioa.s.u-tokyo.ac.jp> - 1.93.0-4
- Recreate gettext .mo files (by using this itself)

Koji build:
F-10: http://koji.fedoraproject.org/koji/taskinfo?taskID=865979
F-9:  http://koji.fedoraproject.org/koji/taskinfo?taskID=865981
Comment 7 Orcan Ogetbil 2008-10-07 16:28:41 EDT
Thanks for the update!

* I have one suggestion. The two rpmlint errors:
   rubygem-gettext-doc.noarch: E: zero-length /usr/share/doc/rubygem-gettext-doc-1.93.0/test/rails/public/favicon.ico                      
   rubygem-gettext-doc.noarch: E: zero-length /usr/share/doc/rubygem-gettext-doc-1.93.0/samples/rails/public/favicon.ico 
These are favicons (the icon (image) that shows in your browser to the left of the addressbar when you go to a webpage). These are probably for the .html doc files.

Having them at zero-length is most likely a mistake of upstream. I would let them know. You can also create a favicon from their logo yourself (with gimp or imagemagick or so) and replace with the in the package. I leave this up to you.

--------------------------------------------------------------------------
* One last thing I noticed. If I go to 
   /usr/share/doc/rubygem-gettext-doc-1.93.0/samples/rails/public/
or
   /usr/share/doc/rubygem-gettext-doc-1.93.0/test/rails/public/
the index.html there has a broken link:
"About your applications environment" points to some non-existing 
file:///usr/share/doc/rubygem-gettext-doc-1.93.0/{samples,test}/rails/public/rails/info/properties

Did you forget to package something, or is this an upstream mistake again?
--------------------------------------------------------------------------

But otherwise the package seems good. Check these issues and see if there's anything we can do. I'm now approving it.



--------------------------------------------------
This package (rubygem-gettext) is APPROVED by oget
--------------------------------------------------
Comment 8 Orcan Ogetbil 2008-10-07 16:30:39 EDT
Oh, if you decide to remove the favicons completely that won't break anything. I think you should do it this way if you don't want to build them yourself.
Comment 9 Mamoru TASAKA 2008-10-07 23:06:20 EDT
Thanks!

- For zero length ico, for now I just keep these as they are.
- About info/properties:
  This is related to "ruby on rails", however I am not familiar with
  it..

New Package CVS Request
=======================
Package Name:          rubygem-gettext
Short Description:     RubyGem of Localization Library and Tools for Ruby
Owners:                mtasaka
Branches:              F-9 F-8
InitialCC:             sseago
Comment 10 Mamoru TASAKA 2008-10-07 23:07:51 EDT
Ah...

New Package CVS Request
=======================
Package Name:          rubygem-gettext
Short Description:     RubyGem of Localization Library and Tools for Ruby
Owners:                mtasaka sseago
Branches:              F-9 F-8
InitialCC:
Comment 11 Huzaifa S. Sidhpurwala 2008-10-08 05:23:37 EDT
cvs done
Comment 12 Mamoru TASAKA 2008-10-08 07:26:50 EDT
Rebuilt on all branches, submitted push requests on bodhi, closing.
Thank you for the review and CVS procedure.
Comment 13 Michael Stahnke 2010-12-27 11:09:31 EST
Package Change Request
======================
Package Name: rubygem-gettext
New Branches: el5 el6
Owners: 
InitialCC: 
Mamoru has stated I may maintain any of his ruby packages in EPEL.
Comment 14 Michael Stahnke 2010-12-27 11:11:41 EST
Package Change Request
======================
Package Name: rubygem-gettext
New Branches: el5 el6
Owners: stahnma
InitialCC: 
Mamoru has stated I may maintain any of his ruby packages in EPEL
Comment 15 Jason Tibbitts 2010-12-27 19:48:06 EST
Git done (by process-git-requests).

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