Bug 439051 - Review Request: wacomexpresskeys - map Wacom tablets ExpressKeys and Touch Strips to key events
Review Request: wacomexpresskeys - map Wacom tablets ExpressKeys and Touch St...
Product: Red Hat Enterprise Linux 4
Classification: Red Hat
Component: Package Review (Show other bugs)
All Linux
low Severity low
: rc
: ---
Assigned To: Jarod Wilson
: 440448 (view as bug list)
Depends On:
Blocks: 439062 441894 RHEL4.0-ACCEPT
  Show dependency treegraph
Reported: 2008-03-26 14:18 EDT by Aristeu Rozanski
Modified: 2009-02-26 09:36 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2009-02-26 09:36:32 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---

Attachments (Terms of Use)

  None (edit)
Description Aristeu Rozanski 2008-03-26 14:18:31 EDT
Spec URL: http://people.redhat.com/arozansk/wacomexpresskeys/wacomexpresskeys.spec
Description: Configuration utility to assign Wacom tablet's ExpressKeys and
Touch Strips to keyboard events.
Comment 1 Jarod Wilson 2008-04-01 19:34:26 EDT
Will try to give this a good look over tomorrow.
Comment 2 Ludek Smid 2008-04-03 12:19:04 EDT
*** Bug 440448 has been marked as a duplicate of this bug. ***
Comment 3 Jarod Wilson 2008-04-07 11:56:21 EDT
Apologies for the delay, working on this now...
Comment 4 Jarod Wilson 2008-04-07 12:01:44 EDT
Review notes:

1) I'd shuffle the tarname define around a little bit, but not absolutely necessary.

2) I can only find evidence of a GPLv2+ license on the source.

3) Rather than using ExclusiveArch with such a long list, probably better to
just use ExcludeArch for the few arches it shouldn't build on.

4) Should use _smp_mflags, if possible.

5) Use of %makeinstall is frowned upon. Use make install
DESTDIR="$RPM_BUILD_ROOT" instead (tested it, works just fine).

Spec diff of suggested changes for the above:

--- wacomexpresskeys.spec       2008-04-07 11:10:54.000000000 -0400
+++ wacomexpresskeys-jw.spec	2008-04-07 11:57:38.000000000 -0400
@@ -1,6 +1,7 @@
 %define _x11dir		/usr/X11R6
 %define _x11libdir	%{_x11dir}/%{_lib}
 %define _x11sdkdir	%{_x11libdir}/Server
+%define tarname		expresskeys
 Name:		wacomexpresskeys
 Version:	0.4.1
@@ -8,22 +9,21 @@ Release:	1%{?dist}
 Summary:	Wacom ExpressKeys and Touch Strips configuration utility
 Group:		System Environment/Base
-License:	GPL/X11
+License:	GPLv2+
 URL:		http://web.telia.com/~u45802013/wacom/
-Source0:	http://web.telia.com/~u45802013/wacom/expresskeys-%{version}.tar.gz
-%define		tarname expresskeys-%{version}
+Source0:	http://web.telia.com/~u45802013/wacom/%{tarname}-%{version}.tar.gz
 BuildRoot:	%{_tmppath}/%{name}-%{version}-%{release}
 BuildRequires:	xorg-x11-devel, xorg-x11-sdk
 Requires:	xorg-x11
-ExclusiveArch:  %{ix86} x86_64 alpha ia64 ppc sparc sparc64
+ExcludeArch:    s390 s390x
 Configuration utility to bind Wacom tablet's ExpressKeys and Touch Strips to
 generate other input events.
-%setup -q -n %{tarname}
+%setup -q -n %{tarname}-%{version}
@@ -38,11 +38,11 @@ export RPM_LIBDIR=%{_lib}
+make %{?_smp_mflags}
+make install DESTDIR="$RPM_BUILD_ROOT"
Comment 5 Aristeu Rozanski 2008-04-07 12:59:09 EDT
Thanks Jarod, changes applied. Also, I added ppc64 to exclude list.
Updated spec file and SRPM on:

Comment 6 Jarod Wilson 2008-04-07 13:09:33 EDT
Only remaining thing I'd suggest is to rename 'test.patch' in the updated build
to something more descriptive.

Otherwise, looks good to me, ship it!
Comment 7 Aristeu Rozanski 2008-04-07 13:22:27 EDT
Sorry, I'm waiting for the customer to give feedback about that patch before I
actually make it official.
Fixing it now.
Thanks Jarod
Comment 8 Aristeu Rozanski 2008-04-07 15:55:56 EDT
Updated spec and RPM at
Comment 9 Jarod Wilson 2008-04-07 16:05:15 EDT
Can't remember what needs to be done now to move to the next step (getting the
package into brew), but package approved...
Comment 10 Aristeu Rozanski 2008-04-07 17:39:41 EDT
I think you should move it to MODIFIED/CLOSED. There's another bug depending on
this one: 439062
Comment 11 Jarod Wilson 2008-04-07 17:53:16 EDT
I'll go with MODIFIED, I think its only to be closed once the package is
imported and built.
Comment 14 Aristeu Rozanski 2009-02-26 09:36:32 EST
The package is in. closing the bug.

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