Bug 439051
| Summary: | Review Request: wacomexpresskeys - map Wacom tablets ExpressKeys and Touch Strips to key events | ||
|---|---|---|---|
| Product: | Red Hat Enterprise Linux 4 | Reporter: | Aristeu Rozanski <arozansk> |
| Component: | Package Review | Assignee: | Jarod Wilson <jarod> |
| Status: | CLOSED CURRENTRELEASE | QA Contact: | |
| Severity: | low | Docs Contact: | |
| Priority: | low | ||
| Version: | 4.7 | CC: | lsmid, notting, pm-rhel |
| Target Milestone: | rc | ||
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | Bug Fix | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2009-02-26 14:36:32 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: | 439062, 441894, 456739 | ||
|
Description
Aristeu Rozanski
2008-03-26 18:18:31 UTC
Will try to give this a good look over tomorrow. *** Bug 440448 has been marked as a duplicate of this bug. *** Apologies for the delay, working on this now... 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
%description
Configuration utility to bind Wacom tablet's ExpressKeys and Touch Strips to
generate other input events.
%prep
-%setup -q -n %{tarname}
+%setup -q -n %{tarname}-%{version}
%build
@@ -38,11 +38,11 @@ export RPM_LIBDIR=%{_lib}
$XSERVER64
export CFLAGS="$RPM_OPT_FLAGS"
-make
+make %{?_smp_mflags}
%install
rm -rf $RPM_BUILD_ROOT
-%makeinstall
+make install DESTDIR="$RPM_BUILD_ROOT"
%clean
rm -rf $RPM_BUILD_ROOT
Thanks Jarod, changes applied. Also, I added ppc64 to exclude list. Updated spec file and SRPM on: http://people.redhat.com/arozansk/wacomexpresskeys/ 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! *blush* Sorry, I'm waiting for the customer to give feedback about that patch before I actually make it official. Fixing it now. Thanks Jarod Updated spec and RPM at http://people.redhat.com/arozansk/wacomexpresskeys/ Can't remember what needs to be done now to move to the next step (getting the package into brew), but package approved... I think you should move it to MODIFIED/CLOSED. There's another bug depending on this one: 439062 I'll go with MODIFIED, I think its only to be closed once the package is imported and built. The package is in. closing the bug. |