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

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

> -----Original Message-----
> From: richard.purdie@linuxfoundation.org
> <richard.purdie@linuxfoundation.org>
> Sent: den 18 december 2018 22:17
> To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>; bitbake-
> devel@lists.openembedded.org
> Subject: Re: [bitbake-devel] [PATCH 2/3] data_smart: Allow numeric
> characters in overrides
> 
> On Tue, 2018-12-18 at 21:05 +0000, Peter Kjellerstedt wrote:
> > 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?
> 
> The commit is clearly wrong. The intent of the commit message is
> correct, it doesn't match what that regex actually does :(
> 
> Cheers,
> 
> Richard

I don't understand the commit message either. It says that there are 
problems with x86-64 because it contains digits, but x86 (which the 
commit message says works) contains digits too, so that cannot really 
be the case. If it is actually the dash in x86-64 that is supposedly 
a cause of problems, then islower() ignores both digits and non-alpha 
characters like the dash so that cannot really be a reason to change 
the code either. All in all I'm confused by this commit and what it 
is trying to solve...

//Peter