[PATCH 1/3] build: Catch and error upon circular task references

[PATCH 1/3] build: Catch and error upon circular task references

[Richard Purdie]

If there are circular task references, error on them rather than show
a recursion error. A simple reproducer is:

"""
do_packageswu () {
       :
}

addtask do_packageswu after do_image_complete before do_image_qa
"""

into image_types.bbclass. There is code in runqueue to detect these but
we never get that far with the current codebase.

[YOCTO #13140]

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 lib/bb/build.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/bb/build.py b/lib/bb/build.py
index 1e062adb51..4249c0be2b 100644
--- a/lib/bb/build.py
+++ b/lib/bb/build.py
@@ -1034,6 +1034,8 @@ def tasksbetween(task_start, task_end, d):
     def follow_chain(task, endtask, chain=None):
         if not chain:
             chain = []
+        if task in chain:
+            bb.fatal("Circular task dependencies as %s depends itself via the chain %s" % (task, " -> ".join(chain)))
         chain.append(task)
         for othertask in tasks:
             if othertask == task:
-- 
2.32.0

[PATCH 2/3] data_smart: Improve error display for handled exceptions

[Richard Purdie]

We don't need tracebacks for BBHandledException. Reduces confusing output like:

ERROR: /meta/recipes-core/images/core-image-tiny-initramfs.bb: Circular task dependencies as do_image_complete depends itself via the chain do_image_complete -> do_packageswu -> do_image_qa -> do_image -> do_image_cpio
ERROR: ExpansionError during parsing /meta/recipes-core/images/core-image-tiny-initramfs.bb
Traceback (most recent call last):
  File "/bitbake/lib/bb/build.py", line 1050, in follow_chain(task='do_image_qa', endtask='do_build', chain=['do_image_complete', 'do_packageswu', 'do_image_qa', 'do_image', 'do_image_cpio']):
                     if task in deps:
    >                    follow_chain(othertask, endtask, chain)
             chain.pop()
  File "/bitbake/lib/bb/build.py", line 1050, in follow_chain(task='do_image', endtask='do_build', chain=['do_image_complete', 'do_packageswu', 'do_image_qa', 'do_image', 'do_image_cpio']):
                     if task in deps:
    >                    follow_chain(othertask, endtask, chain)
             chain.pop()
  File "/bitbake/lib/bb/build.py", line 1050, in follow_chain(task='do_image_cpio', endtask='do_build', chain=['do_image_complete', 'do_packageswu', 'do_image_qa', 'do_image', 'do_image_cpio']):
                     if task in deps:
    >                    follow_chain(othertask, endtask, chain)
             chain.pop()
  File "/bitbake/lib/bb/build.py", line 1038, in follow_chain(task='do_image_complete', endtask='do_build', chain=['do_image_complete', 'do_packageswu', 'do_image_qa', 'do_image', 'do_image_cpio']):
             if task in chain:
    >            bb.fatal("Circular task dependencies as %s depends itself via the chain %s?!" % (task, " -> ".join(chain)))
             chain.append(task)
  File "/bitbake/lib/bb/__init__.py", line 165, in fatal:
         mainlogger.critical(''.join(args), extra=kwargs)
    >    raise BBHandledException()

to the real error:

ERROR: /media/build1/poky/meta/recipes-core/images/core-image-tiny-initramfs.bb: Circular task dependencies as do_image_complete depends itself via the chain do_image_complete -> do_packageswu -> do_image_qa -> do_image -> do_image_cpio

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 lib/bb/data_smart.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/bb/data_smart.py b/lib/bb/data_smart.py
index 515d195fe8..e4afac64b2 100644
--- a/lib/bb/data_smart.py
+++ b/lib/bb/data_smart.py
@@ -418,6 +418,8 @@ class DataSmart(MutableMapping):
                 raise
             except bb.parse.SkipRecipe:
                 raise
+            except bb.BBHandledException:
+                raise
             except Exception as exc:
                 tb = sys.exc_info()[2]
                 raise ExpansionError(varname, s, exc).with_traceback(tb) from exc
-- 
2.32.0

[PATCH 3/3] fetch2: Add recursion guard

[Richard Purdie]

Users sometimes put ${S} references in ${SRC_URI} without realising this can be
problematic. Improve the error messages if they accidentally do.

[YOCTO #11593]

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 lib/bb/fetch2/__init__.py | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
index 47a4943369..d9e1599a05 100644
--- a/lib/bb/fetch2/__init__.py
+++ b/lib/bb/fetch2/__init__.py
@@ -754,6 +754,11 @@ def get_srcrev(d, method_name='sortable_revision'):
     that fetcher provides a method with the given name and the same signature as sortable_revision.
     """
 
+    recursion = d.getVar("__BBINSRCREV")
+    if recursion:
+        raise FetchError("There are recursive references in fetcher variables, likely through SRC_URI")
+    d.setVar("__BBINSRCREV", True)
+
     scms = []
     fetcher = Fetch(d.getVar('SRC_URI').split(), d)
     urldata = fetcher.ud
@@ -768,6 +773,7 @@ def get_srcrev(d, method_name='sortable_revision'):
         autoinc, rev = getattr(urldata[scms[0]].method, method_name)(urldata[scms[0]], d, urldata[scms[0]].names[0])
         if len(rev) > 10:
             rev = rev[:10]
+        d.delVar("__BBINSRCREV")
         if autoinc:
             return "AUTOINC+" + rev
         return rev
@@ -802,6 +808,7 @@ def get_srcrev(d, method_name='sortable_revision'):
     if seenautoinc:
         format = "AUTOINC+" + format
 
+    d.delVar("__BBINSRCREV")
     return format
 
 def localpath(url, d):
-- 
2.32.0

Re: [bitbake-devel] [PATCH 1/3] build: Catch and error upon circular task references

[Quentin Schulz]

Hi Richard,

On Fri, Sep 10, 2021 at 03:07:38PM +0100, Richard Purdie wrote:
> If there are circular task references, error on them rather than show
> a recursion error. A simple reproducer is:
> 
> """
> do_packageswu () {
>        :
> }
> 
> addtask do_packageswu after do_image_complete before do_image_qa
> """
> 
> into image_types.bbclass. There is code in runqueue to detect these but
> we never get that far with the current codebase.
> 
> [YOCTO #13140]
> 
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> ---
>  lib/bb/build.py | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/lib/bb/build.py b/lib/bb/build.py
> index 1e062adb51..4249c0be2b 100644
> --- a/lib/bb/build.py
> +++ b/lib/bb/build.py
> @@ -1034,6 +1034,8 @@ def tasksbetween(task_start, task_end, d):
>      def follow_chain(task, endtask, chain=None):
>          if not chain:
>              chain = []
> +        if task in chain:
> +            bb.fatal("Circular task dependencies as %s depends itself via the chain %s" % (task, " -> ".join(chain)))

depends +on itself?

Cheers,
Quentin

Re: [bitbake-devel] [PATCH 3/3] fetch2: Add recursion guard

[Quentin Schulz]

Hi Richard,

On Fri, Sep 10, 2021 at 03:07:40PM +0100, Richard Purdie wrote:
> Users sometimes put ${S} references in ${SRC_URI} without realising this can be
> problematic. Improve the error messages if they accidentally do.
> 

I think we could also add this check to
insane.bbclass:package_qa_check_src_uri ?

Cheers,
Quentin

Re: [bitbake-devel] [PATCH 3/3] fetch2: Add recursion guard

[Richard Purdie]

On Fri, 2021-09-10 at 16:48 +0200, Quentin Schulz wrote:
> Hi Richard,
> 
> On Fri, Sep 10, 2021 at 03:07:40PM +0100, Richard Purdie wrote:
> > Users sometimes put ${S} references in ${SRC_URI} without realising this can be
> > problematic. Improve the error messages if they accidentally do.
> > 
> 
> I think we could also add this check to
> insane.bbclass:package_qa_check_src_uri ?

It is a nice idea but would be problematic as it causes a parsing failure
unfortunately.

Cheers,

Richard

Re: [bitbake-devel] [PATCH 1/3] build: Catch and error upon circular task references

[Richard Purdie]

On Fri, 2021-09-10 at 16:42 +0200, Quentin Schulz wrote:
> Hi Richard,
> 
> On Fri, Sep 10, 2021 at 03:07:38PM +0100, Richard Purdie wrote:
> > If there are circular task references, error on them rather than show
> > a recursion error. A simple reproducer is:
> > 
> > """
> > do_packageswu () {
> >        :
> > }
> > 
> > addtask do_packageswu after do_image_complete before do_image_qa
> > """
> > 
> > into image_types.bbclass. There is code in runqueue to detect these but
> > we never get that far with the current codebase.
> > 
> > [YOCTO #13140]
> > 
> > Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> > ---
> >  lib/bb/build.py | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/lib/bb/build.py b/lib/bb/build.py
> > index 1e062adb51..4249c0be2b 100644
> > --- a/lib/bb/build.py
> > +++ b/lib/bb/build.py
> > @@ -1034,6 +1034,8 @@ def tasksbetween(task_start, task_end, d):
> >      def follow_chain(task, endtask, chain=None):
> >          if not chain:
> >              chain = []
> > +        if task in chain:
> > +            bb.fatal("Circular task dependencies as %s depends itself via the chain %s" % (task, " -> ".join(chain)))
> 
> depends +on itself?

Well spotted, tweaked in master-next, thanks.

Cheers,

Richard