[PATCH 0/3] isar-sstate improvements

[PATCH 0/3] isar-sstate improvements

[Adriaan Schmidt]

Some minor improvements to the isar-sstate script. p1 is somewhat important
when migrating to bitbake 2.0, as is avoids crashes when having a cache with
both pre and post bitbake 2.0 signature files.

Adriaan Schmidt (3):
  fix(isar-sstate): catch errors in signature comparison
  isar-sstate: refactor exit codes and error messages
  docs(isar-sstate): update documentation of lint command

 scripts/isar-sstate | 42 ++++++++++++++++++++++++------------------
 1 file changed, 24 insertions(+), 18 deletions(-)

-- 
2.30.2

[PATCH 1/3] fix(isar-sstate): catch errors in signature comparison

[Adriaan Schmidt]

Those errors happen when the sstate cache contains both pre and post
bitbake-2.0 signatures, which have a different format (pickle vs. compressed json).

Signed-off-by: Adriaan Schmidt <adriaan.schmidt@siemens.com>
---
 scripts/isar-sstate | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/scripts/isar-sstate b/scripts/isar-sstate
index 8ea85edc..9f5c17b0 100755
--- a/scripts/isar-sstate
+++ b/scripts/isar-sstate
@@ -781,7 +781,10 @@ def sstate_analyze(source, target, **kwargs):
 
             local_file = source.download(s.path)
             remote_file = target.download(t.path)
-            out = compare_sigfiles(remote_file, local_file, recursecb, color=True)
+            try:
+                out = compare_sigfiles(remote_file, local_file, recursecb, color=True)
+            except:
+                out = ["Failed to compare signatures."]
             source.release(local_file)
             target.release(remote_file)
             # shorten hashes from 64 to 8 characters for better readability
-- 
2.30.2

[PATCH 2/3] isar-sstate: refactor exit codes and error messages

[Adriaan Schmidt]

The behavior of isar-sstate in error conditions is not consistent.
This patch tries to fix that:
- If an operation cannot be performed because source/target cannot be accessed
  it's a WARNING (useful when running in CI, so scripts can continue).
- The exit code returned by the script is either 0 (no error), or 1 (error), but
  never -1.
- Issues found be the lint operation are not returned as WARNINGS.

Signed-off-by: Adriaan Schmidt <adriaan.schmidt@siemens.com>
Signed-off-by: Felix Moessbauer <felix.moessbauer@siemens.com>
---
 scripts/isar-sstate | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/scripts/isar-sstate b/scripts/isar-sstate
index 9f5c17b0..b62aec47 100755
--- a/scripts/isar-sstate
+++ b/scripts/isar-sstate
@@ -602,10 +602,9 @@ def sstate_upload(source, target, verbose, **kwargs):
     if not os.path.isdir(source):
         print(f"WARNING: source {source} does not exist. Not uploading.")
         return 0
-
     if not target.exists() and not target.create():
-        print(f"ERROR: target {target} does not exist and could not be created.")
-        return -1
+        print(f"WARNING: target {target} does not exist and could not be created. Not uploading.")
+        return 0
 
     print(f"INFO: uploading {source} to {target}")
     os.chdir(source)
@@ -636,8 +635,7 @@ def sstate_clean(target, max_age, max_sig_age, verbose, **kwargs):
         seconds_per_unit = {'s': 1, 'm': 60, 'h': 3600, 'd': 86400, 'w': 604800}
         m = re.match(r'^(\d+)(w|d|h|m|s)?', x)
         if m is None:
-            print(f"ERROR: cannot parse MAX_AGE '{max_age}', needs to be a number followed by w|d|h|m|s")
-            sys.exit(-1)
+            return None
         unit = m.group(2)
         if unit is None:
             print("WARNING: MAX_AGE without unit, assuming 'days'")
@@ -645,12 +643,15 @@ def sstate_clean(target, max_age, max_sig_age, verbose, **kwargs):
         return int(m.group(1)) * seconds_per_unit[unit]
 
     max_age_seconds = convert_to_seconds(max_age)
+    if max_age_seconds is None:
+        print(f"ERROR: cannot parse MAX_AGE '{max_age}', needs to be a number followed by w|d|h|m|s")
+        return 1
     if max_sig_age is None:
         max_sig_age = max_age
     max_sig_age_seconds = max(max_age_seconds, convert_to_seconds(max_sig_age))
 
     if not target.exists():
-        print(f"INFO: cannot access target {target}. Nothing to clean.")
+        print(f"WARNING: cannot access target {target}. Nothing to clean.")
         return 0
 
     print(f"INFO: scanning {target}")
@@ -679,7 +680,7 @@ def sstate_clean(target, max_age, max_sig_age, verbose, **kwargs):
 
 def sstate_info(target, verbose, **kwargs):
     if not target.exists():
-        print(f"INFO: cannot access target {target}. No info to show.")
+        print(f"WARNING: cannot access target {target}. No info to show.")
         return 0
 
     print(f"INFO: scanning {target}")
@@ -720,11 +721,11 @@ def sstate_info(target, verbose, **kwargs):
 
 def sstate_analyze(source, target, **kwargs):
     if not os.path.isdir(source):
-        print(f"ERROR: source {source} does not exist. Nothing to analyze.")
-        return -1
+        print(f"WARNING: source {source} does not exist. Nothing to analyze.")
+        return 0
     if not target.exists():
-        print(f"ERROR: target {target} does not exist. Nothing to analyze.")
-        return -1
+        print(f"INFO: target {target} does not exist. Nothing to analyze.")
+        return 0
 
     source = SstateFileTarget(source)
     target.enable_cache()
