[Bug 249059] Review Request: wdaemon - hotplug helper for wacom x.org driver

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: wdaemon - hotplug helper for wacom x.org driver


https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=249059


jwilson at redhat.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|nobody at fedoraproject.org    |jwilson at redhat.com




------- Additional Comments From jwilson at redhat.com  2007-07-23 16:44 EST -------
First pass through the spec...

1) Why the use of ExclusiveArch, then list just about all possible arches? I
think an ExcludeArch for the arch or arches it doesn't build on might be better.
So far as I can see, that list is really just s390/s390x, no?

2) %makeinstall is a big no-no :)
http://fedoraproject.org/wiki/Packaging/Guidelines#head-fcaf3e6fcbd51194a5d0dbcfbdd2fcb7791dd002

3) the CFLAGS aren't being honored.

I've actually got a patch in hand that'll make 'make install
DESTDIR=$RPM_BUILD_ROOT' work, as well as the CFLAGS honored:

----8<----
--- wdaemon-0.10/Makefile       2007-07-20 12:07:44.000000000 -0400
+++ wdaemon-0.10.updated/Makefile       2007-07-23 16:30:06.000000000 -0400
@@ -1,4 +1,4 @@
-CFLAGS = -O0 -g -Wall
+CFLAGS ?= -O0 -g -Wall
 OBJS = hotplug.o \
        input.o \
        monitored.o \
@@ -24,10 +24,10 @@ wdaemon: $(OBJS)
        gcc $(CFLAGS) -c -o $@ $<
 
 install:
-       mkdir -p $(bindir)
-       cp wdaemon $(bindir)/
-       mkdir -p $(sysconfdir)/rc.d/init.d/
-       cp wdaemon.initrd $(sysconfdir)/rc.d/init.d/wdaemon
+       mkdir -p $(DESTDIR)$(bindir)
+       cp wdaemon $(DESTDIR)$(bindir)/
+       mkdir -p $(DESTDIR)$(sysconfdir)/rc.d/init.d/
+       cp wdaemon.initrd $(DESTDIR)$(sysconfdir)/rc.d/init.d/wdaemon
 
 clean:
        rm -f *.o wdaemon core
----8<----

4) I'd install is_uinput.sh mode 755 instead of 644, which also eliminates the
need for the %attr stuff on that file in %files.

That's all I've got so far... rpmlint output is fairly clean, just two warnings
about the udev rules files not being marked as config files. Not sure yet if
they should be, or if we just ignore those.

-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.