[PATCH 03/18] evdev: automatic configuration of mtdev library support

[sardemff7+wayland at sardemff7.net]

On 21/06/2013 11:52, Marc Chalain wrote:
> 2013/6/21 <sardemff7+wayland at sardemff7.net>
>
>> On 21/06/2013 10:39, mchalain [marc.chalain at gmail.com] wrote:
>>
>>> From: mchalain <marc.chalain at gmail.com>
>>>
>>>    it allows the --disable-mtdev option at configuration.
>>>    evdev.c creates empty macros for mtdev functions not availlables
>>>
>>> ---
>>>    weston/configure.ac |   17 ++++++++++++++---
>>>    weston/src/evdev.c  |   11 ++++++++---
>>>    2 files changed, 22 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/weston/configure.ac b/weston/configure.ac
>>> index b625221..2bdff35 100644
>>> --- a/weston/configure.ac
>>> +++ b/weston/configure.ac
>>> @@ -132,7 +132,7 @@ AC_ARG_ENABLE(drm-compositor, [
>>>   --enable-drm-compositor],,
>>>    AM_CONDITIONAL(ENABLE_DRM_**COMPOSITOR, test x$enable_drm_compositor =
>>> xyes -a x$enable_egl = xyes)
>>>    if test x$enable_drm_compositor = xyes -a x$enable_egl = xyes; then
>>>      AC_DEFINE([BUILD_DRM_**COMPOSITOR], [1], [Build the DRM compositor])
>>> -  PKG_CHECK_MODULES(DRM_**COMPOSITOR, [libudev >= 136 libdrm >= 2.4.30
>>> gbm mtdev >= 1.1.0])
>>> +  PKG_CHECK_MODULES(DRM_**COMPOSITOR, [libudev >= 136 libdrm >= 2.4.30
>>> gbm])
>>>    fi
>>>
>>>
>>> @@ -161,7 +161,7 @@ AM_CONDITIONAL(ENABLE_RPI_**COMPOSITOR, test
>>> "x$enable_rpi_compositor" = "xyes")
>>>    have_bcm_host="no"
>>>    if test "x$enable_rpi_compositor" = "xyes"; then
>>>      AC_DEFINE([BUILD_RPI_**COMPOSITOR], [1], [Build the compositor for
>>> Raspberry Pi])
>>> -  PKG_CHECK_MODULES(RPI_**COMPOSITOR, [libudev >= 136 mtdev >= 1.1.0])
>>> +  PKG_CHECK_MODULES(RPI_**COMPOSITOR, [libudev >= 136])
>>>      PKG_CHECK_MODULES(RPI_BCM_**HOST, [bcm_host],
>>>                        [have_bcm_host="yes"
>>>                         AC_DEFINE([HAVE_BCM_HOST], [1], [have Raspberry Pi
>>> BCM headers])],
>>> @@ -176,7 +176,7 @@ AM_CONDITIONAL([ENABLE_FBDEV_**COMPOSITOR],
>>>                   [test x$enable_fbdev_compositor = xyes])
>>>    AS_IF([test x$enable_fbdev_compositor = xyes], [
>>>      AC_DEFINE([BUILD_FBDEV_**COMPOSITOR], [1], [Build the fbdev
>>> compositor])
>>> -  PKG_CHECK_MODULES([FBDEV_**COMPOSITOR], [libudev >= 136 mtdev >=
>>> 1.1.0])
>>> +  PKG_CHECK_MODULES([FBDEV_**COMPOSITOR], [libudev >= 136 ])
>>>    ])
>>>
>>>    AC_ARG_ENABLE([rdp-compositor]**, [  --enable-rdp-compositor],,
>>> @@ -203,6 +203,17 @@ PKG_CHECK_MODULES(WEBP, [libwebp], [have_webp=yes],
>>> [have_webp=no])
>>>    AS_IF([test "x$have_webp" = "xyes"],
>>>          [AC_DEFINE([HAVE_WEBP], [1], [Have webp])])
>>>
>>> +AC_ARG_ENABLE(mtdev,
>>> +              AS_HELP_STRING([--disable-**mtdev],
>>> +                             [do not support multitouch library
>>> libmtdev]),,
>>> +             enable_mtdev=auto)
>>> +AM_CONDITIONAL(ENABLE_MTDEV, test "x$enable_mtdev" = "xyes")
>>>
>>
>> If you do not use this, you should probably not set it.
>>
>>
>
>>
>>
>>   +if test "x$enable_mtdev" != "xno"; then
>>> +       PKG_CHECK_MODULES(MTDEV, [mtdev >= 1.1.0], [have_mtdev=yes],
>>> [have_mtdev=no])
>>>
>>
>> Where do you use MTDEV_{CFLAGS,LIBS}?
>
> Nowhere at this moment. But I don't know the future and this was done in
> the prevsious version.

Why do you removed this usage then? I’m not sure what you mean by that. 
If you don’t know the future, you should even more use the provided 
flags. Specifically the LIBS so that mtdev is properly linked everywhere 
is it used. (And CFLAGS in case headers are not in a default location.)


>>
>>
>>   +       AS_IF([test "x$have_mtdev" = "xyes"],
>>> +                 [AC_DEFINE([HAVE_MTDEV], [1], [Have Multitouch device
>>> support])])
>>>
>>
>> What if the user specified --enable-mtdev but the package is not
>> available? It should be an error, not a silent ignore. :-)
>>
>>
> If you set  --enable-mtde, you know that mtdev is available. After the
> automatic check is the normal way in my opinion.

Not at all. You want mtdev support, but you might have missed the 
installation. Also, we (packagers) want to ensure that a specific 
feature has all its deps checked. Please, add an error if $enable_mtdev 
is yes and $have_mtdev is no. :-)


>>
>>   +fi
>>> +
>>>    AC_CHECK_LIB([jpeg], [jpeg_CreateDecompress], have_jpeglib=yes)
>>>    if test x$have_jpeglib = xyes; then
>>>      JPEG_LIBS="-ljpeg"
>>> diff --git a/weston/src/evdev.c b/weston/src/evdev.c
>>> index 122a2d9..59af038 100644
>>> --- a/weston/src/evdev.c
>>> +++ b/weston/src/evdev.c
>>> @@ -27,7 +27,15 @@
>>>    #include <linux/input.h>
>>>    #include <unistd.h>
>>>    #include <fcntl.h>
>>> +#ifdef HAVE_MTDEV
>>>    #include <mtdev.h>
>>> +#else
>>> +struct mtdev
>>> +{ int none;};
>>> +#define mtdev_close_delete(...) do{}while(0)
>>> +#define mtdev_get(...) 0
>>> +#define mtdev_new_open(...) NULL
>>> +#endif
>>>
>>>    #include "compositor.h"
>>>    #include "evdev.h"
>>> @@ -565,11 +573,8 @@ evdev_device_create(struct weston_seat *seat, const
>>> char *path, int device_fd)
>>>          if (device->dispatch == NULL)
>>>                  goto err1;
>>>
>>> -
>>>          if (device->is_mt) {
>>>                  device->mtdev = mtdev_new_open(device->fd);
>>> -               if (!device->mtdev)
>>> -                       weston_log("mtdev failed to open for %s\n", path);
>>>
>>
>> IMO, it should still log it if mtdev support is enabled.
>>
>>
>>
>>           }
>>>
>>>          device->source = wl_event_loop_add_fd(ec->**input_loop,
>>> device->fd,
>>>
>>>
>>
>> --
>>
>> Quentin “Sardem FF7” Glidic
>>
>


-- 

Quentin “Sardem FF7” Glidic