From: Simon Glass <simon.glass@canonical.com> If a newer version of patman has upgraded the database schema beyond what the current patman understands, it goes into an infinite loop, trying to migrate. Exit with an error message instead. Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/patman/database.py | 6 ++++++ tools/patman/test_cseries.py | 17 +++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/tools/patman/database.py b/tools/patman/database.py index e6684f990da..4557c4ff3fc 100644 --- a/tools/patman/database.py +++ b/tools/patman/database.py @@ -86,6 +86,12 @@ class Database: # pylint:disable=R0904 def start(self): """Open the database read for use, migrate to latest schema""" self.open_it() + old_version = self.get_schema_version() + if old_version > LATEST: + self.close() + tout.fatal( + f'Database version {old_version} is too new (max' + f' {LATEST}); please update patman') self.migrate_to(LATEST) def open_it(self): diff --git a/tools/patman/test_cseries.py b/tools/patman/test_cseries.py index 11724b1378f..3a9411033a6 100644 --- a/tools/patman/test_cseries.py +++ b/tools/patman/test_cseries.py @@ -3318,6 +3318,23 @@ Date: .* self.assertEqual(version, db.get_schema_version()) self.assertEqual(4, database.LATEST) + def test_migrate_future_version(self): + """Test that a database newer than patman is rejected""" + db = database.Database(f'{self.tmpdir}/.patman.db') + with terminal.capture(): + db.start() + + # Set the schema version beyond what patman supports + db.cur.execute( + f'UPDATE schema_version SET version = {database.LATEST + 1}') + db.commit() + db.close() + + with self.assertRaises(SystemExit): + with terminal.capture() as (_, err): + db.start() + self.assertIn('is too new', err.getvalue()) + def test_series_scan(self): """Test scanning a series for updates""" cser, _ = self.setup_second() -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> Series are not currently associated with a particular upstream. Add an 'upstream' column to the series table (schema v5) so each series can record which upstream it targets. This stores the upstream name as a string reference to the upstream table's name column. Add series_set_upstream() for updating an existing series, and extend series_add(), series_get_info() and _get_series_list() to handle the new column. Update Series.from_fields() to require the upstream parameter. Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/patman/cser_helper.py | 9 +++++---- tools/patman/database.py | 31 +++++++++++++++++++++++-------- tools/patman/series.py | 3 ++- tools/patman/test_cseries.py | 30 ++++++++++++++++++++++++++++++ 4 files changed, 60 insertions(+), 13 deletions(-) diff --git a/tools/patman/cser_helper.py b/tools/patman/cser_helper.py index 4289593b64b..1441904686c 100644 --- a/tools/patman/cser_helper.py +++ b/tools/patman/cser_helper.py @@ -246,6 +246,7 @@ class CseriesHelper: Return: tuple: str: Series name str: Series description + str or None: Upstream name Raises: ValueError: Series is not found @@ -400,9 +401,9 @@ class CseriesHelper: idnum = self.db.series_find_by_name(name, include_archived) if not idnum: return None - name, desc = self.db.series_get_info(idnum) + name, desc, ups = self.db.series_get_info(idnum) - return Series.from_fields(idnum, name, desc) + return Series.from_fields(idnum, name, desc, ups) def _get_branch_name(self, name, version): """Get the branch name for a particular version @@ -1441,7 +1442,7 @@ class CseriesHelper: int: Number of version which need a 'scan' """ max_vers = self._series_max_version(ser.idnum) - name, desc = self._get_series_info(ser.idnum) + name, desc, ups = self._get_series_info(ser.idnum) coloured = self.col.build(self.col.BLACK, desc, bright=False, back=self.col.YELLOW) versions = self._get_version_list(ser.idnum) @@ -1487,7 +1488,7 @@ class CseriesHelper: all series """ max_vers = self._series_max_version(ser.idnum) - name, desc = self._get_series_info(ser.idnum) + name, desc, ups = self._get_series_info(ser.idnum) stats, pwc = self._series_get_version_stats(ser.idnum, max_vers) states = {x.state for x in pwc.values()} state = 'accepted' diff --git a/tools/patman/database.py b/tools/patman/database.py index 2861ca69fa0..e3df4257f8c 100644 --- a/tools/patman/database.py +++ b/tools/patman/database.py @@ -295,10 +295,11 @@ class Database: # pylint:disable=R0904 list of Series """ res = self.execute( - 'SELECT id, name, desc FROM series ' + + 'SELECT id, name, desc, upstream FROM series ' + ('WHERE archived = 0' if not include_archived else '')) - return [Series.from_fields(idnum=idnum, name=name, desc=desc) - for idnum, name, desc in res.fetchall()] + return [Series.from_fields(idnum=idnum, name=name, desc=desc, + ups=ups) + for idnum, name, desc, ups in res.fetchall()] # series functions @@ -349,12 +350,14 @@ class Database: # pylint:disable=R0904 Return: tuple: str: Series name str: Series description + str or None: Upstream name Raises: ValueError: Series is not found """ - res = self.execute('SELECT name, desc FROM series WHERE id = ?', - (idnum,)) + res = self.execute( + 'SELECT name, desc, upstream FROM series WHERE id = ?', + (idnum,)) recs = res.fetchall() if len(recs) != 1: raise ValueError(f'No series found (id {idnum} len {len(recs)})') @@ -417,7 +420,7 @@ class Database: # pylint:disable=R0904 'GROUP BY series_id') return res.fetchall() - def series_add(self, name, desc): + def series_add(self, name, desc, ups=None): """Add a new series record The new record is set to not archived @@ -425,13 +428,14 @@ class Database: # pylint:disable=R0904 Args: name (str): Series name desc (str): Series description + ups (str or None): Name of the upstream for this series Return: int: ID num of the new series record """ self.execute( - 'INSERT INTO series (name, desc, archived) ' - f"VALUES ('{name}', '{desc}', 0)") + 'INSERT INTO series (name, desc, archived, upstream) ' + 'VALUES (?, ?, 0, ?)', (name, desc, ups)) return self.lastrowid() def series_remove(self, idnum): @@ -481,6 +485,17 @@ class Database: # pylint:disable=R0904 self.execute( 'UPDATE series SET name = ? WHERE id = ?', (name, series_idnum)) + def series_set_upstream(self, series_idnum, ups): + """Update upstream for a series + + Args: + series_idnum (int): ID num of the series + ups (str or None): Name of the upstream, or None to clear + """ + self.execute( + 'UPDATE series SET upstream = ? WHERE id = ?', + (ups, series_idnum)) + # ser_ver functions def ser_ver_get_link(self, series_idnum, version): diff --git a/tools/patman/series.py b/tools/patman/series.py index 517fd4304f5..9985533ee52 100644 --- a/tools/patman/series.py +++ b/tools/patman/series.py @@ -72,11 +72,12 @@ class Series(dict): return self[name] @staticmethod - def from_fields(idnum, name, desc): + def from_fields(idnum, name, desc, ups): ser = Series() ser.idnum = idnum ser.name = name ser.desc = desc + ser.upstream = ups return ser def AddTag(self, commit, line, name, value): diff --git a/tools/patman/test_cseries.py b/tools/patman/test_cseries.py index 5719d9b6ddc..fc29e9a7bef 100644 --- a/tools/patman/test_cseries.py +++ b/tools/patman/test_cseries.py @@ -1879,6 +1879,36 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak self.run_args('upstream', 'list') self.assertFalse(out.getvalue().strip()) + def test_series_upstream(self): + """Test upstream field in the series table""" + cser = self.get_cser() + + # Add a series without upstream + cser.db.series_add('first', 'my desc') + cser.db.commit() + slist = cser.db.series_get_dict() + self.assertIsNone(slist['first'].upstream) + + # Add a series with upstream + cser.db.series_add('second', 'desc2', ups='us') + cser.db.commit() + slist = cser.db.series_get_dict() + self.assertIsNone(slist['first'].upstream) + self.assertEqual('us', slist['second'].upstream) + + # Update upstream on existing series + idnum = cser.db.series_find_by_name('first') + cser.db.series_set_upstream(idnum, 'ci') + cser.db.commit() + slist = cser.db.series_get_dict() + self.assertEqual('ci', slist['first'].upstream) + + # Clear upstream + cser.db.series_set_upstream(idnum, None) + cser.db.commit() + slist = cser.db.series_get_dict() + self.assertIsNone(slist['first'].upstream) + def test_series_add_mark(self): """Test marking a cseries with Change-Id fields""" cser = self.get_cser() -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> When the database is upgraded to v5, try to auto-detect the upstream for each series that has a NULL upstream. This queries the git branch's remote tracking configuration and sets the upstream if a real remote (not local tracking with '.') is found. Series where auto-detection fails are still reported as a warning so the user can set them manually. Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/patman/cser_helper.py | 35 +++++++++++++++++++++++++++++++---- tools/patman/test_cseries.py | 32 ++++++++++++++++++++++---------- 2 files changed, 53 insertions(+), 14 deletions(-) diff --git a/tools/patman/cser_helper.py b/tools/patman/cser_helper.py index 0b29d52bc00..74bccfcfbc6 100644 --- a/tools/patman/cser_helper.py +++ b/tools/patman/cser_helper.py @@ -118,12 +118,39 @@ class CseriesHelper: self.db.open_it() def _check_null_upstreams(self): - """Warn about series that have no upstream set""" + """Detect and warn about series that have no upstream set + + For each series without an upstream, try to detect it from the + git branch's remote tracking configuration. Any series that + cannot be auto-detected are reported as a warning. + """ names = self.db.series_get_null_upstream() - if names: + if not names: + return + + still_null = [] + git_dir = self.gitdir or '.git' + for name in names: + remote_name = None + if gitutil.check_branch(name, git_dir=self.gitdir): + us_ref, _ = gitutil.get_upstream(git_dir, name) + if us_ref and '/' in us_ref and not us_ref.startswith( + 'refs/'): + remote_name = us_ref.split('/')[0] + if remote_name: + idnum = self.db.series_find_by_name(name) + self.db.series_set_upstream(idnum, remote_name) + tout.progress(f"Set upstream for series '{name}' to " + f"'{remote_name}'", trailer='') + else: + still_null.append(name) + + if len(still_null) < len(names): + self.db.commit() + if still_null: tout.warning( - f'{len(names)} series without an upstream:') - for name in names: + f'{len(still_null)} series without an upstream:') + for name in still_null: tout.warning(f' {name}') def close_database(self): diff --git a/tools/patman/test_cseries.py b/tools/patman/test_cseries.py index cf0d8cfdab6..23527cfa8f0 100644 --- a/tools/patman/test_cseries.py +++ b/tools/patman/test_cseries.py @@ -3367,11 +3367,20 @@ Date: .* def test_migrate_upstream_warning(self): """Test that migrating to v5 warns about series without upstream""" + self.make_git_tree() + + # Set 'first' branch to track a remote-style upstream so that + # auto-detection can find it + self.repo.config.set_multivar('branch.first.remote', '', 'origin') + self.repo.config.set_multivar('branch.first.merge', '', + 'refs/heads/main') + db = database.Database(f'{self.tmpdir}/.patman2.db') with terminal.capture(): db.open_it() - # Create a v4 database with some series + # Create a v4 database with some series; 'first' has a matching + # branch with a detectable remote, 'second' and 'third' do not with terminal.capture() as (out, _): db.migrate_to(4) self.assertEqual( @@ -3390,10 +3399,9 @@ Date: .* db.commit() db.close() - # Now open via CseriesHelper which triggers migration and check - self.make_git_tree() cser = cseries.Cseries(self.tmpdir, terminal.COLOR_NEVER) cser.topdir = self.tmpdir + cser.gitdir = self.gitdir # Point at our v4 database database.Database.instances = {} @@ -3403,18 +3411,22 @@ Date: .* old_version = cser.db.start() self.assertEqual(4, old_version) - # Set upstream on one series so only two are reported - idnum = cser.db.series_find_by_name('second') - cser.db.series_set_upstream(idnum, 'us') - cser.db.commit() - - with terminal.capture() as (_, err): + # 'first' should be auto-detected, 'second' and 'third' have no + # matching branch with a remote upstream + with terminal.capture() as (out, err): cser._check_null_upstreams() + self.assertIn("Set upstream for series 'first' to 'origin'", + out.getvalue()) lines = err.getvalue().strip().splitlines() self.assertEqual('2 series without an upstream:', lines[0]) - self.assertEqual(' first', lines[1]) + self.assertEqual(' second', lines[1]) self.assertEqual(' third', lines[2]) + # Check that 'first' was actually updated in the database + slist = cser.db.series_get_dict() + self.assertEqual('origin', slist['first'].upstream) + self.assertIsNone(slist['second'].upstream) + cser.db.close() def test_series_scan(self): -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> Add a 'series set-upstream' subcommand to allow setting or changing the upstream for an existing series. This uses the existing series_set_upstream() database method and follows the same pattern as the rename command. Usage: patman series -s <name> set-upstream -u <upstream> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/patman/cmdline.py | 4 ++++ tools/patman/control.py | 3 +++ tools/patman/cseries.py | 25 +++++++++++++++++++++++++ tools/patman/test_cseries.py | 17 +++++++++++++++++ 4 files changed, 49 insertions(+) diff --git a/tools/patman/cmdline.py b/tools/patman/cmdline.py index f80323b7853..65b9dfab4bb 100644 --- a/tools/patman/cmdline.py +++ b/tools/patman/cmdline.py @@ -298,6 +298,10 @@ def add_series_subparser(subparsers): ren.add_argument('-N', '--new-name', help='New name for the series') series_subparsers.add_parser('rm') + + sup = series_subparsers.add_parser('set-upstream') + sup.add_argument('upstream_name', nargs='?', + help='Name of the upstream for this series') series_subparsers.add_parser('rm-version', aliases=ALIASES['rm-version']) scan = series_subparsers.add_parser('scan') diff --git a/tools/patman/control.py b/tools/patman/control.py index c6fede4305b..1a65dfc2cc6 100644 --- a/tools/patman/control.py +++ b/tools/patman/control.py @@ -191,6 +191,9 @@ def do_series(args, test_db=None, pwork=None, cser=None): cser.version_remove(args.series, args.version, dry_run=args.dry_run) elif args.subcmd == 'rename': cser.rename(args.series, args.new_name, dry_run=args.dry_run) + elif args.subcmd == 'set-upstream': + cser.set_upstream(args.series, args.upstream_name, + dry_run=args.dry_run) elif args.subcmd == 'scan': cser.scan(args.series, mark=args.mark, allow_unmarked=args.allow_unmarked, end=args.upstream, diff --git a/tools/patman/cseries.py b/tools/patman/cseries.py index 6c918185387..246610215ed 100644 --- a/tools/patman/cseries.py +++ b/tools/patman/cseries.py @@ -770,6 +770,31 @@ class Cseries(cser_helper.CseriesHelper): if dry_run: tout.info('Dry run completed') + def set_upstream(self, series, ups, dry_run=False): + """Set the upstream for a series + + Args: + series (str): Name of series to use, or None to use current branch + ups (str): Name of the upstream to set + dry_run (bool): True to do a dry run + """ + if not ups: + raise ValueError('Please specify the upstream name') + ser, _ = self._parse_series_and_version(series, None) + if not ser.idnum: + raise ValueError(f"Series '{ser.name}' not found in database") + + self.db.series_set_upstream(ser.idnum, ups) + + if not dry_run: + self.commit() + else: + self.rollback() + + tout.notice(f"Set upstream for series '{ser.name}' to '{ups}'") + if dry_run: + tout.info('Dry run completed') + def scan(self, branch_name, mark=False, allow_unmarked=False, end=None, dry_run=False): """Scan a branch and make updates to the database if it has changed diff --git a/tools/patman/test_cseries.py b/tools/patman/test_cseries.py index e529d9fe61b..c6b90d5223d 100644 --- a/tools/patman/test_cseries.py +++ b/tools/patman/test_cseries.py @@ -1909,6 +1909,23 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak slist = cser.db.series_get_dict() self.assertIsNone(slist['first'].upstream) + def test_series_set_upstream(self): + """Test setting upstream via the set-upstream command""" + cser = self.get_cser() + with terminal.capture(): + cser.add('first', '', allow_unmarked=True) + + self.db_close() + with terminal.capture() as (out, _): + self.run_args('series', '-s', 'first', 'set-upstream', + 'origin') + self.assertIn("Set upstream for series 'first' to 'origin'", + out.getvalue()) + + self.db_open() + slist = cser.db.series_get_dict() + self.assertEqual('origin', slist['first'].upstream) + def test_series_add_mark(self): """Test marking a cseries with Change-Id fields""" cser = self.get_cser() -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> Allow project_set() and project_get() to accept an upstream name so that each upstream can be associated with a different patchwork project. When no upstream is specified, the existing behaviour is preserved. Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/patman/cseries.py | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/tools/patman/cseries.py b/tools/patman/cseries.py index 5d4a9592de9..56adcc5b4cd 100644 --- a/tools/patman/cseries.py +++ b/tools/patman/cseries.py @@ -637,12 +637,13 @@ class Cseries(cser_helper.CseriesHelper): 'subjects which mismatch their patches and need to be ' 'scanned') - def project_set(self, pwork, name, quiet=False): - """Set the name of the project + def project_set(self, pwork, name, ups=None, quiet=False): + """Set the name of the project for an upstream Args: pwork (Patchwork): Patchwork object to use name (str): Name of the project to use in patchwork + ups (str or None): Upstream name to associate with quiet (bool): True to skip writing the message """ tout.detail(f"Patchwork URL '{pwork.url}': finding name '{name}'") @@ -659,14 +660,20 @@ class Cseries(cser_helper.CseriesHelper): tout.detail(f'Name match: ID {proj_id}') if not proj_id: raise ValueError(f"Unknown project name '{name}'") - self.db.patchwork_update(name, proj_id, link_name) + self.db.patchwork_update(name, proj_id, link_name, ups) self.commit() if not quiet: - tout.notice(f"Project '{name}' patchwork-ID {proj_id} " - f"link-name '{link_name}'") + msg = f"Project '{name}' patchwork-ID {proj_id} " + msg += f"link-name '{link_name}'" + if ups: + msg += f" upstream '{ups}'" + tout.notice(msg) + + def project_get(self, ups=None): + """Get the details of the project for an upstream - def project_get(self): - """Get the details of the project + Args: + ups (str or None): Upstream name to look up, or None for any Returns: tuple or None if there are no settings: @@ -674,7 +681,7 @@ class Cseries(cser_helper.CseriesHelper): proj_id (int): Patchworks project ID for this project link_name (str): Patchwork's link-name for the project """ - return self.db.patchwork_get() + return self.db.patchwork_get(ups) def remove(self, name, dry_run=False): """Remove a series from the database -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> Add -u/--upstream-name to set-project and get-project so the user can specify which upstream a project belongs to. In do_series(), look up the series' upstream and use it to find the matching patchwork project, falling back to any configured project if there is no upstream-specific one. Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/patman/cmdline.py | 8 +++++++- tools/patman/control.py | 26 +++++++++++++++++++------- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/tools/patman/cmdline.py b/tools/patman/cmdline.py index 65b9dfab4bb..74571fd7220 100644 --- a/tools/patman/cmdline.py +++ b/tools/patman/cmdline.py @@ -159,10 +159,16 @@ def add_patchwork_subparser(subparsers): ['set-project', 'U-Boot'], ] patchwork_subparsers = patchwork.add_subparsers(dest='subcmd') - patchwork_subparsers.add_parser('get-project') + gproj = patchwork_subparsers.add_parser('get-project') + gproj.add_argument( + '-u', '--upstream-name', + help='Upstream to get the project for') uset = patchwork_subparsers.add_parser('set-project') uset.add_argument( 'project_name', help="Patchwork project name, e.g. 'U-Boot'") + uset.add_argument( + '-u', '--upstream-name', + help='Upstream to associate this project with') return patchwork diff --git a/tools/patman/control.py b/tools/patman/control.py index 1a65dfc2cc6..191cfc134d3 100644 --- a/tools/patman/control.py +++ b/tools/patman/control.py @@ -135,11 +135,15 @@ def do_series(args, test_db=None, pwork=None, cser=None): if args.subcmd in needs_patchwork: if not pwork: pwork = Patchwork(args.patchwork_url) - proj = cser.project_get() + ups = cser.get_series_upstream(args.series) + proj = cser.project_get(ups) + if not proj: + proj = cser.project_get() if not proj: raise ValueError( - "Please set project ID with 'patman patchwork set-project'") - _, proj_id, link_name = cser.project_get() + "Please set project ID with " + "'patman patchwork set-project'") + _, proj_id, link_name = proj pwork.project_set(proj_id, link_name) elif pwork and pwork is not True: raise ValueError( @@ -270,13 +274,21 @@ def patchwork(args, test_db=None, pwork=None): if args.subcmd == 'set-project': if not pwork: pwork = Patchwork(args.patchwork_url) - cser.project_set(pwork, args.project_name) + cser.project_set(pwork, args.project_name, + ups=args.upstream_name) elif args.subcmd == 'get-project': - info = cser.project_get() + ups = args.upstream_name + info = cser.project_get(ups) if not info: - raise ValueError("Project has not been set; use 'patman patchwork set-project'") + 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}'") + msg = (f"Project '{name}' patchwork-ID {pwid} " + f"link-name '{link_name}'") + if ups: + msg += f" upstream '{ups}'" + print(msg) else: raise ValueError(f"Unknown patchwork subcommand '{args.subcmd}'") finally: -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> A project has multiple upstreams, so the remote must always be specified when setting or getting a project. Change the parameter to a required positional argument and rename it from 'upstream' to 'remote' for clarity. Usage: patman pw set-project U-Boot us patman pw get-project us Also rename the project list column header from 'Upstream' to 'Remote'. Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/patman/cmdline.py | 10 +++++----- tools/patman/control.py | 10 +++++++--- tools/patman/cseries.py | 4 ++-- tools/patman/test_cseries.py | 16 +++++++++++----- 4 files changed, 25 insertions(+), 15 deletions(-) diff --git a/tools/patman/cmdline.py b/tools/patman/cmdline.py index f08287641dd..a1697994571 100644 --- a/tools/patman/cmdline.py +++ b/tools/patman/cmdline.py @@ -156,19 +156,19 @@ def add_patchwork_subparser(subparsers): 'patchwork', aliases=ALIASES['patchwork'], help='Manage patchwork connection') patchwork.defaults_cmds = [ - ['set-project', 'U-Boot'], + ['set-project', 'U-Boot', 'us'], ] patchwork_subparsers = patchwork.add_subparsers(dest='subcmd') gproj = patchwork_subparsers.add_parser('get-project') gproj.add_argument( - '-u', '--upstream-name', - help='Upstream to get the project for') + 'remote', nargs='?', + help='Remote to get the project for') uset = patchwork_subparsers.add_parser('set-project') uset.add_argument( 'project_name', help="Patchwork project name, e.g. 'U-Boot'") uset.add_argument( - '-u', '--upstream-name', - help='Upstream to associate this project with') + 'remote', nargs='?', + help='Remote to associate with this project') patchwork_subparsers.add_parser('list') return patchwork diff --git a/tools/patman/control.py b/tools/patman/control.py index bd9e0f8b9bd..4363802cb3b 100644 --- a/tools/patman/control.py +++ b/tools/patman/control.py @@ -272,12 +272,16 @@ def patchwork(args, test_db=None, pwork=None): try: cser.open_database() if args.subcmd == 'set-project': + if not args.remote: + raise ValueError('Please specify the remote name') if not pwork: pwork = Patchwork(args.patchwork_url) cser.project_set(pwork, args.project_name, - ups=args.upstream_name) + ups=args.remote) elif args.subcmd == 'get-project': - ups = args.upstream_name + if not args.remote: + raise ValueError('Please specify the remote name') + ups = args.remote info = cser.project_get(ups) if not info: raise ValueError( @@ -287,7 +291,7 @@ def patchwork(args, test_db=None, pwork=None): msg = (f"Project '{name}' patchwork-ID {pwid} " f"link-name '{link_name}'") if ups: - msg += f" upstream '{ups}'" + msg += f" remote '{ups}'" print(msg) elif args.subcmd == 'list': cser.project_list() diff --git a/tools/patman/cseries.py b/tools/patman/cseries.py index fdf48a4eabd..3d24dd62fae 100644 --- a/tools/patman/cseries.py +++ b/tools/patman/cseries.py @@ -666,7 +666,7 @@ class Cseries(cser_helper.CseriesHelper): msg = f"Project '{name}' patchwork-ID {proj_id} " msg += f"link-name '{link_name}'" if ups: - msg += f" upstream '{ups}'" + msg += f" remote '{ups}'" tout.notice(msg) def project_get(self, ups=None): @@ -689,7 +689,7 @@ class Cseries(cser_helper.CseriesHelper): if not settings: print('No patchwork projects configured') return - print(f"{'Project':20} {'ID':>4} {'Link name':15} Upstream") + print(f"{'Project':20} {'ID':>4} {'Link name':15} Remote") border = f"{'-' * 20} {'-' * 4} {'-' * 15} {'-' * 15}" print(border) for name, proj_id, link_name, ups in settings: diff --git a/tools/patman/test_cseries.py b/tools/patman/test_cseries.py index f21fda8d2fa..b9be1680199 100644 --- a/tools/patman/test_cseries.py +++ b/tools/patman/test_cseries.py @@ -2483,23 +2483,29 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak self.assertFalse(cser.project_get()) + cser.db.upstream_add('us', 'https://us.example.com') + cser.db.commit() + pwork = Patchwork.for_testing(self._fake_patchwork_cser) with terminal.capture() as (out, _): self.run_args('-P', 'https://url', 'patchwork', 'set-project', - 'U-Boot', pwork=pwork) + 'U-Boot', 'us', 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} " + f"link-name 'uboot' remote 'us'", out.getvalue().strip()) - name, pwid, link_name = cser.project_get() + name, pwid, link_name = cser.project_get('us') self.assertEqual('U-Boot', name) self.assertEqual(6, pwid) self.assertEqual('uboot', link_name) with terminal.capture() as (out, _): - self.run_args('-P', 'https://url', 'patchwork', 'get-project') + self.run_args('-P', 'https://url', 'patchwork', 'get-project', + 'us') self.assertEqual( - f"Project 'U-Boot' patchwork-ID {self.PROJ_ID} link-name 'uboot'", + f"Project 'U-Boot' patchwork-ID {self.PROJ_ID} " + f"link-name 'uboot' remote 'us'", out.getvalue().strip()) def test_patchwork_list(self): -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> When adding an upstream with 'us add', allow an optional project name to be specified. This creates the settings row linking the remote to its patchwork project in a single step, rather than requiring a separate 'pw set-project' call. Usage: patman us add origin https://example.com 'U-Boot' Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/patman/cmdline.py | 5 ++++- tools/patman/control.py | 6 +++++- tools/patman/cseries.py | 12 ++++++++++-- 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/tools/patman/cmdline.py b/tools/patman/cmdline.py index a1697994571..8df9966ec6d 100644 --- a/tools/patman/cmdline.py +++ b/tools/patman/cmdline.py @@ -406,7 +406,7 @@ def add_upstream_subparser(subparsers): upstream = subparsers.add_parser('upstream', aliases=ALIASES['upstream'], help='Manage upstream destinations') upstream.defaults_cmds = [ - ['add', 'us', 'http://fred'], + ['add', 'us', 'http://fred', 'U-Boot'], ['delete', 'us'], ] upstream_subparsers = upstream.add_subparsers(dest='subcmd') @@ -416,6 +416,9 @@ def add_upstream_subparser(subparsers): uadd.add_argument( 'url', help='URL to use for this upstream, e.g. ' "'https://gitlab.denx.de/u-boot/u-boot.git'") + uadd.add_argument( + 'project_name', nargs='?', + help="Patchwork project name, e.g. 'U-Boot'") udel = upstream_subparsers.add_parser('delete') udel.add_argument( 'remote_name', diff --git a/tools/patman/control.py b/tools/patman/control.py index 4363802cb3b..c8d7405f7e8 100644 --- a/tools/patman/control.py +++ b/tools/patman/control.py @@ -239,7 +239,11 @@ def upstream(args, test_db=None): try: cser.open_database() if args.subcmd == 'add': - cser.upstream_add(args.remote_name, args.url) + pwork = None + if args.project_name: + pwork = Patchwork(args.patchwork_url) + cser.upstream_add(args.remote_name, args.url, + args.project_name, pwork) elif args.subcmd == 'default': if args.unset: cser.upstream_set_default(None) diff --git a/tools/patman/cseries.py b/tools/patman/cseries.py index 7237dd575cd..c48267964a1 100644 --- a/tools/patman/cseries.py +++ b/tools/patman/cseries.py @@ -1145,16 +1145,24 @@ class Cseries(cser_helper.CseriesHelper): self.rollback() tout.info('Dry run completed') - def upstream_add(self, name, url): + def upstream_add(self, name, url, project=None, pwork=None): """Add a new upstream tree Args: name (str): Name of the tree url (str): URL for the tree + project (str or None): Patchwork project name to associate + pwork (Patchwork or None): Patchwork object for looking up + the project """ self.db.upstream_add(name, url) + if project: + self.project_set(pwork, project, ups=name, quiet=True) self.commit() - tout.notice(f"Added upstream '{name}' ({url})") + msg = f"Added upstream '{name}' ({url})" + if project: + msg += f" project '{project}'" + tout.notice(msg) def upstream_list(self): """List the upstream repos -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> Currently patman uses a single global --patchwork-url flag. Add a patchwork_url field to each upstream so the URL can be looked up automatically from the database. Update upstream_add() and upstream_get_dict() to handle the new field and add upstream_get_patchwork_url() for direct lookup. Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/patman/cmdline.py | 6 +++- tools/patman/control.py | 28 +++++++++++++++---- tools/patman/cseries.py | 27 ++++++++++++++---- tools/patman/database.py | 34 +++++++++++++++++++---- tools/patman/test_cseries.py | 53 ++++++++++++++++++++++++++++++------ 5 files changed, 122 insertions(+), 26 deletions(-) diff --git a/tools/patman/cmdline.py b/tools/patman/cmdline.py index 090d9845ec1..f24d0aacdff 100644 --- a/tools/patman/cmdline.py +++ b/tools/patman/cmdline.py @@ -406,7 +406,7 @@ def add_upstream_subparser(subparsers): upstream = subparsers.add_parser('upstream', aliases=ALIASES['upstream'], help='Manage upstream destinations') upstream.defaults_cmds = [ - ['add', 'us', 'http://fred', 'U-Boot'], + ['add', 'us', 'http://fred', '-p', 'http://pw', 'U-Boot'], ['delete', 'us'], ] upstream_subparsers = upstream.add_subparsers(dest='subcmd') @@ -416,6 +416,10 @@ def add_upstream_subparser(subparsers): uadd.add_argument( 'url', help='URL to use for this upstream, e.g. ' "'https://gitlab.denx.de/u-boot/u-boot.git'") + uadd.add_argument( + '-p', '--patchwork-url', + help='URL of patchwork server for this upstream, e.g. ' + "'https://patchwork.ozlabs.org'") uadd.add_argument( 'project_name', nargs='?', help="Patchwork project name, e.g. 'U-Boot'") diff --git a/tools/patman/control.py b/tools/patman/control.py index c65fd3d5f1f..e1f52300a17 100644 --- a/tools/patman/control.py +++ b/tools/patman/control.py @@ -134,8 +134,18 @@ def do_series(args, test_db=None, pwork=None, cser=None): cser.open_database() if args.subcmd in needs_patchwork: if not pwork: - pwork = Patchwork(args.patchwork_url) ups = cser.get_series_upstream(args.series) + pw_url = None + if ups: + pw_url = cser.db.upstream_get_patchwork_url(ups) + if not pw_url: + pw_url = args.patchwork_url + if not pw_url: + raise ValueError( + 'No patchwork URL found for upstream ' + f"'{ups}'; use 'patman upstream add' with " + '-p or pass --patchwork-url') + pwork = Patchwork(pw_url) proj = cser.project_get(ups) if not proj: proj = cser.project_get() @@ -239,11 +249,9 @@ def upstream(args, test_db=None): try: cser.open_database() if args.subcmd == 'add': - pwork = None - if args.project_name: - pwork = Patchwork(args.patchwork_url) cser.upstream_add(args.remote_name, args.url, - args.project_name, pwork) + args.project_name, + patchwork_url=args.patchwork_url) elif args.subcmd == 'default': if args.unset: cser.upstream_set_default(None) @@ -279,7 +287,15 @@ def patchwork(args, test_db=None, pwork=None): if not args.remote: raise ValueError('Please specify the remote name') if not pwork: - pwork = Patchwork(args.patchwork_url) + pw_url = cser.db.upstream_get_patchwork_url(args.remote) + if not pw_url: + pw_url = args.patchwork_url + if not pw_url: + raise ValueError( + f"No patchwork URL for remote '{args.remote}'" + "; use 'patman upstream add' with -p" + ' or pass --patchwork-url') + pwork = Patchwork(pw_url) cser.project_set(pwork, args.project_name, ups=args.remote) elif args.subcmd == 'get-project': diff --git a/tools/patman/cseries.py b/tools/patman/cseries.py index c48267964a1..612ccfda7dc 100644 --- a/tools/patman/cseries.py +++ b/tools/patman/cseries.py @@ -16,6 +16,7 @@ from u_boot_pylib import terminal from u_boot_pylib import tout from patman import patchstream +from patman.patchwork import Patchwork from patman import cser_helper from patman.cser_helper import AUTOLINK, oid from patman import send @@ -1145,7 +1146,8 @@ class Cseries(cser_helper.CseriesHelper): self.rollback() tout.info('Dry run completed') - def upstream_add(self, name, url, project=None, pwork=None): + def upstream_add(self, name, url, project=None, pwork=None, + patchwork_url=None): """Add a new upstream tree Args: @@ -1154,12 +1156,21 @@ class Cseries(cser_helper.CseriesHelper): project (str or None): Patchwork project name to associate pwork (Patchwork or None): Patchwork object for looking up the project + patchwork_url (str or None): URL of the patchwork server for + this upstream """ - self.db.upstream_add(name, url) + self.db.upstream_add(name, url, patchwork_url) if project: + if not pwork: + if not patchwork_url: + raise ValueError( + 'Patchwork URL is required when setting a project') + pwork = Patchwork(patchwork_url) self.project_set(pwork, project, ups=name, quiet=True) self.commit() msg = f"Added upstream '{name}' ({url})" + if patchwork_url: + msg += f" patchwork '{patchwork_url}'" if project: msg += f" project '{project}'" tout.notice(msg) @@ -1167,14 +1178,20 @@ class Cseries(cser_helper.CseriesHelper): def upstream_list(self): """List the upstream repos - Shows a list of the repos, obtained from the database + Shows a list of the repos, obtained from the database, along with + any associated patchwork project """ udict = self.get_upstream_dict() for name, items in udict.items(): - url, is_default = items + url, is_default, patchwork_url = items default = 'default' if is_default else '' - print(f'{name:15.15} {default:8} {url}') + proj = self.db.patchwork_get(name) + proj_name = proj[0] if proj else '' + line = f'{name:10.10} {default:8} {proj_name:20} {url}' + if patchwork_url: + line += f' pw:{patchwork_url}' + print(line) def upstream_set_default(self, name): """Set the default upstream target diff --git a/tools/patman/database.py b/tools/patman/database.py index 2f21e62605f..f7ab2d84877 100644 --- a/tools/patman/database.py +++ b/tools/patman/database.py @@ -791,19 +791,21 @@ class Database: # pylint:disable=R0904 # upstream functions - def upstream_add(self, name, url): + def upstream_add(self, name, url, patchwork_url=None): """Add a new upstream record Args: name (str): Name of the tree url (str): URL for the tree + patchwork_url (str or None): URL of the patchwork server Raises: ValueError if the name already exists in the database """ try: self.execute( - 'INSERT INTO upstream (name, url) VALUES (?, ?)', (name, url)) + 'INSERT INTO upstream (name, url, patchwork_url) ' + 'VALUES (?, ?, ?)', (name, url, patchwork_url)) except sqlite3.IntegrityError as exc: if 'UNIQUE constraint failed: upstream.name' in str(exc): raise ValueError(f"Upstream '{name}' already exists") from exc @@ -853,18 +855,38 @@ class Database: # pylint:disable=R0904 self.rollback() raise ValueError(f"No such upstream '{name}'") + def upstream_get_patchwork_url(self, name): + """Get the patchwork URL for an upstream + + Args: + name (str): Upstream name + + Return: + str or None: Patchwork URL, or None if not set + """ + res = self.execute( + 'SELECT patchwork_url FROM upstream WHERE name = ?', (name,)) + rec = res.fetchone() + if rec: + return rec[0] + return None + def upstream_get_dict(self): """Get a list of upstream entries from the database Return: OrderedDict: key (str): upstream name - value (str): url + value: tuple: + str: url + bool: is_default + str or None: patchwork_url """ - res = self.execute('SELECT name, url, is_default FROM upstream') + res = self.execute( + 'SELECT name, url, is_default, patchwork_url FROM upstream') udict = OrderedDict() - for name, url, is_default in res.fetchall(): - udict[name] = url, is_default + for name, url, is_default, patchwork_url in res.fetchall(): + udict[name] = url, is_default, patchwork_url return udict # patchwork functions diff --git a/tools/patman/test_cseries.py b/tools/patman/test_cseries.py index 083727889f8..b14c6c22ddc 100644 --- a/tools/patman/test_cseries.py +++ b/tools/patman/test_cseries.py @@ -1730,14 +1730,14 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak cser.upstream_add('us', 'https://one') ulist = cser.get_upstream_dict() self.assertEqual(1, len(ulist)) - self.assertEqual(('https://one', None), ulist['us']) + self.assertEqual(('https://one', None, None), ulist['us']) 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']) - self.assertEqual(('git@two', None), ulist['ci']) + self.assertEqual(('https://one', None, None), ulist['us']) + self.assertEqual(('git@two', None, None), ulist['ci']) # Try to add a duplicate with self.assertRaises(ValueError) as exc: @@ -1748,8 +1748,39 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak cser.upstream_list() lines = out.getvalue().splitlines() self.assertEqual(2, len(lines)) - self.assertEqual('us https://one', lines[0]) - self.assertEqual('ci git@two', lines[1]) + self.assertEqual( + 'us https://one', + lines[0]) + self.assertEqual( + 'ci git@two', + lines[1]) + + def test_upstream_add_patchwork_url(self): + """Test adding an upstream with a patchwork URL""" + cser = self.get_cser() + + with terminal.capture(): + cser.upstream_add('us', 'https://one', + patchwork_url='https://pw.example.com') + ulist = cser.get_upstream_dict() + self.assertEqual(1, len(ulist)) + self.assertEqual( + ('https://one', None, 'https://pw.example.com'), ulist['us']) + + # Check that the patchwork URL shows in the list + with terminal.capture() as (out, _): + cser.upstream_list() + lines = out.getvalue().splitlines() + self.assertEqual(1, len(lines)) + self.assertIn('pw:https://pw.example.com', lines[0]) + + # Check database lookup + pw_url = cser.db.upstream_get_patchwork_url('us') + self.assertEqual('https://pw.example.com', pw_url) + + # Non-existent upstream returns None + pw_url = cser.db.upstream_get_patchwork_url('nonexistent') + self.assertIsNone(pw_url) def test_upstream_add_cmdline(self): """Test adding an upsream with cmdline""" @@ -1760,7 +1791,9 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak self.run_args('upstream', 'list') lines = out.getvalue().splitlines() self.assertEqual(1, len(lines)) - self.assertEqual('us https://one', lines[0]) + self.assertEqual( + 'us https://one', + lines[0]) def test_upstream_default(self): """Operation of the default upstream""" @@ -1791,8 +1824,12 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak cser.upstream_list() lines = out.getvalue().splitlines() self.assertEqual(2, len(lines)) - self.assertEqual('us https://one', lines[0]) - self.assertEqual('ci default git@two', lines[1]) + self.assertEqual( + 'us https://one', + lines[0]) + self.assertEqual( + 'ci default git@two', + lines[1]) cser.upstream_set_default(None) self.assertIsNone(cser.upstream_get_default()) -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> The series, upstream and patchwork subcommands each have a list command, but use inconsistent names ('ls' for series, 'list' for the others). Add aliases so both 'ls' and 'list' work everywhere. Since argparse stores the alias name rather than the primary name, handle both in the dispatch. Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/patman/cmdline.py | 9 +++++---- tools/patman/control.py | 4 ++-- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/tools/patman/cmdline.py b/tools/patman/cmdline.py index 57c56b099e1..3fef37e738d 100644 --- a/tools/patman/cmdline.py +++ b/tools/patman/cmdline.py @@ -27,10 +27,11 @@ ALIASES = { 'patchwork': ['pw'], 'upstream': ['us'], - # Series aliases + # Subcommand aliases 'archive': ['ar'], 'autolink': ['au'], 'gather': ['g'], + 'ls': ['list'], 'open': ['o'], 'progress': ['p', 'pr', 'prog'], 'rm-version': ['rmv'], @@ -169,7 +170,7 @@ def add_patchwork_subparser(subparsers): uset.add_argument( 'remote', nargs='?', help='Remote to associate with this project') - patchwork_subparsers.add_parser('list') + patchwork_subparsers.add_parser('ls', aliases=['list']) return patchwork @@ -279,7 +280,7 @@ def add_series_subparser(subparsers): series_subparsers.add_parser('get-link') series_subparsers.add_parser('inc') - ls = series_subparsers.add_parser('ls') + ls = series_subparsers.add_parser('ls', aliases=['list']) _add_archived(ls) mar = series_subparsers.add_parser('mark') @@ -429,7 +430,7 @@ def add_upstream_subparser(subparsers): udel.add_argument( 'remote_name', help="Git remote name used for this upstream, e.g. 'us'") - upstream_subparsers.add_parser('list') + upstream_subparsers.add_parser('ls', aliases=['list']) udef = upstream_subparsers.add_parser('default') udef.add_argument('-u', '--unset', action='store_true', help='Unset the default upstream') diff --git a/tools/patman/control.py b/tools/patman/control.py index 7f162b4aadb..352c76bd14e 100644 --- a/tools/patman/control.py +++ b/tools/patman/control.py @@ -263,7 +263,7 @@ def upstream(args, test_db=None): print(result if result else 'unset') elif args.subcmd == 'delete': cser.upstream_delete(args.remote_name) - elif args.subcmd == 'list': + elif args.subcmd == 'ls': cser.upstream_list() else: raise ValueError(f"Unknown upstream subcommand '{args.subcmd}'") @@ -314,7 +314,7 @@ def patchwork(args, test_db=None, pwork=None): if ups: msg += f" remote '{ups}'" print(msg) - elif args.subcmd == 'list': + elif args.subcmd == 'ls': cser.project_list() else: raise ValueError(f"Unknown patchwork subcommand '{args.subcmd}'") -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> Add identity parameter to gitutil.email_patches() and send.email_patches(), passing --identity to git send-email when set. In send.send(), apply series_to from upstream settings: if no Series-to tag is present, use the upstream's configured alias. Warn if the tag does not match the expected value. Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/patman/send.py | 20 +++++++++++++++++--- tools/u_boot_pylib/gitutil.py | 5 ++++- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/tools/patman/send.py b/tools/patman/send.py index 08a916aff1a..51d2c533540 100644 --- a/tools/patman/send.py +++ b/tools/patman/send.py @@ -47,7 +47,8 @@ def check_patches(series, patch_files, run_checkpatch, verbose, use_tree, cwd): def email_patches(col, series, cover_fname, patch_files, process_tags, its_a_go, ignore_bad_tags, add_maintainers, get_maintainer_script, limit, - dry_run, in_reply_to, thread, smtp_server, cwd=None): + dry_run, in_reply_to, thread, smtp_server, identity=None, + cwd=None): """Email patches to the recipients This emails out the patches and cover letter using 'git send-email'. Each @@ -86,6 +87,7 @@ def email_patches(col, series, cover_fname, patch_files, process_tags, its_a_go, thread (bool): True to add --thread to git send-email (make all patches reply to cover-letter or first patch in series) smtp_server (str): SMTP server to use to send patches (None for default) + identity (str or None): Git sendemail identity to use cwd (str): Path to use for patch files (None to use current dir) Return: @@ -101,7 +103,8 @@ def email_patches(col, series, cover_fname, patch_files, process_tags, its_a_go, cmd = gitutil.email_patches( series, cover_fname, patch_files, dry_run, not ignore_bad_tags, cc_file, alias=settings.alias, in_reply_to=in_reply_to, - thread=thread, smtp_server=smtp_server, cwd=cwd) + thread=thread, smtp_server=smtp_server, identity=identity, + cwd=cwd) else: print(col.build(col.RED, "Not sending emails due to errors/warnings")) @@ -182,6 +185,16 @@ def send(args, git_dir=None, cwd=None): col, args.branch, args.count, args.start, args.end, args.ignore_binary, args.add_signoff, keep_change_id=args.keep_change_id, git_dir=git_dir, cwd=cwd) + + series_to = getattr(args, 'series_to', None) + if series_to: + to_list = series.get('to', []) + if to_list and series_to not in to_list: + print(f"WARNING: Series-to tag {to_list} does not include " + f"expected '{series_to}' from upstream settings") + if not to_list: + series['to'] = [series_to] + ok = check_patches(series, patch_files, args.check_patch, args.verbose, args.check_patch_use_tree, cwd) @@ -192,6 +205,7 @@ def send(args, git_dir=None, cwd=None): col, series, cover_fname, patch_files, args.process_tags, its_a_go, args.ignore_bad_tags, args.add_maintainers, args.get_maintainer_script, args.limit, args.dry_run, - args.in_reply_to, args.thread, args.smtp_server, cwd=cwd) + args.in_reply_to, args.thread, args.smtp_server, + identity=getattr(args, 'identity', None), cwd=cwd) return cmd and its_a_go and not args.dry_run diff --git a/tools/u_boot_pylib/gitutil.py b/tools/u_boot_pylib/gitutil.py index 34b4dbb4839..c96cd251776 100644 --- a/tools/u_boot_pylib/gitutil.py +++ b/tools/u_boot_pylib/gitutil.py @@ -459,7 +459,7 @@ def check_suppress_cc_config(): def email_patches(series, cover_fname, args, dry_run, warn_on_error, cc_fname, alias, self_only=False, in_reply_to=None, thread=False, - smtp_server=None, cwd=None): + smtp_server=None, identity=None, cwd=None): """Email a patch series. Args: @@ -479,6 +479,7 @@ def email_patches(series, cover_fname, args, dry_run, warn_on_error, cc_fname, thread (bool): True to add --thread to git send-email (make all patches reply to cover-letter or first patch in series) smtp_server (str or None): SMTP server to use to send patches + identity (str or None): Git sendemail identity to use cwd (str): Path to use for patch files (None to use current dir) Returns: @@ -537,6 +538,8 @@ send --cc-cmd cc-fname" cover p1 p2' warn_on_error) cc = [] cmd = ['git', 'send-email', '--annotate'] + if identity: + cmd.append(f'--identity={identity}') if smtp_server: cmd.append(f'--smtp-server={smtp_server}') if in_reply_to: -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> Add a 'set' subcommand for upstream that allows updating individual fields on an existing upstream without having to delete and re-add it. Supports -p/--patchwork-url, -I/--identity, -t/--series-to, -m/--no-maintainers, --maintainers, --no-tags and --tags. For example: patman upstream set us -I chromium -t concept -m Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/patman/cmdline.py | 25 ++++++++++++++++++ tools/patman/control.py | 19 ++++++++++++++ tools/patman/cseries.py | 14 ++++++++++ tools/patman/database.py | 34 ++++++++++++++++++++++++ tools/patman/test_cseries.py | 50 ++++++++++++++++++++++++++++++++++++ 5 files changed, 142 insertions(+) diff --git a/tools/patman/cmdline.py b/tools/patman/cmdline.py index 6141f106fde..5bf917e4712 100644 --- a/tools/patman/cmdline.py +++ b/tools/patman/cmdline.py @@ -411,6 +411,7 @@ def add_upstream_subparser(subparsers): upstream.defaults_cmds = [ ['add', 'us', 'http://fred', '-p', 'http://pw', 'U-Boot'], ['delete', 'us'], + ['set', 'us'], ] upstream_subparsers = upstream.add_subparsers(dest='subcmd') uadd = upstream_subparsers.add_parser('add') @@ -443,6 +444,30 @@ def add_upstream_subparser(subparsers): 'remote_name', help="Git remote name used for this upstream, e.g. 'us'") upstream_subparsers.add_parser('ls', aliases=['list']) + uset = upstream_subparsers.add_parser('set') + uset.add_argument('remote_name', + help="Git remote name used for this upstream, e.g. 'us'") + uset.add_argument( + '-p', '--patchwork-url', + help='URL of patchwork server for this upstream') + uset.add_argument( + '-I', '--identity', + help="Git sendemail identity to use, e.g. 'chromium'") + uset.add_argument( + '-t', '--series-to', + help="Patman alias for the To address, e.g. 'u-boot'") + uset.add_argument( + '-m', '--no-maintainers', action='store_true', default=None, + help='Skip get_maintainer.pl for this upstream') + uset.add_argument( + '--maintainers', action='store_true', default=None, + help='Enable get_maintainer.pl for this upstream') + uset.add_argument( + '--no-tags', action='store_true', default=None, + help='Skip subject-tag alias processing for this upstream') + uset.add_argument( + '--tags', action='store_true', default=None, + help='Enable subject-tag alias processing for this upstream') udef = upstream_subparsers.add_parser('default') udef.add_argument('-u', '--unset', action='store_true', help='Unset the default upstream') diff --git a/tools/patman/control.py b/tools/patman/control.py index 689fd732dec..cabd2138bb3 100644 --- a/tools/patman/control.py +++ b/tools/patman/control.py @@ -267,6 +267,25 @@ def upstream(args, test_db=None): print(result if result else 'unset') elif args.subcmd == 'delete': cser.upstream_delete(args.remote_name) + elif args.subcmd == 'set': + kwargs = {} + if args.patchwork_url is not None: + kwargs['patchwork_url'] = args.patchwork_url + if args.identity is not None: + kwargs['identity'] = args.identity + if args.series_to is not None: + kwargs['series_to'] = args.series_to + if args.no_maintainers: + kwargs['no_maintainers'] = True + elif args.maintainers: + kwargs['no_maintainers'] = False + if args.no_tags: + kwargs['no_tags'] = True + elif args.tags: + kwargs['no_tags'] = False + if not kwargs: + raise ValueError('No settings to update') + cser.upstream_set(args.remote_name, **kwargs) elif args.subcmd == 'ls': cser.upstream_list() else: diff --git a/tools/patman/cseries.py b/tools/patman/cseries.py index 36f0b432073..588e83298bd 100644 --- a/tools/patman/cseries.py +++ b/tools/patman/cseries.py @@ -1237,6 +1237,20 @@ class Cseries(cser_helper.CseriesHelper): line += ' no-tags' print(line) + def upstream_set(self, name, **kwargs): + """Update settings on an existing upstream + + See Database.upstream_set() for permitted kwargs. + + Args: + name (str): Name of the upstream remote to update + kwargs: Fields to update + """ + self.db.upstream_set(name, **kwargs) + self.commit() + parts = [f'{k}={v!r}' for k, v in kwargs.items()] + tout.notice(f"Updated upstream '{name}': {', '.join(parts)}") + def upstream_set_default(self, name): """Set the default upstream target diff --git a/tools/patman/database.py b/tools/patman/database.py index 19df3c3bc82..dcd39ea2c69 100644 --- a/tools/patman/database.py +++ b/tools/patman/database.py @@ -863,6 +863,40 @@ class Database: # pylint:disable=R0904 self.rollback() raise ValueError(f"No such upstream '{name}'") + def upstream_set(self, name, **kwargs): + """Update fields on an existing upstream + + Args: + name (str): Name of the upstream remote to update + kwargs: Fields to update, each being one of: + patchwork_url (str): URL of the patchwork server, e.g. + 'patchwork.ozlabs.org' + identity (str): Git sendemail identity to use when + sending, corresponding to a [sendemail "<identity>"] + section in .gitconfig + series_to (str): Mailing-list address to use as the + default To: for this upstream + no_maintainers (bool): True to skip + get_maintainer.pl when sending + no_tags (bool): True to skip processing of subject + tags (e.g. 'dm:') when sending + + Raises: + ValueError: Upstream does not exist or invalid field + """ + valid = {'patchwork_url', 'identity', 'series_to', + 'no_maintainers', 'no_tags'} + invalid = set(kwargs) - valid + if invalid: + raise ValueError(f"Invalid upstream field(s): {invalid}") + for field, value in kwargs.items(): + self.execute( + f'UPDATE upstream SET {field} = ? WHERE name = ?', + (value, name)) + if self.rowcount() != 1: + self.rollback() + raise ValueError(f"No such upstream '{name}'") + def upstream_get_patchwork_url(self, name): """Get the patchwork URL for an upstream diff --git a/tools/patman/test_cseries.py b/tools/patman/test_cseries.py index 8f56627958e..fcfe6610321 100644 --- a/tools/patman/test_cseries.py +++ b/tools/patman/test_cseries.py @@ -1813,6 +1813,56 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak 'us https://one', lines[0]) + def test_upstream_set(self): + """Test updating settings on an existing upstream""" + cser = self.get_cser() + + with terminal.capture(): + cser.upstream_add('us', 'https://one') + + # Set identity and series_to + with terminal.capture(): + cser.upstream_set('us', identity='chromium', series_to='concept') + settings = cser.db.upstream_get_send_settings('us') + self.assertEqual('chromium', settings[0]) + self.assertEqual('concept', settings[1]) + + # Set boolean flags + with terminal.capture(): + cser.upstream_set('us', no_maintainers=True, no_tags=True) + settings = cser.db.upstream_get_send_settings('us') + self.assertTrue(settings[2]) + self.assertTrue(settings[3]) + + # Clear boolean flags + with terminal.capture(): + cser.upstream_set('us', no_maintainers=False, no_tags=False) + settings = cser.db.upstream_get_send_settings('us') + self.assertFalse(settings[2]) + self.assertFalse(settings[3]) + + # Non-existent upstream + with self.assertRaises(ValueError) as exc: + cser.upstream_set('nonexistent', identity='x') + self.assertIn('nonexistent', str(exc.exception)) + + def test_upstream_set_cmdline(self): + """Test upstream set via the command line""" + with terminal.capture(): + self.run_args('upstream', 'add', 'us', 'https://one') + + with terminal.capture(): + self.run_args('upstream', 'set', 'us', '-I', 'chromium', + '-t', 'concept', '-m', '--no-tags') + + with terminal.capture() as (out, _): + self.run_args('upstream', 'list') + line = out.getvalue().strip() + self.assertIn('id:chromium', line) + self.assertIn('to:concept', line) + self.assertIn('no-maintainers', line) + self.assertIn('no-tags', line) + def test_upstream_default(self): """Operation of the default upstream""" cser = self.get_cser() -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> Add a header row and separator to 'upstream list' output, matching the style used by 'series ls'. Use narrower columns and show the default upstream with '*' instead of 'default'. Collect optional fields (patchwork URL, identity, series-to, flags) into an Options column. Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/patman/cseries.py | 21 +++++++++++++-------- tools/patman/test_cseries.py | 35 ++++++++++++++--------------------- 2 files changed, 27 insertions(+), 29 deletions(-) diff --git a/tools/patman/cseries.py b/tools/patman/cseries.py index 588e83298bd..47da57acc0f 100644 --- a/tools/patman/cseries.py +++ b/tools/patman/cseries.py @@ -1218,24 +1218,29 @@ class Cseries(cser_helper.CseriesHelper): """ udict = self.get_upstream_dict() + print(f"{'Name':6} {'Def':3} {'Project':10} {'URL':40} Options") + border = (f"{'-' * 6} {'-' * 3} {'-' * 10} {'-' * 40} " + f"{'-' * 20}") + print(border) for name, items in udict.items(): (url, is_default, patchwork_url, identity, series_to, no_maintainers, no_tags) = items - default = 'default' if is_default else '' + default = '*' if is_default else '' proj = self.db.patchwork_get(name) proj_name = proj[0] if proj else '' - line = f'{name:10.10} {default:8} {proj_name:20} {url}' + opts = [] if patchwork_url: - line += f' pw:{patchwork_url}' + opts.append(f'pw:{patchwork_url}') if identity: - line += f' id:{identity}' + opts.append(f'id:{identity}') if series_to: - line += f' to:{series_to}' + opts.append(f'to:{series_to}') if no_maintainers: - line += ' no-maintainers' + opts.append('no-maintainers') if no_tags: - line += ' no-tags' - print(line) + opts.append('no-tags') + print(f'{name:6} {default:3} {proj_name:10} {url:40} ' + f'{" ".join(opts)}') def upstream_set(self, name, **kwargs): """Update settings on an existing upstream diff --git a/tools/patman/test_cseries.py b/tools/patman/test_cseries.py index fcfe6610321..3d475956ff9 100644 --- a/tools/patman/test_cseries.py +++ b/tools/patman/test_cseries.py @@ -1764,13 +1764,10 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak with terminal.capture() as (out, _): cser.upstream_list() lines = out.getvalue().splitlines() - self.assertEqual(2, len(lines)) - self.assertEqual( - 'us https://one', - lines[0]) - self.assertEqual( - 'ci git@two', - lines[1]) + self.assertEqual(4, len(lines)) + self.assertIn('Name', lines[0]) + self.assertIn('https://one', lines[2]) + self.assertIn('git@two', lines[3]) def test_upstream_add_patchwork_url(self): """Test adding an upstream with a patchwork URL""" @@ -1789,8 +1786,8 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak with terminal.capture() as (out, _): cser.upstream_list() lines = out.getvalue().splitlines() - self.assertEqual(1, len(lines)) - self.assertIn('pw:https://pw.example.com', lines[0]) + self.assertEqual(3, len(lines)) + self.assertIn('pw:https://pw.example.com', lines[2]) # Check database lookup pw_url = cser.db.upstream_get_patchwork_url('us') @@ -1808,10 +1805,9 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak with terminal.capture() as (out, _): self.run_args('upstream', 'list') lines = out.getvalue().splitlines() - self.assertEqual(1, len(lines)) - self.assertEqual( - 'us https://one', - lines[0]) + self.assertEqual(3, len(lines)) + self.assertIn('us', lines[2]) + self.assertIn('https://one', lines[2]) def test_upstream_set(self): """Test updating settings on an existing upstream""" @@ -1891,13 +1887,9 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak with terminal.capture() as (out, _): cser.upstream_list() lines = out.getvalue().splitlines() - self.assertEqual(2, len(lines)) - self.assertEqual( - 'us https://one', - lines[0]) - self.assertEqual( - 'ci default git@two', - lines[1]) + self.assertEqual(4, len(lines)) + self.assertNotIn('*', lines[2]) + self.assertIn('*', lines[3]) cser.upstream_set_default(None) self.assertIsNone(cser.upstream_get_default()) @@ -1982,7 +1974,8 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak self.run_args('upstream', 'delete', 'ci') with terminal.capture() as (out, _): self.run_args('upstream', 'list') - self.assertFalse(out.getvalue().strip()) + lines = out.getvalue().splitlines() + self.assertEqual(2, len(lines)) def test_series_upstream(self): """Test upstream field in the series table""" -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> Move the sendemail identity message from upstream lookup to the send path, so it appears closer to the actual send. Change the Series-to mismatch from a warning to an error, since sending to the wrong list is likely unintentional. Widen the URL column in upstream list to avoid truncation. Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/patman/cseries.py | 8 +++----- tools/patman/send.py | 11 ++++++++--- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/tools/patman/cseries.py b/tools/patman/cseries.py index 0f1766f5539..81a8712b3de 100644 --- a/tools/patman/cseries.py +++ b/tools/patman/cseries.py @@ -941,8 +941,6 @@ class Cseries(cser_helper.CseriesHelper): identity, series_to, no_maintainers, no_tags = settings if identity and not getattr(args, 'identity', None): args.identity = identity - print(f"Using sendemail identity '{identity}'" - f" from upstream '{ups}'") if series_to: args.series_to = series_to if no_maintainers: @@ -1220,8 +1218,8 @@ class Cseries(cser_helper.CseriesHelper): """ udict = self.get_upstream_dict() - print(f"{'Name':6} {'Def':3} {'Project':10} {'URL':40} Options") - border = (f"{'-' * 6} {'-' * 3} {'-' * 10} {'-' * 40} " + print(f"{'Name':6} {'Def':3} {'Project':10} {'URL':44} Options") + border = (f"{'-' * 6} {'-' * 3} {'-' * 10} {'-' * 44} " f"{'-' * 20}") print(border) for name, items in udict.items(): @@ -1241,7 +1239,7 @@ class Cseries(cser_helper.CseriesHelper): opts.append('no-maintainers') if no_tags: opts.append('no-tags') - print(f'{name:6} {default:3} {proj_name:10} {url:40} ' + print(f'{name:6} {default:3} {proj_name:10} {url:44} ' f'{" ".join(opts)}') def upstream_set(self, name, **kwargs): diff --git a/tools/patman/send.py b/tools/patman/send.py index 51d2c533540..eb9a8e0da2e 100644 --- a/tools/patman/send.py +++ b/tools/patman/send.py @@ -190,8 +190,9 @@ def send(args, git_dir=None, cwd=None): if series_to: to_list = series.get('to', []) if to_list and series_to not in to_list: - print(f"WARNING: Series-to tag {to_list} does not include " - f"expected '{series_to}' from upstream settings") + raise ValueError( + f"Series-to tag {to_list} does not match " + f"expected '{series_to}' from upstream settings") if not to_list: series['to'] = [series_to] @@ -200,12 +201,16 @@ def send(args, git_dir=None, cwd=None): ok = ok and gitutil.check_suppress_cc_config() + identity = getattr(args, 'identity', None) + if identity: + print(f"Using sendemail identity '{identity}'") + its_a_go = ok or args.ignore_errors cmd = email_patches( col, series, cover_fname, patch_files, args.process_tags, its_a_go, args.ignore_bad_tags, args.add_maintainers, args.get_maintainer_script, args.limit, args.dry_run, args.in_reply_to, args.thread, args.smtp_server, - identity=getattr(args, 'identity', None), cwd=cwd) + identity=identity, cwd=cwd) return cmd and its_a_go and not args.dry_run -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> Strip Co-developed-by and Co-Authored-By tags that use noreply@ email addresses, since these indicate AI-generated contributions rather than human co-developers. Human co-developer tags are preserved. Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/patman/patchstream.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tools/patman/patchstream.py b/tools/patman/patchstream.py index 429bbfb3bac..27cd1980e38 100644 --- a/tools/patman/patchstream.py +++ b/tools/patman/patchstream.py @@ -23,6 +23,10 @@ from u_boot_pylib import gitutil RE_REMOVE = re.compile(r'^BUG=|^TEST=|^BRANCH=|^Review URL:' r'|Reviewed-on:|Commit-\w*:') +# AI co-developer tags to remove (noreply@ indicates non-human) +RE_AI_TAG = re.compile(r'^(Co-developed-by|Co-Authored-By):.*noreply@', + re.IGNORECASE) + # Lines which are allowed after a TEST= line RE_ALLOWED_AFTER_TEST = re.compile('^Signed-off-by:') @@ -445,7 +449,8 @@ class PatchStream: self.commit.subject = line # Detect the tags we want to remove, and skip blank lines - elif RE_REMOVE.match(line) and not commit_tag_match: + elif (RE_REMOVE.match(line) or RE_AI_TAG.match(line) + ) and not commit_tag_match: self.skip_blank = True # TEST= should be the last thing in the commit, so remove -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> Add a step-by-step guide covering how to configure multiple upstreams with different patchwork servers, sendemail identities, To addresses and send options. Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/patman/patman.rst | 167 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 167 insertions(+) diff --git a/tools/patman/patman.rst b/tools/patman/patman.rst index 549e203c254..87e0984c0d7 100644 --- a/tools/patman/patman.rst +++ b/tools/patman/patman.rst @@ -936,6 +936,173 @@ Here is a sample 'progress' view: :width: 800 :alt: Patman showing the progress view +Multiple upstreams +~~~~~~~~~~~~~~~~~~ + +If you send patches to more than one upstream tree (e.g. U-Boot mainline and +the U-Boot concept tree), you can configure patman with multiple upstreams so +that each series automatically uses the correct send settings for its +destination. The trees may use different mailing lists, patchwork servers and +SMTP credentials. + +The key concepts are: + +**Upstream** + A remote git repository that you send patches to. Each upstream has a name + (e.g. ``us``, ``ci``) and a URL. Upstreams can also carry send settings: + a patchwork URL, a sendemail identity, a To address and flags controlling + ``get_maintainer.pl`` and subject-tag processing. Use ``patman upstream`` + commands to manage these. + +**Patchwork project** + The patchwork server tracks patches for a project (e.g. ``U-Boot``). + Each upstream can point to a different patchwork server, and patman needs + to know the project name on that server so it can look up series links. + Use ``patman patchwork set-project`` to configure this per upstream. + +**Sendemail identity** + Git supports multiple SMTP configurations via ``[sendemail "<name>"]`` + sections in ``.gitconfig``. An upstream can reference one of these + identities so that patman passes ``--identity`` to ``git send-email`` + automatically. + +**Series upstream** + Each series can be associated with an upstream. When you send the series, + patman looks up the upstream's settings and applies them. This means the + correct identity, To address and patchwork server are used without any + extra flags on the command line. + +Here is a step-by-step guide for a developer who sends to both U-Boot +mainline and the U-Boot concept tree. + +Step 1: Add your upstreams +.......................... + +First, add each upstream with its git remote URL, patchwork URL and send +settings:: + + patman upstream add us https://source.denx.de/u-boot/u-boot.git \ + -p https://patchwork.ozlabs.org -t u-boot + + patman upstream add ci https://concept.u-boot.org/u-boot/u-boot.git \ + -p https://patchwork.u-boot.org -t concept -I concept -m --no-tags + +The options are: + +``-p`` / ``--patchwork-url`` + URL of the patchwork server for this upstream + +``-t`` / ``--series-to`` + Patman alias for the To address (from your ``~/.patman`` alias file) + +``-I`` / ``--identity`` + Git sendemail identity (selects ``[sendemail "<identity>"]`` from + ``.gitconfig``) + +``-m`` / ``--no-maintainers`` + Skip running ``get_maintainer.pl`` + +``--no-tags`` + Skip subject-tag alias processing + +You can check your upstreams with:: + + patman upstream ls + +You can also set a default upstream:: + + patman upstream default us + +Step 2: Configure git sendemail identities +.......................................... + +If your upstreams use different SMTP servers or credentials, set up git +sendemail identities in your ``.gitconfig``. Each identity is a +``[sendemail "<name>"]`` section that overrides the base ``[sendemail]`` +settings. + +For example, to use one SMTP server by default and a different one for the +concept tree (server names and credentials are just examples; substitute your +own):: + + [sendemail] + smtpserver = smtp.denx.de + smtpserverport = 587 + smtpencryption = tls + + [sendemail "concept"] + smtpserver = smtp.gmail.com + smtpserverport = 587 + smtpencryption = tls + smtpuser = user@gmail.com + +The base ``[sendemail]`` settings are used when no identity is specified. When +you add ``-I concept`` to an upstream, patman passes ``--identity=concept`` to +``git send-email``, which selects the matching section. + +Step 3: Set up patchwork projects +................................. + +Each upstream needs a patchwork project so that patman can find your series on +the server:: + + patman patchwork set-project U-Boot us + patman patchwork set-project U-Boot ci + +This looks up the project on the patchwork server associated with the upstream +and stores the project ID locally. + +Step 4: Set up aliases +...................... + +Add To-address aliases to your ``~/.patman`` file:: + + [alias] + u-boot: U-Boot Mailing List <u-boot@lists.denx.de> + concept: U-Boot Concept <concept@u-boot.org> + +These are the names referenced by ``--series-to`` above. + +Step 5: Add series with an upstream +.................................... + +When adding a series, specify which upstream it targets:: + + patman series add -S us + +If you omit ``-S``, you can set it later with:: + + patman series set-upstream <series> <upstream> + +Step 6: Send +............ + +When you send a series, patman automatically applies the upstream's settings:: + + patman series send + +This looks up the upstream for the series and: + +- passes ``--identity`` to ``git send-email`` if configured +- sets the To address from ``--series-to`` if no ``Series-to:`` tag is present +- skips ``get_maintainer.pl`` if ``--no-maintainers`` is set +- skips tag processing if ``--no-tags`` is set + +If the series has a ``Series-to:`` tag that does not match the upstream's +expected To address, patman raises an error. This prevents accidentally sending +to the wrong mailing list. + +Updating upstream settings +.......................... + +To change settings on an existing upstream, use ``upstream set``:: + + patman upstream set ci -I chromium + patman upstream set us -p https://patchwork.ozlabs.org + +The same flags as ``upstream add`` are available, plus ``--maintainers`` and +``--tags`` to re-enable options that were previously disabled. + General points -------------- -- 2.43.0