Message ID | 20240402172813.418770-7-chris.larson@siemens.com |
---|---|
State | Superseded, archived |
Headers | show |
Series | Add more signature cachability tests to the testsuite | expand |
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 > >
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" 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