Bug 3314 - Simplify getVar() syntax
Summary: Simplify getVar() syntax
Status: RESOLVED FIXED
Alias: None
Product: BitBake
Classification: Build System, Metadata & Runtime
Component: bitbake (show other bugs)
Version: unspecified
Hardware: x86 Multiple
: Low enhancement
Target Milestone: Future
Assignee: Richard Purdie
QA Contact:
URL:
Whiteboard:
Depends on:
Blocks:
 
Reported: 2012-10-20 16:07 UTC by Evadeflow
Modified: 2016-12-08 14:16 UTC (History)
4 users (show)

See Also:
OS type for building Yocto: ---
Type of Regression: ---
Verified:
Documentation change: Yes (doc changes required)


Attachments

Note You need to log in before you can comment on or make changes to this bug.
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.