[gentoo-dev] [PATCH systemd.eclass] Introduce systemd_install_serviced().

[gentoo-dev] [PATCH systemd.eclass] Introduce systemd_install_serviced().

[Michał Górny]

This function can be used to install service configuration templates.
Usage:

  systemd_install_serviced "${FILESDIR}"/foo.service.conf

or:

  systemd_install_serviced "${FILESDIR}"/barbaz foo.service

with the latter specifying related service name explicitly, former
expecting it to match ${basename%.conf}. The files are installed as:

  /etc/systemd/system/foo.service.d/00gentoo.conf

They should be commented out templates that users can use to customize
the service easily.
---
 gx86/eclass/systemd.eclass | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/gx86/eclass/systemd.eclass b/gx86/eclass/systemd.eclass
index 4566631..1575b78 100644
--- a/gx86/eclass/systemd.eclass
+++ b/gx86/eclass/systemd.eclass
@@ -131,6 +131,32 @@ systemd_newunit() {
 	newins "${@}"
 }
 
+# @FUNCTION: systemd_install_serviced
+# @USAGE: <conf-file> [<service.d>]
+# @DESCRIPTION:
+# Install the file <conf-file> as service.d/00gentoo.conf template.
+# The <service.d> argument specifies the configured service name.
+# If not specified, the configuration file name will be used with .conf
+# suffix stripped (e.g. foo.service.conf -> foo.service).
+systemd_install_serviced() {
+	debug-print-function ${FUNCNAME} "${@}"
+
+	local src=${1}
+	local service=${2}
+
+	if [[ ! ${service} ]]; then
+		[[ ${src} == *.conf ]] || die "Source file needs .conf suffix"
+		service=${src##*/}
+		service=${service%.conf}
+	fi
+	# avoid potentially common mistake
+	[[ ${service} != *.d ]] || die "Service must not have .d suffix"
+
+	local INSDESTTREE
+	insinto /etc/systemd/system/"${service}".d
+	newins "${src}" 00gentoo.conf
+}
+
 # @FUNCTION: systemd_dotmpfilesd
 # @USAGE: tmpfilesd1 [...]
 # @DESCRIPTION:
-- 
1.8.3.2

Re: [gentoo-dev] [PATCH systemd.eclass] Introduce systemd_install_serviced().

[Gilles Dartiguelongue]

Le dimanche 08 septembre 2013 à 13:12 +0200, Michał Górny a écrit :
> This function can be used to install service configuration templates.
> Usage:
> 
>   systemd_install_serviced "${FILESDIR}"/foo.service.conf
> 
> or:
> 
>   systemd_install_serviced "${FILESDIR}"/barbaz foo.service
> 
> with the latter specifying related service name explicitly, former
> expecting it to match ${basename%.conf}. The files are installed as:
> 
>   /etc/systemd/system/foo.service.d/00gentoo.conf
> 
> They should be commented out templates that users can use to customize
> the service easily.

Looks like a good idea, do you have a few example packages where that
could be used ?

> ---
>  gx86/eclass/systemd.eclass | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/gx86/eclass/systemd.eclass b/gx86/eclass/systemd.eclass
> index 4566631..1575b78 100644
> --- a/gx86/eclass/systemd.eclass
> +++ b/gx86/eclass/systemd.eclass
> @@ -131,6 +131,32 @@ systemd_newunit() {
>  	newins "${@}"
>  }
>  
> +# @FUNCTION: systemd_install_serviced
> +# @USAGE: <conf-file> [<service.d>]
> +# @DESCRIPTION:
> +# Install the file <conf-file> as service.d/00gentoo.conf template.
> +# The <service.d> argument specifies the configured service name.
> +# If not specified, the configuration file name will be used with .conf
> +# suffix stripped (e.g. foo.service.conf -> foo.service).
> +systemd_install_serviced() {
> +	debug-print-function ${FUNCNAME} "${@}"
> +
> +	local src=${1}
> +	local service=${2}
> +
> +	if [[ ! ${service} ]]; then
> +		[[ ${src} == *.conf ]] || die "Source file needs .conf suffix"
> +		service=${src##*/}
> +		service=${service%.conf}
> +	fi
> +	# avoid potentially common mistake
> +	[[ ${service} != *.d ]] || die "Service must not have .d suffix"
> +
> +	local INSDESTTREE

I guess this is a leftover ?

> +	insinto /etc/systemd/system/"${service}".d
> +	newins "${src}" 00gentoo.conf
> +}
> +
>  # @FUNCTION: systemd_dotmpfilesd
>  # @USAGE: tmpfilesd1 [...]
>  # @DESCRIPTION:

-- 
Gilles Dartiguelongue <eva@gentoo.org>
Gentoo

Re: [gentoo-dev] [PATCH systemd.eclass] Introduce systemd_install_serviced().

[Pacho Ramos]

El dom, 08-09-2013 a las 17:14 +0200, Gilles Dartiguelongue escribió:
> Le dimanche 08 septembre 2013 à 13:12 +0200, Michał Górny a écrit :
> > This function can be used to install service configuration templates.
> > Usage:
> > 
> >   systemd_install_serviced "${FILESDIR}"/foo.service.conf
> > 
> > or:
> > 
> >   systemd_install_serviced "${FILESDIR}"/barbaz foo.service
> > 
> > with the latter specifying related service name explicitly, former
> > expecting it to match ${basename%.conf}. The files are installed as:
> > 
> >   /etc/systemd/system/foo.service.d/00gentoo.conf
> > 
> > They should be commented out templates that users can use to customize
> > the service easily.
> 
> Looks like a good idea, do you have a few example packages where that
> could be used ?

For example distcc or ntp will use them to tell them what IPs should be
allowed or what servers should be used to sync time. This are for some
cases where we need to let people pass some options to the services
without using conf.d file (that some times is not fully compatible with
systemd).

Re: [gentoo-dev] [PATCH systemd.eclass] Introduce systemd_install_serviced().

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 2567 bytes --]

Dnia 2013-09-08, o godz. 17:14:54
Gilles Dartiguelongue <eva@gentoo.org> napisał(a):

> Le dimanche 08 septembre 2013 à 13:12 +0200, Michał Górny a écrit :
> > This function can be used to install service configuration templates.
> > Usage:
> > 
> >   systemd_install_serviced "${FILESDIR}"/foo.service.conf
> > 
> > or:
> > 
> >   systemd_install_serviced "${FILESDIR}"/barbaz foo.service
> > 
> > with the latter specifying related service name explicitly, former
> > expecting it to match ${basename%.conf}. The files are installed as:
> > 
> >   /etc/systemd/system/foo.service.d/00gentoo.conf
> > 
> > They should be commented out templates that users can use to customize
> > the service easily.
> 
> Looks like a good idea, do you have a few example packages where that
> could be used ?

Pacho answered this one for me ;).

