From mboxrd@z Thu Jan  1 00:00:00 1970
From: Dan Nicholson <dbn.lists@gmail.com>
Date: Wed, 03 Mar 2010 14:23:53 +0000
Subject: Re: [PATCH] split --disable-extras into multiple options
Message-Id: <91705d081003030623o2eb64d59waba02515db3006e@mail.gmail.com>
List-Id: <linux-hotplug.vger.kernel.org>
References: <20100211111209.GD30615@pengutronix.de>
In-Reply-To: <20100211111209.GD30615@pengutronix.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
To: linux-hotplug@vger.kernel.org

On Thu, Feb 11, 2010 at 3:12 AM, Michael Olbrich
<m.olbrich@pengutronix.de> wrote:
> Hi,
>
> This patch patch makes it possible to enable all extras with external
> dependencies individually. It replaces:
>        --disable-extras
> with
>        --disable-gudev
>        --disable-bluetooth
>        --disable-keymap
>        --disable-acl
>        --disable-usbdb
>        --disable-pcidb
>        --disable-modem-modeswitch
> as configure options.
> I need a patch like this for the embedded system I'm working on. I need
> gudev for other stuff, but I don't have libacl and some of the other
> dependencies.
>
> Any opinions?
>
> Regards,
> Michael
>
> ---
>  Makefile.am  |   25 +++++++++++++++++++++----
>  configure.ac |   59 ++++++++++++++++++++++++++++++++++++++++++++++++-----------
>  2 files changed, 69 insertions(+), 15 deletions(-)
>
> Index: b/Makefile.am
> =================================> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -288,7 +288,8 @@ extras_v4l_id_v4l_id_LDADD = libudev/lib
>  libexec_PROGRAMS += extras/v4l_id/v4l_id
>  dist_udevrules_DATA += extras/v4l_id/60-persistent-v4l.rules
>
> -if ENABLE_EXTRAS
> +if ENABLE_USBDB
> +if ENABLE_PCIDB
>  # ------------------------------------------------------------------------------
>  # conditional extras (need glib, libusb, libacl, ...)
>  # ------------------------------------------------------------------------------
> @@ -296,7 +297,12 @@ dist_udevrules_DATA += \
>        rules/rules.d/75-net-description.rules \
>        rules/rules.d/75-tty-description.rules \
>        rules/rules.d/78-sound-card.rules
> +endif # ENABLE_PCIDB
> +endif # ENABLE_USBDB
>
> +BUILT_SOURCES > +
> +if ENABLE_GUDEV
>  # ------------------------------------------------------------------------------
>  # GUdev - libudev gobject interface
>  # ------------------------------------------------------------------------------
> @@ -334,7 +340,7 @@ dist_extras_gudev_libgudev_1_0_la_SOURCE
>        extras/gudev/gudevmarshal.c \
>        extras/gudev/gudevenumtypes.h \
>        extras/gudev/gudevenumtypes.c
> -BUILT_SOURCES = $(dist_extras_gudev_libgudev_1_0_la_SOURCES)
> +BUILT_SOURCES += $(dist_extras_gudev_libgudev_1_0_la_SOURCES)
>
>  extras_gudev_libgudev_1_0_la_CPPFLAGS = \
>        $(AM_CPPFLAGS) \
> @@ -410,7 +416,9 @@ typelibs_DATA = extras/gudev/GUdev-1.0.t
>
>  CLEANFILES += $(gir_DATA) $(typelibs_DATA)
>  endif # ENABLE_INTROSPECTION
> +endif # ENABLE_GUDEV

I'm not that familiar with the udev setup, but I'd like to caution
against surrounding these whole sections in an AM_CONDITIONAL. The
preferred thing to do is surround just the parts that result in
commands being run. For instance, I can see that you've here put
dist_extras_gudev_libgudev_1_0_la_SOURCES under ENABLE_GUDEV. This
will eventually break "make dist" if there person doing the
distributing isn't building GUdev.

Instead, you want to put the conditional around the directives that
generate things. For a simple example:

if SOME_OPTIONAL_DEP
bin_PROGRAMS = prog
noinst_LTLIBRARIES = libfoo.la
endif

prog_SOURCES = ...
prog_LDADD = libfoo.la
libfoo_la_SOURCES = ...

This will mean that the program and library won't actually be built
when the conditional is false, but the definitions of their prereqs
are not commented out. Otherwise, this patch looks very good from an
autotools perspective.

--
Dan