Bug 3314

Summary:	Simplify getVar() syntax
Product:	[Build System, Metadata & Runtime] BitBake	Reporter:	Evadeflow <evadeflow>
Component:	bitbake	Assignee:	Richard Purdie <richard.purdie>
Status:	RESOLVED FIXED	QA Contact:
Severity:	enhancement
Priority:	Low	CC:	poky.bs.watcher, poky.watcher, sgw, Viktor.Engelmann
Version:	unspecified
Target Milestone:	Future
Hardware:	x86
OS:	Multiple
Whiteboard:
OS type for building Yocto:	---	Type of Regression:	---
Verified:		Documentation change:	Yes (doc changes required)

Description Evadeflow 2012-10-20 16:07:36 UTC

For Yocto 1.4, it would be nice to address the following...

If you search for occurrences of getVar() in every Yocto file matching the following patterns:

  - *.bb
  - *.inc
  - *.bbclass
  - *.bbappend
  - *.conf

there are 1417 hits. Of those, 1304 of them pass True or 1. That leaves some 113 instances that either do not pass an argument, or pass False/0.  I'm not sure why the default value for the 'expand' argument was chosen to be False, but it definitely decreases the readability of recipes.  Lines like this (from meta/recipes-devtools/gcc/gcc-common.inc) are hard for humans to parse:

> if d.getVar('ARMPKGSFX_EABI', True) == "hf" and d.getVar('TRANSLATED_TARGET_ARCH', True) == "arm":
 
A much nicer syntax would be:

> if d.get('ARMPKGSFX_EABI') == "hf" and d.get('TRANSLATED_TARGET_ARCH') == "arm":

or even better:

> if d['ARMPKGSFX_EABI'] == "hf" and d['TRANSLATED_TARGET_ARCH'] == "arm":


Unfortunately, I see that the DataSmart class already defines this function:

>    def __getitem__(self, item):
>        value = self.getVar(item, False)
>        if value is None:
>            raise KeyError(item)
>        else:
>            return value

So, the ['foo'] syntax is supported, but it doesn't do the right thing for the vast majority of cases?

Can we somehow add a more user-friendly getVar syntax while still maintaining backward-compatibility with old recipes? Maybe get() and getx() overloads? I dunno. All I can say is, the current syntax leaves a lot to be desired, IMHO...

Comment 1 Richard Purdie 2012-10-23 08:50:04 UTC

For what its worth you will hear no argument from anyone that the default should have been True. Changing that is now rather hard with some serious backwards compatibility issues however so we're relcutant to touch this issue, particularly when its more cosmetic and there are bigger issues out there.

Comment 2 Evadeflow 2012-10-23 22:14:51 UTC

(In reply to comment #1)
> For what its worth you will hear no argument from anyone that the default
> should have been True. Changing that is now rather hard with some serious
> backwards compatibility issues however so we're relcutant to touch this
> issue, particularly when its more cosmetic and there are bigger issues out
> there.

I can totally understand this being a low priority. And it would have to be done very carefully. It seems solvable by introducing new functions with the desired semantics, but leaving the old functions in place for a couple of releases, with deprecation warnings. (The warnings could even be off by default, enabled only by an appropriate commandline arg.)

Perhaps there are other readability enhancements in this same vein that could be lumped together in a future release. Building a cross toolchain and compiling an entire distribution for (potentially) multiple embedded targets is so complex that occasional wrong turns are inevitable. Having a semi-formal mechanism for deprecating certain parts of the API might be useful going forward, time and resources permitting.

Comment 3 Richard Purdie 2016-03-20 09:59:41 UTC

For what its worth, we've at least made the expansion parameter mandatory for this release (2.1) which means we have options in future.

Comment 4 Viktor Engelmann 2016-12-06 13:54:07 UTC

In 2.2, expand doesn't even have a default value. Because of that, I get lots of ExpansionError exceptions, saying getVar() missing 1 required positional argument: 'expand'.

Comment 5 Richard Purdie 2016-12-08 14:16:11 UTC

For 2.3 we've made the default to expand. This completes the transition, the only remaining piece is to remove the default values which are no longer needed.