diff mbox series

siggen: Fix dump of non-runtime sigdata

Message ID 20251027-fix-dumpsig-v1-1-465674c4fac3@pbarker.dev
State New
Headers show
Series siggen: Fix dump of non-runtime sigdata | expand

Commit Message

Paul Barker Oct. 27, 2025, 8:25 a.m. UTC
If the debug code is uncommented in SignatureGeneratorBasic.finalise(),
sigtask stamp file are created without runtime data such as
'runtaskdeps' and 'file_checksum_values'. If we then try to run
bitbake-dumpsig on one of these files, we will see a traceback as the
expected keys are not present in the sigdata dictionary:

    Traceback (most recent call last):
      File ".../bitbake/bin/bitbake-dumpsig", line 195, in <module>
        output = bb.siggen.dump_sigfile(options.sigdatafile1)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      File ".../bitbake/lib/bb/siggen.py", line 1266, in dump_sigfile
        computed_taskhash = calc_taskhash(a_data)
                            ^^^^^^^^^^^^^^^^^^^^^
      File ".../bitbake/lib/bb/siggen.py", line 1199, in calc_taskhash
        for dep in sigdata['runtaskdeps']:
                   ~~~~~~~^^^^^^^^^^^^^^^
    KeyError: 'runtaskdeps'

To fix this, treat these entries as empty lists if they are not present.

Signed-off-by: Paul Barker <paul@pbarker.dev>
---
Richard,

This patch attempts to fix an issue reported on IRC last week [1]. It
prevents the traceback, but I'll have to defer to you on whether the
taskhash calculated without 'runtimedeps' and 'file_checksum_values' is
meaningful.

I guess an alternative approach would be to call calc_taskhash in a try
block and not print out a task hash if the call fails.

Thanks!

[1]: https://irc.yoctoproject.org/irc/%23yocto.2025-10-24.log.html#yocto.2025-10-24.log.html#t2025-10-24T11:09:20
---
 lib/bb/siggen.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)


---
base-commit: 35a421cdcafb3595b9de5489ffdc567825400d26
change-id: 20251027-fix-dumpsig-e945637ad89f

Best regards,

Comments

Richard Purdie Oct. 27, 2025, 8:58 a.m. UTC | #1
On Mon, 2025-10-27 at 08:25 +0000, Paul Barker wrote:
> If the debug code is uncommented in SignatureGeneratorBasic.finalise(),
> sigtask stamp file are created without runtime data such as
> 'runtaskdeps' and 'file_checksum_values'. If we then try to run
> bitbake-dumpsig on one of these files, we will see a traceback as the
> expected keys are not present in the sigdata dictionary:
> 
>     Traceback (most recent call last):
>       File ".../bitbake/bin/bitbake-dumpsig", line 195, in <module>
>         output = bb.siggen.dump_sigfile(options.sigdatafile1)
>                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>       File ".../bitbake/lib/bb/siggen.py", line 1266, in dump_sigfile
>         computed_taskhash = calc_taskhash(a_data)
>                             ^^^^^^^^^^^^^^^^^^^^^
>       File ".../bitbake/lib/bb/siggen.py", line 1199, in calc_taskhash
>         for dep in sigdata['runtaskdeps']:
>                    ~~~~~~~^^^^^^^^^^^^^^^
>     KeyError: 'runtaskdeps'
> 
> To fix this, treat these entries as empty lists if they are not present.
> 
> Signed-off-by: Paul Barker <paul@pbarker.dev>
> ---
> Richard,
> 
> This patch attempts to fix an issue reported on IRC last week [1]. It
> prevents the traceback, but I'll have to defer to you on whether the
> taskhash calculated without 'runtimedeps' and 'file_checksum_values' is
> meaningful.
> 
> I guess an alternative approach would be to call calc_taskhash in a try
> block and not print out a task hash if the call fails.

Rather than compute a hash which will never work, we should just skip
calculating/printing it or print something indicating it can't be
calculated. I've not looked into the details yet but I will at some
point unless someone beats me to it with a different patch.

Cheers,

Richard

