Gentoo Archives: gentoo-dev

From: Alec Warner <antarus@g.o>
To: gentoo-dev@l.g.o
Subject: Re: [gentoo-dev] Re: gstreamer eclass review
Date: Sun, 18 Nov 2012 23:16:46
Message-Id: CAAr7Pr97C9y=+-zaH_5FRErBYMqsG9RgD_hnc42CMi7X8m_vUQ@mail.gmail.com
In Reply to: [gentoo-dev] Re: gstreamer eclass review by Duncan <1i5t5.duncan@cox.net>
1 On Sun, Nov 18, 2012 at 12:45 PM, Duncan <1i5t5.duncan@×××.net> wrote:
2 > Gilles Dartiguelongue posted on Sun, 18 Nov 2012 20:06:30 +0100 as
3 > excerpted:
4 >
5 > It's admittedly a style thing thus pretty much up to the author, purely
6 > bikeshedding in the original sense of the essay's trivial color choice,
7 > but...
8 >
9 >> # Even though xz-utils are in @system, they must still be added to DEPEND; see
10 >> # http://archives.gentoo.org/gentoo-dev/msg_a0d4833eb314d1be5d5802a3b710e0a4.xml
11 >> if [[ ${GST_TARBALL_SUFFIX} == "xz" ]]; then
12 >> DEPEND="${DEPEND} app-arch/xz-utils"
13 >> fi
14 >
15 > Single-clause conditional tests (without an else clause) such as the above
16 > can be trivially rewritten like so:
17 >
18 > [[ ${GST_TARBALL_SUFFIX} == "xz" ]] && \
19 > DEPEND="${DEPEND} app-arch/xz-utils"
20 >
21 > Two lines (or one if short enough) instead of three and more concise (fi
22 > line omitted entirely, "if" omitted, "; then" replaced with &&).
23
24 http://www.quickmeme.com/meme/3ru1zx/
25
26 >
27 > It's possible to do similar with if/then/else -> &&/||, but that may
28 > require {} for clarity if not bash parsing, and IMO is not nearly as
29 > clear as the more straightforward no-else-clause case. So if there's
30 > an else clause, I prefer if/then/else; if not, I prefer the && or || logic.
31 >
32 >> if [[ ${PN} != ${GST_ORG_MODULE} ]]; then
33 >> # Do not run test phase for invididual plugin ebuilds.
34 >> RESTRICT="test"
35 >> fi
36 >
37 > Another example, this one definitely a one-liner (plus comment,
38 > "invididual" typo left intact)...
39 >
40 > # Do not run test phase for invididual plugin ebuilds.
41 > [[ ${PN} != ${GST_ORG_MODULE} ]] && RESTRICT="test"
42 >
43 > Or, avoiding that ! in the test and changing the && to ||...
44 >
45 > [[ ${PN} == ${GST_ORG_MODULE} ]] || RESTRICT="test"
46 >
47 > Also, while we're here, "test" is a string-literal with no possibility of
48 > spaces or anything else that could otherwise confuse bash, so needs no
49 > quotes. And I'm not sure if the != pattern matching trigger was
50 > deliberate or not (see bash's "help [[" output). Thus...
51 >
52 > [[ ${PN} = ${GST_ORG_MODULE} ]] || RESTRICT=test
53 >
54 > ... or...
55 >
56 > [[ ${PN} == ${GST_ORG_MODULE} ]] || RESTRICT=test
57 >
58 > ... with the single vs. double equals making the string match (single) vs
59 > pattern match (double) explicit. If the test-internal-not form is
60 > retained, the string match form would be...
61 >
62 > [[ ${PN} ! = ${GST_ORG_MODULE} ]] && RESTRICT=test
63 >
64 > ... while the pattern match form would retain the != that was used
65 > (the distinction being the separating space, != is a pattern match
66 > as is ==, ! = is a string match as is a single = ).
67 >
68 >
69 >> # added to remove circular deps # 6/2/2006 - zaheerm
70 > if [[ ${PN} != ${GST_ORG_MODULE} ]]; then
71 >> RDEPEND="${RDEPEND} media-libs/${GST_ORG_MODULE}:${SLOT}"
72 >> fi
73 >
74 > Ditto... Further examples skipped.
75 >
76 >> # @FUNCTION: gst-plugins10_src_install gst-plugins10_src_install() {
77 >> gst-plugins10_find_plugin_dir
78 >>
79 >> if has "${EAPI:-0}" 0 1 2 3 ; then
80 >> emake install DESTDIR="${D}" || die
81 >> [[ -e README ]] && dodoc README
82 >> else
83 >> default
84 >> fi
85 >>
86 >> [[ ${GST_LA_PUNT} = "yes" ]] && prune_libtool_files --modules
87 >> }
88 >
89 > Here (last line before the function-closing "}", as I said above, I prefer
90 > if/then/else where there's an else clause, so wouldn't change the upper
91 > conditional) we see a counter-example, doing it just as I suggested.
92 > The if/then version (keeping function indent) would look like this.
93 >
94 > if [[ ${GST_LA_PUNT} = "yes" ]]; then
95 > prune_libtool_files --modules
96 > fi
97 >
98 > Of course regardless of that, the quotes around "yes" may be omitted as
99 > it's a string literal. (And here, the explicit single-equals string-
100 > literal match is used, as opposed to the double-equals pattern match. Of
101 > course either one would work here as it's a trivial pattern, just noting
102 > it for consistency.)
103 >
104 > [[ ${GST_LA_PUNT} = yes ]] && prune_libtool_files --modules
105 >
106 >
107 > But as I said up top, that's (mostly, the pattern matching vs string
108 > matching will occasionally bite if you're not on the lookout for it)
109 > trivial style stuff. You may well prefer the way it is now. But in that
110 > case, why the counter-example, omitting if/then? (You may of course
111 > fairly point out that consistency is a trivial style thing too. =:^)
112 >
113 > --
114 > Duncan - List replies preferred. No HTML msgs.
115 > "Every nonfree program has a lord, a master --
116 > and if you use the program, he is your master." Richard Stallman
117 >
118 >