Gentoo Archives: gentoo-dev

From: "Michał Górny" <mgorny@g.o>
To: gentoo-dev@l.g.o
Cc: slong@××××××××××××××××××.uk
Subject: Re: [gentoo-dev] Re: [PATCH systemd.eclass] Introduce systemd_install_serviced().
Date: Tue, 10 Sep 2013 16:37:12
Message-Id: 20130910183609.6063f7a3@gentoo.org
In Reply to: [gentoo-dev] Re: [PATCH systemd.eclass] Introduce systemd_install_serviced(). by "Steven J. Long"
1 Dnia 2013-09-10, o godz. 11:57:31
2 "Steven J. Long" <slong@××××××××××××××××××.uk> napisał(a):
3
4 > Michał Górny wrote:
5 > > +systemd_install_serviced() {
6 > > + debug-print-function ${FUNCNAME} "${@}"
7 > > +
8 > > + local src=${1}
9 > > + local service=${2}
10 > > +
11 > > + if [[ ! ${service} ]]; then
12 > > + [[ ${src} == *.conf ]] || die "Source file needs .conf suffix"
13 >
14 > I would hoist this check to before the if.
15
16 Any suffix is allowed if ${service} is provided. Specific naming is
17 required only for service guessing.
18
19 > > + service=${src##*/}
20 > > + service=${service%.conf}
21 > > + fi
22 > > + # avoid potentially common mistake
23 > > + [[ ${service} != *.d ]] || die "Service must not have .d suffix"
24 >
25 > Double-negative reads badly:
26 >
27 > [[ $service = *.d ]] && die "$FUNCNAME: Service must not have .d suffix"
28
29 I agree.
30
31 > > Nope. 'insinto' sets INSDESTTREE. Due to lack of proper scoping
32 > > support in bash, we need to localize this variable to restore previous
33 > > 'insinto' scope after leaving the function.
34 >
35 > Actually the only reason you are able to do that, is *because* bash has proper
36 > scoping support.
37
38 Proper scoping support would actually either disallow something like
39 this or disallow something designed as 'insinto'. The 'local' in bash
40 in the worst thing you could expect.
41
42 --
43 Best regards,
44 Michał Górny

Attachments

File name MIME type
signature.asc application/pgp-signature

Replies