[bitbake-devel,dunfell,1.46,1/6] taskdata: Improve handling of regex in ASSUME_PROVIDED

Submitted by Steve Sakoman on June 30, 2020, 3:08 a.m. | Patch ID: 174070

Details

Message ID 7ee38d7f6eae1c938842ddcc3aae49e462569df3.1593486375.git.steve@sakoman.com
State New
Headers show

Commit Message

Steve Sakoman June 30, 2020, 3:08 a.m.
From: Richard Purdie <richard.purdie@linuxfoundation.org>

ASSUME_PROVIDED can take regexs however the current way of handling
this in code is suboptimal. It means that you can add something like:

DEPENDS += "texinfo-nativejunk-that-does-not-exist"

and if texinfo-native is in ASSUME_PROVIDED, no error will occur.

Update the code to only treat something as a regex if a start or end
anchor character is present (which wouldn't be valid in a recipe name).

[YOCTO #13893]

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
(cherry picked from commit 3d72e23109990970fbb1086923277af752168b4a)
Signed-off-by: Steve Sakoman <steve@sakoman.com>
---
 lib/bb/taskdata.py | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

Patch hide | download patch | download mbox

diff --git a/lib/bb/taskdata.py b/lib/bb/taskdata.py
index d13a1249..ffbaf362 100644
--- a/lib/bb/taskdata.py
+++ b/lib/bb/taskdata.py
@@ -21,8 +21,13 @@  def re_match_strings(target, strings):
     Whether or not the string 'target' matches
     any one string of the strings which can be regular expression string
     """
-    return any(name == target or re.match(name, target)
-               for name in strings)
+    for name in strings:
+        if name.startswith("^") or name.endswith("$"):
+            if re.match(name, target):
+                return True
+        elif name == target:
+            return True
+    return False
 
 class TaskEntry:
     def __init__(self):

Comments

Paul Barker June 30, 2020, 1:27 p.m.
On Tue, 30 Jun 2020 at 04:08, Steve Sakoman <steve@sakoman.com> wrote:
>
> From: Richard Purdie <richard.purdie@linuxfoundation.org>
>
> ASSUME_PROVIDED can take regexs however the current way of handling
> this in code is suboptimal. It means that you can add something like:
>
> DEPENDS += "texinfo-nativejunk-that-does-not-exist"
>
> and if texinfo-native is in ASSUME_PROVIDED, no error will occur.
>
> Update the code to only treat something as a regex if a start or end
> anchor character is present (which wouldn't be valid in a recipe name).
>
> [YOCTO #13893]
>
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> (cherry picked from commit 3d72e23109990970fbb1086923277af752168b4a)
> Signed-off-by: Steve Sakoman <steve@sakoman.com>
> ---
>  lib/bb/taskdata.py | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/lib/bb/taskdata.py b/lib/bb/taskdata.py
> index d13a1249..ffbaf362 100644
> --- a/lib/bb/taskdata.py
> +++ b/lib/bb/taskdata.py
> @@ -21,8 +21,13 @@ def re_match_strings(target, strings):
>      Whether or not the string 'target' matches
>      any one string of the strings which can be regular expression string
>      """
> -    return any(name == target or re.match(name, target)
> -               for name in strings)
> +    for name in strings:
> +        if name.startswith("^") or name.endswith("$"):
> +            if re.match(name, target):
> +                return True
> +        elif name == target:
> +            return True
> +    return False
>
>  class TaskEntry:
>      def __init__(self):
> --
> 2.17.1

I'm not sure we should be changing how ASSUME_PROVIDED is parsed on a
stable branch. If we do backport this we should at least issue a
warning where the behaviour changes (i.e. if re.match(name, target) is
true but name != target and name doesn't start with ^ or end with $).
Steve Sakoman June 30, 2020, 2:11 p.m.
On Tue, Jun 30, 2020 at 3:27 AM Paul Barker <pbarker@konsulko.com> wrote:
>
> On Tue, 30 Jun 2020 at 04:08, Steve Sakoman <steve@sakoman.com> wrote:
> >
> > From: Richard Purdie <richard.purdie@linuxfoundation.org>
> >
> > ASSUME_PROVIDED can take regexs however the current way of handling
> > this in code is suboptimal. It means that you can add something like:
> >
> > DEPENDS += "texinfo-nativejunk-that-does-not-exist"
> >
> > and if texinfo-native is in ASSUME_PROVIDED, no error will occur.
> >
> > Update the code to only treat something as a regex if a start or end
> > anchor character is present (which wouldn't be valid in a recipe name).
> >
> > [YOCTO #13893]
> >
> > Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> > (cherry picked from commit 3d72e23109990970fbb1086923277af752168b4a)
> > Signed-off-by: Steve Sakoman <steve@sakoman.com>
> > ---
> >  lib/bb/taskdata.py | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/bb/taskdata.py b/lib/bb/taskdata.py
> > index d13a1249..ffbaf362 100644
> > --- a/lib/bb/taskdata.py
> > +++ b/lib/bb/taskdata.py
> > @@ -21,8 +21,13 @@ def re_match_strings(target, strings):
> >      Whether or not the string 'target' matches
> >      any one string of the strings which can be regular expression string
> >      """
> > -    return any(name == target or re.match(name, target)
> > -               for name in strings)
> > +    for name in strings:
> > +        if name.startswith("^") or name.endswith("$"):
> > +            if re.match(name, target):
> > +                return True
> > +        elif name == target:
> > +            return True
> > +    return False
> >
> >  class TaskEntry:
> >      def __init__(self):
> > --
> > 2.17.1
>
> I'm not sure we should be changing how ASSUME_PROVIDED is parsed on a
> stable branch. If we do backport this we should at least issue a
> warning where the behaviour changes (i.e. if re.match(name, target) is
> true but name != target and name doesn't start with ^ or end with $).

I went back and forth on taking this patch since it is kind of a bug
fix. I decided to add it to the review list and see if anyone had
comments.

I'm inclined now to drop it since the bug condition is rare and it is
a behaviour change.

So I'll be removing it from the final pull request.

Thanks for reviewing the patch set, it is much appreciated.

Steve
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.

View/Reply Online (#11470): https://lists.openembedded.org/g/bitbake-devel/message/11470
Mute This Topic: https://lists.openembedded.org/mt/75207174/3617530
Group Owner: bitbake-devel+owner@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/bitbake-devel/unsub  [oe-patchwork@oe-patch.openembedded.org]
-=-=-=-=-=-=-=-=-=-=-=-