Bug 247227 (nabi) - Review Request: nabi - hangul and hanja X input method
Summary: Review Request: nabi - hangul and hanja X input method
Alias: nabi
Product: Fedora
Classification: Fedora
Component: Package Review   
(Show other bugs)
Version: rawhide
Hardware: All
OS: Linux
Target Milestone: ---
Assignee: Jens Petersen
QA Contact: Fedora Extras Quality Assurance
Depends On:
TreeView+ depends on / blocked
Reported: 2007-07-06 05:27 UTC by subhransu behera
Modified: 2007-11-30 22:12 UTC (History)
7 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2007-09-20 06:06:23 UTC
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
petersen: fedora-review+
kevin: fedora-cvs+

Attachments (Terms of Use)
nabi.spec-4.patch (2.47 KB, patch)
2007-07-30 07:18 UTC, Jens Petersen
no flags Details | Diff
nabi.desktop (165 bytes, text/plain)
2007-07-30 07:19 UTC, Jens Petersen
no flags Details
nabi.spec-5.patch (1.11 KB, patch)
2007-08-07 02:52 UTC, Jens Petersen
no flags Details | Diff
nabi.spec-6.patch (923 bytes, patch)
2007-08-09 04:03 UTC, Jens Petersen
no flags Details | Diff
nabi-remove-bad-themes (189 bytes, text/plain)
2007-08-17 01:03 UTC, Jens Petersen
no flags Details

Description subhransu behera 2007-07-06 05:27:54 UTC
Spec URL: 





Nabi is a Korean word that means butterfly. Nabi (the program) is an easy to use Korean X input method. It allows you to enter phonetic Korean characters (hangul) and pictographic Korean characters (hanja.)

Additional Information: 

This package was there in FC-3.

Reference: http://nabi.kldp.net/document_en.html

Comment 1 Mamoru TASAKA 2007-07-06 11:21:26 UTC
Well, you may want to check the spec files of other packages.
For example, there is scim spec:

Please check the packaging guidelines:

* SourceURL must be given with full URL.
* Don't rebuild in mock. At least, libhangul-devel is missing for
* Fedora specific compilation flags are not honored. Perhaps
  this can be resolved by using %configure macro.
* Please support parallel make when possible.
* "install-strip" is forbidden. This disables to create debuginfo
* Please keep timestamps. When using "install" command, use it
  with '-p' option.
* Use macros (e.g. /usr/share -> %{_datadir})
* Categories 'Application' 'X-Red-Hat-Base' are deprecated and
  should be removed
* desktop file must be installed by using desktop-file-install
  (BR: desktop-file-utils is needed)
* The binaries which are not under normal users' paths must be
  specified with full path.
* gettext .mo files must be registered in %files entry by using
  %find_lang macro.
* files under %_sysconfdir should be marked as %config(noreplace)
* We recommend %defattr(-,root,root,-)
* Please remove unused macros.

Comment 2 Mamoru TASAKA 2007-07-06 11:34:55 UTC
* %{_sbindir}/alternatives must be added to Requires(....)

Comment 3 Mamoru TASAKA 2007-07-12 05:27:16 UTC

Comment 4 subhransu behera 2007-07-12 09:38:58 UTC
Hi Mamoru,

Thanks for pointing out the probable mistakes. As nabi-0.18 is released by
upstream I am planning to build the srpm for the same.


Comment 5 Jens Petersen 2007-07-20 08:09:15 UTC
Ok, please update with the new .spec file and srpm url when it is ready.

Comment 6 Mamoru TASAKA 2007-07-23 12:52:05 UTC
By the way are there any information when upstream will
release new version?

Comment 7 Jens Petersen 2007-07-23 23:32:58 UTC
It was already released on the 8th, see:


Comment 8 Jens Petersen 2007-07-23 23:59:39 UTC
Subhransu, if you can update your submission fixing the issues raised so far
I am happy to review this.

Comment 9 subhransu behera 2007-07-24 07:15:15 UTC
I am waiting for newer version of libhangul (0.18) to be built.

Comment 10 Jens Petersen 2007-07-24 07:17:18 UTC
Right probably nabi-0.18 requires libhangul-0.0.6 and the current version
in fedora in 0.0.4.  I asked the owner to update it.

Comment 11 subhransu behera 2007-07-26 07:26:26 UTC
Upgraded to 0.18 release.

Spec URL: http://oriya.sourceforge.net/nabi.spec

SRPM URL: http://oriya.sourceforge.net/nabi-0.18-1.fc7.src.rpm

Comment 12 Jens Petersen 2007-07-30 07:18:17 UTC
Created attachment 160216 [details]

some more fixes - please check I didn't break anything. :)

Comment 13 Jens Petersen 2007-07-30 07:19:07 UTC
Created attachment 160217 [details]

Comment 14 Mamoru TASAKA 2007-07-30 14:06:42 UTC
(In reply to comment #11)
> Upgraded to 0.18 release.
> Spec URL: http://oriya.sourceforge.net/nabi.spec

(In reply to comment #12)
> Created an attachment (id=160216) [edit]
> nabi.spec-4.patch
> some more fixes - please check I didn't break anything. :)

(I only glanced at both spec files)
More cleanup:
* Please use full path for the commands which are not under
  normal users' paths.

* The following is smarter
desktop-file-install --dir=${RPM_BUILD_ROOT}%{_datadir}/applications/ \
! Note: desktop-file-install automatically fix the following,
        however Categories must be seperated by semicolon: i.e.

* What is the use of the line:

? Question
  Which package should own %_sysconfdir/X11/xinit/xinput.d ?

Comment 15 Jens Petersen 2007-08-06 05:58:01 UTC
(In reply to comment #14)
>   Which package should own %_sysconfdir/X11/xinit/xinput.d ?

I think it should be owned by im-chooser now.

Comment 16 Jens Petersen 2007-08-06 06:13:39 UTC
So actually it should be the nabi config file listed there not the directory.

Comment 17 subhransu behera 2007-08-06 07:57:23 UTC
Upgraded with required fixes.

Spec URL: http://oriya.sourceforge.net/0.18-3/nabi.spec

SRPM URL: http://oriya.sourceforge.net/0.18-3/nabi-0.18-3.fc7.src.rpm

Comment 18 Jens Petersen 2007-08-07 00:24:13 UTC
(In reply to comment #15)
> >   Which package should own %_sysconfdir/X11/xinit/xinput.d ?
> I think it should be owned by im-chooser now.

BTW Tagoh already kindly fixed this in bug 250960. :)

Comment 19 Jens Petersen 2007-08-07 02:52:34 UTC
Created attachment 160792 [details]

This should hopefully address the rest of the points raised by Tasaka-san.
Please check them and update package.

Comment 20 subhransu behera 2007-08-07 07:17:58 UTC
Package Updated.

Spec URL: http://oriya.sourceforge.net/0.18-4/nabi.spec

SRPM URL: http://oriya.sourceforge.net/0.18-4/nabi-0.18-4.fc7.src.rpm

Comment 21 Mamoru TASAKA 2007-08-07 11:52:20 UTC
* rpmlint
  - rpmlint complains about:
W: nabi invalid-license GPL
W: nabi rpm-buildroot-usage %prep rm -rf $RPM_BUILD_ROOT
    - License tag policy is updated so please change license
      tag accordingly.
      As far as I checked the source code, this is GPLv2+.
    - "rm -rf $RPM_BUILD_ROOT" is not needed for %prep

Comment 22 Mamoru TASAKA 2007-08-07 11:52:50 UTC
Other things seems okay for me.

Comment 23 Jens Petersen 2007-08-08 02:04:28 UTC
Thanks, Tasaka-san, for your careful checking: seems the new version of
rpmlint is more strict.

Ok, Subhransu, if you can do one more update I'd like to move into the final
formal next.

Comment 24 subhransu behera 2007-08-08 06:41:38 UTC
Package Updated.

Spec URL: http://oriya.sourceforge.net/0.18-5/nabi.spec

SRPM URL: http://oriya.sourceforge.net/0.18-5/nabi-0.18-5.fc7.src.rpm

Comment 25 Mamoru TASAKA 2007-08-08 16:19:34 UTC
Seems okay to me.

Comment 26 Jens Petersen 2007-08-09 03:59:59 UTC
Here is my review:

+ rpmlint clean
+ package follows naming and packaging guidelines
+ specifies GPL version and includes COPYING
+ spec file is legible
+ source is pristine
0d0fba8851a1ac367d7b52840e6bef3e  nabi-0.18.tar.gz
+ builds in mock and runs correctly
+ buildreqs listed
+ uses %find_lang
+ file and dir ownership looks correct
+ consistent macro usage
+ does not contain devel files
+ scriplets reasonable

Need attention:
- actually thinking more, I realised that the desktop is not needed
  since we don't normally run input methods from the desktop menu -
  they should be configured with im-chooser instead.
- minor, but I think ChangeLog can be dropped from %doc

Apart from that all MUST items from ReviewGuidelines are satisfied. :)

Comment 27 Jens Petersen 2007-08-09 04:03:05 UTC
Created attachment 160954 [details]

this patch fixes the above:

- remove unnecessary desktop file
- tidy doc filelist

Comment 28 Jens Petersen 2007-08-09 23:47:34 UTC
I contacted upstream about the questionable themes which we are excluding from
the package.  I hope they can be removed in a future release.

Comment 29 subhransu behera 2007-08-10 06:24:18 UTC
Package Updated.

Spec URL: http://oriya.sourceforge.net/0.18-6/nabi.spec

SRPM URL: http://oriya.sourceforge.net/0.18-6/nabi-0.18-6.fc8.src.rpm

Comment 30 Jens Petersen 2007-08-17 00:42:33 UTC
I suggest preparing a new tarball with the unappropriate images removed.
Upstream has agreed to remove them in the next release.

The new tarball should have a distinct name from the upstream one:
say something like nabi-0.18_fedora.tar.gz.

Probably need to patch some of the makefiles too for the removed themes.

Comment 31 Jens Petersen 2007-08-17 01:03:46 UTC
Created attachment 161708 [details]

A small shellscript does the theme removal.

Comment 32 Mamoru TASAKA 2007-08-24 09:51:00 UTC
Well, what is the status of this bug?

Comment 33 subhransu behera 2007-08-24 10:04:15 UTC
Hi Mamoru San,

We're waiting for upstream to remove the unappropriate images till the 
end of the this month.

Or else, we can go ahead as per as the suggestion of Jens.

Comment 34 Mamoru TASAKA 2007-08-24 10:41:42 UTC
Okay, thank you for imformation.

Comment 35 Jens Petersen 2007-09-04 00:21:56 UTC
Since it is taking a bit longer for the upstream release than expected let's
go ahead with just removing the questionable themes from the tarball as per

Comment 36 subhransu behera 2007-09-04 06:39:12 UTC
Package updated with version: 0.18-7

Spec URL: http://oriya.sourceforge.net/0.18-7/nabi.spec

SRPM URL: http://oriya.sourceforge.net/0.18-7/nabi-0.18-7.fc8.src.rpm

Comment 37 Jens Petersen 2007-09-05 07:12:07 UTC
Here is the source diff output for the record:
$ diff -ur nabi-0.18 nabi-0.18_fedora
Only in nabi-0.18/themes: MSWindows
Only in nabi-0.18/themes: MSWindowsXP
Only in nabi-0.18/themes: Mac

All issues have been addressed and the package looks good to me.

Thanks Subhransu for your patience and work on this.
I am happy to approve the package. :)

BTW note if you want to build the package for F7 then it will have to
be with the older release since f7 libhangul is still 0.0.4 which is not
new enough to build the latest nabi.


Comment 38 Jens Petersen 2007-09-05 07:42:36 UTC
It would help if you could do one so-called package "pre-review"
and I am happy to sponsor you for cvsextras. :-)

Comment 39 subhransu behera 2007-09-07 06:21:33 UTC
New Package CVS Request
Package Name: nabi
Short Description: Hangul X Input Method 
Owners: sbehera
Branches: F-7
InitialCC: petersen
Cvsextras Commits: Yes

Comment 40 Kevin Fenzi 2007-09-07 18:37:46 UTC
cvs done.

Comment 41 Parag AN(पराग) 2007-09-20 06:06:23 UTC
Closing this review as package already got built successfully on koji.

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