> > ---
> >  gx86/eclass/systemd.eclass | 26 ++++++++++++++++++++++++++
> >  1 file changed, 26 insertions(+)
> > 
> > diff --git a/gx86/eclass/systemd.eclass b/gx86/eclass/systemd.eclass
> > index 4566631..1575b78 100644
> > --- a/gx86/eclass/systemd.eclass
> > +++ b/gx86/eclass/systemd.eclass
> > @@ -131,6 +131,32 @@ systemd_newunit() {
> >  	newins "${@}"
> >  }
> >  
> > +# @FUNCTION: systemd_install_serviced
> > +# @USAGE: <conf-file> [<service.d>]
> > +# @DESCRIPTION:
> > +# Install the file <conf-file> as service.d/00gentoo.conf template.
> > +# The <service.d> argument specifies the configured service name.
> > +# If not specified, the configuration file name will be used with .conf
> > +# suffix stripped (e.g. foo.service.conf -> foo.service).
> > +systemd_install_serviced() {
> > +	debug-print-function ${FUNCNAME} "${@}"
> > +
> > +	local src=${1}
> > +	local service=${2}
> > +
> > +	if [[ ! ${service} ]]; then
> > +		[[ ${src} == *.conf ]] || die "Source file needs .conf suffix"
> > +		service=${src##*/}
> > +		service=${service%.conf}
> > +	fi
> > +	# avoid potentially common mistake
> > +	[[ ${service} != *.d ]] || die "Service must not have .d suffix"
> > +
> > +	local INSDESTTREE
> 
> I guess this is a leftover ?

Nope. 'insinto' sets INSDESTTREE. Due to lack of proper scoping
support in bash, we need to localize this variable to restore previous
'insinto' scope after leaving the function.

> > +	insinto /etc/systemd/system/"${service}".d
> > +	newins "${src}" 00gentoo.conf
> > +}
> > +
> >  # @FUNCTION: systemd_dotmpfilesd
> >  # @USAGE: tmpfilesd1 [...]
> >  # @DESCRIPTION:

-- 
Best regards,
Michał Górny

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 966 bytes --]

Re: [gentoo-dev] [PATCH systemd.eclass] Introduce systemd_install_serviced().

[Alexis Ballier]

