On 27/12/16 23:15, Christian Hesse wrote:
> From: Christian Hesse <m...@eworm.de>
> 
> If systemd is enabled we install unit files to $libdir/systemd/system
> (or the path specified by SYSTEMD_UNIT_DIR).
> The unit files are generated on the fly with matching $sbindir.
> 
> Signed-off-by: Christian Hesse <m...@eworm.de>
> ---
>  configure.ac                                       | 10 ++++++++
>  distro/Makefile.am                                 |  4 +---
>  distro/systemd/.gitignore                          |  1 +
>  distro/systemd/Makefile.am                         | 27 
> ++++++++++++++++++++++
>  ...-client@.service => openvpn-cli...@.service.in} |  2 +-
>  ...-server@.service => openvpn-ser...@.service.in} |  2 +-
>  6 files changed, 41 insertions(+), 5 deletions(-)
>  create mode 100644 distro/systemd/.gitignore
>  create mode 100644 distro/systemd/Makefile.am
>  rename distro/systemd/{openvpn-client@.service => 
> openvpn-cli...@.service.in} (89%)
>  rename distro/systemd/{openvpn-server@.service => 
> openvpn-ser...@.service.in} (83%)


Finally had some time to look at this!  Some comments below.

[...snip...]

> --- /dev/null
> +++ b/distro/systemd/.gitignore
> @@ -0,0 +1 @@
> +*.service
> \ No newline at end of file

I think it is better to put all of these things into the .gitignore file
in the project root directory.  I see that this have slipped through a
few times (./vendor, ./test/unit_tests and sample/sample-keys/) ... but
as do ignore directories in the "master" .gitignore, I think we should
have everything there.  It will be easier to know where to look.  And
rather split things up when that master file gets too long and complicated.


> diff --git a/distro/systemd/Makefile.am b/distro/systemd/Makefile.am
> new file mode 100644
> index 0000000..53a88c9
> --- /dev/null
> +++ b/distro/systemd/Makefile.am
> @@ -0,0 +1,27 @@
> +#
> +#  OpenVPN -- An application to securely tunnel IP networks
> +#             over a single 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. <sa...@openvpn.net>
> +#
> +
> +%.service: %.service.in Makefile
> +     $(AM_V_GEN)sed -e 's|\@sbindir\@|$(sbindir)|' \
> +             $< > $@.tmp && mv $@.tmp $@
> +
> +EXTRA_DIST = \
> +     openvpn-cli...@.service.in \
> +     openvpn-ser...@.service.in
> +
> +if ENABLE_SYSTEMD
> +systemdunitdir = $(systemdunitdir)

This conflicts with AC_SUBST([systemdunitdir]) in configure.ac.  So this
line should not be here.

[...snip....]

> diff --git a/distro/systemd/openvpn-client@.service 
> b/distro/systemd/openvpn-cli...@.service.in
> similarity index 89%
> rename from distro/systemd/openvpn-client@.service
> rename to distro/systemd/openvpn-cli...@.service.in
> index 5618af3..d9fd6b0 100644
> --- a/distro/systemd/openvpn-client@.service
> +++ b/distro/systemd/openvpn-cli...@.service.in
> @@ -12,7 +12,7 @@ PrivateTmp=true
>  RuntimeDirectory=openvpn-client
>  RuntimeDirectoryMode=0710
>  WorkingDirectory=/etc/openvpn/client
> -ExecStart=/usr/sbin/openvpn --suppress-timestamps --nobind --config %i.conf
> +ExecStart=@sbindir@ --suppress-timestamps --nobind --config %i.conf

It should be: ExecStart=@sbindir@/openvpn .... otherwise the generated
files are pointing at a only a directory.

[...snip....]

> diff --git a/distro/systemd/openvpn-server@.service 
> b/distro/systemd/openvpn-ser...@.service.in
> similarity index 83%
> rename from distro/systemd/openvpn-server@.service
> rename to distro/systemd/openvpn-ser...@.service.in
> index b9b4dba..a270982 100644
> --- a/distro/systemd/openvpn-server@.service
> +++ b/distro/systemd/openvpn-ser...@.service.in
> @@ -12,7 +12,7 @@ PrivateTmp=true
>  RuntimeDirectory=openvpn-server
>  RuntimeDirectoryMode=0710
>  WorkingDirectory=/etc/openvpn/server
> -ExecStart=/usr/sbin/openvpn --status %t/openvpn-server/status-%i.log 
> --status-version 2 --suppress-timestamps --config %i.conf
> +ExecStart=@sbindir@ --status %t/openvpn-server/status-%i.log 
> --status-version 2 --suppress-timestamps --config %i.conf

Same as above.


-- 
kind regards,

David Sommerseth
OpenVPN Technologies, Inc


Attachment: signature.asc
Description: OpenPGP digital signature

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to