Bug 201337

Summary: Review Request: gcin - Chinese input method for Traditional Chinese
Product: [Fedora] Fedora Reporter: Chung-Yen Chang <candyz0416>
Component: Package ReviewAssignee: Jens Petersen <petersen>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: eng-i18n-bugs, i, j, mtasaka
Target Milestone: ---Flags: i: fedora-review+
gwync: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2006-08-24 14:04:12 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 163779    
Attachments:
Description Flags
build log of gcin-1.2.1-4 in mock
none
Log from failing x86_64 build of gcin-1.2.1-7 in mock
none
gcin.spec-3.patch
none
gcin-5.patch
none
gcin-7.patch
none
Build log of gcin-1.2.2-8 on fc6-devel with fedora undefined
none
gcin.spec-10.patch
none
gcin.spec-10.1.patch none

Description Chung-Yen Chang 2006-08-04 14:20:42 UTC
Spec URL: http://cle.linux.org.tw/candyz/gcin.spec
SRPM URL: http://cle.linux.org.tw/candyz/gcin-1.2.1-2.src.rpm
Description: gcin is a Chinese input method server for Traditional Chinese. It features
a better GTK user interface.
Also see http://cle.linux.org.tw/gcin/

Comment 1 Mamoru TASAKA 2006-08-14 08:11:32 UTC
Hello.

=======================================================
I cannot sponsor you formally because I do not have a member
of sponsors. You may ask someone to review you on fedora-extras
mailing list, for example. 
=======================================================

However, I do a kind of pre-review of your package. 
Note: my review cannot be a formal review.

First, please read http://fedoraproject.org/wiki/Packaging/Guidelines .
Then:

* rpmlint is not silent. Fix below unless you have a opinion not
  to do so.

gcin-1.2.1-2.i386.rpm:
W: gcin summary-not-capitalized gcin - Chinese input method server
W: gcin no-version-in-last-changelog
W: gcin no-soname /usr/lib/libgcin-im-client.so
E: gcin script-without-shellbang /etc/X11/xinit/xinput.d/gcin
W: gcin devel-file-in-non-devel-package /usr/include/gcin-im-client.h

gcin-1.2.1-2.src.rpm:
W: gcin summary-not-capitalized gcin - Chinese input method server
E: gcin unknown-key GPG#476a8659
W: gcin hardcoded-packager-tag Chung-Yen
W: gcin setup-not-quiet
E: gcin use-of-RPM_SOURCE_DIR
W: gcin mixed-use-of-spaces-and-tabs

gcin-debuginfo-1.2.1-2.i386.rpm:
W: gcin-debuginfo no-version-in-last-changelog

* This package doesn't be rebuilt in mock. Check the missing BuildRequires.
  In mock, this build of this package shows:

