Re: [Openvpn-devel] [PATCH 2/2] allow to enable plugin lookup with configure argument

2016-11-29 Thread Christian Hesse
David Sommerseth  on Tue, 2016/11/29 00:47:
> On 28/11/16 17:16, Christian Hesse wrote:
> > From: Christian Hesse 
> > 
> > For plugin lookup (give relative path to plugin directory in
> > configuration) we had to configure with something like this:
> > 
> > CFLAGS="$CFLAGS
> > -DPLUGIN_LIBDIR=\\\"/usr/lib/openvpn/plugins\\\"" ./configure
> > 
> > This allows to pass --enable-plugin-lookup to configure to achieve the
> > same. As a bonus we can be sure that install path and lookup path in
> > openvpn binary are the same.  
> 
> 
> Thank you for your patch.  Unfortunately I'm not convinced this is the
> proper way to do it.
> 
> First of all, to achieve what I believe is your goal, you need to flip
> things around:
> 
>   ./configure CFLAGS="$CFLAGS
> -DPLUGIN_LIBDIR=\\\"/usr/lib/openvpn/plugins\\\""
> 
> This will ensure that whenever 'make' decides to re-run ./configure
> automatically it will keep all variables provided to the command line.

Ah, right...
I used this to build a binary package, so configure is run only once and it
works for this case. :D

> Secondly, I believe the proper way to configure PLUGIN_LIBDIR without
> going via CFLAGS is to use a similar approach to what is used by
> IFCONFIG, ROUTE, IPROUTE and NETSTAT.  They are configured via AC_ARG_VAR.
> 
> I'd recommend just adding these two lines to configure.ac instead:
> 
> AC_ARG_VAR([PLUGINDIR], [Path of default plug-in search directory])
> AC_DEFINE_UNQUOTED([PLUGIN_LIBDIR], ["$PLUGINDIR"])
> 
> (these lines are not tested, but that should give some pointer towards a
> better direction.
> 
> With these lines in place, it is expected that you can do:
> 
>   ./configure PLUGINDIR=/usr/lib/openvpn/plugins
> 
> Which should result in defining PLUGIN_LIBDIR in config.h.

I decided to go another way... Wanted to make the plugin search work out of
the box. Or do we want to keep that optional?

(AC_DEFINE_UNQUOTED() fails if PLUGINDIR is not defined and you try
${libdir}/openvpn/plugins instead. That evaluates to
"${exec_prefix}/lib/openvpn/plugins" and breaks the path in config.h.)
-- 
main(a){char*c=/*Schoene Gruesse */"B?IJj;MEH"
"CX:;",b;for(a/*Best regards my address:*/=0;b=c[a++];)
putchar(b-1/(/*Chriscc -ox -xc - && ./x*/b/42*2-3)*42);}


pgpDcQgOS59i6.pgp
Description: OpenPGP digital signature
--
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH 2/2] allow to enable plugin lookup with configure argument

2016-11-28 Thread David Sommerseth
On 28/11/16 17:16, Christian Hesse wrote:
> From: Christian Hesse 
> 
> For plugin lookup (give relative path to plugin directory in
> configuration) we had to configure with something like this:
> 
> CFLAGS="$CFLAGS -DPLUGIN_LIBDIR=\\\"/usr/lib/openvpn/plugins\\\"" ./configure
> 
> This allows to pass --enable-plugin-lookup to configure to achieve the
> same. As a bonus we can be sure that install path and lookup path in
> openvpn binary are the same.


Thank you for your patch.  Unfortunately I'm not convinced this is the
proper way to do it.

First of all, to achieve what I believe is your goal, you need to flip
things around:

  ./configure CFLAGS="$CFLAGS
-DPLUGIN_LIBDIR=\\\"/usr/lib/openvpn/plugins\\\""

This will ensure that whenever 'make' decides to re-run ./configure
automatically it will keep all variables provided to the command line.


Secondly, I believe the proper way to configure PLUGIN_LIBDIR without
going via CFLAGS is to use a similar approach to what is used by
IFCONFIG, ROUTE, IPROUTE and NETSTAT.  They are configured via AC_ARG_VAR.

I'd recommend just adding these two lines to configure.ac instead:

AC_ARG_VAR([PLUGINDIR], [Path of default plug-in search directory])
AC_DEFINE_UNQUOTED([PLUGIN_LIBDIR], ["$PLUGINDIR"])

(these lines are not tested, but that should give some pointer towards a
better direction.

With these lines in place, it is expected that you can do:

  ./configure PLUGINDIR=/usr/lib/openvpn/plugins

Which should result in defining PLUGIN_LIBDIR in config.h.


-- 
kind regards,

David Sommerseth
OpenVPN Technologies, Inc



> ---
>  configure.ac| 8 
>  src/openvpn/Makefile.am | 4 
>  2 files changed, 12 insertions(+)
> 
> diff --git a/configure.ac b/configure.ac
> index d0fe889..193b5f0 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -308,6 +308,14 @@ AC_ARG_WITH(
>   [with_plugindir="\$(libdir)/openvpn/plugins"]
>  )
>  
> +AC_ARG_ENABLE(
> + [plugin-lookup],
> + [AS_HELP_STRING([--enable-plugin-lookup], [enable plugin lookup in 
> plugin directory @<:@default=no@:>@])],
> + [enable_plugin_lookup="$enableval"],
> + [enable_plugin_lookup="no"]
> +)
> +AM_CONDITIONAL([ENABLE_PLUGIN_LOOKUP], [test x$enable_plugin_lookup = xyes])
> +
>  
>  AC_DEFINE_UNQUOTED([TARGET_ALIAS], ["${host}"], [A string representing our 
> host])
>  case "$host" in
> diff --git a/src/openvpn/Makefile.am b/src/openvpn/Makefile.am
> index 4c18449..46afc9a 100644
> --- a/src/openvpn/Makefile.am
> +++ b/src/openvpn/Makefile.am
> @@ -33,6 +33,10 @@ if WIN32
>  AM_CFLAGS += -municode -UUNICODE
>  endif
>  
> +if ENABLE_PLUGIN_LOOKUP
> +AM_CFLAGS += -DPLUGIN_LIBDIR=\"$(plugindir)\"
> +endif
> +
>  sbin_PROGRAMS = openvpn
>  
>  openvpn_SOURCES = \
> 




signature.asc
Description: OpenPGP digital signature
--
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel