[collectd] [PATCH] Let configure bail out on missing dependencies

Bruno Prémont bonbons at linux-vserver.org
Mon Oct 6 13:59:20 CEST 2008


Hi Sebastian,

On Mon, 06 October 2008 Sebastian Harl <sh at tokkee.org> wrote:
> Hi Bruno,
> 
> On Sun, Oct 05, 2008 at 02:09:08PM +0200, Bruno Prémont wrote:
> > The patch below adds check in AC_PLUGIN() that verifies if the
> > dependencies are met for any enabled plugin.
> 
> Thanks for your patch!
> 
> > A later addition would be to list requirements (dependencies,
> > OS/Kernel restrictions) for all plugins in an easily accessible
> > location (e.g. INSTALL file)
> 
> The list of prerequisites in the README file already list all
> dependencies and should also mention all plugins that use those deps.
> I'm not sure if it makes sense to add another list which is just
> sorted differently but would have to be maintained in parallel.

I did not re-read the README recently so if the list was already there,
months ago I've forgotten since then.
Just reading it now, looks fine though at least for libiptc (iptables
plugin) it does not mention that collectd packages an own version as
it is mentioning for e.g. liboping..

> > --- collectd-4.5.0/configure.in	2008-09-05
> > 10:53:08.000000000 +0200 +++ collectd-4.5.0-new/configure.in
> > 2008-10-05 13:59:24.030892180 +0200 @@ -2477,9 +2477,12 @@ AC_DEFUN(
> >  	     enable_plugin="no"
> >       fi
> >      ])
> > -    if test "x$enable_plugin" = "xyes"
> > +    if test "x$enable_plugin" = "xyes" && test "x$2" = "xyes"
> >      then
> >  	    AC_DEFINE([HAVE_PLUGIN_]my_toupper([$1]), 1, [Define
> > to 1 if the $1 plugin is enabled.])
> > +    else
> > +    	    dependency_error="yes"
> > +	    test "x$enable_plugin" = "xyes" &&
> > enable_plugin="failed (missing dependency)"
> 
> Hrm ... this would also bail out if the user specifies
> --disable-<plugin> while the dependencies would be fulfilled. I.e.
> $enable_plugin would then equal "no" but $2 would be "yes", so the
> test fails.
> 
> So, the else part should be changed to:
> 
>   else if test "x$enable_plugin" = "xyes" && test "x$2" = "xno"
>   then
>       ...
>   fi; fi

Oops yes, I was not careful enough.
The other way would be:

  if test "x$enable_plugin" = "xyes"; then
    if test "x$2" = "xyes"; then
      AC_DEFINE([...])
    else
      dependency_error="yes"
      enable_plugin="failed (missing dependency, see README for details)"
    fi
  fi

Cheers,
Bruno



More information about the collectd mailing list