> 
> Thanks!
> 
> [1]: https://irc.yoctoproject.org/irc/%23yocto.2025-10-24.log.html#yocto.2025-10-24.log.html#t2025-10-24T11:09:20
> ---
>  lib/bb/siggen.py | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/bb/siggen.py b/lib/bb/siggen.py
> index 41eb6430122f..5355f499074f 100644
> --- a/lib/bb/siggen.py
> +++ b/lib/bb/siggen.py
> @@ -1196,10 +1196,10 @@ def calc_basehash(sigdata):
>  def calc_taskhash(sigdata):
>      data = sigdata['basehash']
>  
> -    for dep in sigdata['runtaskdeps']:
> +    for dep in sigdata.get('runtaskdeps', []):
>          data = data + sigdata['runtaskhashes'][dep]
>  
> -    for c in sigdata['file_checksum_values']:
> +    for c in sigdata.get('file_checksum_values', []):
>          if c[1]:
>              if "./" in c[0]:
>                  data = data + c[0]
> 
> ---
> base-commit: 35a421cdcafb3595b9de5489ffdc567825400d26
> change-id: 20251027-fix-dumpsig-e945637ad89f
> 
> Best regards,
Paul Barker Oct. 27, 2025, 9:16 a.m. UTC | #2
On Mon, 2025-10-27 at 08:58 +0000, Richard Purdie wrote:
> On Mon, 2025-10-27 at 08:25 +0000, Paul Barker wrote:
> > If the debug code is uncommented in SignatureGeneratorBasic.finalise(),
> > sigtask stamp file are created without runtime data such as
> > 'runtaskdeps' and 'file_checksum_values'. If we then try to run
> > bitbake-dumpsig on one of these files, we will see a traceback as the
> > expected keys are not present in the sigdata dictionary:
> > 
> >     Traceback (most recent call last):
> >       File ".../bitbake/bin/bitbake-dumpsig", line 195, in <module>
> >         output = bb.siggen.dump_sigfile(options.sigdatafile1)
> >                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >       File ".../bitbake/lib/bb/siggen.py", line 1266, in dump_sigfile
> >         computed_taskhash = calc_taskhash(a_data)
> >                             ^^^^^^^^^^^^^^^^^^^^^
> >       File ".../bitbake/lib/bb/siggen.py", line 1199, in calc_taskhash
> >         for dep in sigdata['runtaskdeps']:
> >                    ~~~~~~~^^^^^^^^^^^^^^^
> >     KeyError: 'runtaskdeps'
> > 
> > To fix this, treat these entries as empty lists if they are not present.
> > 
> > Signed-off-by: Paul Barker <paul@pbarker.dev>
> > ---
> > Richard,
> > 
> > This patch attempts to fix an issue reported on IRC last week [1]. It
> > prevents the traceback, but I'll have to defer to you on whether the
> > taskhash calculated without 'runtimedeps' and 'file_checksum_values' is
> > meaningful.
> > 
> > I guess an alternative approach would be to call calc_taskhash in a try
> > block and not print out a task hash if the call fails.
> 
> Rather than compute a hash which will never work, we should just skip
> calculating/printing it or print something indicating it can't be
> calculated. I've not looked into the details yet but I will at some
> point unless someone beats me to it with a different patch.

Is calc_taskhash likely to change in the forseeable future to depend on
additional keys in sigdata? If not, we could just check for 'runtimedeps' and
'file_checksum_values' in dump_sigfile before calling calc_taskhash and say
"unable to compute task hash" if they're missing.

I can send a v2 implementing that.

Thanks,
Richard Purdie Oct. 27, 2025, 9:59 a.m. UTC | #3
On Mon, 2025-10-27 at 09:16 +0000, Paul Barker wrote:
> On Mon, 2025-10-27 at 08:58 +0000, Richard Purdie wrote:
> > On Mon, 2025-10-27 at 08:25 +0000, Paul Barker wrote:
> > > If the debug code is uncommented in SignatureGeneratorBasic.finalise(),
> > > sigtask stamp file are created without runtime data such as
> > > 'runtaskdeps' and 'file_checksum_values'. If we then try to run
> > > bitbake-dumpsig on one of these files, we will see a traceback as the
> > > expected keys are not present in the sigdata dictionary:
> > > 
> > >     Traceback (most recent call last):
> > >       File ".../bitbake/bin/bitbake-dumpsig", line 195, in <module>
> > >         output = bb.siggen.dump_sigfile(options.sigdatafile1)
> > >                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > >       File ".../bitbake/lib/bb/siggen.py", line 1266, in dump_sigfile
> > >         computed_taskhash = calc_taskhash(a_data)
> > >                             ^^^^^^^^^^^^^^^^^^^^^
> > >       File ".../bitbake/lib/bb/siggen.py", line 1199, in calc_taskhash
> > >         for dep in sigdata['runtaskdeps']:
> > >                    ~~~~~~~^^^^^^^^^^^^^^^
> > >     KeyError: 'runtaskdeps'
> > > 
> > > To fix this, treat these entries as empty lists if they are not present.
> > > 
> > > Signed-off-by: Paul Barker <paul@pbarker.dev>
> > > ---
> > > Richard,
> > > 
> > > This patch attempts to fix an issue reported on IRC last week [1]. It
> > > prevents the traceback, but I'll have to defer to you on whether the
> > > taskhash calculated without 'runtimedeps' and 'file_checksum_values' is
> > > meaningful.
> > > 
> > > I guess an alternative approach would be to call calc_taskhash in a try
> > > block and not print out a task hash if the call fails.
> > 
> > Rather than compute a hash which will never work, we should just skip
> > calculating/printing it or print something indicating it can't be
> > calculated. I've not looked into the details yet but I will at some
> > point unless someone beats me to it with a different patch.
> 
> Is calc_taskhash likely to change in the forseeable future to depend on
> additional keys in sigdata? If not, we could just check for 'runtimedeps' and
> 'file_checksum_values' in dump_sigfile before calling calc_taskhash and say
> "unable to compute task hash" if they're missing.
> 
> I can send a v2 implementing that.

Potentially it can change but it wouldn't lose the runtimedeps so
keying off that is probably safe enough.

Cheers,

Richard
diff mbox series

Patch

diff --git a/lib/bb/siggen.py b/lib/bb/siggen.py
index 41eb6430122f..5355f499074f 100644
--- a/lib/bb/siggen.py
+++ b/lib/bb/siggen.py
@@ -1196,10 +1196,10 @@  def calc_basehash(sigdata):
 def calc_taskhash(sigdata):
     data = sigdata['basehash']
 
-    for dep in sigdata['runtaskdeps']:
+    for dep in sigdata.get('runtaskdeps', []):
         data = data + sigdata['runtaskhashes'][dep]
 
-    for c in sigdata['file_checksum_values']:
+    for c in sigdata.get('file_checksum_values', []):
         if c[1]:
             if "./" in c[0]:
                 data = data + c[0]