From: Simon Glass <simon.glass@canonical.com> Some checkpatch warnings lack file/line information and are not covered by the no_file_match list. This causes spurious messages on stderr when running 'patman test': patman: failed to find file / line information: ERROR:NO_AUTHOR_SIGN_OFF: Missing Signed-off-by: ... WARNING:PRIV_AUTO: struct 'fred' should have a _priv suffix Broaden the Signed-off-by match to cover the NO_AUTHOR_SIGN_OFF variant, and add a pattern for the struct suffix warnings. Co-developed-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/patman/checkpatch.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tools/patman/checkpatch.py b/tools/patman/checkpatch.py index fef07299acf..6f99a36a08d 100644 --- a/tools/patman/checkpatch.py +++ b/tools/patman/checkpatch.py @@ -92,8 +92,9 @@ def check_patch_parse_one_message(message): file_match = RE_FILE.search(message) # some messages have no file, catch those here no_file_match = any(s in message for s in [ - '\nSubject:', 'Missing Signed-off-by: line(s)', - 'does MAINTAINERS need updating' + '\nSubject:', 'Missing Signed-off-by:', + 'does MAINTAINERS need updating', + 'should have a', ]) if file_match: -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> When scanning commits for a series, show a summary notice with the total count and how many were added or removed. This makes it easier to see at a glance what changed during a scan operation. Co-developed-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/patman/cseries.py | 15 +++++++++++++++ tools/patman/test_cseries.py | 4 +++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/tools/patman/cseries.py b/tools/patman/cseries.py index c94daf59e36..c5620985a09 100644 --- a/tools/patman/cseries.py +++ b/tools/patman/cseries.py @@ -803,13 +803,17 @@ class Cseries(cser_helper.CseriesHelper): if i is not None: del to_remove[i] + removed = 0 + added = 0 for seq, cmt in enumerate(ser.commits): if seq in to_remove: _show_item('-', seq, to_remove[seq].subject) del to_remove[seq] + removed += 1 if seq in to_add: _show_item('+', seq, to_add[seq].subject) del to_add[seq] + added += 1 else: _show_item(' ', seq, cmt.subject) seq = len(ser.commits) @@ -823,6 +827,17 @@ class Cseries(cser_helper.CseriesHelper): self._add_series_commits(ser, svid) if not dry_run: self.commit() + seq = len(ser.commits) + msg = '' + if added: + msg += f'{added} added' + if removed: + if msg: + msg += ', ' + msg += f'{removed} removed' + if msg: + msg = f' ({msg})' + tout.notice(f'Scanned {seq} commit{self.plural(seq)}{msg}') else: self.rollback() tout.info('Dry run completed') diff --git a/tools/patman/test_cseries.py b/tools/patman/test_cseries.py index b25debec3fe..a7750525ea6 100644 --- a/tools/patman/test_cseries.py +++ b/tools/patman/test_cseries.py @@ -3335,7 +3335,9 @@ Date: .* with terminal.capture() as (out, _): self.run_args('series', 'scan', '-M', pwork=True) - self.assertEqual(expect, out.getvalue()) + self.assertEqual( + expect + 'Scanned 3 commits (1 added, 1 removed)\n', + out.getvalue()) new_pcdict = cser.get_pcommit_dict(svid).values() self.assertEqual(len(old_pcdict), len(new_pcdict)) -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> Some patchwork instances return project names with trailing whitespace, which causes the exact string match to fail. Strip whitespace before comparing. Also add debug logging to show the projects being checked and which one matches, to help diagnose future issues. Co-developed-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/patman/control.py | 2 +- tools/patman/cseries.py | 13 +++++++++---- tools/patman/test_cseries.py | 8 ++++---- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/tools/patman/control.py b/tools/patman/control.py index 338a58f4d75..c6fede4305b 100644 --- a/tools/patman/control.py +++ b/tools/patman/control.py @@ -273,7 +273,7 @@ def patchwork(args, test_db=None, pwork=None): if not info: raise ValueError("Project has not been set; use 'patman patchwork set-project'") name, pwid, link_name = info - print(f"Project '{name}' patchwork-ID {pwid} link-name {link_name}") + print(f"Project '{name}' patchwork-ID {pwid} link-name '{link_name}'") else: raise ValueError(f"Unknown patchwork subcommand '{args.subcmd}'") finally: diff --git a/tools/patman/cseries.py b/tools/patman/cseries.py index c5620985a09..e8d40394608 100644 --- a/tools/patman/cseries.py +++ b/tools/patman/cseries.py @@ -634,20 +634,25 @@ class Cseries(cser_helper.CseriesHelper): name (str): Name of the project to use in patchwork quiet (bool): True to skip writing the message """ + tout.detail(f"Patchwork URL '{pwork.url}': finding name '{name}'") res = self.loop.run_until_complete(pwork.get_projects()) proj_id = None link_name = None for proj in res: - if proj['name'] == name: - proj_id = proj['id'] + pid, pname = proj['id'], proj['name'] + ok = pname.strip() == name + tout.detail(f"{pid:3} '{pname}'") + if ok: + proj_id = pid link_name = proj['link_name'] + tout.detail(f'Name match: ID {proj_id}') if not proj_id: raise ValueError(f"Unknown project name '{name}'") self.db.settings_update(name, proj_id, link_name) self.commit() if not quiet: - tout.info(f"Project '{name}' patchwork-ID {proj_id} " - f'link-name {link_name}') + tout.notice(f"Project '{name}' patchwork-ID {proj_id} " + f"link-name '{link_name}'") def project_get(self): """Get the details of the project diff --git a/tools/patman/test_cseries.py b/tools/patman/test_cseries.py index a7750525ea6..3aa8e0e739e 100644 --- a/tools/patman/test_cseries.py +++ b/tools/patman/test_cseries.py @@ -2389,7 +2389,7 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak with terminal.capture() as (out, _): cser.project_set(pwork, 'U-Boot') self.assertEqual( - f"Project 'U-Boot' patchwork-ID {self.PROJ_ID} link-name uboot", + f"Project 'U-Boot' patchwork-ID {self.PROJ_ID} link-name 'uboot'", out.getvalue().strip()) def test_patchwork_project_get(self): @@ -2400,7 +2400,7 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak with terminal.capture() as (out, _): cser.project_set(pwork, 'U-Boot') self.assertEqual( - f"Project 'U-Boot' patchwork-ID {self.PROJ_ID} link-name uboot", + f"Project 'U-Boot' patchwork-ID {self.PROJ_ID} link-name 'uboot'", out.getvalue().strip()) name, pwid, link_name = cser.project_get() @@ -2419,7 +2419,7 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak self.run_args('-P', 'https://url', 'patchwork', 'set-project', 'U-Boot', pwork=pwork) self.assertEqual( - f"Project 'U-Boot' patchwork-ID {self.PROJ_ID} link-name uboot", + f"Project 'U-Boot' patchwork-ID {self.PROJ_ID} link-name 'uboot'", out.getvalue().strip()) name, pwid, link_name = cser.project_get() @@ -2430,7 +2430,7 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak with terminal.capture() as (out, _): self.run_args('-P', 'https://url', 'patchwork', 'get-project') self.assertEqual( - f"Project 'U-Boot' patchwork-ID {self.PROJ_ID} link-name uboot", + f"Project 'U-Boot' patchwork-ID {self.PROJ_ID} link-name 'uboot'", out.getvalue().strip()) def check_series_list_patches(self): -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> Several series subcommands complete silently or only log at the info level, which is not visible with the default NOTICE log level. Add a tout.notice() summary to each action subcommand so the user sees confirmation of what was done. For subcommands that already have a summary at info level, promote it to notice. For those with no summary at all, add one. Also update tests to suppress the new output where needed. Co-developed-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/patman/cseries.py | 40 ++++++++++++----- tools/patman/test_cseries.py | 85 +++++++++++++++++++++++------------- 2 files changed, 83 insertions(+), 42 deletions(-) diff --git a/tools/patman/cseries.py b/tools/patman/cseries.py index 937af58ba3e..c90020b7eb6 100644 --- a/tools/patman/cseries.py +++ b/tools/patman/cseries.py @@ -142,7 +142,7 @@ class Cseries(cser_helper.CseriesHelper): del_branch = repo.lookup_branch(del_name) branch_oid = del_branch.peel(pygit2.enums.ObjectType.COMMIT).oid del_branch.delete() - print(f"Deleted branch '{del_name}' {oid(branch_oid)}") + tout.info(f"Deleted branch '{del_name}' {oid(branch_oid)}") self.db.ser_ver_remove(ser.idnum, max_vers) if not dry_run: @@ -150,6 +150,8 @@ class Cseries(cser_helper.CseriesHelper): else: self.rollback() + tout.notice(f"Decremented series '{ser.name}' to v{new_max}") + def increment(self, series_name, dry_run=False): """Increment a series to the next version and create a new branch @@ -191,7 +193,7 @@ class Cseries(cser_helper.CseriesHelper): self.rollback() # repo.head.set_target(amended) - tout.info(f'Added new branch {new_name}') + tout.notice(f"Incremented series '{ser.name}' to v{vers}") if dry_run: tout.info('Dry run completed') @@ -372,7 +374,7 @@ class Cseries(cser_helper.CseriesHelper): msg += f', {no_desc} missing description' if failed: msg += f', {failed} updated failed' - tout.info(msg + f' ({requests} requests)') + tout.notice(msg + f' ({requests} requests)') tout.info('') tout.info(f"{'Name':15} Version {'Description':40} Result") @@ -466,6 +468,9 @@ class Cseries(cser_helper.CseriesHelper): f'Marked commits {len(bad)}/{len(ser.commits)}') new_oid = self._mark_series(in_name, ser, dry_run=dry_run) + count = len(ser.commits) + tout.notice(f"Marked {count} commit{self.plural(count)}" + f" in series '{name}'") if dry_run: tout.info('Dry run completed') return new_oid @@ -510,6 +515,9 @@ class Cseries(cser_helper.CseriesHelper): else: vals.info = 'no mark' + count = len(ser.commits) + tout.notice(f"Unmarked {count} commit{self.plural(count)}" + f" in series '{name}'") if dry_run: tout.info('Dry run completed') return vals.oid @@ -756,7 +764,7 @@ class Cseries(cser_helper.CseriesHelper): else: self.rollback() - tout.info(f"Renamed series '{series}' to '{name}'") + tout.notice(f"Renamed series '{series}' to '{name}'") if dry_run: tout.info('Dry run completed') @@ -916,6 +924,9 @@ class Cseries(cser_helper.CseriesHelper): self.db.series_set_archived(ser.idnum, True) self.commit() + count = len(tag_info) + tout.notice(f"Archived series '{ser.name}'" + f" ({count} version{self.plural(count)})") def unarchive(self, series): """Unarchive a series @@ -958,6 +969,9 @@ class Cseries(cser_helper.CseriesHelper): self.db.ser_ver_set_archive_tag(idnum, None) self.commit() + count = len(tag_info) + tout.notice(f"Unarchived series '{ser.name}'" + f" ({count} version{self.plural(count)})") def status(self, pwork, series, version, show_comments, show_cover_comments=False): @@ -1029,9 +1043,9 @@ class Cseries(cser_helper.CseriesHelper): updated, updated_cover = self._sync_one( svid, ser.name, version, show_comments, show_cover_comments, gather_tags, cover, patches, dry_run) - tout.info(f"{updated} patch{'es' if updated != 1 else ''}" - f"{' and cover letter' if updated_cover else ''} " - f'updated ({stats.request_count} requests)') + tout.notice(f"{updated} patch{'es' if updated != 1 else ''}" + f"{' and cover letter' if updated_cover else ''} " + f'updated ({stats.request_count} requests)') if not dry_run: self.commit() @@ -1064,7 +1078,7 @@ class Cseries(cser_helper.CseriesHelper): add_newline = gather_tags tout.info('') - tout.info( + tout.notice( f"{tot_updated} patch{'es' if tot_updated != 1 else ''} and " f"{tot_cover} cover letter{'s' if tot_cover != 1 else ''} " f'updated, {missing} missing ' @@ -1084,6 +1098,7 @@ class Cseries(cser_helper.CseriesHelper): """ self.db.upstream_add(name, url) self.commit() + tout.notice(f"Added upstream '{name}' ({url})") def upstream_list(self): """List the upstream repos @@ -1106,6 +1121,8 @@ class Cseries(cser_helper.CseriesHelper): """ self.db.upstream_set_default(name) self.commit() + if name: + tout.notice(f"Set default upstream to '{name}'") def upstream_get_default(self): """Get the default upstream target @@ -1123,6 +1140,7 @@ class Cseries(cser_helper.CseriesHelper): """ self.db.upstream_delete(name) self.commit() + tout.notice(f"Deleted upstream '{name}'") def version_remove(self, name, version, dry_run=False): """Remove a version of a series from the database @@ -1147,7 +1165,7 @@ class Cseries(cser_helper.CseriesHelper): else: self.rollback() - tout.info(f"Removed version {version} from series '{name}'") + tout.notice(f"Removed version {version} from series '{name}'") if dry_run: tout.info('Dry run completed') @@ -1194,7 +1212,7 @@ class Cseries(cser_helper.CseriesHelper): else: self.rollback() - tout.info(f"Changed version {version} in series '{ser.name}' " - f"to {new_version} named '{new_name}'") + tout.notice(f"Changed version {version} in series '{ser.name}' " + f"to {new_version} named '{new_name}'") if dry_run: tout.info('Dry run completed') diff --git a/tools/patman/test_cseries.py b/tools/patman/test_cseries.py index 3aa8e0e739e..c8af0bb1db7 100644 --- a/tools/patman/test_cseries.py +++ b/tools/patman/test_cseries.py @@ -772,7 +772,8 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak def test_series_list_archived(self): """Archive a series and test listing it""" self.setup_second() - self.cser.archive('first') + with terminal.capture(): + self.cser.archive('first') with terminal.capture() as (out, _): self.run_args('series', 'ls', pwork=True) lines = out.getvalue().splitlines() @@ -877,7 +878,7 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak f'- add v2: {HASH_RE} as {HASH_RE} spi: SPI fixes') self.assertRegex( next(itr), f'Updating branch first2 from {HASH_RE} to {HASH_RE}') - self.assertEqual('Added new branch first2', next(itr)) + self.assertEqual("Incremented series 'first' to v2", next(itr)) return itr def test_series_link(self): @@ -1532,9 +1533,11 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak cser = next(cor) # Archive it and make sure it is invisible - cser.archive('first') + with terminal.capture(): + cser.archive('first') cser = next(cor) - cser.unarchive('first') + with terminal.capture(): + cser.unarchive('first') self.assertFalse(next(cor)) cor.close() @@ -1544,11 +1547,13 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak cser = next(cor) # Archive it and make sure it is invisible - self.run_args('series', '-s', 'first', 'archive', pwork=True, - cser=cser) + with terminal.capture(): + self.run_args('series', '-s', 'first', 'archive', pwork=True, + cser=cser) next(cor) - self.run_args('series', '-s', 'first', 'unarchive', pwork=True, - cser=cser) + with terminal.capture(): + self.run_args('series', '-s', 'first', 'unarchive', pwork=True, + cser=cser) self.assertFalse(next(cor)) cor.close() @@ -1696,10 +1701,11 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak with terminal.capture() as (out, _): cser.decrement('first') lines = out.getvalue().splitlines() - self.assertEqual(2, len(lines)) + self.assertEqual(3, len(lines)) self.assertEqual("Removing series 'first' v2", lines[0]) self.assertEqual( f"Deleted branch 'first2' {str(branch_oid)[:10]}", lines[1]) + self.assertEqual("Decremented series 'first' to v1", lines[2]) svdict = cser.get_ser_ver_dict() self.assertEqual(1, len(svdict)) @@ -1720,12 +1726,14 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak """Test adding an upsream""" cser = self.get_cser() - cser.upstream_add('us', 'https://one') + with terminal.capture(): + cser.upstream_add('us', 'https://one') ulist = cser.get_upstream_dict() self.assertEqual(1, len(ulist)) self.assertEqual(('https://one', None), ulist['us']) - cser.upstream_add('ci', 'git@two') + with terminal.capture(): + cser.upstream_add('ci', 'git@two') ulist = cser.get_upstream_dict() self.assertEqual(2, len(ulist)) self.assertEqual(('https://one', None), ulist['us']) @@ -1762,17 +1770,21 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak cser.upstream_set_default('us') self.assertEqual("No such upstream 'us'", str(exc.exception)) - cser.upstream_add('us', 'https://one') - cser.upstream_add('ci', 'git@two') + with terminal.capture(): + cser.upstream_add('us', 'https://one') + cser.upstream_add('ci', 'git@two') self.assertIsNone(cser.upstream_get_default()) - cser.upstream_set_default('us') + with terminal.capture(): + cser.upstream_set_default('us') self.assertEqual('us', cser.upstream_get_default()) - cser.upstream_set_default('us') + with terminal.capture(): + cser.upstream_set_default('us') - cser.upstream_set_default('ci') + with terminal.capture(): + cser.upstream_set_default('ci') self.assertEqual('ci', cser.upstream_get_default()) with terminal.capture() as (out, _): @@ -1792,19 +1804,22 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak self.assertEqual("patman: ValueError: No such upstream 'us'", out.getvalue().strip().splitlines()[-1]) - self.run_args('upstream', 'add', 'us', 'https://one') - self.run_args('upstream', 'add', 'ci', 'git@two') + with terminal.capture(): + self.run_args('upstream', 'add', 'us', 'https://one') + self.run_args('upstream', 'add', 'ci', 'git@two') with terminal.capture() as (out, _): self.run_args('upstream', 'default') self.assertEqual('unset', out.getvalue().strip()) - self.run_args('upstream', 'default', 'us') + with terminal.capture(): + self.run_args('upstream', 'default', 'us') with terminal.capture() as (out, _): self.run_args('upstream', 'default') self.assertEqual('us', out.getvalue().strip()) - self.run_args('upstream', 'default', 'ci') + with terminal.capture(): + self.run_args('upstream', 'default', 'ci') with terminal.capture() as (out, _): self.run_args('upstream', 'default') self.assertEqual('ci', out.getvalue().strip()) @@ -1825,14 +1840,17 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak cser.upstream_delete('us') self.assertEqual("No such upstream 'us'", str(exc.exception)) - cser.upstream_add('us', 'https://one') - cser.upstream_add('ci', 'git@two') + with terminal.capture(): + cser.upstream_add('us', 'https://one') + cser.upstream_add('ci', 'git@two') - cser.upstream_set_default('us') - cser.upstream_delete('us') + with terminal.capture(): + cser.upstream_set_default('us') + cser.upstream_delete('us') self.assertIsNone(cser.upstream_get_default()) - cser.upstream_delete('ci') + with terminal.capture(): + cser.upstream_delete('ci') ulist = cser.get_upstream_dict() self.assertFalse(ulist) @@ -1843,17 +1861,20 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak self.assertEqual("patman: ValueError: No such upstream 'us'", out.getvalue().strip().splitlines()[-1]) - self.run_args('us', 'add', 'us', 'https://one') - self.run_args('us', 'add', 'ci', 'git@two') + with terminal.capture(): + self.run_args('us', 'add', 'us', 'https://one') + self.run_args('us', 'add', 'ci', 'git@two') - self.run_args('upstream', 'default', 'us') - self.run_args('upstream', 'delete', 'us') + with terminal.capture(): + self.run_args('upstream', 'default', 'us') + self.run_args('upstream', 'delete', 'us') with terminal.capture() as (out, _): self.run_args('upstream', 'default', 'us', expect_ret=1) self.assertEqual("patman: ValueError: No such upstream 'us'", out.getvalue().strip()) - self.run_args('upstream', 'delete', 'ci') + with terminal.capture(): + self.run_args('upstream', 'delete', 'ci') with terminal.capture() as (out, _): self.run_args('upstream', 'list') self.assertFalse(out.getvalue().strip()) @@ -2003,6 +2024,7 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak f'- unmarked: {HASH_RE} as {HASH_RE} spi: SPI fixes') self.assertRegex( next(itr), f'Updating branch first from {HASH_RE} to {HASH_RE}') + self.assertEqual("Unmarked 2 commits in series 'first'", next(itr)) self.assertEqual('Dry run completed', next(itr)) with self.stage('unmark'): @@ -3086,7 +3108,8 @@ Date: .* def test_series_progress_all_archived(self): """Test showing progress for all cseries including archived ones""" self.setup_second() - self.cser.archive('first') + with terminal.capture(): + self.cser.archive('first') with self.stage('progress without archived'): with terminal.capture() as (out, _): -- 2.43.0