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

["Michał Górny" <mgorny@gentoo.org>]

[-- 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 --]