Bug 228243 - Review Request: eblook - EB and EPWING dictionary search program
Review Request: eblook - EB and EPWING dictionary search program
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Parag AN(पराग)
Fedora Package Reviews List
:
Depends On: 228241
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2007-02-11 21:56 EST by Jens Petersen
Modified: 2007-11-30 17:11 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-02-20 07:18:39 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description Jens Petersen 2007-02-11 21:56:22 EST
Spec URL: http://people.redhat.com/petersen/extras/eblook.spec
SRPM URL: http://people.redhat.com/petersen/extras/eblook-1.6.1-1.src.rpm
Description: 
eblook is a command-line EB and EPWING dictionary search program.

http://openlab.ring.gr.jp/edict/eblook/
Comment 1 Parag AN(पराग) 2007-02-12 07:33:27 EST
mockbuild failed as zlib-devel is not present in BuildRequires
When added above BR, mock build is fine but rpmlint on RPM gave
W: eblook file-not-utf8 /usr/share/info/eblook.info.gz
The character encoding of this file is not UTF-8.  Consider converting it
in the specfile for example using iconv(1).

Comment 2 Michael Schwendt 2007-02-12 07:50:22 EST
Better, create a patch (with the help of iconv) and apply the
patch in the specfile.
Comment 3 Mamoru TASAKA 2007-02-12 08:07:06 EST
(In reply to comment #2)
> Better, create a patch (with the help of iconv) and apply the
> patch in the specfile.

It is quite usual for Japanese document to use simply iconv.
Creating patch results in making the patch contain both EUC-JP 
and UTF-8 characters and the patch cannot be seen easily.
Comment 4 Michael Schwendt 2007-02-12 08:18:18 EST
In general, running iconv can silently break the file, e.g. when it
is encoded in the target encoding already and when iconv doesn't
encounter any illegal encoding.
Comment 5 Mamoru TASAKA 2007-02-12 08:35:44 EST
(In reply to comment #4)
> e.g. when it
> is encoded in the target encoding already and when iconv doesn't
> encounter any illegal encoding.

I think you are raising an example which won't usually happen...
Anyway, it is usual to use iconv, i.e.

iconv -f EUC-JP -t UTF-8 foo.txt > foo.txt.tmp && \
  mv -f foo.txt.tmp foo.txt || rm -f foo.txt.tmp

Comment 6 Michael Schwendt 2007-02-12 08:52:30 EST
Try that when "foo.txt" is UTF-8 already, because for example
upstream syncs with changes found in an rpm. iconv either
creates invalid output silently or throws an error. The latter
is good, the former is bad. Using a patch avoids that case.

Btw, my comment is not specific to iconv. It also applies to other
spec inline hacks (sed, Perl, and more). Breakage we've encountered
before in Fedora packages.
Comment 7 Mamoru TASAKA 2007-02-12 09:04:47 EST
(In reply to comment #6)
> Try that when "foo.txt" is UTF-8 already, 
So.. this should not happen. Why don't you check the encoding
beforehand?
Comment 8 Mamoru TASAKA 2007-02-12 09:09:55 EST
By the way..
EUC-JP and UTF-8 encondings are quite different, so
when trying to "already UTF-8 encoded" Japanese text from
EUC-JP to UTF-8, this fails 100%.
Comment 9 Michael Schwendt 2007-02-12 09:34:38 EST
> Why don't you check the encoding beforehand?

Because the spec file would not do that either. It would run
iconv with hardcoded options, regardless of what encoding is used
in the input file actually.

Anyway, I believe my message has become clear. Whether all sequences
in an UTF-8 stream are invalid EUC-JP encodings is beyond the scope
of my initial comment. That is an assertion that doesn't hold true
for a few common other conversions, e.g. ISO Latin-1 to UTF-8.
Comment 10 Jens Petersen 2007-02-14 01:38:52 EST
(In reply to comment #1)
> mockbuild failed as zlib-devel is not present in BuildRequires
> When added above BR, mock build is fine but rpmlint on RPM gave

Thanks for catching that.  I made eb-devel require zlib-devel.

> W: eblook file-not-utf8 /usr/share/info/eblook.info.gz
> The character encoding of this file is not UTF-8.  Consider converting it
> in the specfile for example using iconv(1).

I think in this case using iconv should be pretty safe since it will
choke on utf8 when trying to convert from eucjp.

Updated to:
Spec URL: http://people.redhat.com/petersen/extras/eblook.spec
SRPM URL: http://people.redhat.com/petersen/extras/eblook-1.6.1-2.src.rpm
Comment 11 Parag AN(पराग) 2007-02-14 04:20:37 EST
Review:
+ package builds in mock (development i386).
+ rpmlint is silent for SRPM and RPM
+ source files match upstream.
c570ce70697e6653d4d086fa3ad97e19  eblook-1.6.1.tar.gz
+ package meets naming and packaging guidelines.
+ specfile is properly named, is cleanly written
+ Spec file is written in American English.
+ Spec file is legible.
+ dist tag is present.
+ build root is correct.
+ license is open source-compatible.
+ License text is included in package.
+ %doc is small; no -doc subpackage required.
+ %doc does not affect runtime.
+ BuildRequires are proper.
+ %clean is present.
+ package installed properly.
+ Macro use appears rather consistent.
+ Package contains code, not content.
+ no static libraries.
+ no .pc file present.
+ no -devel subpackage exists.
+ no .la files.
+ no translations are available.
+ Does owns the directories it creates.
+ no duplicates in %files.
+ file permissions are appropriate.
APPROVED.
Comment 12 Jens Petersen 2007-02-14 18:58:36 EST
Thanks!  Requested CVSSyncNeeded.
Comment 13 Jens Petersen 2007-02-20 07:18:39 EST
Packages for devel, FC-5 and FC-6 have been built.

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