Bug 251536 - Review Request: xorg-x11-drv-diamondtouch - Xorg diamondtouch input driver
Review Request: xorg-x11-drv-diamondtouch - Xorg diamondtouch input driver
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Kevin Fenzi
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-08-09 12:36 EDT by Adam Jackson
Modified: 2008-02-21 13:53 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-02-21 13:53:20 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
kevin: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Adam Jackson 2007-08-09 12:36:41 EDT
Spec URL: http://people.fedoraproject.org/~ajax/diamondtouch/xorg-x11-drv-diamondtouch.spec
SRPM URL: http://people.fedoraproject.org/~ajax/diamondtouch/xorg-x11-drv-diamondtouch-0.2.0-0.1.fc8.src.rpm

Description:
The MERL Diamondtouch is a multi-user touchscreen.  See demo video:

http://wearables.unisa.edu.au/mpx/?q=node/86

This is the X driver for same.
Comment 1 Kevin Fenzi 2007-08-10 00:35:28 EDT
I'd be happy to review this package, although I don't have the hardware. 
Look for a full review in a bit. 
Comment 2 Kevin Fenzi 2007-08-10 00:51:31 EDT
This package doesn't seem to build for me under devel mock: 

 gcc -DHAVE_CONFIG_H -I. -I. -I.. -Wall -I../include -O2 -g -pipe -Wall
-Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4
-m64 -mtune=generic -I/usr/include/xorg -I../src -MT
diamondtouch_drv_la-diamondtouch.lo -MD -MP -MF
.deps/diamondtouch_drv_la-diamondtouch.Tpo -c diamondtouch.c  -fPIC -DPIC -o
.libs/diamondtouch_drv_la-diamondtouch.o
diamondtouch.c: In function 'DtReadInput':
diamondtouch.c:417: warning: implicit declaration of function 'xf86DisableDevice'
diamondtouch.c: In function '_dt_init_axes':
diamondtouch.c:698: error: 'GetMotionHistory' undeclared (first use in this
function)
diamondtouch.c:698: error: (Each undeclared identifier is reported only once
diamondtouch.c:698: error: for each function it appears in.)
diamondtouch.c:699: warning: implicit declaration of function 'GetMotionHistorySize'
diamondtouch.c:704: warning: implicit declaration of function
'InitAbsoluteClassDeviceStruct'
make[2]: *** [diamondtouch_drv_la-diamondtouch.lo] Error 1
make[2]: Leaving directory `/builddir/build/BUILD/xf86-input-diamondtouch-0.2.0/src'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory `/builddir/build/BUILD/xf86-input-diamondtouch-0.2.0'
make: *** [all] Error 2
error: Bad exit status from /var/tmp/rpm-tmp.91130 (%build)

Missing BuildRequires? Or something else going on? 
Comment 3 Adam Jackson 2007-08-11 20:34:13 EDT
Mmm.  Might require either MPX branch of X, or else new input stuff that'll be
in 1.4, and it only built for me because I had sdk pollution.

Feel free to ignore this for a while.
Comment 4 Kevin Fenzi 2007-11-30 23:13:04 EST
Any further news on this?
Comment 5 Adam Jackson 2008-02-13 15:01:45 EST
This builds with the xorg-x11-server-devel in rawhide now.
Comment 6 Kevin Fenzi 2008-02-14 23:05:26 EST
I of course don't have the hardware to test this, but
I can check the package otherwise.

OK - Package meets naming and packaging guidelines
OK - Spec file matches base package name.
OK - Spec has consistant macro usage.
OK - Meets Packaging Guidelines.
OK - License (MIT)
OK - License field in spec matches
See below - License file included in package
OK - Spec in American English
OK - Spec is legible.
OK - Sources match upstream md5sum:
198d2c738c16537c273546f2e74538bd  xf86-input-diamondtouch-0.2.0.tar.bz2
198d2c738c16537c273546f2e74538bd  xf86-input-diamondtouch-0.2.0.tar.bz2.1
OK - BuildRequires correct
OK - Package has %defattr and permissions on files is good.
OK - Package has a correct %clean section.
OK - Package has correct buildroot
OK - Package is code or permissible content.
OK - Packages %doc files don't affect runtime.
OK - Package has rm -rf RPM_BUILD_ROOT at top of %install

See below - Headers/static libs in -devel subpackage.

OK - Package compiles and builds on at least one arch.
OK - Package has no duplicate files in %files.
OK - Package doesn't own any directories other packages own.
OK - Package owns all the directories it creates.
See below - No rpmlint output.
OK - final provides and requires are sane.

SHOULD Items:

OK - Should build in mock.
OK - Should build on all supported archs
OK - Should have dist tag
OK - Should package latest version

Issues:

1. Might ask upstream to include a copy of the license they use.
Not a blocker of course.

2. rpmlint complains:

xorg-x11-drv-diamondtouch.x86_64: W: devel-file-in-non-devel-package
/usr/include/diamondtouch/ee_defs.h

Is there any point in shipping this header file?

I'll leave it up to you if you want to ship that header or not,
otherwise this package is APPROVED.
Comment 7 Adam Jackson 2008-02-18 10:50:35 EST
Yeah, the header probably shouldn't ship, I can't think of a reason for
including it.

New Package CVS Request
=======================
Package Name: xorg-x11-drv-diamondtouch
Short Description: Xorg diamondtouch input driver
Owners: ajax
Branches: 
InitialCC: 
Cvsextras Commits: yes
Comment 8 Kevin Fenzi 2008-02-18 12:22:33 EST
cvs done.
Comment 9 Adam Jackson 2008-02-21 13:53:20 EST
Built in rawhide, thanks!

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