On Sun, 8 Sep 2013 17:46:28 +0200
Michał Górny <mgorny@gentoo.org> wrote:
> > > suffix" +
> > > +	local INSDESTTREE
> > 
> > I guess this is a leftover ?
> 
> Nope. 'insinto' sets INSDESTTREE. Due to lack of proper scoping
> support in bash, we need to localize this variable to restore previous
> 'insinto' scope after leaving the function.

its much better to use a subshell like the other eclasses for that

[gentoo-dev] Re: [PATCH systemd.eclass] Introduce systemd_install_serviced().

[Steven J. Long]

Michał Górny wrote:
> +systemd_install_serviced() {
> +	debug-print-function ${FUNCNAME} "${@}"
> +
> +	local src=${1}
> +	local service=${2}
> +
> +	if [[ ! ${service} ]]; then
> +		[[ ${src} == *.conf ]] || die "Source file needs .conf suffix"

I would hoist this check to before the if.

> +		service=${src##*/}
> +		service=${service%.conf}
> +	fi
> +	# avoid potentially common mistake
> +	[[ ${service} != *.d ]] || die "Service must not have .d suffix"

Double-negative reads badly:

  [[ $service = *.d ]] && die "$FUNCNAME: Service must not have .d suffix"

> Nope. 'insinto' sets INSDESTTREE. Due to lack of proper scoping
> support in bash, we need to localize this variable to restore previous
> 'insinto' scope after leaving the function.

Actually the only reason you are able to do that, is *because* bash has proper
scoping support.

-- 
#friendly-coders -- We're friendly, but we're not /that/ friendly ;-)

Re: [gentoo-dev] Re: [PATCH systemd.eclass] Introduce systemd_install_serviced().

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 1368 bytes --]

Dnia 2013-09-10, o godz. 11:57:31
"Steven J. Long" <slong@rathaus.eclipse.co.uk> napisał(a):

> Michał Górny wrote:
> > +systemd_install_serviced() {
> > +	debug-print-function ${FUNCNAME} "${@}"
> > +
> > +	local src=${1}
> > +	local service=${2}
> > +
> > +	if [[ ! ${service} ]]; then
> > +		[[ ${src} == *.conf ]] || die "Source file needs .conf suffix"
> 
> I would hoist this check to before the if.

Any suffix is allowed if ${service} is provided. Specific naming is
required only for service guessing.

> > +		service=${src##*/}
> > +		service=${service%.conf}
> > +	fi
> > +	# avoid potentially common mistake
> > +	[[ ${service} != *.d ]] || die "Service must not have .d suffix"
> 
> Double-negative reads badly:
> 
>   [[ $service = *.d ]] && die "$FUNCNAME: Service must not have .d suffix"

I agree.

> > Nope. 'insinto' sets INSDESTTREE. Due to lack of proper scoping
> > support in bash, we need to localize this variable to restore previous
> > 'insinto' scope after leaving the function.
> 
> Actually the only reason you are able to do that, is *because* bash has proper
> scoping support.

Proper scoping support would actually either disallow something like
this or disallow something designed as 'insinto'. The 'local' in bash
in the worst thing you could expect.

-- 
Best regards,
Michał Górny

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 966 bytes --]

Re: [gentoo-dev] Re: [PATCH systemd.eclass] Introduce systemd_install_serviced().

[Mike Gilbert]

On Tue, Sep 10, 2013 at 12:36 PM, Michał Górny <mgorny@gentoo.org> wrote:
>> > Nope. 'insinto' sets INSDESTTREE. Due to lack of proper scoping
>> > support in bash, we need to localize this variable to restore previous
>> > 'insinto' scope after leaving the function.
>>
>> Actually the only reason you are able to do that, is *because* bash has proper
>> scoping support.
>
> Proper scoping support would actually either disallow something like
> this or disallow something designed as 'insinto'. The 'local' in bash
> in the worst thing you could expect.
>

From what I can remember from college, It seems like a pretty typical
case of dynamic scope. Why do you find this to be bad?

Re: [gentoo-dev] [PATCH systemd.eclass] Introduce systemd_install_serviced().

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 703 bytes --]

Dnia 2013-09-08, o godz. 13:12:26
Michał Górny <mgorny@gentoo.org> napisał(a):

> This function can be used to install service configuration templates.
> Usage:
> 
>   systemd_install_serviced "${FILESDIR}"/foo.service.conf
> 
> or:
> 
>   systemd_install_serviced "${FILESDIR}"/barbaz foo.service
> 
> with the latter specifying related service name explicitly, former
> expecting it to match ${basename%.conf}. The files are installed as:
> 
>   /etc/systemd/system/foo.service.d/00gentoo.conf
> 
> They should be commented out templates that users can use to customize
> the service easily.

Improved wrt received feedback and committed.

-- 
Best regards,
Michał Górny

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 966 bytes --]