@@ -795,8 +796,8 @@ def sstate_analyze(source, target, **kwargs):
 def sstate_lint(target, verbose, sources_dir, build_dir, exit_code, **kwargs):
     ADDITIONAL_IGNORED_VARNAMES = 'PP'.split()
     if not target.exists():
-        print(f"ERROR: target {target} does not exist. Nothing to analyze.")
-        return -1
+        print(f"WARNING: target {target} does not exist. Nothing to analyze.")
+        return 0
 
     cache_sigs = {s.hash: s for s in target.list_all() if s.suffix.endswith('.siginfo')}
 
@@ -839,7 +840,7 @@ def sstate_lint(target, verbose, sources_dir, build_dir, exit_code, **kwargs):
     if sum_hits == 0:
         print(f'no cachability issues found (scanned {len(cache_sigs)} signatures)')
     else:
-        print(f'warning: found cachability issues (scanned {len(cache_sigs)} signatures)')
+        print(f'found cachability issues (scanned {len(cache_sigs)} signatures)')
         print(f'-> absolute paths: sources-dir {hits_srcdir}, build-dir {hits_builddir}, other {hits_other}')
     if exit_code is not None:
         return exit_code
-- 
2.30.2

[PATCH 3/3] docs(isar-sstate): update documentation of lint command

[Adriaan Schmidt]

Signed-off-by: Adriaan Schmidt <adriaan.schmidt@siemens.com>
---
 scripts/isar-sstate | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/scripts/isar-sstate b/scripts/isar-sstate
index b62aec47..c462a5cb 100755
--- a/scripts/isar-sstate
+++ b/scripts/isar-sstate
@@ -22,7 +22,7 @@ sstate cache:
     `SSTATE_DIR`. To share them, you need to explicitly upload them to
     the shared location, which is what isar-sstate is for.
 
-isar-sstate implements four commands (upload, clean, info, analyze),
+isar-sstate implements five commands (upload, clean, info, analyze, lint),
 and supports three remote backends (filesystem, http/webdav, AWS S3).
 
 ## Commands
@@ -64,8 +64,10 @@ format as `bitbake-diffsigs`.
 
 ### lint
 
-The `lint` command searches form common flaws that reduce the
-cachability of a layer.
+The `lint` command searches for common flaws that reduce the cachability
+of a layer, e.g., signatures containing absolute paths from the host
+environment. Issues found this way usually indicate errors in recipes
+or in Isar itself.
 
 ## Backends
 
-- 
2.30.2

Re: [PATCH 1/3] fix(isar-sstate): catch errors in signature comparison

[Henning Schild]

Am Tue, 19 Jul 2022 08:16:26 +0200
schrieb Adriaan Schmidt <adriaan.schmidt@siemens.com>:

> Those errors happen when the sstate cache contains both pre and post
> bitbake-2.0 signatures, which have a different format (pickle vs.
> compressed json).
> 
> Signed-off-by: Adriaan Schmidt <adriaan.schmidt@siemens.com>
> ---
>  scripts/isar-sstate | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/isar-sstate b/scripts/isar-sstate
> index 8ea85edc..9f5c17b0 100755
> --- a/scripts/isar-sstate
> +++ b/scripts/isar-sstate
> @@ -781,7 +781,10 @@ def sstate_analyze(source, target, **kwargs):
>  
>              local_file = source.download(s.path)
>              remote_file = target.download(t.path)
> -            out = compare_sigfiles(remote_file, local_file,
> recursecb, color=True)
> +            try:
> +                out = compare_sigfiles(remote_file, local_file,
> recursecb, color=True)
> +            except:
> +                out = ["Failed to compare signatures."]

Maybe there is a python exception for just that, or an error code from
which we can take strerr so we do not invent our own.

Henning

>              source.release(local_file)
>              target.release(remote_file)
>              # shorten hashes from 64 to 8 characters for better
> readability

RE: [PATCH 1/3] fix(isar-sstate): catch errors in signature comparison

[Schmidt, Adriaan]

Schild, Henning, Dienstag, 19. Juli 2022 09:05:
> Am Tue, 19 Jul 2022 08:16:26 +0200
> schrieb Adriaan Schmidt <adriaan.schmidt@siemens.com>:
> 
> > Those errors happen when the sstate cache contains both pre and post
> > bitbake-2.0 signatures, which have a different format (pickle vs.
> > compressed json).
> >
> > Signed-off-by: Adriaan Schmidt <adriaan.schmidt@siemens.com>
> > ---
> >  scripts/isar-sstate | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/scripts/isar-sstate b/scripts/isar-sstate
> > index 8ea85edc..9f5c17b0 100755
> > --- a/scripts/isar-sstate
> > +++ b/scripts/isar-sstate
> > @@ -781,7 +781,10 @@ def sstate_analyze(source, target, **kwargs):
> >
> >              local_file = source.download(s.path)
> >              remote_file = target.download(t.path)
> > -            out = compare_sigfiles(remote_file, local_file,
> > recursecb, color=True)
> > +            try:
> > +                out = compare_sigfiles(remote_file, local_file,
> > recursecb, color=True)
> > +            except:
> > +                out = ["Failed to compare signatures."]
> 
> Maybe there is a python exception for just that, or an error code from
> which we can take strerr so we do not invent our own.

The exceptions we see here come from different locations during file reading.
I'm seeing things like EOFError or _pickle.UnpicklingError, and when we move
to bitbake 2.0 it may be something compression or json related.
The strings returned are not really useful (e.g. from the unpickler it
might be "Memo value not found at index 97"), that's why I went for
"catch anything and move on".

But I just noticed that sstate_lint() also needs a try/catch when reading
signature files. Will send update.

Adriaan


> Henning
> 
> >              source.release(local_file)
> >              target.release(remote_file)
> >              # shorten hashes from 64 to 8 characters for better
> > readability