[OE-core] style suggestions for that whole "_append" and leading space thing

[Patrick Ohly]

On Mon, 2017-02-27 at 06:07 -0500, Robert P. J. Day wrote:
>   getting pedantic (as i am wont to do), occasionally i run across
> things like this, from meta/conf/distro/include/no-static-libs.inc:
> 
>   EXTRA_OECONF_append = "${DISABLE_STATIC}"
> 
> which, at first glance, simply seems wrong given the lack of a leading
> space, until one looks higher up in that same file to read:
> 
>   DISABLE_STATIC = " --disable-static"
> 
> where one finds the leading space as part of the variable itself. i
> find this potentially *really* confusing; consistent style suggests
> variables should be assigned their values without regard as to how
> they might be used later. subsequent references or usages of those
> variables should be responsible for making sure they're included
> properly, no?

In principle you are right, but probably it was done this way here to
avoid adding a space to EXTRA_OECONF in those cases where DISABLE_STATIC
is overridden to be empty. Consider DISABLE_STATIC an implementation
detail and then it becomes okay to set it as needed by the
implementation.

>   and the last line in that same file also suggests potential misuse:
> 
>   EXTRA_OECMAKE_append_pn-libical = "-DSHARED_ONLY=True"

Yes, that's just plain wrong and just happens to work by accident.

> p.s. would the same logic hold with lines like this?
> 
> meta/conf/machine/include/tune-ppce6500.inc:
> 
>   MACHINE_FEATURES_BACKFILL_CONSIDERED_append =
>      "${@bb.utils.contains('TUNE_FEATURES', 'e6500', ' qemu-usermode', '', d)}"
> 
> would it not be clearer if one were to write:
> 
>   MACHINE_FEATURES_BACKFILL_CONSIDERED_append =
>      " ${@bb.utils.contains('TUNE_FEATURES', 'e6500', 'qemu-usermode', '', d)}"

The existing line is better because it avoids adding a space when
nothing needs to be added.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.