[gentoo-dev] Re: gstreamer eclass review

[Duncan]

Gilles Dartiguelongue posted on Sun, 18 Nov 2012 20:06:30 +0100 as
excerpted:

It's admittedly a style thing thus pretty much up to the author, purely
bikeshedding in the original sense of the essay's trivial color choice,
but...

> # Even though xz-utils are in @system, they must still be added to DEPEND; see
> # http://archives.gentoo.org/gentoo-dev/msg_a0d4833eb314d1be5d5802a3b710e0a4.xml
> if [[ ${GST_TARBALL_SUFFIX} == "xz" ]]; then
> 	DEPEND="${DEPEND} app-arch/xz-utils"
> fi

Single-clause conditional tests (without an else clause) such as the above
can be trivially rewritten like so:

[[ ${GST_TARBALL_SUFFIX} == "xz" ]] && \
 	DEPEND="${DEPEND} app-arch/xz-utils"

Two lines (or one if short enough) instead of three and more concise (fi
line omitted entirely, "if" omitted, "; then" replaced with &&).

It's possible to do similar with if/then/else -> &&/||, but that may
require {} for clarity if not bash parsing, and IMO is not nearly as
clear as the more straightforward no-else-clause case.  So if there's
an else clause, I prefer if/then/else; if not, I prefer the && or || logic.

> if [[ ${PN} != ${GST_ORG_MODULE} ]]; then
> 	# Do not run test phase for invididual plugin ebuilds.
>	RESTRICT="test"
> fi

Another example, this one definitely a one-liner (plus comment,
"invididual" typo left intact)...

# Do not run test phase for invididual plugin ebuilds.
[[ ${PN} != ${GST_ORG_MODULE} ]] && RESTRICT="test"

Or, avoiding that ! in the test and changing the && to ||...

[[ ${PN} == ${GST_ORG_MODULE} ]] || RESTRICT="test"

Also, while we're here, "test" is a string-literal with no possibility of
spaces or anything else that could otherwise confuse bash, so needs no
quotes.  And I'm not sure if the != pattern matching trigger was
deliberate or not (see bash's "help [[" output).  Thus...

[[ ${PN} = ${GST_ORG_MODULE} ]] || RESTRICT=test

... or...

[[ ${PN} == ${GST_ORG_MODULE} ]] || RESTRICT=test

... with the single vs. double equals making the string match (single) vs
pattern match (double) explicit.  If the test-internal-not form is
retained, the string match form would be...

[[ ${PN} ! = ${GST_ORG_MODULE} ]] && RESTRICT=test

... while the pattern match form would retain the != that was used
(the distinction being the separating space, != is a pattern match
as is ==, ! = is a string match as is a single = ).


> # added to remove circular deps # 6/2/2006 - zaheerm
if [[ ${PN} != ${GST_ORG_MODULE} ]]; then
> 	RDEPEND="${RDEPEND} media-libs/${GST_ORG_MODULE}:${SLOT}"
> fi

Ditto... Further examples skipped.

> # @FUNCTION: gst-plugins10_src_install gst-plugins10_src_install() {
> 	gst-plugins10_find_plugin_dir
>
> 	if has "${EAPI:-0}" 0 1 2 3 ; then
> 		emake install DESTDIR="${D}" || die
>		[[ -e README ]] && dodoc README
> 	else
> 		default
> 	fi
>
> 	[[ ${GST_LA_PUNT} = "yes" ]] && prune_libtool_files --modules
> }

Here (last line before the function-closing "}", as I said above, I prefer
if/then/else where there's an else clause, so wouldn't change the upper
conditional) we see a counter-example, doing it just as I suggested.
The if/then version (keeping function indent) would look like this.

	if [[ ${GST_LA_PUNT} = "yes" ]]; then
		prune_libtool_files --modules
	fi

Of course regardless of that, the quotes around "yes" may be omitted as
it's a string literal.  (And here, the explicit single-equals string-
literal match is used, as opposed to the double-equals pattern match.  Of
course either one would work here as it's a trivial pattern, just noting
it for consistency.)

	[[ ${GST_LA_PUNT} = yes ]] && prune_libtool_files --modules


But as I said up top, that's (mostly, the pattern matching vs string
matching will occasionally bite if you're not on the lookout for it)
trivial style stuff.  You may well prefer the way it is now.  But in that
case, why the counter-example, omitting if/then?  (You may of course
fairly point out that consistency is a trivial style thing too. =:^)

-- 
Duncan - List replies preferred.   No HTML msgs.
"Every nonfree program has a lord, a master --
and if you use the program, he is your master."  Richard Stallman