show correct default for plugin dir in configure help

PATCH In review Christian Hesse 4 participants 23 messages Nov 28, 2016 – Jan 27, 2017

lore

Share & tools

lore

Christian Hesse

author

2016-11-28 16:16 UTC #0 lore raw

PATCH 1/2 show correct default for plugin dir in configure help

From: Christian Hesse <[email protected]...>

Signed-off-by Christian Hesse <[email protected]...>

---
 configure.ac | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

configure.ac +1 -1

index f4073d0..d0fe889 100644
@@ -303,7 +303,7 @@ AC_ARG_WITH(
 
 AC_ARG_WITH(
 	[plugindir],
-	[AS_HELP_STRING([--with-plugindir], [plugin directory @<:@default=LIBDIR/openvpn@:>@])],
+	[AS_HELP_STRING([--with-plugindir], [plugin directory @<:@default=LIBDIR/openvpn/plugins@:>@])],
 	,
 	[with_plugindir="\$(libdir)/openvpn/plugins"]
 )

Signature · 1 line

2.10.2

Christian Hesse

author

2016-11-28 16:16 UTC #1 lore raw

PATCH 2/2 allow to enable plugin lookup with configure argument

From: Christian Hesse <[email protected]...>

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.

Signed-off-by Christian Hesse <[email protected]...>

---
 configure.ac            | 8 ++++++++
 src/openvpn/Makefile.am | 4 ++++
 2 files changed, 12 insertions(+)

configure.ac +8 -0

index d0fe889..193b5f0 100644
@@ -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

src/openvpn/Makefile.am +4 -0

index 4c18449..46afc9a 100644
@@ -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 · 1 line

2.10.2

David Sommerseth

2016-11-28 23:47 UTC #2 lore raw

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

Quoted 10 lines · Christian Hesse

From: Christian Hesse <[email protected]...>
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.

Signature · 4 lines

kind regards,

David Sommerseth
OpenVPN Technologies, Inc

Quoted 40 lines

---
 configure.ac            | 8 ++++++++
 src/openvpn/Makefile.am | 4 ++++
 2 files changed, 12 insertions(+)

configure.ac +8 -0

index d0fe889..193b5f0 100644
@@ -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

src/openvpn/Makefile.am +4 -0

index 4c18449..46afc9a 100644
@@ -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 = \

Christian Hesse

author

2016-11-29 11:05 UTC #3 lore raw

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

David Sommerseth <[email protected]...> on Tue, 2016/11/29 00:47:

Quoted 17 lines · Christian Hesse

Quoted 11 lines
From: Christian Hesse <[email protected]...>
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.

Quoted 8 lines

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

Quoted 9 lines

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"])

Quoted 8 lines

(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.)

Signature · 3 lines

main(a){char*c=/*    Schoene Gruesse                         */"B?IJj;MEH"
"CX:;",b;for(a/*    Best regards             my address:    */=0;b=c[a++];)
putchar(b-1/(/*    Chris            cc -ox -xc - && ./x    */b/42*2-3)*42);}

Christian Hesse

author

2016-11-29 11:07 UTC #4 lore raw

PATCH 1/1 Clean up plugin path handling

From: Christian Hesse <[email protected]...>

Drop --with-plugindir, instead use an environment variable PLUGINDIR to specify the plugin directory.

This always defines PLUGIN_LIBDIR and enables plugin search path.

Signed-off-by Christian Hesse <[email protected]...>

---
 configure.ac            | 14 ++++++--------
 src/openvpn/Makefile.am |  3 ++-
 2 files changed, 8 insertions(+), 9 deletions(-)

configure.ac +6 -8

index f4073d0..5fe652e 100644
@@ -301,13 +301,12 @@ AC_ARG_WITH(
 	[with_crypto_library="openssl"]
 )
 
-AC_ARG_WITH(
-	[plugindir],
-	[AS_HELP_STRING([--with-plugindir], [plugin directory @<:@default=LIBDIR/openvpn@:>@])],
-	,
-	[with_plugindir="\$(libdir)/openvpn/plugins"]
-)
-
+AC_ARG_VAR([PLUGINDIR], [Path of plug-in directory @<:@default=LIBDIR/openvpn/plugins@:>@])
+if test -n "${PLUGINDIR}"; then
+	plugindir="${PLUGINDIR}"
+else
+	plugindir="\${libdir}/openvpn/plugins"
+fi
 
 AC_DEFINE_UNQUOTED([TARGET_ALIAS], ["${host}"], [A string representing our host])
 case "$host" in

@@ -1245,7 +1244,6 @@ AM_CONDITIONAL([ENABLE_PLUGIN_AUTH_PAM], [test "${enable_plugin_auth_pam}" = "ye
 AM_CONDITIONAL([ENABLE_PLUGIN_DOWN_ROOT], [test "${enable_plugin_down_root}" = "yes"])
 AM_CONDITIONAL([ENABLE_CRYPTO], [test "${enable_crypto}" = "yes"])
 
-plugindir="${with_plugindir}"
 sampledir="\$(docdir)/sample"
 AC_SUBST([plugindir])
 AC_SUBST([sampledir])

src/openvpn/Makefile.am +2 -1

index 4c18449..188834a 100644
@@ -27,7 +27,8 @@ AM_CFLAGS = \
 	$(OPTIONAL_CRYPTO_CFLAGS) \
 	$(OPTIONAL_LZO_CFLAGS) \
 	$(OPTIONAL_LZ4_CFLAGS) \
-	$(OPTIONAL_PKCS11_HELPER_CFLAGS)
+	$(OPTIONAL_PKCS11_HELPER_CFLAGS) \
+	-DPLUGIN_LIBDIR=\"${plugindir}\"
 if WIN32
 # we want unicode entry point but not the macro
 AM_CFLAGS += -municode -UUNICODE

Signature · 1 line

2.10.2

Christian Hesse

author

2016-11-29 13:47 UTC #5 lore raw

Re: [Openvpn-devel] [PATCH 1/1] Clean up plugin path handling

Christian Hesse <[email protected]...> on Tue, 2016/11/29 12:07:

Quoted 51 lines

From: Christian Hesse <[email protected]...>

Drop --with-plugindir, instead use an environment variable PLUGINDIR to specify the plugin directory.

This always defines PLUGIN_LIBDIR and enables plugin search path.

Signed-off-by Christian Hesse <[email protected]...>

---
 configure.ac            | 14 ++++++--------
 src/openvpn/Makefile.am |  3 ++-
 2 files changed, 8 insertions(+), 9 deletions(-)

configure.ac +1 -7

index f4073d0..5fe652e 100644
@@ -301,13 +301,12 @@ AC_ARG_WITH(
 	[with_crypto_library="openssl"]
 )
 
-AC_ARG_WITH(
-	[plugindir],
-	[AS_HELP_STRING([--with-plugindir], [plugin directory
 @<:@default=LIBDIR/openvpn@:>@])],
-	,
-	[with_plugindir="\$(libdir)/openvpn/plugins"]
-)
-
+AC_ARG_VAR([PLUGINDIR], [Path of plug-in directory

@<:@default=LIBDIR/openvpn/plugins@:>@]) +if test -n "${PLUGINDIR}"; then

configure.ac +4 -0

+	plugindir="${PLUGINDIR}"
+else
+	plugindir="\${libdir}/openvpn/plugins"
+fi
 
 AC_DEFINE_UNQUOTED([TARGET_ALIAS], ["${host}"], [A string representing our

host]) case "$host" in

configure.ac +0 -1

@@ -1245,7 +1244,6 @@ AM_CONDITIONAL([ENABLE_PLUGIN_AUTH_PAM], [test
 "${enable_plugin_auth_pam}" = "ye AM_CONDITIONAL([ENABLE_PLUGIN_DOWN_ROOT],
 [test "${enable_plugin_down_root}" = "yes"])
 AM_CONDITIONAL([ENABLE_CRYPTO], [test "${enable_crypto}" = "yes"]) 
-plugindir="${with_plugindir}"
 sampledir="\$(docdir)/sample"
 AC_SUBST([plugindir])
 AC_SUBST([sampledir])

src/openvpn/Makefile.am +0 -0

index 4c18449..188834a 100644

src/openvpn/Makefile.am AM_CFLAGS = \ · line 34

Quoted 9 lines

src/openvpn/Makefile.am +2 -1

@@ -27,7 +27,8 @@ AM_CFLAGS = \
 	$(OPTIONAL_CRYPTO_CFLAGS) \
 	$(OPTIONAL_LZO_CFLAGS) \
 	$(OPTIONAL_LZ4_CFLAGS) \
-	$(OPTIONAL_PKCS11_HELPER_CFLAGS)
+	$(OPTIONAL_PKCS11_HELPER_CFLAGS) \
+	-DPLUGIN_LIBDIR=\"${plugindir}\"
 if WIN32
 # we want unicode entry point but not the macro
 AM_CFLAGS += -municode -UUNICODE

The alternative would look something like this:

configure.ac +7 -8

@@ -301,13 +301,13 @@ AC_ARG_WITH(
 	[with_crypto_library="openssl"]
 )
 
-AC_ARG_WITH(
-	[plugindir],
-	[AS_HELP_STRING([--with-plugindir], [plugin directory @<:@default=LIBDIR/openvpn@:>@])],
-	,
-	[with_plugindir="\$(libdir)/openvpn/plugins"]
-)
-
+AC_ARG_VAR([PLUGINDIR], [Path of plug-in directory @<:@default=LIBDIR/openvpn/plugins@:>@])
+if test -n "${PLUGINDIR}"; then
+	plugindir="${PLUGINDIR}"
+	AC_DEFINE_UNQUOTED([PLUGIN_LIBDIR], ["${PLUGINDIR}"], [Path of plug-in search directory])
+else
+	plugindir="\${libdir}/openvpn/plugins"
+fi
 
 AC_DEFINE_UNQUOTED([TARGET_ALIAS], ["${host}"], [A string representing our host])
 case "$host" in

@@ -1245,7 +1245,6 @@ AM_CONDITIONAL([ENABLE_PLUGIN_AUTH_PAM], [test "${enable_plugin_auth_pam}" = "ye
 AM_CONDITIONAL([ENABLE_PLUGIN_DOWN_ROOT], [test "${enable_plugin_down_root}" = "yes"])
 AM_CONDITIONAL([ENABLE_CRYPTO], [test "${enable_crypto}" = "yes"])
 
-plugindir="${with_plugindir}"
 sampledir="\$(docdir)/sample"
 AC_SUBST([plugindir])
 AC_SUBST([sampledir])

However you *have* to give PLUGINDIR this way to enable the search path. I would like to avoid that. And I did not find a way to move AC_DEFINE_UNQUOTED() below the condition due to the nested variables in $libdir.

Signature · 3 lines

main(a){char*c=/*    Schoene Gruesse                         */"B?IJj;MEH"
"CX:;",b;for(a/*    Best regards             my address:    */=0;b=c[a++];)
putchar(b-1/(/*    Chris            cc -ox -xc - && ./x    */b/42*2-3)*42);}

David Sommerseth

2016-12-09 21:37 UTC #6 lore raw

Re: [Openvpn-devel] [PATCH 1/1] Clean up plugin path handling

In reply to Christian Hesse

Quoted 24 lines · Christian Hesse

From: Christian Hesse <[email protected]...>
Drop --with-plugindir, instead use an environment variable PLUGINDIR to specify the plugin directory.
This always defines PLUGIN_LIBDIR and enables plugin search path.
Signed-off-by Christian Hesse <[email protected]...>
---
 configure.ac            | 14 ++++++--------
 src/openvpn/Makefile.am |  3 ++-
 2 files changed, 8 insertions(+), 9 deletions(-)
configure.ac +0 -3
index f4073d0..5fe652e 100644
@@ -301,13 +301,12 @@ AC_ARG_WITH(
 	[with_crypto_library="openssl"]
 )
 
-AC_ARG_WITH(
-	[plugindir],
-	[AS_HELP_STRING([--with-plugindir], [plugin directory @<:@default=LIBDIR/openvpn@:>@])],

configure.ac AC_ARG_WITH( · line 309

Quoted 10 lines

configure.ac +6 -4

-	,
-	[with_plugindir="\$(libdir)/openvpn/plugins"]
-)
-
+AC_ARG_VAR([PLUGINDIR], [Path of plug-in directory @<:@default=LIBDIR/openvpn/plugins@:>@])
+if test -n "${PLUGINDIR}"; then
+	plugindir="${PLUGINDIR}"
+else
+	plugindir="\${libdir}/openvpn/plugins"
+fi

Finally had some time to dig into this one. I like the idea here, I think it makes sense. But I'm not sure ${libdir} is correct by default.
I think that should be /usr/local/lib64 by default on a 64bit system.

My google-foo isn't helpful for me today ... This is somewhat related, especially towards the end of this doc page:
<https://www.gnu.org/software/autoconf/manual/autoconf-2.69/html_node/Site-Defaults.html>

But I believe there are better ways to do this.

I haven't checked this in detail what happens with 'make install'. We should ensure that the plug-ins we ship (./src/plugins) which are built are installed as well into this directory.

Signature · 4 lines

kind regards,

David Sommerseth
OpenVPN Technologies, Inc

Christian Hesse

author

2016-12-09 21:54 UTC #7 lore raw

Re: [Openvpn-devel] [PATCH 1/1] Clean up plugin path handling

David Sommerseth <[email protected]...> on Fri, 2016/12/09 22:37:

Quoted 45 lines · Christian Hesse

Quoted 31 lines
From: Christian Hesse <[email protected]...>
Drop --with-plugindir, instead use an environment variable PLUGINDIR to specify the plugin directory.
This always defines PLUGIN_LIBDIR and enables plugin search path.
Signed-off-by Christian Hesse <[email protected]...>
---
 configure.ac            | 14 ++++++--------
 src/openvpn/Makefile.am |  3 ++-
 2 files changed, 8 insertions(+), 9 deletions(-)
configure.ac +1 -7
index f4073d0..5fe652e 100644
@@ -301,13 +301,12 @@ AC_ARG_WITH(
 	[with_crypto_library="openssl"]
 )
 
-AC_ARG_WITH(
-	[plugindir],
-	[AS_HELP_STRING([--with-plugindir], [plugin directory
 @<:@default=LIBDIR/openvpn@:>@])],
-	,
-	[with_plugindir="\$(libdir)/openvpn/plugins"]
-)
-
+AC_ARG_VAR([PLUGINDIR], [Path of plug-in directory
@<:@default=LIBDIR/openvpn/plugins@:>@]) +if test -n "${PLUGINDIR}"; then
configure.ac AC_ARG_WITH( · line 309
Quoted 4 lines
configure.ac +4 -0
+	plugindir="${PLUGINDIR}"
+else
+	plugindir="\${libdir}/openvpn/plugins"
+fi  
Finally had some time to dig into this one. I like the idea here, I think it makes sense. But I'm not sure ${libdir} is correct by default.
I think that should be /usr/local/lib64 by default on a 64bit system.
My google-foo isn't helpful for me today ... This is somewhat related, especially towards the end of this doc page:
<https://www.gnu.org/software/autoconf/manual/autoconf-2.69/html_node/Site-Defaults.html>

Quoted 5 lines

But I believe there are better ways to do this.
I haven't checked this in detail what happens with 'make install'. We should ensure that the plug-ins we ship (./src/plugins) which are built are installed as well into this directory.

${libdir} is where plugins are installed to... That's why I choose it. ;)

Installing anything to $prefix/lib64/ does not make sense imho. Never.

32bit systems install to $prefix/lib/, 64bit system install to $prefix/lib/. The only valid extra is $prefix/lib32/ for multilib on 64bit systemd.

Signature · 3 lines

main(a){char*c=/*    Schoene Gruesse                         */"B?IJj;MEH"
"CX:;",b;for(a/*    Best regards             my address:    */=0;b=c[a++];)
putchar(b-1/(/*    Chris            cc -ox -xc - && ./x    */b/42*2-3)*42);}

David Sommerseth

2016-12-09 22:40 UTC #8 lore raw

Re: [Openvpn-devel] [PATCH 1/1] Clean up plugin path handling

Quoted 48 lines · Christian Hesse

David Sommerseth <[email protected]...> on Fri, 2016/12/09 22:37:
Quoted 47 lines · Christian Hesse
Quoted 31 lines
From: Christian Hesse <[email protected]...>
Drop --with-plugindir, instead use an environment variable PLUGINDIR to specify the plugin directory.
This always defines PLUGIN_LIBDIR and enables plugin search path.
Signed-off-by Christian Hesse <[email protected]...>
---
 configure.ac            | 14 ++++++--------
 src/openvpn/Makefile.am |  3 ++-
 2 files changed, 8 insertions(+), 9 deletions(-)
configure.ac +1 -7
index f4073d0..5fe652e 100644
@@ -301,13 +301,12 @@ AC_ARG_WITH(
 	[with_crypto_library="openssl"]
 )
 
-AC_ARG_WITH(
-	[plugindir],
-	[AS_HELP_STRING([--with-plugindir], [plugin directory
 @<:@default=LIBDIR/openvpn@:>@])],
-	,
-	[with_plugindir="\$(libdir)/openvpn/plugins"]
-)
-
+AC_ARG_VAR([PLUGINDIR], [Path of plug-in directory
@<:@default=LIBDIR/openvpn/plugins@:>@]) +if test -n "${PLUGINDIR}"; then
configure.ac AC_ARG_WITH( · line 309
Quoted 4 lines
configure.ac +4 -0
+	plugindir="${PLUGINDIR}"
+else
+	plugindir="\${libdir}/openvpn/plugins"
+fi  
Finally had some time to dig into this one. I like the idea here, I think it makes sense. But I'm not sure ${libdir} is correct by default.
I think that should be /usr/local/lib64 by default on a 64bit system.
My google-foo isn't helpful for me today ... This is somewhat related, especially towards the end of this doc page:
<https://www.gnu.org/software/autoconf/manual/autoconf-2.69/html_node/Site-Defaults.html>
But I believe there are better ways to do this.

Quoted 7 lines

Quoted 3 lines
I haven't checked this in detail what happens with 'make install'. We should ensure that the plug-ins we ship (./src/plugins) which are built are installed as well into this directory.
${libdir} is where plugins are installed to... That's why I choose it. ;)
Installing anything to $prefix/lib64/ does not make sense imho. Never.

Well, that is the default for Fedora/RHEL families. 32bit systems uses /usr/lib, 64bit uses /usr/lib64.

$ uname -m
x86_64
$ cat /etc/redhat-release
Red Hat Enterprise Linux Server release 7.3 (Maipo)
$ rpm --eval '%{_libdir}'
/usr/lib64

I've tested this on Fedora 24 and Scientific Linux 6 and 7 as well, with the same result.

And you'll find the same in openSUSE too:
<https://en.opensuse.org/openSUSE:Packaging_Conventions_RPM_Macros#.25_lib>

Unless it has changed in Debian/Ubuntu, it is a similar policy there too.

Signature · 4 lines

kind regards,

David Sommerseth
OpenVPN Technologies, Inc

Christian Hesse

author

2016-12-09 23:30 UTC #9 lore raw

Re: [Openvpn-devel] [PATCH 1/1] Clean up plugin path handling

David Sommerseth <[email protected]...> on Fri, 2016/12/09 23:40:

Quoted 69 lines · Christian Hesse

Quoted 50 lines
David Sommerseth <[email protected]...> on Fri, 2016/12/09 22:37:
Quoted 48 lines · Christian Hesse
Quoted 32 lines
From: Christian Hesse <[email protected]...>
Drop --with-plugindir, instead use an environment variable PLUGINDIR to specify the plugin directory.
This always defines PLUGIN_LIBDIR and enables plugin search path.
Signed-off-by Christian Hesse <[email protected]...>
---
 configure.ac            | 14 ++++++--------
 src/openvpn/Makefile.am |  3 ++-
 2 files changed, 8 insertions(+), 9 deletions(-)
configure.ac +1 -7
index f4073d0..5fe652e 100644
@@ -301,13 +301,12 @@ AC_ARG_WITH(
 	[with_crypto_library="openssl"]
 )
 
-AC_ARG_WITH(
-	[plugindir],
-	[AS_HELP_STRING([--with-plugindir], [plugin directory
 @<:@default=LIBDIR/openvpn@:>@])],
-	,
-	[with_plugindir="\$(libdir)/openvpn/plugins"]
-)
-
+AC_ARG_VAR([PLUGINDIR], [Path of plug-in directory
 @<:@default=LIBDIR/openvpn/plugins@:>@]) +if test -n "${PLUGINDIR}";
then
configure.ac AC_ARG_WITH( · line 309
Quoted 4 lines
configure.ac +4 -0
+	plugindir="${PLUGINDIR}"
+else
+	plugindir="\${libdir}/openvpn/plugins"
+fi    
Finally had some time to dig into this one. I like the idea here, I think it makes sense. But I'm not sure ${libdir} is correct by default.
I think that should be /usr/local/lib64 by default on a 64bit system.
My google-foo isn't helpful for me today ... This is somewhat related, especially towards the end of this doc page:
<https://www.gnu.org/software/autoconf/manual/autoconf-2.69/html_node/Site-Defaults.html>
But I believe there are better ways to do this.
Quoted 7 lines
Quoted 3 lines
I haven't checked this in detail what happens with 'make install'. We should ensure that the plug-ins we ship (./src/plugins) which are built are installed as well into this directory.
${libdir} is where plugins are installed to... That's why I choose it. ;)
Installing anything to $prefix/lib64/ does not make sense imho. Never.
Well, that is the default for Fedora/RHEL families. 32bit systems uses /usr/lib, 64bit uses /usr/lib64.
$ uname -m
x86_64
$ cat /etc/redhat-release
Red Hat Enterprise Linux Server release 7.3 (Maipo)
$ rpm --eval '%{_libdir}'
/usr/lib64

Quoted 7 lines

I've tested this on Fedora 24 and Scientific Linux 6 and 7 as well, with the same result.
And you'll find the same in openSUSE too:
<https://en.opensuse.org/openSUSE:Packaging_Conventions_RPM_Macros#.25_lib>
Unless it has changed in Debian/Ubuntu, it is a similar policy there too.

Uh, interesting... Probably I did not look at these distributions for too long... (And/or relied on the package manager.)

For Arch Linux we use /usr/lib/, though /usr/lib64/ is provided by a symlink to lib nowadays.

Signature · 3 lines

main(a){char*c=/*    Schoene Gruesse                         */"B?IJj;MEH"
"CX:;",b;for(a/*    Best regards             my address:    */=0;b=c[a++];)
putchar(b-1/(/*    Chris            cc -ox -xc - && ./x    */b/42*2-3)*42);}

Christian Hesse

author

2016-12-27 22:24 UTC #10 lore raw

PATCH v2 1/1 Clean up plugin path handling

In reply to Christian Hesse

From: Christian Hesse <[email protected]...>

Drop --with-plugindir, instead use an environment variable PLUGINDIR to specify the plugin directory.

This generates a header file src/openvpn/plugindir.h which contains a define for the plugindir.

v2: The configure script can not evaluate the final $libdir path. So
use make to create a header file on the containing the final path.

Signed-off-by Christian Hesse <[email protected]...>

---
 configure.ac               | 14 ++++++--------
 src/openvpn/.gitignore     |  1 +
 src/openvpn/Makefile.am    | 12 ++++++++++--
 src/openvpn/plugin.c       |  1 +
 src/openvpn/plugindir.h.in | 34 ++++++++++++++++++++++++++++++++++
 5 files changed, 52 insertions(+), 10 deletions(-)
 create mode 100644 src/openvpn/.gitignore
 create mode 100644 src/openvpn/plugindir.h.in

configure.ac +6 -8

index 43487b0..182ea21 100644
@@ -301,13 +301,12 @@ AC_ARG_WITH(
 	[with_crypto_library="openssl"]
 )
 
-AC_ARG_WITH(
-	[plugindir],
-	[AS_HELP_STRING([--with-plugindir], [plugin directory @<:@default=LIBDIR/openvpn@:>@])],
-	,
-	[with_plugindir="\$(libdir)/openvpn/plugins"]
-)
-
+AC_ARG_VAR([PLUGINDIR], [Path of plug-in directory @<:@default=LIBDIR/openvpn/plugins@:>@])
+if test -n "${PLUGINDIR}"; then
+	plugindir="${PLUGINDIR}"
+else
+	plugindir="\${libdir}/openvpn/plugins"
+fi
 
 AC_DEFINE_UNQUOTED([TARGET_ALIAS], ["${host}"], [A string representing our host])
 case "$host" in

@@ -1245,7 +1244,6 @@ AM_CONDITIONAL([ENABLE_PLUGIN_AUTH_PAM], [test "${enable_plugin_auth_pam}" = "ye
 AM_CONDITIONAL([ENABLE_PLUGIN_DOWN_ROOT], [test "${enable_plugin_down_root}" = "yes"])
 AM_CONDITIONAL([ENABLE_CRYPTO], [test "${enable_crypto}" = "yes"])
 
-plugindir="${with_plugindir}"
 sampledir="\$(docdir)/sample"
 AC_SUBST([plugindir])
 AC_SUBST([sampledir])

src/openvpn/.gitignore new file +1 -0

new file mode 100644
index 0000000..450575a
@@ -0,0 +1 @@
+plugindir.h
\ No newline at end of file

src/openvpn/Makefile.am +10 -2

index bea294b..11854b6 100644
@@ -11,12 +11,19 @@
 
 include $(top_srcdir)/build/ltrc.inc
 
+plugindir.h: plugindir.h.in Makefile
+	$(AM_V_GEN)sed -e 's|\@PLUGINDIR\@|$(plugindir)|' \
+		$< > [email protected] && mv [email protected] $@
+
+plugin.c: plugindir.h
+
 MAINTAINERCLEANFILES = \
 	$(srcdir)/Makefile.in
 
 EXTRA_DIST = \
 	openvpn.vcxproj \
-	openvpn.vcxproj.filters
+	openvpn.vcxproj.filters \
+	plugindir.h.in
 
 AM_CPPFLAGS = \
 	-I$(top_srcdir)/include \

@@ -89,7 +96,7 @@ openvpn_SOURCES = \
 	perf.c perf.h \
 	pf.c pf.h pf-inline.h \
 	ping.c ping.h ping-inline.h \
-	plugin.c plugin.h \
+	plugin.c plugin.h plugindir.h \
 	pool.c pool.h \
 	proto.c proto.h \
 	proxy.c proxy.h \

@@ -131,3 +138,4 @@ if WIN32
 openvpn_SOURCES += openvpn_win32_resources.rc block_dns.c block_dns.h
 openvpn_LDADD += -lgdi32 -lws2_32 -lwininet -lcrypt32 -liphlpapi -lwinmm -lfwpuclnt -lrpcrt4
 endif
+

src/openvpn/plugin.c +1 -0

index 17eb2d8..a891ef4 100644
@@ -43,6 +43,7 @@
 #include "error.h"
 #include "misc.h"
 #include "plugin.h"
+#include "plugindir.h"
 #include "ssl_backend.h"
 #include "win32.h"
 #include "memdbg.h"

src/openvpn/plugindir.h.in new file +34 -0

new file mode 100644
index 0000000..6fa7b3e
@@ -0,0 +1,34 @@
+/*
+ *  OpenVPN -- An application to securely tunnel IP networks
+ *             over a single TCP/UDP port, with support for SSL/TLS-based
+ *             session authentication and key exchange,
+ *             packet encryption, packet authentication, and
+ *             packet compression.
+ *
+ *  Copyright (C) 2017 OpenVPN Technologies, Inc. <[email protected]...>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2
+ *  as published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program (see the file COPYING included with this
+ *  distribution); if not, write to the Free Software Foundation, Inc.,
+ *  59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+/*
+ * define the plugin directory
+ */
+
+#ifndef OPENVPN_PLUGINDIR_H
+#define OPENVPN_PLUGINDIR_H
+
+#define PLUGIN_LIBDIR "@PLUGINDIR@"
+
+#endif /* OPENVPN_PLUGINDIR_H */

Signature · 1 line

2.11.0

Christian Hesse

author

2017-01-25 16:04 UTC #11 lore raw

PATCH v3 1/1 Clean up plugin path handling

From: Christian Hesse <[email protected]...>

Drop --with-plugindir, instead use an environment variable PLUGINDIR to specify the plugin directory.

This generates a header file src/openvpn/plugindir.h which contains a define file the plugindir.

v2: The configure script can not evaluate the final $libdir path. So
use make to create a header file on the containing the final path.

v3: Fix whitespace errors and gitignore location.

Signed-off-by Christian Hesse <[email protected]...>

---
 .gitignore                 |  1 +
 configure.ac               | 14 ++++++--------
 src/openvpn/Makefile.am    | 11 +++++++++--
 src/openvpn/plugin.c       |  1 +
 src/openvpn/plugindir.h.in | 34 ++++++++++++++++++++++++++++++++++
 5 files changed, 51 insertions(+), 10 deletions(-)
 create mode 100644 src/openvpn/plugindir.h.in

.gitignore +1 -0

index e6da21c..28df0f8 100644
@@ -62,6 +62,7 @@ t_client_ips.rc
 tests/unit_tests/**/*_testdriver
 
 src/openvpn/openvpn
+src/openvpn/plugindir.h
 include/openvpn-plugin.h
 config-version.h
 nbproject

configure.ac +6 -8

index 8783109..f4c6bda 100644
@@ -301,13 +301,12 @@ AC_ARG_WITH(
 	[with_crypto_library="openssl"]
 )
 
-AC_ARG_WITH(
-	[plugindir],
-	[AS_HELP_STRING([--with-plugindir], [plugin directory @<:@default=LIBDIR/openvpn@:>@])],
-	,
-	[with_plugindir="\$(libdir)/openvpn/plugins"]
-)
-
+AC_ARG_VAR([PLUGINDIR], [Path of plug-in directory @<:@default=LIBDIR/openvpn/plugins@:>@])
+if test -n "${PLUGINDIR}"; then
+	plugindir="${PLUGINDIR}"
+else
+	plugindir="\${libdir}/openvpn/plugins"
+fi
 
 AC_DEFINE_UNQUOTED([TARGET_ALIAS], ["${host}"], [A string representing our host])
 case "$host" in

@@ -1270,7 +1269,6 @@ AM_CONDITIONAL([ENABLE_PLUGIN_DOWN_ROOT], [test "${enable_plugin_down_root}" = "
 AM_CONDITIONAL([ENABLE_CRYPTO], [test "${enable_crypto}" = "yes"])
 AM_CONDITIONAL([HAVE_LD_WRAP_SUPPORT], [test "${have_ld_wrap_support}" = "yes"])
 
-plugindir="${with_plugindir}"
 sampledir="\$(docdir)/sample"
 AC_SUBST([plugindir])
 AC_SUBST([sampledir])

src/openvpn/Makefile.am +9 -2

index bea294b..53f4676 100644
@@ -11,12 +11,19 @@
 
 include $(top_srcdir)/build/ltrc.inc
 
+plugindir.h: plugindir.h.in Makefile
+	$(AM_V_GEN)sed -e 's|\@PLUGINDIR\@|$(plugindir)|' \
+		$< > [email protected] && mv [email protected] $@
+
+plugin.c: plugindir.h
+
 MAINTAINERCLEANFILES = \
 	$(srcdir)/Makefile.in
 
 EXTRA_DIST = \
 	openvpn.vcxproj \
-	openvpn.vcxproj.filters
+	openvpn.vcxproj.filters \
+	plugindir.h.in
 
 AM_CPPFLAGS = \
 	-I$(top_srcdir)/include \

@@ -89,7 +96,7 @@ openvpn_SOURCES = \
 	perf.c perf.h \
 	pf.c pf.h pf-inline.h \
 	ping.c ping.h ping-inline.h \
-	plugin.c plugin.h \
+	plugin.c plugin.h plugindir.h \
 	pool.c pool.h \
 	proto.c proto.h \
 	proxy.c proxy.h \

src/openvpn/plugin.c +1 -0

index e530c0c..5af6d1e 100644
@@ -43,6 +43,7 @@
 #include "error.h"
 #include "misc.h"
 #include "plugin.h"
+#include "plugindir.h"
 #include "ssl_backend.h"
 #include "win32.h"
 #include "memdbg.h"

src/openvpn/plugindir.h.in new file +34 -0

new file mode 100644
index 0000000..6fa7b3e
@@ -0,0 +1,34 @@
+/*
+ *  OpenVPN -- An application to securely tunnel IP networks
+ *             over a single TCP/UDP port, with support for SSL/TLS-based
+ *             session authentication and key exchange,
+ *             packet encryption, packet authentication, and
+ *             packet compression.
+ *
+ *  Copyright (C) 2017 OpenVPN Technologies, Inc. <[email protected]...>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2
+ *  as published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program (see the file COPYING included with this
+ *  distribution); if not, write to the Free Software Foundation, Inc.,
+ *  59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+/*
+ * define plugindir
+ */
+
+#ifndef OPENVPN_PLUGINDIR_H
+#define OPENVPN_PLUGINDIR_H
+
+#define PLUGIN_LIBDIR "@PLUGINDIR@"
+
+#endif /* OPENVPN_PLUGINDIR_H */

Signature · 1 line

2.11.0

David Sommerseth

2017-01-25 17:06 UTC #12 lore raw

Re: [Openvpn-devel] [PATCH v3 1/1] Clean up plugin path handling

Quoted 14 lines · Christian Hesse

From: Christian Hesse <[email protected]...>
Drop --with-plugindir, instead use an environment variable PLUGINDIR to specify the plugin directory.
This generates a header file src/openvpn/plugindir.h which contains a define file the plugindir.
v2: The configure script can not evaluate the final $libdir path. So
use make to create a header file on the containing the final path.
v3: Fix whitespace errors and gitignore location.
Signed-off-by Christian Hesse <[email protected]...>

Quoted 8 lines

---
 .gitignore                 |  1 +
 configure.ac               | 14 ++++++--------
 src/openvpn/Makefile.am    | 11 +++++++++--
 src/openvpn/plugin.c       |  1 +
 src/openvpn/plugindir.h.in | 34 ++++++++++++++++++++++++++++++++++
 5 files changed, 51 insertions(+), 10 deletions(-)
 create mode 100644 src/openvpn/plugindir.h.in

Okay, some more feedback to this thread :)

As I've said, I think this makes sense. It provides a predictable plug-in directory which is easily accessible within the C code ... and it will always be set. So, Feature-ACK.

But I am wondering about the rationale of adding another header file (plugindir.h). Why not just add this information to openvpn-plugin.h.in, which results into openvpn-plugin.h ... and which plugin.c already includes via plugin.h ... I actually think this path can be quite useful for third-party plug-ins, being built outside of the openvpn source tree.

Further, we should then also kick out the #ifdef PLUGIN_LIBDIR in plugin_init_item() [plugin.c:237]. And we need to check what the #if defined(ENABLE_PLUGIN_SEARCH) is all about ... I do not see any possibilities to define the ENABLE_PLUGIN_SEARCH macro without changing a .c/.h file. Does this block even make sense to enable at all?

So, as this patch got "clean-up" and "plugin path handling" in the subject, lets cover a few more aspects of this clean-up :)

Signature · 4 lines

kind regards,

David Sommerseth
OpenVPN Technologies, Inc

Gert Doering

2017-01-25 17:37 UTC #13 lore raw

Re: [Openvpn-devel] [PATCH v3 1/1] Clean up plugin path handling

Hi,

Quoted 6 lines · David Sommerseth

But I am wondering about the rationale of adding another header file (plugindir.h). Why not just add this information to openvpn-plugin.h.in, which results into openvpn-plugin.h ... and which plugin.c already includes via plugin.h ... I actually think this path can be quite useful for third-party plug-ins, being built outside of the openvpn source tree.

gert

Signature · 4 lines

USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...

Christian Hesse

author

2017-01-25 18:57 UTC #14 lore raw

Re: [Openvpn-devel] [PATCH v3 1/1] Clean up plugin path handling

In reply to David Sommerseth

David Sommerseth <[email protected]...> on Wed, 2017/01/25 18:06:

Quoted 31 lines · Christian Hesse

Quoted 14 lines
From: Christian Hesse <[email protected]...>
Drop --with-plugindir, instead use an environment variable PLUGINDIR to specify the plugin directory.
This generates a header file src/openvpn/plugindir.h which contains a define file the plugindir.
v2: The configure script can not evaluate the final $libdir path. So
use make to create a header file on the containing the final path.
v3: Fix whitespace errors and gitignore location.
Signed-off-by Christian Hesse <[email protected]...>
Quoted 8 lines
---
 .gitignore                 |  1 +
 configure.ac               | 14 ++++++--------
 src/openvpn/Makefile.am    | 11 +++++++++--
 src/openvpn/plugin.c       |  1 +
 src/openvpn/plugindir.h.in | 34 ++++++++++++++++++++++++++++++++++
 5 files changed, 51 insertions(+), 10 deletions(-)
 create mode 100644 src/openvpn/plugindir.h.in  
Okay, some more feedback to this thread :)
As I've said, I think this makes sense. It provides a predictable plug-in directory which is easily accessible within the C code ... and it will always be set. So, Feature-ACK.

Quoted 6 lines

But I am wondering about the rationale of adding another header file (plugindir.h). Why not just add this information to openvpn-plugin.h.in, which results into openvpn-plugin.h ... and which plugin.c already includes via plugin.h ... I actually think this path can be quite useful for third-party plug-ins, being built outside of the openvpn source tree.

I did not want to be too intrusive. ;) Happy to hear that... About to prepare a new patch, stay tuned!

Signature · 3 lines

main(a){char*c=/*    Schoene Gruesse                         */"B?IJj;MEH"
"CX:;",b;for(a/*    Best regards             my address:    */=0;b=c[a++];)
putchar(b-1/(/*    Chris            cc -ox -xc - && ./x    */b/42*2-3)*42);}

Christian Hesse

author

2017-01-25 19:37 UTC #15 lore raw

PATCH v4 1/1 Clean up plugin path handling

From: Christian Hesse <[email protected]...>

Drop --with-plugindir, instead use an environment variable PLUGINDIR to specify the plugin directory.

This makes src/openvpn/plugin.h a template (moved the file to src/openvpn/plugin.h.in). The real header file is generated on the fly, including a define for the plugin path.
As the path is always available remove the compile time condition (and dead code) from src/openvpn/plugin.c.

v2: The configure script can not evaluate the final $libdir path. So
use make to create a header file on the containing the final path.

v3: Fix whitespace errors and gitignore location.

v4: No extra header file, generate src/openvpn/plugin.h on the fly.
Remove condition and dead code.

Signed-off-by Christian Hesse <[email protected]...>

---
 .gitignore                            |  1 +
 configure.ac                          | 14 ++++++--------
 src/openvpn/Makefile.am               |  7 ++++++-
 src/openvpn/plugin.c                  | 10 +---------
 src/openvpn/{plugin.h => plugin.h.in} |  2 ++
 5 files changed, 16 insertions(+), 18 deletions(-)
 rename src/openvpn/{plugin.h => plugin.h.in} (99%)

.gitignore +1 -0

index 30e289b..780be68 100644
@@ -63,6 +63,7 @@ t_client_ips.rc
 tests/unit_tests/**/*_testdriver
 
 src/openvpn/openvpn
+src/openvpn/plugin.h
 include/openvpn-plugin.h
 config-version.h
 nbproject

configure.ac +6 -8

index 79fb1ba..b29f8b4 100644
@@ -301,13 +301,12 @@ AC_ARG_WITH(
 	[with_crypto_library="openssl"]
 )
 
-AC_ARG_WITH(
-	[plugindir],
-	[AS_HELP_STRING([--with-plugindir], [plugin directory @<:@default=LIBDIR/openvpn@:>@])],
-	,
-	[with_plugindir="\$(libdir)/openvpn/plugins"]
-)
-
+AC_ARG_VAR([PLUGINDIR], [Path of plug-in directory @<:@default=LIBDIR/openvpn/plugins@:>@])
+if test -n "${PLUGINDIR}"; then
+	plugindir="${PLUGINDIR}"
+else
+	plugindir="\${libdir}/openvpn/plugins"
+fi
 
 AC_DEFINE_UNQUOTED([TARGET_ALIAS], ["${host}"], [A string representing our host])
 case "$host" in

@@ -1284,7 +1283,6 @@ AM_CONDITIONAL([ENABLE_PLUGIN_DOWN_ROOT], [test "${enable_plugin_down_root}" = "
 AM_CONDITIONAL([ENABLE_CRYPTO], [test "${enable_crypto}" = "yes"])
 AM_CONDITIONAL([HAVE_LD_WRAP_SUPPORT], [test "${have_ld_wrap_support}" = "yes"])
 
-plugindir="${with_plugindir}"
 sampledir="\$(docdir)/sample"
 AC_SUBST([plugindir])
 AC_SUBST([sampledir])

src/openvpn/Makefile.am +6 -1

index bea294b..4f8a315 100644
@@ -11,12 +11,17 @@
 
 include $(top_srcdir)/build/ltrc.inc
 
+plugin.h: plugin.h.in Makefile
+	$(AM_V_GEN)sed -e 's|\@PLUGINDIR\@|$(plugindir)|' \
+		$< > [email protected] && mv [email protected] $@
+
 MAINTAINERCLEANFILES = \
 	$(srcdir)/Makefile.in
 
 EXTRA_DIST = \
 	openvpn.vcxproj \
-	openvpn.vcxproj.filters
+	openvpn.vcxproj.filters \
+	plugin.h.in
 
 AM_CPPFLAGS = \
 	-I$(top_srcdir)/include \

src/openvpn/plugin.c +1 -9

index e530c0c..f777027 100644
@@ -234,23 +234,15 @@ plugin_init_item(struct plugin *p, const struct plugin_option *o)
 #ifndef _WIN32
 
     p->handle = NULL;
-#if defined(PLUGIN_LIBDIR)
+
     if (!absolute_pathname(p->so_pathname))
     {
         char full[PATH_MAX];
 
         openvpn_snprintf(full, sizeof(full), "%s/%s", PLUGIN_LIBDIR, p->so_pathname);
         p->handle = dlopen(full, RTLD_NOW);
-#if defined(ENABLE_PLUGIN_SEARCH)
-        if (!p->handle)
-        {
-            rel = true;
-            p->handle = dlopen(p->so_pathname, RTLD_NOW);
-        }
-#endif
     }
     else
-#endif
     {
         rel = !absolute_pathname(p->so_pathname);
         p->handle = dlopen(p->so_pathname, RTLD_NOW);

src/openvpn/plugin.h.in renamed from src/openvpn/plugin.h +2 -0

similarity index 99%
rename from src/openvpn/plugin.h
rename to src/openvpn/plugin.h.in
index 4ded529..3d6e312 100644
@@ -43,6 +43,8 @@
 
 #define MAX_PLUGINS 16
 
+#define PLUGIN_LIBDIR "@PLUGINDIR@"
+
 struct plugin_option {
     const char *so_pathname;
     const char **argv;

Signature · 1 line

2.11.0

Christian Hesse

author

2017-01-25 20:19 UTC #16 lore raw

PATCH v5 1/1 Clean up plugin path handling

From: Christian Hesse <[email protected]...>

Drop --with-plugindir, instead use an environment variable PLUGINDIR to specify the plugin directory.

This puts a define into include/openvpn-plugin.h.in which has the plugin directory.
The configure script does not know about the final plugin path. Thus we have to make make generate the final header file for us.

As the path is always available remove the compile time condition (and dead code) from src/openvpn/plugin.c.

v2: The configure script can not evaluate the final $libdir path. So
use make to create a header file on the containing the final path.

v3: Fix whitespace errors and gitignore location.

v4: No extra header file, generate src/openvpn/plugin.h on the fly.
Remove condition and dead code.

v5: Move the define to include/openvpn-plugin.h.in and let make generate
the final header file.

Signed-off-by Christian Hesse <[email protected]...>

---
 configure.ac                | 16 +++++++---------
 include/Makefile.am         |  7 +++++++
 include/openvpn-plugin.h.in |  2 ++
 src/openvpn/plugin.c        | 10 +---------
 4 files changed, 17 insertions(+), 18 deletions(-)

configure.ac +7 -9

index 79fb1ba..f305c54 100644
@@ -38,7 +38,7 @@ AC_DEFINE([OPENVPN_VERSION_MINOR], [PRODUCT_VERSION_MINOR], [OpenVPN minor versi
 AC_DEFINE([OPENVPN_VERSION_PATCH], ["PRODUCT_VERSION_PATCH"], [OpenVPN patch level - may be a string or integer])
 
 AC_CONFIG_AUX_DIR([.])
-AC_CONFIG_HEADERS([config.h include/openvpn-plugin.h])
+AC_CONFIG_HEADERS([config.h])
 AC_CONFIG_SRCDIR([src/openvpn/syshead.h])
 AC_CONFIG_MACRO_DIR([m4])

@@ -301,13 +301,12 @@ AC_ARG_WITH(
 	[with_crypto_library="openssl"]
 )
 
-AC_ARG_WITH(
-	[plugindir],
-	[AS_HELP_STRING([--with-plugindir], [plugin directory @<:@default=LIBDIR/openvpn@:>@])],
-	,
-	[with_plugindir="\$(libdir)/openvpn/plugins"]
-)
-
+AC_ARG_VAR([PLUGINDIR], [Path of plug-in directory @<:@default=LIBDIR/openvpn/plugins@:>@])
+if test -n "${PLUGINDIR}"; then
+	plugindir="${PLUGINDIR}"
+else
+	plugindir="\${libdir}/openvpn/plugins"
+fi
 
 AC_DEFINE_UNQUOTED([TARGET_ALIAS], ["${host}"], [A string representing our host])
 case "$host" in

@@ -1284,7 +1283,6 @@ AM_CONDITIONAL([ENABLE_PLUGIN_DOWN_ROOT], [test "${enable_plugin_down_root}" = "
 AM_CONDITIONAL([ENABLE_CRYPTO], [test "${enable_crypto}" = "yes"])
 AM_CONDITIONAL([HAVE_LD_WRAP_SUPPORT], [test "${have_ld_wrap_support}" = "yes"])
 
-plugindir="${with_plugindir}"
 sampledir="\$(docdir)/sample"
 AC_SUBST([plugindir])
 AC_SUBST([sampledir])

include/Makefile.am +7 -0

index a52c427..37962a6 100644
@@ -16,3 +16,10 @@ MAINTAINERCLEANFILES = \
 include_HEADERS = \
 	openvpn-plugin.h \
 	openvpn-msg.h
+
+openvpn-plugin.h: openvpn-plugin.h.in Makefile
+	$(AM_V_GEN)sed -e 's|\@PLUGINDIR\@|$(plugindir)|' \
+		-e 's|\@OPENVPN_VERSION_MAJOR\@|$(OPENVPN_VERSION_MAJOR)|' \
+		-e 's|\@OPENVPN_VERSION_MINOR\@|$(OPENVPN_VERSION_MINOR)|' \
+		-e 's|\@OPENVPN_VERSION_PATCH\@|$(OPENVPN_VERSION_PATCH)|' \
+               $< > [email protected] && mv [email protected] $@

include/openvpn-plugin.h.in +2 -0

index 0b30352..a084fc1 100644
@@ -27,6 +27,8 @@
 
 #define OPENVPN_PLUGIN_VERSION 3
 
+#define PLUGIN_LIBDIR "@PLUGINDIR@"
+
 #ifdef ENABLE_CRYPTO
 #ifdef ENABLE_CRYPTO_MBEDTLS
 #include <mbedtls/x509_crt.h>

src/openvpn/plugin.c +1 -9

index e530c0c..f777027 100644
@@ -234,23 +234,15 @@ plugin_init_item(struct plugin *p, const struct plugin_option *o)
 #ifndef _WIN32
 
     p->handle = NULL;
-#if defined(PLUGIN_LIBDIR)
+
     if (!absolute_pathname(p->so_pathname))
     {
         char full[PATH_MAX];
 
         openvpn_snprintf(full, sizeof(full), "%s/%s", PLUGIN_LIBDIR, p->so_pathname);
         p->handle = dlopen(full, RTLD_NOW);
-#if defined(ENABLE_PLUGIN_SEARCH)
-        if (!p->handle)
-        {
-            rel = true;
-            p->handle = dlopen(p->so_pathname, RTLD_NOW);
-        }
-#endif
     }
     else
-#endif
     {
         rel = !absolute_pathname(p->so_pathname);
         p->handle = dlopen(p->so_pathname, RTLD_NOW);

Signature · 1 line

2.11.0

David Sommerseth

2017-01-25 21:16 UTC #17 lore raw

Re: [Openvpn-devel] [PATCH applied] Clean up plugin path handling

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

ACK. This have been through enough rounds, and I currently don't see a better approach to the openvpn-plugin.h generation either. If we find better ways later on, lets fix it then.

I've tested this using gdb and investigating which code paths are used in different circumstances (--plugin with and without absolute path) and the expected plug-ins are loaded.

Your patch has been applied to the following branches

commit 4590c3831d0400096fab08aa1ed7f909da870ced (master) commit 05baa0ecf841852fbf29ef39ee9cd1d4cac990cf (release/2.4)

Author: Christian Hesse
Date:   Wed Jan 25 21:19:47 2017 +0100

     Clean up plugin path handling

     Signed-off-by: Christian Hesse <[email protected]...>
     Acked-by: David Sommerseth <[email protected]...>
     Message-Id: <[email protected]...>
     URL: http://www.mail-archive.com/search?l=mid&[email protected]...
     Signed-off-by: David Sommerseth <[email protected]...>

- --
kind regards,

David Sommerseth

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBAgAGBQJYiRWlAAoJEIbPlEyWcf3yQGEP/269/VKw/dW5KU/XrLIfwgT+ N5qzzKvalVnvlb8ki2GE4YaMp0dfjElup28iQi7wCbmAfHYiX9inc0KtQK7ggMHQ 6EMzgRnJla0mTVyKnFecSCVNFuDERcLc1a+K/Qq4JeiRUK3eyHbKZCA766xd3C+G /7x2SMPdXAqpAexuxO74Nz+GWuu7KLBY+F3LHYHAC4GiTHtd84pi2GpDveaod4K2 ttRqvnnZRn5JiuVl89To20faU1iw9RHvqFqZMOZfEq4fkRTK+0tJiMhJ5NcH1khm OiS/4x/HQtL79GqjrUkOCr3vyjoMRWd13Lz49eK6U4GFlcsivH09IL8toHfcdMjX DimgaYNHNo0+rxHOV53Jp6ADd97E93580Xe1drk6CUo7Sf0f4C25dtFzQJm67pkV L/45suNEL6jq2UvqCj/KR9AnawEVaGkhI9ikEkEJsDFgxi2fXWthuhe8SOXNugPA j+lyViB9vmTb2OVukEJKartAYBiOKd7dcTUUoE3U/XvEbNKMS7IsBMIDTinTUOlt la9Vu7DucKGXOeEIhbHHYuNV6Mff1eWNr8l7bx42mE1hSDHba5DwF0r0x2oTDZyn esew8w+R/ALVqeXCMx5bzwtdBngqAY0W90ePeuK2FyNFRsy83lZYbS6DVhmkXplQ OpS/rxQtFMDJI09cT+wz
=KBA6
-----END PGP SIGNATURE-----

Gert Doering

2017-01-26 08:05 UTC #18 lore raw

Re: [Openvpn-devel] [PATCH applied] Clean up plugin path handling

Hi,

Quoted 3 lines · David Sommerseth

ACK. This have been through enough rounds, and I currently don't see a better approach to the openvpn-plugin.h generation either. If we find better ways later on, lets fix it then.

This needs a bit of polishing... as you've undoubtly already seen, this change broke building on everything that is not Linux or MacOS...

Since we'll see this more often if developers cannot test on "other platforms", I'm happy to offer accounts on my FreeBSD 10.3 buildslave to regular contributors that want to test their patches on at least one BSD system to ensure there is no hidden bashism or GNU Make-ism (which seems to be the problem here).

Thinking of you and Christian here :-) - so if interested, please mail me a ssh pubkey and a static source IPv4/IPv6 network so I can create an account and open the ACLs (the buildslaves have fairly restricted network access and reachability).

gert

Signature · 4 lines

USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...

Christian Hesse

author

2017-01-27 08:49 UTC #19 lore raw

PATCH 1/1 remove GNUism and fix out-of-tree build

From: Christian Hesse <[email protected]...>

The plugin path handling cleanup (4590c383) introduced GNUism and broke out-of-tree builds.

Revert back to let configure generate the header file. Instead let make add an extra CFLAG that defines PLUGIN_LIBDIR.

Signed-off-by Christian Hesse <[email protected]...>

---
 configure.ac                | 2 +-
 include/Makefile.am         | 7 -------
 include/openvpn-plugin.h.in | 2 --
 src/openvpn/Makefile.am     | 4 +++-
 4 files changed, 4 insertions(+), 11 deletions(-)

configure.ac +1 -1

index f305c54..b29f8b4 100644
@@ -38,7 +38,7 @@ AC_DEFINE([OPENVPN_VERSION_MINOR], [PRODUCT_VERSION_MINOR], [OpenVPN minor versi
 AC_DEFINE([OPENVPN_VERSION_PATCH], ["PRODUCT_VERSION_PATCH"], [OpenVPN patch level - may be a string or integer])
 
 AC_CONFIG_AUX_DIR([.])
-AC_CONFIG_HEADERS([config.h])
+AC_CONFIG_HEADERS([config.h include/openvpn-plugin.h])
 AC_CONFIG_SRCDIR([src/openvpn/syshead.h])
 AC_CONFIG_MACRO_DIR([m4])

include/Makefile.am +0 -7

index 37962a6..a52c427 100644
@@ -16,10 +16,3 @@ MAINTAINERCLEANFILES = \
 include_HEADERS = \
 	openvpn-plugin.h \
 	openvpn-msg.h
-
-openvpn-plugin.h: openvpn-plugin.h.in Makefile
-	$(AM_V_GEN)sed -e 's|\@PLUGINDIR\@|$(plugindir)|' \
-		-e 's|\@OPENVPN_VERSION_MAJOR\@|$(OPENVPN_VERSION_MAJOR)|' \
-		-e 's|\@OPENVPN_VERSION_MINOR\@|$(OPENVPN_VERSION_MINOR)|' \
-		-e 's|\@OPENVPN_VERSION_PATCH\@|$(OPENVPN_VERSION_PATCH)|' \
-               $< > [email protected] && mv [email protected] $@

include/openvpn-plugin.h.in +0 -2

index a084fc1..0b30352 100644
@@ -27,8 +27,6 @@
 
 #define OPENVPN_PLUGIN_VERSION 3
 
-#define PLUGIN_LIBDIR "@PLUGINDIR@"
-
 #ifdef ENABLE_CRYPTO
 #ifdef ENABLE_CRYPTO_MBEDTLS
 #include <mbedtls/x509_crt.h>

src/openvpn/Makefile.am +3 -1

index bea294b..3f97855 100644
@@ -27,7 +27,9 @@ AM_CFLAGS = \
 	$(OPTIONAL_CRYPTO_CFLAGS) \
 	$(OPTIONAL_LZO_CFLAGS) \
 	$(OPTIONAL_LZ4_CFLAGS) \
-	$(OPTIONAL_PKCS11_HELPER_CFLAGS)
+	$(OPTIONAL_PKCS11_HELPER_CFLAGS) \
+	-DPLUGIN_LIBDIR=\"${plugindir}\"
+
 if WIN32
 # we want unicode entry point but not the macro
 AM_CFLAGS += -municode -UUNICODE

Signature · 1 line

2.11.0

Gert Doering

2017-01-27 09:55 UTC #20 lore raw

Re: [Openvpn-devel] [PATCH 1/1] remove GNUism and fix out-of-tree build

Hi,

Quoted 9 lines · Christian Hesse

From: Christian Hesse <[email protected]...>
The plugin path handling cleanup (4590c383) introduced GNUism and broke out-of-tree builds.
Revert back to let configure generate the header file. Instead let make add an extra CFLAG that defines PLUGIN_LIBDIR.
Signed-off-by Christian Hesse <[email protected]...>

ACK. Gets the job done and fixes the fallout :-)

Tested on FreeBSD 10.3, OpenBSD 6.0 and OpenSolaris 11 - so I blatantly assume it will also work on all the rest of the zoo.

For a v7 of this... we might consider whether to print out the plugin search path at "--help" or "--version" time (we already print the whole autoconf sermon)...

gert

Signature · 4 lines

USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...

David Sommerseth

2017-01-27 14:50 UTC #21 lore raw

Re: [Openvpn-devel] [PATCH applied] plugin: Remove GNUism in openvpn-plugin.h generation

In reply to Christian Hesse

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

I reworded the commit message slightly, as this isn't strictly bound to out-of-tree builds (which the original commit message stated), but related to GNU extensions in the Makefile which fails on non-GNU Make.

Your patch has been applied to the following branches

commit 631812fe29c69d0034628ab8321cb4016cb4fc2d (master) commit cb17255b1f7600b3255d1e018cfc426b40da4044 (release/2.4)

Author: Christian Hesse
Date:   Fri Jan 27 09:49:27 2017 +0100

     plugin: Remove GNUism in openvpn-plugin.h generation

     Signed-off-by: Christian Hesse <[email protected]...>
     Acked-by: Gert Doering <[email protected]...>
     Message-Id: <[email protected]...>
     URL: http://www.mail-archive.com/[email protected]/msg13966.html
     Signed-off-by: David Sommerseth <[email protected]...>

- --
kind regards,

David Sommerseth

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBAgAGBQJYi14UAAoJEIbPlEyWcf3y/ccP/j6RXkfFqxHC6aG19WyVPu9g 91N9RvrVFfDQZ6QULWkLzL98At+x35P+iOjs6qQRKWmnKv+U4LGjJbdS+t1AzABM Yhbovsa+0jcs+SfE1Du8asODt/5222aNtqjRO04edq3BBNoMi4u3AnxJ2ExyGXGF QCQ/ZpWaRu9eBCUZU3/9Cf9G8aMTg2xKGlswdvkUjvyEkj5x+YwVCM+htFYshEWD JFIVeVPfgBh60Sv6kpBP8C+2LrdMsY/n7s3EoCH+riwDO/J5ZrfNmUFrWRZ86V7M EomUn8DF2FOVKBfu59O2Ka4X9oLlHdik+YGjCbnRiSTIT1eezdUXgIzUpwBwg7eb klwPtkeDnNR0XUeuyqaXxC1THuQHi7qJQTcdbKTBBuabtIoAfBT3H2COC8dJOmMU MY1Ih5uf8K/kfG9AZMxfFr3IEt+yo/c+iEIg9sHZwfQNXDXnRH3zyyPApe1H5Sgi 9ubh7lK9+5gv+DJY8XNKS42xZYiVaHFb820dh8MMkhJ/j1Vzgcst7HiKo24ntxsa /nx0fyArFa5anYy3hoD36iOmDiZwnC0J1SkzgXqLCpXnO19kP96GBymFBHXrGF59 SrZOllkAeoldrsvp94ArLdMFQe4NSUS+KrPo+ACqAISxndNPUaQsHSSjYmptermN JnNGw9dRuxhuU83DwPko
=/dLr
-----END PGP SIGNATURE-----

David Sommerseth

2016-11-28 23:56 UTC #22 lore raw

In reply to Christian Hesse

Quoted 12 lines · Christian Hesse

From: Christian Hesse <[email protected]...>
Signed-off-by Christian Hesse <[email protected]...>
---
 configure.ac | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
configure.ac +0 -0
index f4073d0..d0fe889 100644

configure.ac AC_ARG_WITH( · line 309

Quoted 8 lines

configure.ac +1 -1

@@ -303,7 +303,7 @@ AC_ARG_WITH(
 
 AC_ARG_WITH(
 	[plugindir],
-	[AS_HELP_STRING([--with-plugindir], [plugin directory @<:@default=LIBDIR/openvpn@:>@])],
+	[AS_HELP_STRING([--with-plugindir], [plugin directory @<:@default=LIBDIR/openvpn/plugins@:>@])],
 	,
 	[with_plugindir="\$(libdir)/openvpn/plugins"]
 )

I think all this --with-pluigndir should be kicked right out and use PLUGINDIR environment variables instead, as setting this variable in this way will preserve it better when ./configure is automatically re-run by make.

This is more likely some left-over clean-up which never happened earlier on when we had the last big autotools overhaul.

Signature · 4 lines

kind regards,

David Sommerseth
OpenVPN Technologies, Inc