gmake[2]: Entering directory `/builddir/build/BUILD/gcin-1.2.1'
gcc  -o gcin gcin.o eve.o win0.o pho.o tsin.o win1.o util.o pho-util.o
gcin-conf.o tsin-util.o win-sym.o intcode.o pho-sy
m.o win-int.o win-pho.o gcin-settings.o table-update.o win-gtab.o gtab.o
gtab-util.o phrase.o win-inmd-switch.o pho-dbg.o
 locale.o win-pho-near.o gcin-switch.o win-status.o IC.o IMdkit/lib/libXimd.a
im-srv/im-srv.a -lXtst -L/lib -lgtk-x11-2.0
 -lgdk-x11-2.0 -latk-1.0 -lgdk_pixbuf-2.0 -lm -lpangocairo-1.0 -lpango-1.0
-lcairo -lgobject-2.0 -lgmodule-2.0 -ldl -lgli
b-2.0   -L/usr/X11R6/lib
/usr/bin/ld: cannot find -lXtst
collect2: ld returned 1 exit status
gmake[2]: *** [gcin] Error 1

* Commands needed for %post and so on which are in the paths normal 
  users don't use (such as /usr/sbin) must be used with full path.

* Why does this package have the definition of %prefix or %Docdir ?
  I strongly suspect this is not necessary. Especially, unless you have a
  reason that this package can be relocatable, removing %prefix is 
  necessary.

* Check files which this package should have. I suspect that this
  package should own %{_datadir}/gcin/ .

* Check %Require. Is a explicit requirement of gtk2 is necessary?

* Don't use Packager. Fedora buildsys overrides it.

* The format of BuildRoot is not preferred. Especially, why use
  %{_builddir}, not %{_tmppath}?

* Perhaps the description of Group is not proper.

* Using of %makeinstall is now discouraged.

* debuginfo rpm includes no files. Perhaps binaries in this package
  is stripped in build or install stages, which is incorrect and needs
  fixing.

* Use macro, instead of using /etc and so on.

* Don't use $RPM_SOURCE_DIR, use %SOURCE?? so that we can check where
  the sources is used easier.

* Specify the location of %Source0 by URL, not by only the filename.

Note: I didn't check this package by using this. I only checked
      packaging issue. 

Comment 2 Mamoru TASAKA 2006-08-14 08:30:14 UTC
Oops!!

debuginfo includes some files, however, apparantly it is not sufficient.
Anyway, check where binaries are stripped in build or install stages.

Comment 3 Chung-Yen Chang 2006-08-16 04:26:14 UTC
I have fixed some rpmlint problems.
The new
Spec URL: http://cle.linux.org.tw/candyz/gcin.spec
SRPM URL: http://cle.linux.org.tw/candyz/gcin-1.2.1-3.src.rpm

I still have problems, but I don't know how to solve it.

E: gcin script-without-shellbang /etc/X11/xinit/xinput.d/gcin
This executable text file does not contain a shebang, thus it cannot be
properly executed.  Often this is a sign of spurious executable bits for a
non-script file, but can also be a case of a missing shebang.  To fix this
error, find out which case of the above it is, and either remove the
executable bits or add the shebang.

W: gcin-devel no-soname /usr/lib/libgcin-im-client.so


Comment 4 Mamoru TASAKA 2006-08-16 08:17:26 UTC
Hello.

--------------------------------------------------------
Again I have to mention that I cannot sponsor you formally
because I am not a member of sponsors. 

You may ask on fedora-extras mailing lists for someone to 
sponsor you.
--------------------------------------------------------

I see lots of rpmlint complaints were removed.
mock builds cleanly.

Then:

* rpmlint:
gcin-1.2.1-3.i386.rpm:
E: gcin script-without-shellbang /etc/X11/xinit/xinput.d/gcin
--- I suspect the permission of /etc/X11/xinit/xinput.d/gcin
    can be 0644 (see /etc/X11/xinit/xinput.d/scim.conf in scim).

gcin-1.2.1-3.src.rpm:
W: gcin strange-permission gcin 0755
--- change the permission of gcin (source 1) to 0644

gcin-debuginfo-1.2.1-3.i386.rpm:

gcin-devel-1.2.1-3.i386.rpm:
W: gcin-devel no-soname /usr/lib/libgcin-im-client.so
--- This is unwilling, however, this is a problem of this
    package, not your packaging. You may tell upstream that
    a proper soname should be added or you can add a soname
    by yourself for a moment.
    By the way, can this file be in devel package, not in
    main package?  (I don't know well about gcin......)

* debuginfo rpm is of no use.
This is because this package....

A. does not accept CFLAGS. i.e. This package is not built with
   debug option "-g". Overwrite OPTFLAGS like
-----------------------------------------------------------------
in %build stage:
   make OPTFLAGS="${RPM_OPT_FLAGS}" %{?_smp_mflags}
-----------------------------------------------------------------

B. This package strips binaries, which is not accepted for fedora
   packages. Fix makefile not to strip binaries, like
-----------------------------------------------------------------
in %prep stage:
   sed -i.strip -e 's|install[ \t][ \t]*-s|install|' Makefile
-----------------------------------------------------------------

With fix by A and B: rpmlint complains:
W: gcin non-conffile-in-etc /etc/X11/xinit/xinput.d/gcin
   -- can be ignored, I think

W: gcin-devel no-soname /usr/lib/libgcin-im-client.so
   -- you may ask upstream to fix this.

E: gcin-devel shlib-with-non-pic-code /usr/lib/libgcin-im-client.so
   -- I don't know well about shared libraries, however, this
      error means:

The listed shared libraries contain object code that was compiled
without -fPIC. All object code in shared libraries should be
recompiled separately from the static libraries with the -fPIC option.

Another common mistake that causes this problem is linking with
``gcc -Wl,-shared'' instead of ``gcc -shared''.

    --- libgcin-im-client.so is made by gcin-im-client.o im-addr.o gcin-conf.o 
        util.o gcin-crypt.o and actually gcin-crypt.o is not compiled with
        -fpic. gcin-crypt.o is used to make another binary and in this case,
        gcin-crypt.o should be compiled without -fpic. So I think gcin-crypt.c
        must be compiled twice with different ways. Fix this.
        For example:
-----------------------------------------------------------------
--- gcin-1.2.1/im-client/Makefile.orig  2006-05-01 18:57:43.000000000 +0900
+++ gcin-1.2.1/im-client/Makefile       2006-08-16 16:55:01.000000000 +0900
@@ -6,7 +6,7 @@
         -DCLIENT_LIB=1 -DGCIN_BIN_DIR=\"$(GCIN_BIN_DIR)\" \
         -DDEBUG="0$(GCIN_DEBUG)" -DGCIN_TABLE_DIR=\"$(GCIN_TABLE_DIR)\" \
         -DFREEBSD=$(FREEBSD)
-OBJS = gcin-im-client.o im-addr.o gcin-conf.o util.o gcin-crypt.o
+OBJS = gcin-im-client.o im-addr.o gcin-conf.o util.o gcin-crypt-fpic.o
 
 .c.E:
        $(CC) $(CFLAGS) -E -o $@ $<
@@ -43,6 +43,9 @@
 im-addr.o: ../im-srv/im-addr.c
        $(CC) -c -fpic $(CFLAGS) -o $@ $<
 
+gcin-crypt-fpic.o: gcin-crypt.c
+       $(CC) -c -fpic $(CFLAGS) -o $@ $<
+
 clean:
        rm -f *.o *.so *~ *.E *.db config.mak tags core.* .depend
 
-------------------------------------------------------------------

E: gcin-debuginfo script-without-shellbang
/usr/src/debug/gcin-1.2.1/IMdkit/include/IMdkit.h
E: gcin-debuginfo script-without-shellbang
/usr/src/debug/gcin-1.2.1/IMdkit/include/Xi18n.h
E: gcin-debuginfo script-without-shellbang
/usr/src/debug/gcin-1.2.1/IMdkit/include/XimProto.h
   --- fix this with:
-------------------------------------------------------------------
in %prep stage:
       find . -name \*.h -o -name \*.c | xargs chmod ugo-x 
-------------------------------------------------------------------

* %post and %postun
Require something to execute %post (and %postun) properly.
e.g. Requires(post): /usr/sbin/alternatives, chkconfig

* Requires:
Requires: gtk2 >= 2.2.4 atk cairo glib2 pango
All of these are not needed because gcin requires
libgtk-x11-2.0.so.0 libatk-1.0.so.0  libglib-2.0.so.0 libpango-1.0.so.0 and
these libraries require the package above. So explicit requirements of
those above are unnecessary.

* Again:
  Check files which this package should have. I suspect that this
  package should own %{_datadir}/gcin/
  The file entry contains:
-------------------------------------------------------------------
%{_datadir}/gcin/*
-------------------------------------------------------------------
  I think that this must be
-------------------------------------------------------------------
%{_datadir}/gcin/
-------------------------------------------------------------------
  , which includes the files under /usr/share/gcin AND the directory
/usr/share/gcin .



Comment 5 Mamoru TASAKA 2006-08-16 08:34:03 UTC
Other issues I noticed now:

* /usr/lib/menu/ : this directory is not owned by other packages.
  So this package should own this directory.
* gcin (main) package requires libgcin-im-client.so, so this library
  should be in main package, not in devel.

Comment 6 Chung-Yen Chang 2006-08-16 15:03:15 UTC
I have fixed the rpmlint problems.
New files:
Spec URL: http://cle.linux.org.tw/candyz/gcin.spec
SRPM URL: http://cle.linux.org.tw/candyz/gcin-1.2.1-4.src.rpm

Mamoru Tasaka thanks a lot.

Comment 7 Mamoru TASAKA 2006-08-16 17:01:49 UTC
Created attachment 134326 [details]
build log of gcin-1.2.1-4 in mock

--------------------------------------------------------
Again I have to mention that I cannot sponsor you formally
because I am not a member of sponsors. 

You may ask on fedora-extras mailing lists for someone to 
sponsor you.
--------------------------------------------------------

Well, 
* rpmlint is clean. 
* debuginfo rpm is created correctly.
I think now you may well ask for someone
to sponsor you because I cannot sponsor you and I cannot approve
your package.

2 points:
* I cannot check this other than on i386 machine (because
  I only have i386 machine), however,
  on x86_64, libraries may be installed on /usr/lib64. You may
  have to configure gcin-i386.conf and also change its name
  according to architecture (for example, by using %{_arch} ).
  For example, qt-3.3.6-12.x86_64 has qt-x86_64.conf with entry
  "/usr/lib64/qt-3.3/lib".
* I cannot build this in mock (local build okay).
  Perhaps qt-devel or so is missing for BR? Please check my build log
  attached.

Again, I only tested packaging issues.

Comment 8 Mamoru TASAKA 2006-08-16 17:30:03 UTC
Well, qt-devel seems sufficient for missing buildrequires.

Comment 9 Mamoru TASAKA 2006-08-16 17:40:08 UTC
Oops!!

* debuginfo rpm is created correctly.

Well, no. It is not. debuginfo contains no debug information.
Build log shows:

gcc -Wall -O -I/usr/include/gtk-2.0 ......

This means that codes are compiled with no debug option (-g).
You must change the optional flag of compilation so that gcc
is called with $RPM_OPT_FLAGS .
You can see from my build log that $RPM_OPT_FLAGS are not used
in compilation.

Comment 10 Chung-Yen Chang 2006-08-16 23:20:59 UTC
I fixed all above problems.
New files:
Spec URL: http://cle.linux.org.tw/candyz/gcin.spec
SRPM URL: http://cle.linux.org.tw/candyz/gcin-1.2.1-5.src.rpm

I only have i386 machine (no x86_64 machine), Tasaka can you help me to test it?
If all is right, then I may start to ask for someone to sponsor me.


Comment 11 John Mahowald 2006-08-17 02:43:26 UTC
Build failed on devel x86_64.

RPM build errors:
    File not found by glob:
/var/tmp/gcin-1.2.1-5-root-mockbuild/usr/lib64/menu/gcin*
    File not found:
/var/tmp/gcin-1.2.1-5-root-mockbuild/usr/lib64/gtk-2.0/immodules/im-gcin.so
    File not found:
/var/tmp/gcin-1.2.1-5-root-mockbuild/usr/lib64/qt-3.3/plugins/inputmethods/libqgcin.so
    File not found:
/var/tmp/gcin-1.2.1-5-root-mockbuild/usr/lib64/gcin/libgcin-im-client.so


As you might guess this is not getting libdir set correctly. From looking at the
custom configure script I believe you can just do  %configure libdir=%{_libdir}
 in your spec.

Comment 12 Chung-Yen Chang 2006-08-17 03:30:59 UTC
I update it for lib64 problems.
New files:
Spec URL: http://cle.linux.org.tw/candyz/gcin.spec
SRPM URL: http://cle.linux.org.tw/candyz/gcin-1.2.1-6.src.rpm

John Mahowald can you help me to build  it on x86_64 again and see if it still failure?

Comment 13 Jason Tibbitts 2006-08-17 03:41:21 UTC
%configure should set libdir automatically (in this case, the macro includes
"--libdir=/usr/lib64"), but some things still ignore it.

In any case, I grabbed and build -6 and now the build doesn't complete.  It gets
down to the qt bit and fails:

gmake[3]: Entering directory `/builddir/build/BUILD/gcin-1.2.1/qt-im'
/usr/lib/qt-3.3/bin/moc qgcininputcontextplugin.h -o
qgcininputcontextplugin.h_moc.cpp
gmake[3]: /usr/lib/qt-3.3/bin/moc: Command not found
gmake[3]: *** [qgcininputcontextplugin.o] Error 127

