Bug 606714 - Review Request: eekboard - A Virtual Keyboard for GNOME
Summary: Review Request: eekboard - A Virtual Keyboard for GNOME
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Mamoru TASAKA
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2010-06-22 09:55 UTC by Daiki Ueno
Modified: 2010-07-30 08:46 UTC (History)
7 users (show)

Fixed In Version: eekboard-0.0.4-2.fc13
Clone Of:
Environment:
Last Closed: 2010-07-02 06:53:53 UTC
Type: ---
Embargoed:
mtasaka: fedora-review+
j: fedora-cvs+


Attachments (Terms of Use)
screenshot of eekboard (163.12 KB, image/png)
2010-06-28 16:22 UTC, Mamoru TASAKA
no flags Details

Description Daiki Ueno 2010-06-22 09:55:06 UTC
Spec URL: http://ueno.fedorapeople.org/eekboard/eekboard.spec
SRPM URL: http://ueno.fedorapeople.org/eekboard/eekboard-0.0.2-1.fc13.src.rpm
Description:
eekboard is a virtual keyboard software package which ships with a
standalone virtual keyboard application (eekboard), and a library to
create keyboard-like UI (libeek).

Comment 1 Chen Lei 2010-06-22 14:54:40 UTC
One suggestion:

It'll be better to merge libxxx subpackages to a -libs subpackage, libxxx subpackage looks like the debian style.

Comment 2 Daiki Ueno 2010-06-23 01:14:49 UTC
Thanks.  Fixed in:
Spec URL: http://ueno.fedorapeople.org/eekboard/eekboard.spec
SRPM URL: http://ueno.fedorapeople.org/eekboard/eekboard-0.0.2-2.fc13.src.rpm

I also reduced the number of subpackages into 4:

- eekboard
- eekboard-libs
- eekboard-devel
- eekboard-devel-docs

Previously I subpackaged them by library dependencies (gtk, clutter, xkl, etc).  Now I think this rough splitting is better because the only application "eekboard" requires all of them.

Comment 4 Mamoru TASAKA 2010-06-26 09:14:42 UTC
Some notes:

* BuildRoot
  - BuildRoot tag is no longer used (even if rpmlint may complain if you
    remove this)
    https://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag

  ! By the way for this package %clean is also unneeded because this
    package won't build on F-12 (c.f.:
    http://koji.fedoraproject.org/koji/taskinfo?taskID=2273814 :
    higher gtk seems needed )
    and %clean is not needed for F-13+:
    https://fedoraproject.org/wiki/Packaging/Guidelines#.25clean

* BR
  - Please try to remove redundant BRs
    - "glib2-devel" is always required by "gtk2-devel"
    - You probably need gobject-introspection"-devel" (not just gobject-introspection)
      ! Note that gobject-introspection-devel is required by clutter-devel
        (and so clutter-gtk-devel)

* build issues
  - The spec file contains:
-----------------------------------------------------
    62  # Prevent the upstream configure script from using clutter-gtk-0.90,
    63  # which is not available in Fedora yet.
    64  export CLUTTER_GTK_CFLAGS=`pkg-config clutter-gtk-0.10 --cflags`
    65  export CLUTTER_GTK_LIBS=`pkg-config clutter-gtk-0.10 --libs`
-----------------------------------------------------
    Why is this needed while configure.ac contains the below?
-----------------------------------------------------
    72    PKG_CHECK_MODULES([CLUTTER_GTK], [clutter-gtk-0.90],
    73      [enable_clutter_gtk=yes])
    74    if test x$enable_clutter_gtk = xno; then
    75      PKG_CHECK_MODULES([CLUTTER_GTK], [clutter-gtk-0.10],
    76        [enable_clutter_gtk=yes])
    77    fi
-----------------------------------------------------
  - Your srpm fails to build on F-13 as:
    http://koji.fedoraproject.org/koji/taskinfo?taskID=2273819
    Note that current dist-f13-build pango is 1.28.0-1.fc13 and
    introspection is enabled from 1.28.0-2

* Desktop file verification
-----------------------------------------------------
   518  /builddir/build/BUILDROOT/eekboard-0.0.3-2.fc14.i386/usr/share/applications/eekboard.desktop: error: (will be fatal in the future): value "GNOME" in key "Categories" in group "Desktop Entry" requires another category to be present among the following categories: GTK
-----------------------------------------------------
  - Please consider to fix this.

* Documents
  - "AUTHORS COPYING README" should be moved to -libs, because
    eekboard depends on -libs and not the opposite.

* Directory ownership issue
  - Please take care of directory ownership issue
------------------------------------------------------
# rpm -qf /usr/include/eek-0.1/ /usr/include/eek-0.1/eek/ /usr/share/vala/ /usr/share/vala/vapi/
file /usr/include/eek-0.1 is not owned by any package
file /usr/include/eek-0.1/eek is not owned by any package
file /usr/share/vala is not owned by any package
file /usr/share/vala/vapi is not owned by any package
------------------------------------------------------

! Functionality
  - By the way (with using rawhide) when I try to
    * launch uxterm
    * launch eekboard from uxterm
    * choose 'ヘルプ' -> '情報', then choose '閉じる' on the popup'ed window
    Then eekboard freezes....

Comment 5 Daiki Ueno 2010-06-28 05:33:16 UTC
Thanks.  Mostly fixed:

Spec URL: http://ueno.fedorapeople.org/eekboard/eekboard.spec
SRPM URL: http://ueno.fedorapeople.org/eekboard/eekboard-0.0.3-3.fc13.src.rpm

- on CLUTTER_GTK_CFLAGS and CLUTTER_GTK_LIBS setting

the upstream configure.ac is wrong (since it does not specify ACTION-IF-NOT-FOUND clause of PKG_CHECK_MODULES, configure aborts if clutter-gtk-0.90 is not found).  I'll fix this in the next release.

- on F-13 build failure related to pango introspection

Pango*.gir is included in gir-repository-devel, so I added it to BR.

- on /usr/share/vala/ ownership

Added vala to eekboard-devel requirements.

- freeze on the About dialog

Couldn't reproduce it on F-13, but I'll try it on rawhide.  Anyway, additional information would be much appreciated.

Comment 6 Mamoru TASAKA 2010-06-28 07:07:06 UTC
Just tried to rebuild but this time your srpm does not build
on rawhide:
http://koji.fedoraproject.org/koji/taskinfo?taskID=2276522

Comment 8 Mamoru TASAKA 2010-06-28 16:22:25 UTC
Created attachment 427454 [details]
screenshot of eekboard

For -4:

* Timestamps
  - Please consider to use
-------------------------------------------------------------
make install DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p"
-------------------------------------------------------------
    to keep timestamps on installed files as much as possible.
    This method usually works for Makefiles generated by recent
    autotools.

* Directory ownership issue
  - Still %{_includedir}/eek-0.1/ is not owned by any packages.

! About "Freeze" of eekboard
  - Well, actually it seems that when I
    * launch eekboard (from terminal)
    * move eekboard window
    Then "virtual keyboard" on eekboard won't be redrawn (see attached)

Comment 9 Daiki Ueno 2010-06-29 01:03:37 UTC
Thanks.  Updated:

Spec URL: http://ueno.fedorapeople.org/eekboard/eekboard.spec
SRPM URL: http://ueno.fedorapeople.org/eekboard/eekboard-0.0.3-5.fc13.src.rpm

about "freeze", I guess you are hitting on the clutter-gtk bug:
http://bugzilla.openedhand.com/show_bug.cgi?id=2169
Could you try the following?

EEKBOARD_DISABLE_CLUTTER=1 eekboard

I'm now thinking of making this behavior default, at least clutter-gtk-0.90 is not found when building.

Comment 10 Mamoru TASAKA 2010-06-29 06:08:18 UTC
Will check later. For now about this comment:

(In reply to comment #9)

> about "freeze", I guess you are hitting on the clutter-gtk bug:
> http://bugzilla.openedhand.com/show_bug.cgi?id=2169
> Could you try the following?
> 
> EEKBOARD_DISABLE_CLUTTER=1 eekboard

It seems this works for me.

Comment 11 Mamoru TASAKA 2010-06-29 07:20:07 UTC
Well, good, except for the "freezing" issue on rawhide.
I think for now setting "EEKBOARD_DISABLE_CLUTTER=1" as default
is preferable, because we want to make the package work just as it is.

Comment 12 Jens Petersen 2010-06-29 09:17:00 UTC
Minor comment - not sure if it is worth subpackaging devel-docs?
Maybe it could be merged into devel?

Comment 13 Parag AN(पराग) 2010-06-29 10:34:51 UTC
I too think devel-docs is not that big to have it subpackaged from -devel.

Comment 14 Daiki Ueno 2010-07-01 05:10:01 UTC
Thanks.  Updated:

Spec URL: http://ueno.fedorapeople.org/eekboard/eekboard.spec
SRPM URL: http://ueno.fedorapeople.org/eekboard/eekboard-0.0.4-1.fc13.src.rpm

about "freeze", I added a workaround instead of disabling Clutter.

Comment 15 Mamoru TASAKA 2010-07-01 07:05:35 UTC
Well, currently rawhide tree has some broken dependencies
around gtk2/gtk3 related packages and now I am trying to resolve
the dependencies at least between packages needed for this package,
please wait....

Comment 16 Mamoru TASAKA 2010-07-01 17:24:49 UTC
Approving.

---------------------------------------------------------
   This package (eekboard) is APPROVED by mtasaka
---------------------------------------------------------

Comment 17 Daiki Ueno 2010-07-02 03:07:43 UTC
Thanks for the review, Tasaka-san.

New Package CVS Request
=======================
Package Name: eekboard
Short Description: A Virtual Keyboard for GNOME
Owners: ueno
Branches: F-13
InitialCC: i18n-team

Comment 18 Jason Tibbitts 2010-07-02 04:50:18 UTC
CVS done (by process-cvs-requests.py).

Comment 19 Fedora Update System 2010-07-02 06:02:45 UTC
eekboard-0.0.4-1.fc13 has been submitted as an update for Fedora 13.
http://admin.fedoraproject.org/updates/eekboard-0.0.4-1.fc13

Comment 20 Mamoru TASAKA 2010-07-02 06:53:53 UTC
Thanks, now closing.

Comment 21 Fedora Update System 2010-07-14 06:23:47 UTC
eekboard-0.0.4-2.fc13 has been submitted as an update for Fedora 13.
http://admin.fedoraproject.org/updates/eekboard-0.0.4-2.fc13

Comment 22 Fedora Update System 2010-07-30 08:46:14 UTC
eekboard-0.0.4-2.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.


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