Bug 251536

Summary: Review Request: xorg-x11-drv-diamondtouch - Xorg diamondtouch input driver
Product: [Fedora] Fedora Reporter: Adam Jackson <ajax>
Component: Package ReviewAssignee: Kevin Fenzi <kevin>
Status: CLOSED RAWHIDE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora-package-review, notting
Target Milestone: ---Flags: kevin: fedora-review+
kevin: 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: 2008-02-21 18:53:20 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:

Description Adam Jackson 2007-08-09 16:36:41 UTC
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 04:35:28 UTC
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 04:51:31 UTC
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-12 00:34:13 UTC
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-12-01 04:13:04 UTC
Any further news on this?

Comment 5 Adam Jackson 2008-02-13 20:01:45 UTC
This builds with the xorg-x11-server-devel in rawhide now.

Comment 6 Kevin Fenzi 2008-02-15 04:05:26 UTC
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 15:50:35 UTC
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 17:22:33 UTC
cvs done.

Comment 9 Adam Jackson 2008-02-21 18:53:20 UTC
Built in rawhide, thanks!