I've no idea what's happening.

Comment 14 Chung-Yen Chang 2006-08-17 04:46:11 UTC
There are many problems about lib64 in the source code, I try my test to make a patch for it.
Hope this time can fix all the problems.
Please help me to build it on x86_64 machine.

New files:
Spec URL: http://cle.linux.org.tw/candyz/gcin.spec
SRPM URL: http://cle.linux.org.tw/candyz/gcin-1.2.1-7.src.rpm

Also, I will report the issues to the upstream, too.

Comment 15 Mamoru TASAKA 2006-08-17 07:31:57 UTC
(In reply to comment #14)

> Spec URL: http://cle.linux.org.tw/candyz/gcin.spec
> SRPM URL: http://cle.linux.org.tw/candyz/gcin-1.2.1-7.src.rpm

These seem OK for i386. Build is fine, rpmlint is clean,
debuginfo is correct (this time it is really okay)
and these seems to work well on i386.

However, I only have i386 machine so I cannot check this on x86_64.


Comment 16 Jason Tibbitts 2006-08-17 16:35:48 UTC
The -7 package still fails for me on x86_64:

RPM build errors:
    File not found by glob:
/var/tmp/gcin-1.2.1-7-root-mockbuild/usr/lib64/menu/gcin*
    File not found:
/var/tmp/gcin-1.2.1-7-root-mockbuild/usr/lib64/gtk-2.0/immodules/im-gcin.so
    File not found:
/var/tmp/gcin-1.2.1-7-root-mockbuild/usr/lib64/qt-3.3/plugins/inputmethods/libqgcin.so
    File not found:
/var/tmp/gcin-1.2.1-7-root-mockbuild/usr/lib64/gcin/libgcin-im-client.so
Error building package from gcin-1.2.1-7.src.rpm, See build log


Comment 17 Mamoru TASAKA 2006-08-17 16:52:06 UTC
(In reply to comment #16)
> The -7 package still fails for me on x86_64:

Dear Jason;
What does your full build log on x86_64 say?
Perhaps a full build log will be useful.
I cannot very this package other than on i386.

Comment 18 Jason Tibbitts 2006-08-17 17:31:27 UTC
Created attachment 134397 [details]
Log from failing x86_64 build of gcin-1.2.1-7 in mock

Comment 19 Chung-Yen Chang 2006-08-18 00:13:21 UTC
So bad, too many problems about x86_64.
The upstream author and I only have i386 machine, so we can not test and debug it on x86_64.
Anyway, I will see the mock build log first then try to fix it.

Comment 20 Chung-Yen Chang 2006-08-18 01:12:20 UTC
Again, the new file:
Spec URL: http://cle.linux.org.tw/candyz/gcin.spec
SRPM URL: http://cle.linux.org.tw/candyz/gcin-1.2.2-2.src.rpm

i386 mock builds cleanly
Hope the x86_64 problems will be fixed.


Comment 21 Chung-Yen Chang 2006-08-18 01:55:45 UTC
I use mock to build i386 and x86_64 and ppc version for FC5, and all those builds are cleanly.
Also, I use mock to build it for FC4, and the builds are cleanly, too.
But I use my i386 machine to build all those, so I don't know will it builds cleanly on x86_64 machines.

Comment 22 Jason Tibbitts 2006-08-18 04:22:18 UTC
1.2.2-2 does indeed build on x86_64 and rpmlint is quiet to boot.

Note that I haven't signed on to review this package, but since I have the build
logs in front of me I'll check a few things.

You don't use the %{?dist} tag.  It's not required but it does make maintenance
across multiple releases much easier and you should probably use it unless you
know that you don't want to.  See
http://fedoraproject.org/wiki/Packaging/DistTag for more information.

You must include the COPYING file in %doc.  You should also consider including
ChangeLog, and putting README in the main package instead of the -devel
subpackage if it includes any end-user documentation.  (I am not capable of
reading it so I can't tell.

The configure script isn't a regular configure script, so I'm not sure it's
productive to call it with the %configure macro, which will call configure with
many flags that it doesn't seem to understand.

It looks like the Makefile ignores CFLAGS, so the compiler is not called with
the proper set of flags.  I think you will need to patch the makefile, or
perhaps the generated config.mak.  I added the following hack to the end of %prep:

perl -pi -e "s/^(OPTFLAGS.*=)/\1 %{optflags} /" config.mak

and it does get the right flags passed to the compiler, but it also causes the
build to fail on my 8CPU machine unless I disable parallel make.  I have no idea
why.  I have a suspicion it has something to do with the "-pipe" option.




Comment 23 Chung-Yen Chang 2006-08-18 05:14:59 UTC
I add the COPYING and Changlog to %dco and remove README from -devel subpackage.
I also remove the configure script. (It is no use)
I add "perl -pi -e "s/^(OPTFLAGS.*=)/\1 %{optflags} /" config.mak" after %configure, then I build fail on my 
SMP machine, too.
I disable parallel make then it build success.
Now, I am reading the DistTag Document, and I will try to use the %{?dist} tag.

Comment 24 Chung-Yen Chang 2006-08-18 06:43:04 UTC
Add COPYING Changelog to %doc
Use Dist Tag

The New files:
Spec URL: http://cle.linux.org.tw/candyz/gcin.spec
SRPM URL: http://cle.linux.org.tw/candyz/gcin-1.2.2-3.src.rpm

The mock build cleanly on my machine.

Comment 25 Jens Petersen 2006-08-19 07:56:19 UTC
Thanks for all your work on this.  I just tested it on my x86_64
and it seems to work ok (though I'm not very familiar with TC).

A couple of comments:

Is the -devel subpackage really necessary?  Are there any extensions to gcin
that would use the include file it contains?

Probably it would be clearer to rename the xinput file from gcin to
xinput-gcin say.

I will attach a patch for the spec file to improve it further.

Comment 26 Jens Petersen 2006-08-19 08:03:31 UTC
Created attachment 134501 [details]
gcin.spec-3.patch

More necessary improvements.

- don't use configure macro
- add .conf suffix to xinput.d file and update install scripts for fc6
- move lib to libdir and drop ld.so.conf.d file
- other minor cleanup

Comment 27 Jens Petersen 2006-08-19 08:35:04 UTC
[I forgot to thanks all the earlier reviewers for their work on this too.:)]

Also you buildrequire desktop-file-utils but don't use it I think.
You probably should to install the .desktop file with it or else remove it entirely.
See <http://fedoraproject.org/wiki/Packaging/Guidelines#desktop>.

Once all these things are fixed I think the package can be accepted.

Comment 28 Jens Petersen 2006-08-19 08:40:00 UTC
Note the changes I made in %post/%preun are only for fc6.
Since like you had is needed for fc5 and earlier btw.
It is hope to you how you want to handle that: either using
%{fedora} and different .spec files on branches say.

Comment 29 Chung-Yen Chang 2006-08-19 09:19:46 UTC
Petersen, thanks for your spec patch
These days I have read all http://fedoraproject.org/wiki/Packaging/* documents.
I merge your spec patch and create a new version, and mock build cleanly for fc6 and fc5.

The New files:
Spec URL: http://cle.linux.org.tw/candyz/gcin.spec
SRPM URL: http://cle.linux.org.tw/candyz/gcin-1.2.2-5.src.rpm

Comment 30 Jens Petersen 2006-08-19 10:46:08 UTC
Thanks.

What do you think about removing the -devel package?

You still need to install the desktop file in %install:
check some other spec files for examples, and then you can 
restore the desktop-file-utils buildrequires.


Comment 31 Jens Petersen 2006-08-19 10:47:50 UTC
Created attachment 134505 [details]
gcin-5.patch

a few more fixes

Comment 32 Chung-Yen Chang 2006-08-19 11:10:28 UTC
I had email to the upstream author and ask him about the header file is necessary?
I am waiting for his reply.
If the header file is not necessay, then the -devel package can be removed.
About desktop file:
"make install" already install "%{_datadir}/applications/gcin-setup.desktop"
What should I do?
Remove gcin-setup.desktop and then use "desktop-file-install" in %install?
Another question, should I use gcin.desktop rather than gcin-setup.desktop?
Thanks.

Comment 33 Chung-Yen Chang 2006-08-19 13:20:46 UTC
I fixed with the gcin-5.patch and use desktop-file-install in %install (ref. from scim's spec file) 

The New files:
Spec URL: http://cle.linux.org.tw/candyz/gcin.spec
SRPM URL: http://cle.linux.org.tw/candyz/gcin-1.2.2-6.src.rpm

Comment 34 Jason Tibbitts 2006-08-19 17:12:33 UTC
Jens, while it's great that you're reviewing this (as I don't really have the
expertise to review it properly) the fact remains that because this is a
FE-NEEDSPONSOR ticket, and according to the current rules for contributorship
the review must be done by a sponsor.  This does sort of put us in a bit of a
quandary, however, as there might not be an available sponsor.  Might you be
willing to co-maintain this package?  (Assuming of course that Chang would agree.)

Comment 35 Chung-Yen Chang 2006-08-20 02:44:45 UTC
I agree.
Jens will you like to co-maintain this package?
And Tibbitts can you sponsor this package?

Another information about -devel subpackage, it seems the gcin-im-client.h only need for rxvt/mrxvt.
If the rxvt/mrxvt was patched with gcin IM client support, then it is need gcin-im-client.h to build rxvt/
mrxvt.
http://www.csie.nctu.edu.tw/~cp76/linux/mrxvt-gcin-0.4.1.tar.bz2 is a mrxvt with gcin IM client 
patched
http://www.csie.nctu.edu.tw/~cp76/linux/rxvt-gcin.tbz is a rxvt with gcin IM client patched
But both mrxvt-gcin-0.4.1.tar.bz2 and rxvt-gcin.tbz are a little bit out-of-date, so I think the -devel 
subpackage is not necessary now.
So now I remove the -devel subpackage.
The New files with no -devel subpackage:
Spec URL: http://cle.linux.org.tw/candyz/gcin.spec
SRPM URL: http://cle.linux.org.tw/candyz/gcin-1.2.2-7.src.rpm

Comment 36 Jens Petersen 2006-08-20 03:18:00 UTC
Created attachment 134521 [details]
gcin-7.patch

Fix changelog. ;)

Otherwise looks good to me.  IMHO the package could now be approved,
but unfortunately as Jason pointed out I don't have Sponsor status either...

I don't really feel qualified or motivated to maintain this package
in the long term, but if there is a strong demand to have it in Extras
asap then I suppose I could take it for now.  Though I'd really rather
we try to find a sponsor for Chung-Yen Chang.

Comment 37 Chung-Yen Chang 2006-08-20 03:30:08 UTC
Fix changelog:
Spec URL: http://cle.linux.org.tw/candyz/gcin.spec
SRPM URL: http://cle.linux.org.tw/candyz/gcin-1.2.2-8.src.rpm

Now I still need to find a sponsor for this package.

Comment 38 Jens Petersen 2006-08-20 04:51:41 UTC
Chung-Yen Chang, have you done any pre-reviews of any other packages?

Comment 39 Chung-Yen Chang 2006-08-20 05:02:35 UTC
Jens, no, I am new to FE bugzilla.
But I will try to do some pre-reviews of other packages.

Comment 40 Jens Petersen 2006-08-22 12:44:52 UTC
Thanks, Chung-Yen Chang, for doing a pre-review of devilspie (bug 203288).
I think I am able to sponsor you to become an Fedora Extras contributor.

Comment 41 Mamoru TASAKA 2006-08-23 14:00:05 UTC
Created attachment 134711 [details]
Build log of gcin-1.2.2-8 on fc6-devel with fedora undefined

Hello.

So I cannot sponsor formally for this package (because
I am not a member), who will be the sponsor for this package?
I think that this package leaves "little" problem, so
it would be better that this package can be released ASAP.

The reason I mentioned "little" is because 
* I can rebuild this package with mock.
* however, I canNOT rebuild this package without mock even
  with proper BR rpms installed
because usually %{fedora} is undefined and so Patch5 is applied
when rebuilt withOUT mock, this is not right for FC5 and above.

Now, FC4 and below FC4 are marked as regacy, so this package
can be released only for FE5 and FE6-devel. So, Patch5 is
not needed, perhaps?

Comment 42 Paul Howarth 2006-08-23 14:26:07 UTC
(In reply to comment #41)
> Created an attachment (id=134711) [edit]
> Build log of gcin-1.2.2-8 on fc6-devel with fedora undefined
> 
> Hello.
> 
> So I cannot sponsor formally for this package (because
> I am not a member), who will be the sponsor for this package?

It is the submitter that is sponsored, not the package.

> I think that this package leaves "little" problem, so
> it would be better that this package can be released ASAP.
> 
> The reason I mentioned "little" is because 
> * I can rebuild this package with mock.
> * however, I canNOT rebuild this package without mock even
>   with proper BR rpms installed
> because usually %{fedora} is undefined and so Patch5 is applied
> when rebuilt withOUT mock, this is not right for FC5 and above.
> 
> Now, FC4 and below FC4 are marked as regacy, so this package
> can be released only for FE5 and FE6-devel. So, Patch5 is
> not needed, perhaps?

You can define the macro manyually if you wish:

$ rpmbuild -ba --define 'fedora 5' --define 'dist .fc5' packagename.spec

I think it would be better though if the default (i.e. with the fedora macro
undefined) was appropriate for the current release (FC5/FC6) and the extra
defines were only needed for the legacy distros,

Comment 43 Mamoru TASAKA 2006-08-23 14:42:36 UTC
(In reply to comment #42)

> It is the submitter that is sponsored, not the package.
Oh, yes. What I meant was who will sponsor Chung-Yen Chang?

> I think it would be better though if the default (i.e. with the fedora macro
> undefined) was appropriate for the current release (FC5/FC6) and the extra
> defines were only needed for the legacy distros,

I have the same opinion. It would be better that the spec file
assumes that the distro is FC-5 or above if %fedora is not defined.


Comment 44 Chung-Yen Chang 2006-08-24 00:57:03 UTC
Tasaka thanks for you review and test.
This is mistake for patch5, but now I think patch5 is not necessary, so I remove it.
Both Howarth's and Tasaka's suggestion are very good.
So I think set fc5/fc6 to default in spec is more appropriate, I will keep in mind.

The new files:
Spec URL: http://cle.linux.org.tw/candyz/gcin.spec
SRPM URL: http://cle.linux.org.tw/candyz/gcin-1.2.2-10.src.rpm

I use mock build for fc6/fc5 are cleanly.

Comment 45 Jens Petersen 2006-08-24 02:07:48 UTC
Created attachment 134765 [details]
gcin.spec-10.patch

If the package is only for fc5 and later, then I think it is better to
just simplify it like this.  (There is no problem to use gcin.conf on
fc4 and earlier afaics.)

As I tried to say in comment 40, I will sponsor Chung-Yen Chang.

Comment 46 Chung-Yen Chang 2006-08-24 02:23:27 UTC
Hi, Petersen,
I try to merge it to spec file (for fc3 fc4 fc5 fc6 and rhel4).
But, after all, I think it is better to simplify it like your patch.
So maybe it is better form me to seperate the spec file for "fc5 and later" and "fc4 and earlier and 
rhel4".

And another question, I know use gcon.conf on fc4 and earlier is no problem, but I think it is better for 
use gcin then gcin.conf on fc4 and earlier.
(It look a little stranger if use gcin.conf)

[candyz:~] im-switch -z zh_TW -l
xinput-zh_TW - status is manual.
 link currently points to gcin
scim - priority 81
oxim - priority 30
gcin.conf - priority 40
Current `best' version is scim.
=======================================================
The following languages currently have input methods configured:
as_IN bn_IN en_US gu_IN hi_IN ja_JP kn_IN ko_KR ml_IN ne_NE or_IN pa_IN si_LK ta_IN te_IN th_TH vi_VN 
zh_CN zh_HK zh_SG zh_TW


Comment 47 Jens Petersen 2006-08-24 03:05:18 UTC
Created attachment 134766 [details]
gcin.spec-10.1.patch

Okay, you're right - I was being too hasty.  Here is a better patch,
which should handle %fedora correctly.

Comment 48 Chung-Yen Chang 2006-08-24 03:16:40 UTC
I merge the gcin.spec-10.1.patch
New files:
Spec URL: http://cle.linux.org.tw/candyz/gcin.spec
SRPM URL: http://cle.linux.org.tw/candyz/gcin-1.2.2-11.src.rpm

Petersen, thanks a lot.

Comment 49 Jens Petersen 2006-08-24 03:19:51 UTC
But sorry, the last patch still doesn't help when %fedora is not defined.
So yes, it is probably easiest just to branch the spec file for fc5 as I originally
thought too.

Comment 50 Chung-Yen Chang 2006-08-24 03:48:58 UTC
Now I branch the spec file.

New files:
Spec URL: http://cle.linux.org.tw/candyz/gcin.spec
SRPM URL: http://cle.linux.org.tw/candyz/gcin-1.2.2-12.src.rpm

Spec for fc4 and earlier URL: http://cle.linux.org.tw/candyz/gcin.spec-branch

Comment 51 Jens Petersen 2006-08-24 04:08:32 UTC
srpm looks good to me and gcin.spec-branch.

(I think you forgot to update gcin.spec.:)

Comment 52 Chung-Yen Chang 2006-08-24 04:13:54 UTC
Sorry, I forgot to update gcin.spec.
Now the gcin.spec updated. :P

Comment 53 Mamoru TASAKA 2006-08-24 05:06:22 UTC
Well, srpm and spec seems well for me. too.
(I only tested for  http://cle.linux.org.tw/candyz/gcin.spec ).

Comment 54 Jens Petersen 2006-08-24 06:12:40 UTC
Package is APPROVED.

When you are ready please follow the instructions carefully
to import into extras cvs.

Thanks for contributing gcin to Fedora Extras.

Comment 55 Chung-Yen Chang 2007-09-12 06:09:01 UTC
Package Change Request
======================
Package Name: gcin
New Branches: EL-4 EL-5

Comment 56 Kevin Fenzi 2007-09-12 17:49:16 UTC
cvs done.

Comment 57 Christopher Meng 2014-04-23 08:54:42 UTC
Package Change Request
======================
Package Name: gcin
New Branches: epel7
Owners: cicku

Comment 58 Gwyn Ciesla 2014-04-24 12:39:52 UTC
Git done (by process-git-requests).