Message ID | GV1PR07MB91208FE4BD9DE31DFD4A7318A8742@GV1PR07MB9120.eurprd07.prod.outlook.com |
---|---|
State | New |
Headers | show |
Series | bb/data_smart: better old override syntax check | expand |
On Sat, 2024-09-28 at 09:00 +0200, Konrad Weihmann via lists.openembedded.org wrote: > when defining a function do_removesomething, > the parser warned about > > Variable do_removesomething contains an operation using the > old override syntax. Please convert this layer/metadata before > attempting to use with a newer bitbake > > correct it would be to search for a override operator at the end > of the string or for the next operator. > > To avoid running the expensive regex search on every setVar call > the current "cheap" check would be have to be triggered, > before the more precise check is run. > > Signed-off-by: Konrad Weihmann <kweihmann@outlook.com> > --- > lib/bb/data_smart.py | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/lib/bb/data_smart.py b/lib/bb/data_smart.py > index 4e15a43c2..0523744de 100644 > --- a/lib/bb/data_smart.py > +++ b/lib/bb/data_smart.py > @@ -539,14 +539,14 @@ class DataSmart(MutableMapping): > return var in self.overridedata > def setVar(self, var, value, **loginfo): > - > if not var.startswith("__anon_") and ("_append" in var or > "_prepend" in var or "_remove" in var): > - info = "%s" % var > - if "file" in loginfo: > - info += " file: %s" % loginfo["file"] > - if "line" in loginfo: > - info += " line: %s" % loginfo["line"] > - bb.fatal("Variable %s contains an operation using the > old > override syntax. Please convert this layer/metadata before attempting > to > use with a newer bitbake." % info) > + if re.search(r"(_append|_prepend|_remove)(_|$)", var) is > not None: > + info = "%s" % var > + if "file" in loginfo: > + info += " file: %s" % loginfo["file"] > + if "line" in loginfo: > + info += " line: %s" % loginfo["line"] > + bb.fatal("Variable %s contains an operation using > the > old override syntax. Please convert this layer/metadata before > attempting to use with a newer bitbake." % info) > shortvar = var.split(":", 1)[0] > if shortvar in self._var_renames: The patch appears line wrapped unfortunately (it appears as quoted above). Also, I would still prefer a pre-compiled regex. I appreciate python just have a last used cache but I'd prefer not to rely on python's implementation details which can and do change. Cheers, Richard
On 30.09.24 11:13, Richard Purdie wrote: > On Sat, 2024-09-28 at 09:00 +0200, Konrad Weihmann via > lists.openembedded.org wrote: >> when defining a function do_removesomething, >> the parser warned about >> >> Variable do_removesomething contains an operation using the >> old override syntax. Please convert this layer/metadata before >> attempting to use with a newer bitbake >> >> correct it would be to search for a override operator at the end >> of the string or for the next operator. >> >> To avoid running the expensive regex search on every setVar call >> the current "cheap" check would be have to be triggered, >> before the more precise check is run. >> >> Signed-off-by: Konrad Weihmann <kweihmann@outlook.com> >> --- >> lib/bb/data_smart.py | 14 +++++++------- >> 1 file changed, 7 insertions(+), 7 deletions(-) >> >> diff --git a/lib/bb/data_smart.py b/lib/bb/data_smart.py >> index 4e15a43c2..0523744de 100644 >> --- a/lib/bb/data_smart.py >> +++ b/lib/bb/data_smart.py >> @@ -539,14 +539,14 @@ class DataSmart(MutableMapping): >> return var in self.overridedata >> def setVar(self, var, value, **loginfo): >> - >> if not var.startswith("__anon_") and ("_append" in var or >> "_prepend" in var or "_remove" in var): >> - info = "%s" % var >> - if "file" in loginfo: >> - info += " file: %s" % loginfo["file"] >> - if "line" in loginfo: >> - info += " line: %s" % loginfo["line"] >> - bb.fatal("Variable %s contains an operation using the >> old >> override syntax. Please convert this layer/metadata before attempting >> to >> use with a newer bitbake." % info) >> + if re.search(r"(_append|_prepend|_remove)(_|$)", var) is >> not None: >> + info = "%s" % var >> + if "file" in loginfo: >> + info += " file: %s" % loginfo["file"] >> + if "line" in loginfo: >> + info += " line: %s" % loginfo["line"] >> + bb.fatal("Variable %s contains an operation using >> the >> old override syntax. Please convert this layer/metadata before >> attempting to use with a newer bitbake." % info) >> shortvar = var.split(":", 1)[0] >> if shortvar in self._var_renames: > > The patch appears line wrapped unfortunately (it appears as quoted > above). O_O - okay that might have been my mail client. Ever since Microsoft decided to drop plain TLS access to SMTP, I don't have any other way of contributing any kind of patches, but to pipe them into a client that supports oauth :-(. (P.s. before anyone jumps in gmail is not an option for me ;-) ) > > Also, I would still prefer a pre-compiled regex. I appreciate python > just have a last used cache but I'd prefer not to rely on python's > implementation details which can and do change. Wouldn't that have an overall penalty on the runtime behavior (I know it's only once)? With this inline expression, it would actually only be applicable on the pattern in question, so for an already converted layer set it runtime penalty is +/- 0, while with the precompiled expression we would add something on top for all users. Just want to make sure that is would be acceptable for you, as my understanding was that any penalty should be avoided > > Cheers, > > Richard
Hi Konrad, On 9/30/24 1:13 PM, Konrad Weihmann via lists.openembedded.org wrote: > > > On 30.09.24 11:13, Richard Purdie wrote: >> On Sat, 2024-09-28 at 09:00 +0200, Konrad Weihmann via >> lists.openembedded.org wrote: >>> when defining a function do_removesomething, >>> the parser warned about >>> >>> Variable do_removesomething contains an operation using the >>> old override syntax. Please convert this layer/metadata before >>> attempting to use with a newer bitbake >>> >>> correct it would be to search for a override operator at the end >>> of the string or for the next operator. >>> >>> To avoid running the expensive regex search on every setVar call >>> the current "cheap" check would be have to be triggered, >>> before the more precise check is run. >>> >>> Signed-off-by: Konrad Weihmann <kweihmann@outlook.com> >>> --- >>> lib/bb/data_smart.py | 14 +++++++------- >>> 1 file changed, 7 insertions(+), 7 deletions(-) >>> >>> diff --git a/lib/bb/data_smart.py b/lib/bb/data_smart.py >>> index 4e15a43c2..0523744de 100644 >>> --- a/lib/bb/data_smart.py >>> +++ b/lib/bb/data_smart.py >>> @@ -539,14 +539,14 @@ class DataSmart(MutableMapping): >>> return var in self.overridedata >>> def setVar(self, var, value, **loginfo): >>> - >>> if not var.startswith("__anon_") and ("_append" in var or >>> "_prepend" in var or "_remove" in var): >>> - info = "%s" % var >>> - if "file" in loginfo: >>> - info += " file: %s" % loginfo["file"] >>> - if "line" in loginfo: >>> - info += " line: %s" % loginfo["line"] >>> - bb.fatal("Variable %s contains an operation using the >>> old >>> override syntax. Please convert this layer/metadata before attempting >>> to >>> use with a newer bitbake." % info) >>> + if re.search(r"(_append|_prepend|_remove)(_|$)", var) is >>> not None: >>> + info = "%s" % var >>> + if "file" in loginfo: >>> + info += " file: %s" % loginfo["file"] >>> + if "line" in loginfo: >>> + info += " line: %s" % loginfo["line"] >>> + bb.fatal("Variable %s contains an operation using >>> the >>> old override syntax. Please convert this layer/metadata before >>> attempting to use with a newer bitbake." % info) >>> shortvar = var.split(":", 1)[0] >>> if shortvar in self._var_renames: >> >> The patch appears line wrapped unfortunately (it appears as quoted >> above). > > O_O - okay that might have been my mail client. > Ever since Microsoft decided to drop plain TLS access to SMTP, I don't > have any other way of contributing any kind of patches, but to pipe them > into a client that supports oauth :-(. > (P.s. before anyone jumps in gmail is not an option for me ;-) ) > bitbake-devel is available on lore.kernel.org so you should be able to send patches via the web endpoint, c.f. https://b4.docs.kernel.org/en/latest/contributor/send.html#web-endpoint (I have not used it this setup, as I use my personal mail address for sending patches because I got tired of MS and my IT deciding what's best for me :) ). >> >> Also, I would still prefer a pre-compiled regex. I appreciate python >> just have a last used cache but I'd prefer not to rely on python's >> implementation details which can and do change. > > Wouldn't that have an overall penalty on the runtime behavior (I know > it's only once)? > With this inline expression, it would actually only be applicable on the > pattern in question, so for an already converted layer set it runtime > penalty is +/- 0, while with the precompiled expression we would add > something on top for all users. > Just want to make sure that is would be acceptable for you, as my > understanding was that any penalty should be avoided > We could have a regex that is precompiled in the if condition so that the penalty only happens if we go into that code path at least once? (I initially only wanted to provide another way of contribution but I guess I can also chime in here). Cheers, Quentin
On Mon, 2024-09-30 at 13:13 +0200, Konrad Weihmann wrote: > > Also, I would still prefer a pre-compiled regex. I appreciate python > > just have a last used cache but I'd prefer not to rely on python's > > implementation details which can and do change. > > Wouldn't that have an overall penalty on the runtime behavior (I know > it's only once)? > With this inline expression, it would actually only be applicable on the > pattern in question, so for an already converted layer set it runtime > penalty is +/- 0, while with the precompiled expression we would add > something on top for all users. > Just want to make sure that is would be acceptable for you, as my > understanding was that any penalty should be avoided You'll see a number of regex compiles at the top of data_smart.py. I'd imagine one more there for this wouldn't be noticed. Cheers, Richard
On 30.09.24 13:19, Quentin Schulz wrote: > Hi Konrad, > > On 9/30/24 1:13 PM, Konrad Weihmann via lists.openembedded.org wrote: >> >> >> On 30.09.24 11:13, Richard Purdie wrote: >>> On Sat, 2024-09-28 at 09:00 +0200, Konrad Weihmann via >>> lists.openembedded.org wrote: >>>> when defining a function do_removesomething, >>>> the parser warned about >>>> >>>> Variable do_removesomething contains an operation using the >>>> old override syntax. Please convert this layer/metadata before >>>> attempting to use with a newer bitbake >>>> >>>> correct it would be to search for a override operator at the end >>>> of the string or for the next operator. >>>> >>>> To avoid running the expensive regex search on every setVar call >>>> the current "cheap" check would be have to be triggered, >>>> before the more precise check is run. >>>> >>>> Signed-off-by: Konrad Weihmann <kweihmann@outlook.com> >>>> --- >>>> lib/bb/data_smart.py | 14 +++++++------- >>>> 1 file changed, 7 insertions(+), 7 deletions(-) >>>> >>>> diff --git a/lib/bb/data_smart.py b/lib/bb/data_smart.py >>>> index 4e15a43c2..0523744de 100644 >>>> --- a/lib/bb/data_smart.py >>>> +++ b/lib/bb/data_smart.py >>>> @@ -539,14 +539,14 @@ class DataSmart(MutableMapping): >>>> return var in self.overridedata >>>> def setVar(self, var, value, **loginfo): >>>> - >>>> if not var.startswith("__anon_") and ("_append" in var or >>>> "_prepend" in var or "_remove" in var): >>>> - info = "%s" % var >>>> - if "file" in loginfo: >>>> - info += " file: %s" % loginfo["file"] >>>> - if "line" in loginfo: >>>> - info += " line: %s" % loginfo["line"] >>>> - bb.fatal("Variable %s contains an operation using the >>>> old >>>> override syntax. Please convert this layer/metadata before attempting >>>> to >>>> use with a newer bitbake." % info) >>>> + if re.search(r"(_append|_prepend|_remove)(_|$)", var) is >>>> not None: >>>> + info = "%s" % var >>>> + if "file" in loginfo: >>>> + info += " file: %s" % loginfo["file"] >>>> + if "line" in loginfo: >>>> + info += " line: %s" % loginfo["line"] >>>> + bb.fatal("Variable %s contains an operation using >>>> the >>>> old override syntax. Please convert this layer/metadata before >>>> attempting to use with a newer bitbake." % info) >>>> shortvar = var.split(":", 1)[0] >>>> if shortvar in self._var_renames: >>> >>> The patch appears line wrapped unfortunately (it appears as quoted >>> above). >> >> O_O - okay that might have been my mail client. >> Ever since Microsoft decided to drop plain TLS access to SMTP, I don't have any other way of contributing any kind of patches, but to pipe them into a client that supports oauth :-(. >> (P.s. before anyone jumps in gmail is not an option for me ;-) ) >> > > bitbake-devel is available on lore.kernel.org so you should be able to send patches via the web endpoint, c.f. https://b4.docs.kernel.org/en/latest/contributor/send.html#web-endpoint (I have not used it this setup, as I use my personal mail address for sending patches because I got tired of MS and my IT deciding what's best for me :) ). Looks promising, but I couldn't get it to work for now. Also I read of the limitation that the web endpoint applies only for projects hosted on kernel.org, not sure if that is the case for openembedded projects. If there is a _b4_submit endpoint available, this might be worth documenting in the contributors guide. I saw various projects starting to add .b4-config files to their repos, maybe that's the way to go. > >>> >>> Also, I would still prefer a pre-compiled regex. I appreciate python >>> just have a last used cache but I'd prefer not to rely on python's >>> implementation details which can and do change. >> >> Wouldn't that have an overall penalty on the runtime behavior (I know it's only once)? >> With this inline expression, it would actually only be applicable on the pattern in question, so for an already converted layer set it runtime penalty is +/- 0, while with the precompiled expression we would add something on top for all users. >> Just want to make sure that is would be acceptable for you, as my understanding was that any penalty should be avoided >> > > We could have a regex that is precompiled in the if condition so that the penalty only happens if we go into that code path at least once? (I initially only wanted to provide another way of contribution but I guess I can also chime in here). > > Cheers, > Quentin
Hi Konrad, On 10/3/24 10:18 AM, Konrad Weihmann wrote: > > > On 30.09.24 13:19, Quentin Schulz wrote: >> Hi Konrad, >> >> On 9/30/24 1:13 PM, Konrad Weihmann via lists.openembedded.org wrote: >>> >>> >>> On 30.09.24 11:13, Richard Purdie wrote: >>>> On Sat, 2024-09-28 at 09:00 +0200, Konrad Weihmann via >>>> lists.openembedded.org wrote: >>>>> when defining a function do_removesomething, >>>>> the parser warned about >>>>> >>>>> Variable do_removesomething contains an operation using the >>>>> old override syntax. Please convert this layer/metadata before >>>>> attempting to use with a newer bitbake >>>>> >>>>> correct it would be to search for a override operator at the end >>>>> of the string or for the next operator. >>>>> >>>>> To avoid running the expensive regex search on every setVar call >>>>> the current "cheap" check would be have to be triggered, >>>>> before the more precise check is run. >>>>> >>>>> Signed-off-by: Konrad Weihmann <kweihmann@outlook.com> >>>>> --- >>>>> lib/bb/data_smart.py | 14 +++++++------- >>>>> 1 file changed, 7 insertions(+), 7 deletions(-) >>>>> >>>>> diff --git a/lib/bb/data_smart.py b/lib/bb/data_smart.py >>>>> index 4e15a43c2..0523744de 100644 >>>>> --- a/lib/bb/data_smart.py >>>>> +++ b/lib/bb/data_smart.py >>>>> @@ -539,14 +539,14 @@ class DataSmart(MutableMapping): >>>>> return var in self.overridedata >>>>> def setVar(self, var, value, **loginfo): >>>>> - >>>>> if not var.startswith("__anon_") and ("_append" in var or >>>>> "_prepend" in var or "_remove" in var): >>>>> - info = "%s" % var >>>>> - if "file" in loginfo: >>>>> - info += " file: %s" % loginfo["file"] >>>>> - if "line" in loginfo: >>>>> - info += " line: %s" % loginfo["line"] >>>>> - bb.fatal("Variable %s contains an operation using the >>>>> old >>>>> override syntax. Please convert this layer/metadata before attempting >>>>> to >>>>> use with a newer bitbake." % info) >>>>> + if re.search(r"(_append|_prepend|_remove)(_|$)", var) is >>>>> not None: >>>>> + info = "%s" % var >>>>> + if "file" in loginfo: >>>>> + info += " file: %s" % loginfo["file"] >>>>> + if "line" in loginfo: >>>>> + info += " line: %s" % loginfo["line"] >>>>> + bb.fatal("Variable %s contains an operation using >>>>> the >>>>> old override syntax. Please convert this layer/metadata before >>>>> attempting to use with a newer bitbake." % info) >>>>> shortvar = var.split(":", 1)[0] >>>>> if shortvar in self._var_renames: >>>> >>>> The patch appears line wrapped unfortunately (it appears as quoted >>>> above). >>> >>> O_O - okay that might have been my mail client. >>> Ever since Microsoft decided to drop plain TLS access to SMTP, I >>> don't have any other way of contributing any kind of patches, but to >>> pipe them into a client that supports oauth :-(. >>> (P.s. before anyone jumps in gmail is not an option for me ;-) ) >>> >> >> bitbake-devel is available on lore.kernel.org so you should be able to >> send patches via the web endpoint, c.f. https:// >> eur02.safelinks.protection.outlook.com/? >> url=https%3A%2F%2Fb4.docs.kernel.org%2Fen%2Flatest%2Fcontributor%2Fsend.html%23web-endpoint&data=05%7C02%7Cquentin.schulz%40cherry.de%7C57edcb28fe7347f1e4d308dce383eb7f%7C5e0e1b5221b54e7b83bb514ec460677e%7C0%7C0%7C638635402949492410%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=9kIULshRlYhS%2F35Q6PKX%2B9wotgKIx%2B92SVKRXtbCa5s%3D&reserved=0 (I have not used it this setup, as I use my personal mail address for sending patches because I got tired of MS and my IT deciding what's best for me :) ). > > Looks promising, but I couldn't get it to work for now. > Also I read of the limitation that the web endpoint applies only for > projects hosted on kernel.org, not sure if that is the case for > openembedded projects. > https://lore.kernel.org/?q=yocto https://lore.kernel.org/?q=openembedded That should cover most of our repos? If a repo you want to contribute to doesn't appear there, I guess we could ask for its ML to be mirrored to lore.kernel.org via our and their sysadmins. > If there is a _b4_submit endpoint available, this might be worth > documenting in the contributors guide. > I saw various projects starting to add .b4-config files to their repos, > maybe that's the way to go. > Yes, but poky is making everything difficult by being a collection of source code from other repos. When one writes something to bitbake/ directory it should go to the bitbake ML, meta/ to OE-Core, docs/ to yocto-docs and most of the rest to poky. We also want to make sure that a patch doesn't make changes in multiple repos at the same time. I haven't looked at it in depth so far, I think it might be doable with some script(s) and a few .b4-config in the appropriate places. But having started to use b4 for every project with contribution by mail, I find it VERY helpful. Once we get that working for poky, it does make a lot of sense to document it everywhere it can be used :) Cheers, Quentin
diff --git a/lib/bb/data_smart.py b/lib/bb/data_smart.py index 4e15a43c2..0523744de 100644 --- a/lib/bb/data_smart.py +++ b/lib/bb/data_smart.py @@ -539,14 +539,14 @@ class DataSmart(MutableMapping): return var in self.overridedata def setVar(self, var, value, **loginfo): - if not var.startswith("__anon_") and ("_append" in var or "_prepend" in var or "_remove" in var): - info = "%s" % var - if "file" in loginfo: - info += " file: %s" % loginfo["file"] - if "line" in loginfo: - info += " line: %s" % loginfo["line"] - bb.fatal("Variable %s contains an operation using the old override syntax. Please convert this layer/metadata before attempting to use with a newer bitbake." % info) + if re.search(r"(_append|_prepend|_remove)(_|$)", var) is not None: + info = "%s" % var + if "file" in loginfo: + info += " file: %s" % loginfo["file"] + if "line" in loginfo: + info += " line: %s" % loginfo["line"] + bb.fatal("Variable %s contains an operation using the
when defining a function do_removesomething, the parser warned about Variable do_removesomething contains an operation using the old override syntax. Please convert this layer/metadata before attempting to use with a newer bitbake correct it would be to search for a override operator at the end of the string or for the next operator. To avoid running the expensive regex search on every setVar call the current "cheap" check would be have to be triggered, before the more precise check is run. Signed-off-by: Konrad Weihmann <kweihmann@outlook.com> --- lib/bb/data_smart.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) old override syntax. Please convert this layer/metadata before attempting to use with a newer bitbake." % info) shortvar = var.split(":", 1)[0] if shortvar in self._var_renames: