[6/9] isar-sstate: add --excluded-tasks argument

From: Christopher Larson <chris.larson@seimens.com>

This allows the user to override the default lists of tasks to ignore
when linting the sstate cache.

Signed-off-by: Christopher Larson <chris.larson@siemens.com>
---
 scripts/isar-sstate | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

On Tue, 2024-04-02 at 17:28 +0000, kergoth@gmail.com wrote:
> From: Christopher Larson <chris.larson@seimens.com>
> 
> This allows the user to override the default lists of tasks to ignore
> when linting the sstate cache.
> 
> Signed-off-by: Christopher Larson <chris.larson@siemens.com>
> ---
>  scripts/isar-sstate | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/scripts/isar-sstate b/scripts/isar-sstate
> index 5270f944..dddfafcb 100755
> --- a/scripts/isar-sstate
> +++ b/scripts/isar-sstate
> @@ -146,7 +146,7 @@ try:
>  except ModuleNotFoundError:
>      s3_supported = False
>  
> -DEFAULT_IGNORED_TASKS = "rootfs_wicenv image_wic image_cpio
> image_tar image_ext4"
> +DEFAULT_IGNORED_TASKS =
> "rootfs_wicenv,image_wic,image_cpio,image_tar,image_ext4"

Also here, it might be good to support wildcards, e.g. 'image_*'.
Apart from that minor detail, this is a valuable addition.

Acked-by: Felix Moessbauer <felix.moessbauer@siemens.com>

Felix

>  
>  SstateCacheEntry = namedtuple(
>          'SstateCacheEntry', 'hash path arch pn task suffix islink
> age size'.split())
> @@ -601,6 +601,9 @@ def arguments():
>      parser.add_argument(
>          '--lint-stamps', default=False, action='store_true',
>          help="lint: assume target is a stamps directory (target must
> be a local path)")
> +    parser.add_argument(
> +        '--excluded-tasks', type=str, default=DEFAULT_IGNORED_TASKS,
> +        help="lint: comma-separated list of tasks to ignore
> (default: %(default)s)")
>  
>      args = parser.parse_args()
>      if args.command in 'upload analyze'.split() and args.source is
> None:
> @@ -609,6 +612,7 @@ def arguments():
>      elif args.command in 'info clean'.split() and args.source is not
> None:
>          print(f"ERROR: '{args.command}' must not have a source (only
> a target)")
>          sys.exit(1)
> +    args.excluded_tasks = args.excluded_tasks.split(',')
>      return args
>  
>  
> @@ -807,13 +811,14 @@ def sstate_analyze(source, target, **kwargs):
>              print('\n'.join(out))
>  
>  
> -def sstate_lint(target, verbose, sources_dir, build_dir, exit_code,
> pedantic, lint_stamps, **kwargs):
> +def sstate_lint(target, verbose, sources_dir, build_dir, exit_code,
> pedantic, lint_stamps,
> +                excluded_tasks, **kwargs):
>      ADDITIONAL_IGNORED_VARNAMES = 'PP'.split()
>      # only list non-cacheable tasks here
>      # note that these still can break caching of other tasks that
> depend on these.
>      # Run in pedantic mode to also look for these issues (e.g. in
> image-in-image builds)
>      # To make a build fully cacheable, avoid absolute paths in
> BBLAYERS
> -    ADDITIONAL_IGNORED_TASKS = list() if pedantic else
> DEFAULT_IGNORED_TASKS.split()
> +    ADDITIONAL_IGNORED_TASKS = list() if pedantic else
> excluded_tasks
>      if not target.exists():
>          print(f"WARNING: target {target} does not exist. Nothing to
> analyze.")
>          return 0
> -- 
> 2.39.2
>

Good idea, I can include that in the v2 if there's no objection.

-----Original Message-----
From: Moessbauer, Felix (T CED OES-DE) <felix.moessbauer@siemens.com> 
Sent: Wednesday, April 3, 2024 12:11 AM
To: kergoth@gmail.com; isar-users@googlegroups.com
Cc: Larson, Chris (DI CTO FDS CES LX MEL) <chris.larson@siemens.com>
Subject: Re: [PATCH 6/9] isar-sstate: add --excluded-tasks argument

On Tue, 2024-04-02 at 17:28 +0000, kergoth@gmail.com wrote:
> From: Christopher Larson <chris.larson@seimens.com>
> 
> This allows the user to override the default lists of tasks to ignore 
> when linting the sstate cache.
> 
> Signed-off-by: Christopher Larson <chris.larson@siemens.com>
> ---
>  scripts/isar-sstate | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/scripts/isar-sstate b/scripts/isar-sstate index 
> 5270f944..dddfafcb 100755
> --- a/scripts/isar-sstate
> +++ b/scripts/isar-sstate
> @@ -146,7 +146,7 @@ try:
>  except ModuleNotFoundError:
>      s3_supported = False
>  
> -DEFAULT_IGNORED_TASKS = "rootfs_wicenv image_wic image_cpio image_tar 
> image_ext4"
> +DEFAULT_IGNORED_TASKS =
> "rootfs_wicenv,image_wic,image_cpio,image_tar,image_ext4"

Also here, it might be good to support wildcards, e.g. 'image_*'.
Apart from that minor detail, this is a valuable addition.

Acked-by: Felix Moessbauer <felix.moessbauer@siemens.com>

Felix

>  
>  SstateCacheEntry = namedtuple(
>          'SstateCacheEntry', 'hash path arch pn task suffix islink age 
> size'.split()) @@ -601,6 +601,9 @@ def arguments():
>      parser.add_argument(
>          '--lint-stamps', default=False, action='store_true',
>          help="lint: assume target is a stamps directory (target must 
> be a local path)")
> +    parser.add_argument(
> +        '--excluded-tasks', type=str, default=DEFAULT_IGNORED_TASKS,
> +        help="lint: comma-separated list of tasks to ignore
> (default: %(default)s)")
>  
>      args = parser.parse_args()
>      if args.command in 'upload analyze'.split() and args.source is
> None:
> @@ -609,6 +612,7 @@ def arguments():
>      elif args.command in 'info clean'.split() and args.source is not
> None:
>          print(f"ERROR: '{args.command}' must not have a source (only 
> a target)")
>          sys.exit(1)
> +    args.excluded_tasks = args.excluded_tasks.split(',')
>      return args
>  
>  
> @@ -807,13 +811,14 @@ def sstate_analyze(source, target, **kwargs):
>              print('\n'.join(out))
>  
>  
> -def sstate_lint(target, verbose, sources_dir, build_dir, exit_code, 
> pedantic, lint_stamps, **kwargs):
> +def sstate_lint(target, verbose, sources_dir, build_dir, exit_code,
> pedantic, lint_stamps,
> +                excluded_tasks, **kwargs):
>      ADDITIONAL_IGNORED_VARNAMES = 'PP'.split()
>      # only list non-cacheable tasks here
>      # note that these still can break caching of other tasks that 
> depend on these.
>      # Run in pedantic mode to also look for these issues (e.g. in 
> image-in-image builds)
>      # To make a build fully cacheable, avoid absolute paths in 
> BBLAYERS
> -    ADDITIONAL_IGNORED_TASKS = list() if pedantic else
> DEFAULT_IGNORED_TASKS.split()
> +    ADDITIONAL_IGNORED_TASKS = list() if pedantic else
> excluded_tasks
>      if not target.exists():
>          print(f"WARNING: target {target} does not exist. Nothing to
> analyze.")
>          return 0
> --
> 2.39.2
>

I have a quick question about this. In the code, there are constants using the "ignored" wording rather than excluded, so my adding the argument using excluded seems like it's probably not consistent. I should probably rename the argument to match, or adjust the variable naming for consistency. Any thoughts?

-----Original Message-----
From: Moessbauer, Felix (T CED OES-DE) <felix.moessbauer@siemens.com> 
Sent: Wednesday, April 3, 2024 12:11 AM
To: kergoth@gmail.com; isar-users@googlegroups.com
Cc: Larson, Chris (DI CTO FDS CES LX MEL) <chris.larson@siemens.com>
Subject: Re: [PATCH 6/9] isar-sstate: add --excluded-tasks argument

On Tue, 2024-04-02 at 17:28 +0000, kergoth@gmail.com wrote:
> From: Christopher Larson <chris.larson@seimens.com>
> 
> This allows the user to override the default lists of tasks to ignore 
> when linting the sstate cache.
> 
> Signed-off-by: Christopher Larson <chris.larson@siemens.com>
> ---
>  scripts/isar-sstate | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/scripts/isar-sstate b/scripts/isar-sstate index 
> 5270f944..dddfafcb 100755
> --- a/scripts/isar-sstate
> +++ b/scripts/isar-sstate
> @@ -146,7 +146,7 @@ try:
>  except ModuleNotFoundError:
>      s3_supported = False
>  
> -DEFAULT_IGNORED_TASKS = "rootfs_wicenv image_wic image_cpio image_tar 
> image_ext4"
> +DEFAULT_IGNORED_TASKS =
> "rootfs_wicenv,image_wic,image_cpio,image_tar,image_ext4"

Also here, it might be good to support wildcards, e.g. 'image_*'.
Apart from that minor detail, this is a valuable addition.

Acked-by: Felix Moessbauer <felix.moessbauer@siemens.com>

Felix

>  
>  SstateCacheEntry = namedtuple(
>          'SstateCacheEntry', 'hash path arch pn task suffix islink age 
> size'.split()) @@ -601,6 +601,9 @@ def arguments():
>      parser.add_argument(
>          '--lint-stamps', default=False, action='store_true',
>          help="lint: assume target is a stamps directory (target must 
> be a local path)")
> +    parser.add_argument(
> +        '--excluded-tasks', type=str, default=DEFAULT_IGNORED_TASKS,
> +        help="lint: comma-separated list of tasks to ignore
> (default: %(default)s)")
>  
>      args = parser.parse_args()
>      if args.command in 'upload analyze'.split() and args.source is
> None:
> @@ -609,6 +612,7 @@ def arguments():
>      elif args.command in 'info clean'.split() and args.source is not
> None:
>          print(f"ERROR: '{args.command}' must not have a source (only 
> a target)")
>          sys.exit(1)
> +    args.excluded_tasks = args.excluded_tasks.split(',')
>      return args
>  
>  
> @@ -807,13 +811,14 @@ def sstate_analyze(source, target, **kwargs):
>              print('\n'.join(out))
>  
>  
> -def sstate_lint(target, verbose, sources_dir, build_dir, exit_code, 
> pedantic, lint_stamps, **kwargs):
> +def sstate_lint(target, verbose, sources_dir, build_dir, exit_code,
> pedantic, lint_stamps,
> +                excluded_tasks, **kwargs):
>      ADDITIONAL_IGNORED_VARNAMES = 'PP'.split()
>      # only list non-cacheable tasks here
>      # note that these still can break caching of other tasks that 
> depend on these.
>      # Run in pedantic mode to also look for these issues (e.g. in 
> image-in-image builds)
>      # To make a build fully cacheable, avoid absolute paths in 
> BBLAYERS
> -    ADDITIONAL_IGNORED_TASKS = list() if pedantic else
> DEFAULT_IGNORED_TASKS.split()
> +    ADDITIONAL_IGNORED_TASKS = list() if pedantic else
> excluded_tasks
>      if not target.exists():
>          print(f"WARNING: target {target} does not exist. Nothing to
> analyze.")
>          return 0
> --
> 2.39.2
>

On Wed, 2024-04-03 at 21:44 +0000, Larson, Chris (DI CTO FDS CES LX
MEL) wrote:
> I have a quick question about this. In the code, there are constants
> using the "ignored" wording rather than excluded, so my adding the
> argument using excluded seems like it's probably not consistent. I
> should probably rename the argument to match, or adjust the variable
> naming for consistency. Any thoughts?

We should at least keep it consistent. Renaming the internal variables
is fine, but please try to not change the external interface (the
parameters).

Felix

> 
> -----Original Message-----
> From: Moessbauer, Felix (T CED OES-DE) <felix.moessbauer@siemens.com>
> Sent: Wednesday, April 3, 2024 12:11 AM
> To: kergoth@gmail.com; isar-users@googlegroups.com
> Cc: Larson, Chris (DI CTO FDS CES LX MEL) <chris.larson@siemens.com>
> Subject: Re: [PATCH 6/9] isar-sstate: add --excluded-tasks argument
> 
> On Tue, 2024-04-02 at 17:28 +0000, kergoth@gmail.com wrote:
> > From: Christopher Larson <chris.larson@seimens.com>
> > 
> > This allows the user to override the default lists of tasks to
> > ignore 
> > when linting the sstate cache.
> > 
> > Signed-off-by: Christopher Larson <chris.larson@siemens.com>
> > ---
> >  scripts/isar-sstate | 11 ++++++++---
> >  1 file changed, 8 insertions(+), 3 deletions(-)
> > 
> > diff --git a/scripts/isar-sstate b/scripts/isar-sstate index 
> > 5270f944..dddfafcb 100755
> > --- a/scripts/isar-sstate
> > +++ b/scripts/isar-sstate
> > @@ -146,7 +146,7 @@ try:
> >  except ModuleNotFoundError:
> >      s3_supported = False
> >  
> > -DEFAULT_IGNORED_TASKS = "rootfs_wicenv image_wic image_cpio
> > image_tar 
> > image_ext4"
> > +DEFAULT_IGNORED_TASKS =
> > "rootfs_wicenv,image_wic,image_cpio,image_tar,image_ext4"
> 
> Also here, it might be good to support wildcards, e.g. 'image_*'.
> Apart from that minor detail, this is a valuable addition.
> 
> Acked-by: Felix Moessbauer <felix.moessbauer@siemens.com>
> 
> Felix
> 
> >  
> >  SstateCacheEntry = namedtuple(
> >          'SstateCacheEntry', 'hash path arch pn task suffix islink
> > age 
> > size'.split()) @@ -601,6 +601,9 @@ def arguments():
> >      parser.add_argument(
> >          '--lint-stamps', default=False, action='store_true',
> >          help="lint: assume target is a stamps directory (target
> > must 
> > be a local path)")
> > +    parser.add_argument(
> > +        '--excluded-tasks', type=str,
> > default=DEFAULT_IGNORED_TASKS,
> > +        help="lint: comma-separated list of tasks to ignore
> > (default: %(default)s)")
> >  
> >      args = parser.parse_args()
> >      if args.command in 'upload analyze'.split() and args.source is
> > None:
> > @@ -609,6 +612,7 @@ def arguments():
> >      elif args.command in 'info clean'.split() and args.source is
> > not
> > None:
> >          print(f"ERROR: '{args.command}' must not have a source
> > (only 
> > a target)")
> >          sys.exit(1)
> > +    args.excluded_tasks = args.excluded_tasks.split(',')
> >      return args
> >  
> >  
> > @@ -807,13 +811,14 @@ def sstate_analyze(source, target, **kwargs):
> >              print('\n'.join(out))
> >  
> >  
> > -def sstate_lint(target, verbose, sources_dir, build_dir,
> > exit_code, 
> > pedantic, lint_stamps, **kwargs):
> > +def sstate_lint(target, verbose, sources_dir, build_dir,
> > exit_code,
> > pedantic, lint_stamps,
> > +                excluded_tasks, **kwargs):
> >      ADDITIONAL_IGNORED_VARNAMES = 'PP'.split()
> >      # only list non-cacheable tasks here
> >      # note that these still can break caching of other tasks that 
> > depend on these.
> >      # Run in pedantic mode to also look for these issues (e.g. in 
> > image-in-image builds)
> >      # To make a build fully cacheable, avoid absolute paths in 
> > BBLAYERS
> > -    ADDITIONAL_IGNORED_TASKS = list() if pedantic else
> > DEFAULT_IGNORED_TASKS.split()
> > +    ADDITIONAL_IGNORED_TASKS = list() if pedantic else
> > excluded_tasks
> >      if not target.exists():
> >          print(f"WARNING: target {target} does not exist. Nothing
> > to
> > analyze.")
> >          return 0
> > --
> > 2.39.2
> > 
> 
> -- 
> Siemens AG, Technology
> Linux Expert Center
> 
>