Re: [PATCH 2/3] data_smart: Allow numeric characters in overrides

[Peter Kjellerstedt <peter.kjellerstedt@axis.com>]

> -----Original Message-----
> From: bitbake-devel-bounces@lists.openembedded.org <bitbake-devel-
> bounces@lists.openembedded.org> On Behalf Of Richard Purdie
> Sent: den 14 december 2018 15:02
> To: bitbake-devel@lists.openembedded.org
> Subject: [bitbake-devel] [PATCH 2/3] data_smart: Allow numeric
> characters in overrides
> 
> We're seeing problems due to the way x86-64 is handled (or not handled)
> as an override. Relax the containts on overrides from being lowercase
> to being lowercase or numeric. This fixes problem where MACHINE=qemux86
> would work but MACHINE=qemux86-64 would fail the same tests.
> 
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> ---
>  lib/bb/data_smart.py | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/bb/data_smart.py b/lib/bb/data_smart.py
> index 297a2f45b4..c342adaa0a 100644
> --- a/lib/bb/data_smart.py
> +++ b/lib/bb/data_smart.py
> @@ -43,6 +43,7 @@ __setvar_regexp__ =
> re.compile(r'(?P<base>.*?)(?P<keyword>_append|_prepend|_remo
>  __expand_var_regexp__ = re.compile(r"\${[^{}@\n\t :]+}")
>  __expand_python_regexp__ = re.compile(r"\${@.+?}")
>  __whitespace_split__ = re.compile(r'(\s)')
> +__override_regexp__ = re.compile(r'[a-z0-9]+')
> 
>  def infer_caller_details(loginfo, parent = False, varval = True):
>      """Save the caller the trouble of specifying everything."""
> @@ -597,7 +598,7 @@ class DataSmart(MutableMapping):
>          # aka pay the cookie monster
>          override = var[var.rfind('_')+1:]
>          shortvar = var[:var.rfind('_')]
> -        while override and override.islower():
> +        while override and __override_regexp__.match(override):
>              if shortvar not in self.overridedata:
>                  self.overridedata[shortvar] = []
>              if [var, override] not in self.overridedata[shortvar]:
> --
> 2.19.1

I do not understand this commit. The commit explanation and the code change 
don't match up. The motivation in the commit message is that there was 
problems with overrides such as x86-64 containing digits and that the 
code should be changed to allow overrides to be lower case and numeric.
However, the modified code changes the test from using islower(), which as 
far as I can tell only validates the alpha-characters in the string and 
ignores all else, to using a regular expression r'[a-z0-9]+', which will 
match as long as the first character is a lower case character or a numeric 
character (note that there is no $ at the end of the regular expression).

Here are tests with islower(), which correctly validates "x86" and "x86-64" 
as lower case overrides, but not "fooBar":

>>> "x86".islower()
True
>>> "x86-64".islower()
True
>>> "fooBar".islower()
False

Here are the corresponding tests using the r'[a-z0-9]+' regular expression.
Note that it only matches "x86" from "x86-64" and "foo" from "fooBar", and 
will incorrectly allow "fooBar" as an override:

>>> re.match(r'[a-z0-9]+', "x86")
<_sre.SRE_Match object; span=(0, 3), match='x86'>
>>> re.match(r'[a-z0-9]+', "x86-64")
<_sre.SRE_Match object; span=(0, 3), match='x86'>
>>> re.match(r'[a-z0-9]+', "fooBar")
<_sre.SRE_Match object; span=(0, 3), match='foo'>

So why change from islower(), which as far as I can tell did the right thing 
from the beginning?

//Peter