From: Simon Glass <simon.glass@canonical.com> Add support for associating each series with an upstream remote, so that patman can automatically select the correct patchwork server, sendemail identity, To address and send options. This avoids having to remember which settings to use for each project. The series includes a v5 schema migration to add the necessary columns, accessor functions, command-line wiring, and documentation for the multi-upstream workflow. Simon Glass (32): patman: Expand the prep_series() comment patman: Reject database versions newer than supported patman: Add upstream and send settings to the database schema patman: Add an upstream column to the series table patman: Warn about series with no upstream after migration patman: Auto-detect upstream for series during migration patman: Show upstream in series list patman: Add set-upstream command for series patman: Associate patchwork projects with upstreams patman: Add upstream parameter to project_set() and project_get() patman: Add get_series_upstream() helper patman: Wire up per-upstream patchwork project in commands patman: Add patchwork list command patman: Make remote a required positional for patchwork project commands patman: Group remotes by project in patchwork list patman: Allow specifying project when adding an upstream patman: Rename -u/--use-commit to -1/--use-first-commit patman: Add per-upstream patchwork URL patman: Allow setting the upstream when adding a series patman: Add 'ls' and 'list' aliases for list subcommands patman: Add per-upstream send settings to the database patman: Support sendemail identity and series-to in the send path patman: Wire up per-upstream send settings in commands patman: Add an 'upstream set' command to update settings patman: Use notice() for database migration messages patman: Add header and tidy columns in upstream list patman: Update series description when adding a new version patman: Improve send feedback and upstream list formatting patman: Improve autolink wait with progress and backoff patman: Filter out AI co-developer tags from patches patman: Update series description on scan patman: Document multi-upstream setup tools/patman/cmdline.py | 72 +++++- tools/patman/control.py | 89 +++++++- tools/patman/cser_helper.py | 79 ++++++- tools/patman/cseries.py | 246 ++++++++++++++++++--- tools/patman/database.py | 292 +++++++++++++++++++++--- tools/patman/patchstream.py | 7 +- tools/patman/patman.rst | 167 ++++++++++++++ tools/patman/send.py | 25 ++- tools/patman/series.py | 3 +- tools/patman/test_common.py | 1 + tools/patman/test_cseries.py | 405 ++++++++++++++++++++++++++++++---- tools/u_boot_pylib/gitutil.py | 5 +- 12 files changed, 1251 insertions(+), 140 deletions(-) -- 2.43.0 base-commit: 0d02e154c4d4b861094ab3a1d389eb7530d6b313 branch: patg
From: Simon Glass <simon.glass@canonical.com> The comment for prep_series() is quite terse. Expand it to describe what the function actually does. Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/patman/cser_helper.py | 9 +++++++-- tools/patman/database.py | 2 +- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/tools/patman/cser_helper.py b/tools/patman/cser_helper.py index ea9881fcf6f..4289593b64b 100644 --- a/tools/patman/cser_helper.py +++ b/tools/patman/cser_helper.py @@ -255,11 +255,16 @@ class CseriesHelper: def prep_series(self, name, end=None): """Prepare to work with a series + Parse the branch name to determine the series name and version + number, then count the commits from the upstream branch (or up to the + end commit if provided) and collect the series metadata from those + commits. + Args: name (str): Branch name with version appended, e.g. 'fix2' end (str or None): Commit to end at, e.g. 'my_branch~16'. Only - commits up to that are processed. None to process commits up to - the upstream branch + commits up to that are processed. Use None to process commits up + to the upstream branch Return: tuple: str: Series name, e.g. 'fix' diff --git a/tools/patman/database.py b/tools/patman/database.py index 9c25b04a720..e6684f990da 100644 --- a/tools/patman/database.py +++ b/tools/patman/database.py @@ -41,7 +41,7 @@ Pcommit = namedtuple( 'idnum,seq,subject,svid,change_id,state,patch_id,num_comments') -class Database: +class Database: # pylint:disable=R0904 """Database of information used by patman""" # dict of databases: -- 2.43.0
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> Add a v5 migration to support per-upstream configuration. This allows each series to be associated with an upstream remote, each upstream to have its own patchwork URL and send settings (identity, series_to, no_maintainers, no_tags) and each series version to have its own description. The settings table is recreated without the UNIQUE constraint on name, since the same patchwork project can be associated with multiple upstreams. Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/patman/database.py | 38 +++++++++++++++++++++++++++++++++++- tools/patman/test_cseries.py | 2 +- 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/tools/patman/database.py b/tools/patman/database.py index 4557c4ff3fc..2861ca69fa0 100644 --- a/tools/patman/database.py +++ b/tools/patman/database.py @@ -19,7 +19,7 @@ from u_boot_pylib import tout from patman.series import Series # Schema version (version 0 means there is no database yet) -LATEST = 4 +LATEST = 5 # Information about a series/version record SerVer = namedtuple( @@ -156,6 +156,40 @@ class Database: # pylint:disable=R0904 """Add an archive tag for each ser_ver""" self.cur.execute('ALTER TABLE ser_ver ADD COLUMN archive_tag') + def _migrate_to_v5(self): + """Add upstream support to series, settings and upstream tables + + - Add upstream column to series table + - Recreate settings table without UNIQUE constraint on name, adding + an upstream column (since the same project can have multiple + remotes) + - Add patchwork_url, identity, series_to, no_maintainers and + no_tags columns to upstream table + - Add desc column to ser_ver table + """ + self.cur.execute('ALTER TABLE series ADD COLUMN upstream') + + self.cur.execute( + 'CREATE TABLE settings_new ' + '(name, proj_id INT, link_name, upstream)') + self.cur.execute( + 'INSERT INTO settings_new SELECT name, proj_id, link_name, NULL ' + 'FROM settings') + self.cur.execute('DROP TABLE settings') + self.cur.execute('ALTER TABLE settings_new RENAME TO settings') + default_ups = self.upstream_get_default() + if default_ups: + self.cur.execute( + 'UPDATE settings SET upstream = ?', (default_ups,)) + + self.cur.execute('ALTER TABLE upstream ADD COLUMN patchwork_url') + self.cur.execute('ALTER TABLE upstream ADD COLUMN identity') + self.cur.execute('ALTER TABLE upstream ADD COLUMN series_to') + self.cur.execute( + 'ALTER TABLE upstream ADD COLUMN no_maintainers BIT') + self.cur.execute('ALTER TABLE upstream ADD COLUMN no_tags BIT') + self.cur.execute('ALTER TABLE ser_ver ADD COLUMN desc') + def migrate_to(self, dest_version): """Migrate the database to the selected version @@ -182,6 +216,8 @@ class Database: # pylint:disable=R0904 self._migrate_to_v3() elif version == 4: self._migrate_to_v4() + elif version == 5: + self._migrate_to_v5() # Save the new version if we have a schema_version table if version > 1: diff --git a/tools/patman/test_cseries.py b/tools/patman/test_cseries.py index 3a9411033a6..5719d9b6ddc 100644 --- a/tools/patman/test_cseries.py +++ b/tools/patman/test_cseries.py @@ -3316,7 +3316,7 @@ Date: .* self.assertEqual(f'Update database to v{version}', out.getvalue().strip()) self.assertEqual(version, db.get_schema_version()) - self.assertEqual(4, database.LATEST) + self.assertEqual(5, database.LATEST) def test_migrate_future_version(self): """Test that a database newer than patman is rejected""" -- 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 from before v5, existing series will have a NULL upstream. Detect this after migration and print a warning listing the affected series, so the user knows to set an upstream for them. Add series_get_null_upstream() to query for series with NULL upstream. Have start() return the old schema version so callers can check whether a migration from a particular version occurred. Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/patman/cser_helper.py | 13 ++++++++- tools/patman/database.py | 20 +++++++++++++- tools/patman/test_cseries.py | 52 ++++++++++++++++++++++++++++++++++++ 3 files changed, 83 insertions(+), 2 deletions(-) diff --git a/tools/patman/cser_helper.py b/tools/patman/cser_helper.py index 1441904686c..0b29d52bc00 100644 --- a/tools/patman/cser_helper.py +++ b/tools/patman/cser_helper.py @@ -110,11 +110,22 @@ class CseriesHelper: # For the first instance, start it up with the expected schema self.db, is_new = Database.get_instance(fname) if is_new: - self.db.start() + old_version = self.db.start() + if old_version is not None and old_version < 5: + self._check_null_upstreams() else: # If a previous test has already checked the schema, just open it self.db.open_it() + def _check_null_upstreams(self): + """Warn about series that have no upstream set""" + names = self.db.series_get_null_upstream() + if names: + tout.warning( + f'{len(names)} series without an upstream:') + for name in names: + tout.warning(f' {name}') + def close_database(self): """Close the database""" if self.db: diff --git a/tools/patman/database.py b/tools/patman/database.py index e3df4257f8c..2bd225e4deb 100644 --- a/tools/patman/database.py +++ b/tools/patman/database.py @@ -84,7 +84,12 @@ class Database: # pylint:disable=R0904 return Database(db_path), True def start(self): - """Open the database read for use, migrate to latest schema""" + """Open the database ready for use, migrate to latest schema + + Return: + int or None: Schema version before migration, or None if no + migration was needed + """ self.open_it() old_version = self.get_schema_version() if old_version > LATEST: @@ -93,6 +98,9 @@ class Database: # pylint:disable=R0904 f'Database version {old_version} is too new (max' f' {LATEST}); please update patman') self.migrate_to(LATEST) + if old_version == LATEST: + return None + return old_version def open_it(self): """Open the database, creating it if necessary""" @@ -496,6 +504,16 @@ class Database: # pylint:disable=R0904 'UPDATE series SET upstream = ? WHERE id = ?', (ups, series_idnum)) + def series_get_null_upstream(self): + """Get a list of series names that have no upstream set + + Return: + list of str: Series names with NULL upstream + """ + res = self.execute( + 'SELECT name FROM series WHERE upstream IS NULL') + return [row[0] for row in res.fetchall()] + # ser_ver functions def ser_ver_get_link(self, series_idnum, version): diff --git a/tools/patman/test_cseries.py b/tools/patman/test_cseries.py index fc29e9a7bef..cf0d8cfdab6 100644 --- a/tools/patman/test_cseries.py +++ b/tools/patman/test_cseries.py @@ -3365,6 +3365,58 @@ Date: .* db.start() self.assertIn('is too new', err.getvalue()) + def test_migrate_upstream_warning(self): + """Test that migrating to v5 warns about series without upstream""" + db = database.Database(f'{self.tmpdir}/.patman2.db') + with terminal.capture(): + db.open_it() + + # Create a v4 database with some series + with terminal.capture() as (out, _): + db.migrate_to(4) + self.assertEqual( + 'Update database to v1\nUpdate database to v2\n' + 'Update database to v3\nUpdate database to v4', + out.getvalue().strip()) + db.execute( + "INSERT INTO series (name, desc, archived) " + "VALUES ('first', 'desc1', 0)") + db.execute( + "INSERT INTO series (name, desc, archived) " + "VALUES ('second', 'desc2', 0)") + db.execute( + "INSERT INTO series (name, desc, archived) " + "VALUES ('third', 'desc3', 0)") + 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 + + # Point at our v4 database + database.Database.instances = {} + cser.db, _ = database.Database.get_instance( + f'{self.tmpdir}/.patman2.db') + with terminal.capture() as (_, err): + 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): + cser._check_null_upstreams() + lines = err.getvalue().strip().splitlines() + self.assertEqual('2 series without an upstream:', lines[0]) + self.assertEqual(' first', lines[1]) + self.assertEqual(' third', lines[2]) + + cser.db.close() + 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> 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> Update 'series ls' to display a two-character upstream abbreviation before the version column, making it easy to see which remote each series targets. Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/patman/cseries.py | 8 +++++--- tools/patman/test_cseries.py | 14 +++++++------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/tools/patman/cseries.py b/tools/patman/cseries.py index c90020b7eb6..6c918185387 100644 --- a/tools/patman/cseries.py +++ b/tools/patman/cseries.py @@ -408,18 +408,20 @@ class Cseries(cser_helper.CseriesHelper): include_archived (bool): True to include archived series also """ sdict = self.db.series_get_dict(include_archived) - print(f"{'Name':15} {'Description':40} Accepted Versions") - border = f"{'-' * 15} {'-' * 40} -------- {'-' * 15}" + print(f"{'Name':15} {'Description':40} Accepted Us Versions") + border = f"{'-' * 15} {'-' * 40} -------- -- {'-' * 15}" print(border) for name in sorted(sdict): ser = sdict[name] versions = self._get_version_list(ser.idnum) stat = self._series_get_version_stats( ser.idnum, self._series_max_version(ser.idnum))[0] + ups = (ser.upstream or '')[:2] vlist = ' '.join([str(ver) for ver in sorted(versions)]) - print(f'{name:16.16} {ser.desc:41.41} {stat.rjust(8)} {vlist}') + print(f'{name:16.16} {ser.desc:41.41} {stat.rjust(8)} ' + f'{ups:2} {vlist}') print(border) def list_patches(self, series, version, show_commit=False, diff --git a/tools/patman/test_cseries.py b/tools/patman/test_cseries.py index 23527cfa8f0..e529d9fe61b 100644 --- a/tools/patman/test_cseries.py +++ b/tools/patman/test_cseries.py @@ -759,14 +759,14 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak self.assertEqual(5, len(lines)) self.assertEqual( 'Name Description ' - 'Accepted Versions', lines[0]) + 'Accepted Us Versions', lines[0]) self.assertTrue(lines[1].startswith('--')) self.assertEqual( 'first ' - ' -/2 1', lines[2]) + ' -/2 1', lines[2]) self.assertEqual( 'second Series for my board ' - ' 1/3 1 2', lines[3]) + ' 1/3 1 2', lines[3]) self.assertTrue(lines[4].startswith('--')) def test_series_list_archived(self): @@ -780,7 +780,7 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak self.assertEqual(4, len(lines)) self.assertEqual( 'second Series for my board ' - ' 1/3 1 2', lines[2]) + ' 1/3 1 2', lines[2]) # Now list including archived series with terminal.capture() as (out, _): @@ -789,10 +789,10 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak self.assertEqual(5, len(lines)) self.assertEqual( 'first ' - ' -/2 1', lines[2]) + ' -/2 1', lines[2]) self.assertEqual( 'second Series for my board ' - ' 1/3 1 2', lines[3]) + ' 1/3 1 2', lines[3]) def test_do_series_add(self): """Add a new cseries""" @@ -821,7 +821,7 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak self.assertTrue(lines[1].startswith('--')) self.assertEqual( 'first my-description ' - '-/2 1', lines[2]) + '-/2 1', lines[2]) def test_do_series_add_cmdline(self): """Add a new cseries using the cmdline""" -- 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> Each upstream typically has its own patchwork project. Add an upstream column to the patchwork table (schema v5) so each upstream can have its own project configuration. Rename the table from 'settings' to 'patchwork' to better reflect its purpose. During migration, any existing row is associated with the default upstream. The patchwork_update() and patchwork_get() methods accept an optional upstream parameter to store and retrieve per-upstream project configuration. Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/patman/cseries.py | 4 +-- tools/patman/database.py | 49 +++++++++++++++++------------ tools/patman/test_cseries.py | 60 ++++++++++++++++++++++++++++++++++++ 3 files changed, 92 insertions(+), 21 deletions(-) diff --git a/tools/patman/cseries.py b/tools/patman/cseries.py index 246610215ed..5d4a9592de9 100644 --- a/tools/patman/cseries.py +++ b/tools/patman/cseries.py @@ -659,7 +659,7 @@ 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.settings_update(name, proj_id, link_name) + self.db.patchwork_update(name, proj_id, link_name) self.commit() if not quiet: tout.notice(f"Project '{name}' patchwork-ID {proj_id} " @@ -674,7 +674,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.settings_get() + return self.db.patchwork_get() def remove(self, name, dry_run=False): """Remove a series from the database diff --git a/tools/patman/database.py b/tools/patman/database.py index 2bd225e4deb..22f21ce715f 100644 --- a/tools/patman/database.py +++ b/tools/patman/database.py @@ -165,12 +165,12 @@ class Database: # pylint:disable=R0904 self.cur.execute('ALTER TABLE ser_ver ADD COLUMN archive_tag') def _migrate_to_v5(self): - """Add upstream support to series, settings and upstream tables + """Add upstream support to series, patchwork and upstream tables - Add upstream column to series table - - Recreate settings table without UNIQUE constraint on name, adding - an upstream column (since the same project can have multiple - remotes) + - Rename and recreate patchwork table (formerly 'settings') without + UNIQUE constraint on name, adding an upstream column (since the + same project can have multiple remotes) - Add patchwork_url, identity, series_to, no_maintainers and no_tags columns to upstream table - Add desc column to ser_ver table @@ -178,17 +178,17 @@ class Database: # pylint:disable=R0904 self.cur.execute('ALTER TABLE series ADD COLUMN upstream') self.cur.execute( - 'CREATE TABLE settings_new ' + 'CREATE TABLE patchwork_new ' '(name, proj_id INT, link_name, upstream)') self.cur.execute( - 'INSERT INTO settings_new SELECT name, proj_id, link_name, NULL ' + 'INSERT INTO patchwork_new SELECT name, proj_id, link_name, NULL ' 'FROM settings') self.cur.execute('DROP TABLE settings') - self.cur.execute('ALTER TABLE settings_new RENAME TO settings') + self.cur.execute('ALTER TABLE patchwork_new RENAME TO patchwork') default_ups = self.upstream_get_default() if default_ups: self.cur.execute( - 'UPDATE settings SET upstream = ?', (default_ups,)) + 'UPDATE patchwork SET upstream = ?', (default_ups,)) self.cur.execute('ALTER TABLE upstream ADD COLUMN patchwork_url') self.cur.execute('ALTER TABLE upstream ADD COLUMN identity') @@ -867,32 +867,43 @@ class Database: # pylint:disable=R0904 udict[name] = url, is_default return udict - # settings functions + # patchwork functions - def settings_update(self, name, proj_id, link_name): - """Set the patchwork settings of the project + def patchwork_update(self, name, proj_id, link_name, ups=None): + """Set the patchwork project details for an upstream Args: name (str): Name of the project to use in patchwork proj_id (int): Project ID for the project link_name (str): Link name for the project + ups (str or None): Upstream name to associate with, or None """ - self.execute('DELETE FROM settings') self.execute( - 'INSERT INTO settings (name, proj_id, link_name) ' - 'VALUES (?, ?, ?)', (name, proj_id, link_name)) + 'DELETE FROM patchwork WHERE upstream IS ?', (ups,)) + self.execute( + 'INSERT INTO patchwork (name, proj_id, link_name, upstream) ' + 'VALUES (?, ?, ?, ?)', (name, proj_id, link_name, ups)) + + def patchwork_get(self, ups=None): + """Get the patchwork project details for an upstream - def settings_get(self): - """Get the patchwork settings 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: + tuple or None if there is no project set: name (str): Project name, e.g. 'U-Boot' proj_id (int): Patchworks project ID for this project link_name (str): Patchwork's link-name for the project """ - res = self.execute("SELECT name, proj_id, link_name FROM settings") + if ups is not None: + res = self.execute( + 'SELECT name, proj_id, link_name FROM patchwork ' + 'WHERE upstream = ?', (ups,)) + else: + res = self.execute( + 'SELECT name, proj_id, link_name FROM patchwork') recs = res.fetchall() - if len(recs) != 1: + if not recs: return None return recs[0] diff --git a/tools/patman/test_cseries.py b/tools/patman/test_cseries.py index c6b90d5223d..0922c3e57f5 100644 --- a/tools/patman/test_cseries.py +++ b/tools/patman/test_cseries.py @@ -2502,6 +2502,66 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak f"Project 'U-Boot' patchwork-ID {self.PROJ_ID} link-name 'uboot'", out.getvalue().strip()) + def test_patchwork_upstream(self): + """Test patchwork project with upstream association""" + cser = self.get_cser() + + # Add two upstreams + cser.db.upstream_add('us', 'https://us.example.com') + cser.db.upstream_add('ci', 'https://ci.example.com') + cser.db.commit() + + # Set project for a specific upstream + cser.db.patchwork_update('U-Boot', 6, 'uboot', 'us') + cser.db.commit() + + # Look up by upstream + info = cser.db.patchwork_get('us') + self.assertEqual(('U-Boot', 6, 'uboot'), info) + + # Different upstream has no project + self.assertIsNone(cser.db.patchwork_get('ci')) + + # No upstream arg returns any match + info = cser.db.patchwork_get() + self.assertEqual(('U-Boot', 6, 'uboot'), info) + + # Set a different project for ci + cser.db.patchwork_update('Linux', 10, 'linux', 'ci') + cser.db.commit() + + self.assertEqual(('Linux', 10, 'linux'), cser.db.patchwork_get('ci')) + self.assertEqual(('U-Boot', 6, 'uboot'), cser.db.patchwork_get('us')) + + def test_migrate_patchwork_upstream(self): + """Test that migrating to v5 renames settings to patchwork""" + db = database.Database(f'{self.tmpdir}/.patman3.db') + with terminal.capture(): + db.open_it() + + # Create a v4 database with an upstream and a patchwork row + with terminal.capture(): + db.migrate_to(4) + db.execute( + "INSERT INTO upstream (name, url, is_default) " + "VALUES ('us', 'https://us.example.com', 1)") + db.execute( + "INSERT INTO settings (name, proj_id, link_name) " + "VALUES ('U-Boot', 6, 'uboot')") + db.commit() + + # Migrate to v5 + with terminal.capture(): + db.migrate_to(5) + + # The existing row should now be in 'patchwork' with the default upstream + res = db.execute( + 'SELECT name, proj_id, link_name, upstream FROM patchwork') + recs = res.fetchall() + self.assertEqual(1, len(recs)) + self.assertEqual(('U-Boot', 6, 'uboot', 'us'), recs[0]) + db.close() + def check_series_list_patches(self): """Test listing the patches for a series""" 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 a method to look up the upstream for a series by name. If no name is given, use the current branch. This is needed so that patchwork commands can find the correct project for the series being operated on. Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/patman/cser_helper.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tools/patman/cser_helper.py b/tools/patman/cser_helper.py index 74bccfcfbc6..a60e02878a5 100644 --- a/tools/patman/cser_helper.py +++ b/tools/patman/cser_helper.py @@ -426,6 +426,22 @@ class CseriesHelper: self.db.pcommit_add_list(svid, to_add) + def get_series_upstream(self, name): + """Get the upstream for a series + + Args: + name (str): Name of series, or None to use current branch + + Return: + str or None: Upstream name, or None if not found + """ + if not name: + name = gitutil.get_branch() + ser = self.get_series_by_name(name) + if ser: + return ser.upstream + return None + def get_series_by_name(self, name, include_archived=False): """Get a Series object from the database by name -- 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> Add a 'patchwork list' (or 'pw list') command to show all configured patchwork projects and which upstream each is associated with. Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/patman/cmdline.py | 1 + tools/patman/control.py | 2 ++ tools/patman/cseries.py | 13 +++++++++++++ tools/patman/database.py | 15 +++++++++++++++ tools/patman/test_cseries.py | 26 ++++++++++++++++++++++++++ 5 files changed, 57 insertions(+) diff --git a/tools/patman/cmdline.py b/tools/patman/cmdline.py index 74571fd7220..f08287641dd 100644 --- a/tools/patman/cmdline.py +++ b/tools/patman/cmdline.py @@ -169,6 +169,7 @@ def add_patchwork_subparser(subparsers): uset.add_argument( '-u', '--upstream-name', help='Upstream to associate this project with') + patchwork_subparsers.add_parser('list') return patchwork diff --git a/tools/patman/control.py b/tools/patman/control.py index 191cfc134d3..bd9e0f8b9bd 100644 --- a/tools/patman/control.py +++ b/tools/patman/control.py @@ -289,6 +289,8 @@ def patchwork(args, test_db=None, pwork=None): if ups: msg += f" upstream '{ups}'" print(msg) + elif args.subcmd == 'list': + cser.project_list() else: raise ValueError(f"Unknown patchwork subcommand '{args.subcmd}'") finally: diff --git a/tools/patman/cseries.py b/tools/patman/cseries.py index 56adcc5b4cd..fdf48a4eabd 100644 --- a/tools/patman/cseries.py +++ b/tools/patman/cseries.py @@ -683,6 +683,19 @@ class Cseries(cser_helper.CseriesHelper): """ return self.db.patchwork_get(ups) + def project_list(self): + """List all patchwork project configurations""" + settings = self.db.patchwork_get_list() + if not settings: + print('No patchwork projects configured') + return + print(f"{'Project':20} {'ID':>4} {'Link name':15} Upstream") + border = f"{'-' * 20} {'-' * 4} {'-' * 15} {'-' * 15}" + print(border) + for name, proj_id, link_name, ups in settings: + print(f'{name:20} {proj_id:4} {link_name:15} {ups or ""}') + print(border) + def remove(self, name, dry_run=False): """Remove a series from the database diff --git a/tools/patman/database.py b/tools/patman/database.py index 22f21ce715f..396cf52aa48 100644 --- a/tools/patman/database.py +++ b/tools/patman/database.py @@ -884,6 +884,21 @@ class Database: # pylint:disable=R0904 'INSERT INTO patchwork (name, proj_id, link_name, upstream) ' 'VALUES (?, ?, ?, ?)', (name, proj_id, link_name, ups)) + def patchwork_get_list(self): + """Get all patchwork project configurations + + Returns: + list of tuple: + name (str): Project name, e.g. 'U-Boot' + proj_id (int): Patchworks project ID for this project + link_name (str): Patchwork's link-name for the project + upstream (str or None): Upstream name + """ + res = self.execute( + 'SELECT name, proj_id, link_name, upstream FROM patchwork ' + 'ORDER BY upstream') + return res.fetchall() + def patchwork_get(self, ups=None): """Get the patchwork project details for an upstream diff --git a/tools/patman/test_cseries.py b/tools/patman/test_cseries.py index 0922c3e57f5..f21fda8d2fa 100644 --- a/tools/patman/test_cseries.py +++ b/tools/patman/test_cseries.py @@ -2502,6 +2502,32 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak f"Project 'U-Boot' patchwork-ID {self.PROJ_ID} link-name 'uboot'", out.getvalue().strip()) + def test_patchwork_list(self): + """Test listing patchwork project configurations""" + cser = self.get_cser() + + # No projects configured + with terminal.capture() as (out, _): + cser.project_list() + self.assertEqual('No patchwork projects configured', + out.getvalue().strip()) + + # Add projects for two upstreams + cser.db.upstream_add('us', 'https://us.example.com') + cser.db.upstream_add('ci', 'https://ci.example.com') + cser.db.patchwork_update('U-Boot', 6, 'uboot', 'us') + cser.db.patchwork_update('Linux', 10, 'linux', 'ci') + cser.db.commit() + + with terminal.capture() as (out, _): + cser.project_list() + lines = out.getvalue().splitlines() + self.assertEqual(5, len(lines)) + self.assertIn('Linux', lines[2]) + self.assertIn('ci', lines[2]) + self.assertIn('U-Boot', lines[3]) + self.assertIn('us', lines[3]) + def test_patchwork_upstream(self): """Test patchwork project with upstream association""" cser = self.get_cser() -- 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> A project can have multiple remotes, so group them on a single line rather than showing one row per remote. Sort by project name then upstream in the patchwork table query. Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/patman/cseries.py | 13 +++++++++++-- tools/patman/database.py | 2 +- tools/patman/test_cseries.py | 9 ++++++--- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/tools/patman/cseries.py b/tools/patman/cseries.py index 3d24dd62fae..7237dd575cd 100644 --- a/tools/patman/cseries.py +++ b/tools/patman/cseries.py @@ -689,11 +689,20 @@ class Cseries(cser_helper.CseriesHelper): if not settings: print('No patchwork projects configured') return - print(f"{'Project':20} {'ID':>4} {'Link name':15} Remote") + print(f"{'Project':20} {'ID':>4} {'Link name':15} Remotes") border = f"{'-' * 20} {'-' * 4} {'-' * 15} {'-' * 15}" print(border) + + # Group remotes by project + projects = OrderedDict() for name, proj_id, link_name, ups in settings: - print(f'{name:20} {proj_id:4} {link_name:15} {ups or ""}') + key = (name, proj_id, link_name) + projects.setdefault(key, []) + if ups: + projects[key].append(ups) + for (name, proj_id, link_name), remotes in projects.items(): + rlist = ' '.join(sorted(remotes)) + print(f'{name:20} {proj_id:4} {link_name:15} {rlist}') print(border) def remove(self, name, dry_run=False): diff --git a/tools/patman/database.py b/tools/patman/database.py index 396cf52aa48..2f21e62605f 100644 --- a/tools/patman/database.py +++ b/tools/patman/database.py @@ -896,7 +896,7 @@ class Database: # pylint:disable=R0904 """ res = self.execute( 'SELECT name, proj_id, link_name, upstream FROM patchwork ' - 'ORDER BY upstream') + 'ORDER BY name, upstream') return res.fetchall() def patchwork_get(self, ups=None): diff --git a/tools/patman/test_cseries.py b/tools/patman/test_cseries.py index b9be1680199..2a31815932c 100644 --- a/tools/patman/test_cseries.py +++ b/tools/patman/test_cseries.py @@ -2518,11 +2518,13 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak self.assertEqual('No patchwork projects configured', out.getvalue().strip()) - # Add projects for two upstreams + # Add two remotes for U-Boot and one for Linux cser.db.upstream_add('us', 'https://us.example.com') cser.db.upstream_add('ci', 'https://ci.example.com') + cser.db.upstream_add('linus', 'https://linus.example.com') cser.db.patchwork_update('U-Boot', 6, 'uboot', 'us') - cser.db.patchwork_update('Linux', 10, 'linux', 'ci') + cser.db.patchwork_update('U-Boot', 6, 'uboot', 'ci') + cser.db.patchwork_update('Linux', 10, 'linux', 'linus') cser.db.commit() with terminal.capture() as (out, _): @@ -2530,8 +2532,9 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak lines = out.getvalue().splitlines() self.assertEqual(5, len(lines)) self.assertIn('Linux', lines[2]) - self.assertIn('ci', lines[2]) + self.assertIn('linus', lines[2]) self.assertIn('U-Boot', lines[3]) + self.assertIn('ci', lines[3]) self.assertIn('us', lines[3]) def test_patchwork_upstream(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> Free up -u for a future upstream option by renaming the flag that uses the first commit's subject as the series description. Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/patman/cmdline.py | 2 +- tools/patman/control.py | 2 +- tools/patman/test_cseries.py | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tools/patman/cmdline.py b/tools/patman/cmdline.py index 8df9966ec6d..090d9845ec1 100644 --- a/tools/patman/cmdline.py +++ b/tools/patman/cmdline.py @@ -237,7 +237,7 @@ def add_series_subparser(subparsers): add.add_argument('-D', '--desc', help='Series description / cover-letter title') add.add_argument( - '-u', '--use-commit', action='store_true', + '-1', '--use-first-commit', action='store_true', help="Use the first commit's subject as series description if needed") add.add_argument( '-f', '--force-version', action='store_true', diff --git a/tools/patman/control.py b/tools/patman/control.py index c8d7405f7e8..c65fd3d5f1f 100644 --- a/tools/patman/control.py +++ b/tools/patman/control.py @@ -151,7 +151,7 @@ def do_series(args, test_db=None, pwork=None, cser=None): if args.subcmd == 'add': cser.add(args.series, args.desc, mark=args.mark, allow_unmarked=args.allow_unmarked, end=args.upstream, - use_commit=args.use_commit, dry_run=args.dry_run) + use_commit=args.use_first_commit, dry_run=args.dry_run) elif args.subcmd == 'archive': cser.archive(args.series) elif args.subcmd == 'autolink': diff --git a/tools/patman/test_cseries.py b/tools/patman/test_cseries.py index 2a31815932c..083727889f8 100644 --- a/tools/patman/test_cseries.py +++ b/tools/patman/test_cseries.py @@ -455,7 +455,7 @@ class TestCseries(unittest.TestCase, TestCommon): str(exc.exception)) with terminal.capture() as (out, _): - self.run_args('series', '-s', 'first', 'add', '--use-commit', + self.run_args('series', '-s', 'first', 'add', '--use-first-commit', '--allow-unmarked', pwork=True) lines = out.getvalue().splitlines() self.assertEqual( @@ -799,7 +799,7 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak self.make_git_tree() args = Namespace(subcmd='add', desc='my-description', series='first', mark=False, allow_unmarked=True, upstream=None, - use_commit=False, dry_run=False) + use_first_commit=False, dry_run=False) with terminal.capture() as (out, _): control.do_series(args, test_db=self.tmpdir, pwork=True) @@ -847,7 +847,7 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak force=True) args = Namespace(subcmd='add', series=None, mark=False, allow_unmarked=True, upstream=None, dry_run=False, - desc=None, use_commit=False) + desc=None, use_first_commit=False) with terminal.capture(): control.do_series(args, test_db=self.tmpdir, pwork=True) -- 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> Adding a series always creates it with no upstream, requiring a separate 'set-upstream' command afterwards. Add a -S/--set-upstream option to 'series add' so the upstream can be specified in one step. If the series already exists (e.g. when adding a new version), the upstream is also updated. Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/patman/cmdline.py | 2 ++ tools/patman/control.py | 3 ++- tools/patman/cseries.py | 17 ++++++++++------- tools/patman/test_cseries.py | 17 +++++++++++++++++ 4 files changed, 31 insertions(+), 8 deletions(-) diff --git a/tools/patman/cmdline.py b/tools/patman/cmdline.py index f24d0aacdff..57c56b099e1 100644 --- a/tools/patman/cmdline.py +++ b/tools/patman/cmdline.py @@ -242,6 +242,8 @@ def add_series_subparser(subparsers): add.add_argument( '-f', '--force-version', action='store_true', help='Change the Series-version on a series to match its branch') + add.add_argument('-S', '--set-upstream', + help='Set the upstream for this series') _add_mark(add) _add_allow_unmarked(add) _upstream_add(add) diff --git a/tools/patman/control.py b/tools/patman/control.py index e1f52300a17..7f162b4aadb 100644 --- a/tools/patman/control.py +++ b/tools/patman/control.py @@ -161,7 +161,8 @@ def do_series(args, test_db=None, pwork=None, cser=None): if args.subcmd == 'add': cser.add(args.series, args.desc, mark=args.mark, allow_unmarked=args.allow_unmarked, end=args.upstream, - use_commit=args.use_first_commit, dry_run=args.dry_run) + use_commit=args.use_first_commit, + ups=args.set_upstream, dry_run=args.dry_run) elif args.subcmd == 'archive': cser.archive(args.series) elif args.subcmd == 'autolink': diff --git a/tools/patman/cseries.py b/tools/patman/cseries.py index 612ccfda7dc..6369b626035 100644 --- a/tools/patman/cseries.py +++ b/tools/patman/cseries.py @@ -39,7 +39,8 @@ class Cseries(cser_helper.CseriesHelper): super().__init__(topdir, colour) def add(self, branch_name, desc=None, mark=False, allow_unmarked=False, - end=None, use_commit=False, force_version=False, dry_run=False): + end=None, use_commit=False, force_version=False, ups=None, + dry_run=False): """Add a series (or new version of a series) to the database Args: @@ -47,13 +48,13 @@ class Cseries(cser_helper.CseriesHelper): desc (str): Description to use, or None to use the series subject mark (str): True to mark each commit with a change ID allow_unmarked (str): True to not require each commit to be marked - end (str): Add only commits up to but exclu - use_commit (bool)): True to use the first commit's subject as the + end (str): Add only commits up to but excluding this commit + use_commit (bool): True to use the first commit's subject as the series description, if none is available in the series or - provided in 'desc') - + provided in 'desc' force_version (bool): True if ignore a Series-version tag that doesn't match its branch name + ups (str or None): Name of the upstream for this series dry_run (bool): True to do a dry run """ name, ser, version, msg = self.prep_series(branch_name, end) @@ -70,7 +71,7 @@ class Cseries(cser_helper.CseriesHelper): raise ValueError(f"Branch '{name}' has no cover letter - " 'please provide description') if not desc: - desc = ser['cover'][0] + desc = ser.cover[0] # pylint: disable=E1136 ser = self._handle_mark(name, ser, version, mark, allow_unmarked, force_version, dry_run) @@ -80,9 +81,11 @@ class Cseries(cser_helper.CseriesHelper): added = False series_id = self.db.series_find_by_name(ser.name) if not series_id: - series_id = self.db.series_add(ser.name, desc) + series_id = self.db.series_add(ser.name, desc, ups=ups) added = True msg += f" series '{ser.name}'" + elif ups: + self.db.series_set_upstream(series_id, ups) if version not in self._get_version_list(series_id): svid = self.db.ser_ver_add(series_id, version, link) diff --git a/tools/patman/test_cseries.py b/tools/patman/test_cseries.py index b14c6c22ddc..98acc1de184 100644 --- a/tools/patman/test_cseries.py +++ b/tools/patman/test_cseries.py @@ -799,6 +799,7 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak self.make_git_tree() args = Namespace(subcmd='add', desc='my-description', series='first', mark=False, allow_unmarked=True, upstream=None, + set_upstream=None, use_first_commit=False, dry_run=False) with terminal.capture() as (out, _): control.do_series(args, test_db=self.tmpdir, pwork=True) @@ -823,6 +824,21 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak 'first my-description ' '-/2 1', lines[2]) + def test_do_series_add_upstream(self): + """Test that series add can set the upstream""" + self.make_git_tree() + args = Namespace(subcmd='add', desc='my-description', series='first', + mark=False, allow_unmarked=True, upstream=None, + set_upstream='origin', + use_first_commit=False, dry_run=False) + with terminal.capture(): + control.do_series(args, test_db=self.tmpdir, pwork=True) + + cser = self.get_database() + slist = cser.db.series_get_dict() + ser = slist.get('first') + self.assertEqual('origin', ser.upstream) + def test_do_series_add_cmdline(self): """Add a new cseries using the cmdline""" self.make_git_tree() @@ -847,6 +863,7 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak force=True) args = Namespace(subcmd='add', series=None, mark=False, allow_unmarked=True, upstream=None, dry_run=False, + set_upstream=None, desc=None, use_first_commit=False) with terminal.capture(): control.do_series(args, test_db=self.tmpdir, pwork=True) -- 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> The send settings are already in the v5 schema. Add accessor functions to use them: upstream_get_send_settings() for looking up all settings at once, and upstream_get_identity() for the identity alone. Update upstream_add() to accept send-settings parameters and upstream_get_dict() to return them. This allows different upstreams to use different email configurations. For example, chromium patches can use a different SMTP identity and skip maintainer lookups, while U-Boot patches use the defaults. Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/patman/cseries.py | 27 ++++++++++++++-- tools/patman/database.py | 62 ++++++++++++++++++++++++++++++++---- tools/patman/test_cseries.py | 9 +++--- 3 files changed, 85 insertions(+), 13 deletions(-) diff --git a/tools/patman/cseries.py b/tools/patman/cseries.py index 6369b626035..871e14458a8 100644 --- a/tools/patman/cseries.py +++ b/tools/patman/cseries.py @@ -932,6 +932,15 @@ class Cseries(cser_helper.CseriesHelper): if not ser.idnum: raise ValueError(f"Series '{ser.name}' not found in database") + if not getattr(args, 'identity', None): + ups = self.get_series_upstream(name) + if ups: + identity = self.db.upstream_get_identity(ups) + if identity: + args.identity = identity + print(f"Using sendemail identity '{identity}'" + f" from upstream '{ups}'") + args.branch = self._get_branch_name(ser.name, version) likely_sent = send.send(args, git_dir=self.gitdir, cwd=self.topdir) @@ -1150,7 +1159,7 @@ class Cseries(cser_helper.CseriesHelper): tout.info('Dry run completed') def upstream_add(self, name, url, project=None, pwork=None, - patchwork_url=None): + patchwork_url=None, identity=None): """Add a new upstream tree Args: @@ -1161,8 +1170,9 @@ class Cseries(cser_helper.CseriesHelper): the project patchwork_url (str or None): URL of the patchwork server for this upstream + identity (str or None): Git sendemail identity to use """ - self.db.upstream_add(name, url, patchwork_url) + self.db.upstream_add(name, url, patchwork_url, identity=identity) if project: if not pwork: if not patchwork_url: @@ -1174,6 +1184,8 @@ class Cseries(cser_helper.CseriesHelper): msg = f"Added upstream '{name}' ({url})" if patchwork_url: msg += f" patchwork '{patchwork_url}'" + if identity: + msg += f" identity '{identity}'" if project: msg += f" project '{project}'" tout.notice(msg) @@ -1187,13 +1199,22 @@ class Cseries(cser_helper.CseriesHelper): udict = self.get_upstream_dict() for name, items in udict.items(): - url, is_default, patchwork_url = items + (url, is_default, patchwork_url, identity, series_to, + no_maintainers, no_tags) = items default = '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}' if patchwork_url: line += f' pw:{patchwork_url}' + if identity: + line += f' id:{identity}' + if series_to: + line += f' to:{series_to}' + if no_maintainers: + line += ' no-maintainers' + if no_tags: + line += ' no-tags' print(line) def upstream_set_default(self, name): diff --git a/tools/patman/database.py b/tools/patman/database.py index f7ab2d84877..19df3c3bc82 100644 --- a/tools/patman/database.py +++ b/tools/patman/database.py @@ -791,21 +791,29 @@ class Database: # pylint:disable=R0904 # upstream functions - def upstream_add(self, name, url, patchwork_url=None): + def upstream_add(self, name, url, patchwork_url=None, identity=None, + series_to=None, no_maintainers=False, no_tags=False): """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 + identity (str or None): Git sendemail identity to use + series_to (str or None): Patman alias for the To address + no_maintainers (bool): True to skip get_maintainer.pl + no_tags (bool): True to skip subject-tag alias processing Raises: ValueError if the name already exists in the database """ try: self.execute( - 'INSERT INTO upstream (name, url, patchwork_url) ' - 'VALUES (?, ?, ?)', (name, url, patchwork_url)) + 'INSERT INTO upstream (name, url, patchwork_url, identity,' + ' series_to, no_maintainers, no_tags) ' + 'VALUES (?, ?, ?, ?, ?, ?, ?)', + (name, url, patchwork_url, identity, series_to, + no_maintainers, no_tags)) except sqlite3.IntegrityError as exc: if 'UNIQUE constraint failed: upstream.name' in str(exc): raise ValueError(f"Upstream '{name}' already exists") from exc @@ -871,6 +879,43 @@ class Database: # pylint:disable=R0904 return rec[0] return None + def upstream_get_identity(self, name): + """Get the sendemail identity for an upstream + + Args: + name (str): Upstream name + + Return: + str or None: Identity name, or None if not set + """ + res = self.execute( + 'SELECT identity FROM upstream WHERE name = ?', (name,)) + rec = res.fetchone() + if rec: + return rec[0] + return None + + def upstream_get_send_settings(self, name): + """Get the send settings for an upstream + + Args: + name (str): Upstream name + + Return: + tuple or None: + str or None: identity + str or None: series_to + bool: no_maintainers + bool: no_tags + """ + res = self.execute( + 'SELECT identity, series_to, no_maintainers, no_tags ' + 'FROM upstream WHERE name = ?', (name,)) + rec = res.fetchone() + if rec: + return rec + return None + def upstream_get_dict(self): """Get a list of upstream entries from the database @@ -881,12 +926,17 @@ class Database: # pylint:disable=R0904 str: url bool: is_default str or None: patchwork_url + str or None: identity + str or None: series_to + bool: no_maintainers + bool: no_tags """ res = self.execute( - 'SELECT name, url, is_default, patchwork_url FROM upstream') + 'SELECT name, url, is_default, patchwork_url, identity,' + ' series_to, no_maintainers, no_tags FROM upstream') udict = OrderedDict() - for name, url, is_default, patchwork_url in res.fetchall(): - udict[name] = url, is_default, patchwork_url + for rec in res.fetchall(): + udict[rec[0]] = rec[1:] return udict # patchwork functions diff --git a/tools/patman/test_cseries.py b/tools/patman/test_cseries.py index 98acc1de184..8f56627958e 100644 --- a/tools/patman/test_cseries.py +++ b/tools/patman/test_cseries.py @@ -1747,14 +1747,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, None), ulist['us']) + self.assertEqual(('https://one', None, None, None, None, 0, 0), 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, None), ulist['us']) - self.assertEqual(('git@two', None, None), ulist['ci']) + self.assertEqual(('https://one', None, None, None, None, 0, 0), ulist['us']) + self.assertEqual(('git@two', None, None, None, None, 0, 0), ulist['ci']) # Try to add a duplicate with self.assertRaises(ValueError) as exc: @@ -1782,7 +1782,8 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak ulist = cser.get_upstream_dict() self.assertEqual(1, len(ulist)) self.assertEqual( - ('https://one', None, 'https://pw.example.com'), ulist['us']) + ('https://one', None, 'https://pw.example.com', None, None, 0, 0), + ulist['us']) # Check that the patchwork URL shows in the list with terminal.capture() as (out, _): -- 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 -I/--identity, -t/--series-to, -m/--no-maintainers and --no-tags options to 'upstream add' and thread them through to the database. In cseries.send(), look up the series' upstream send settings and apply them to args before sending: set identity, series_to, add_maintainers and process_tags as appropriate. Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/patman/cmdline.py | 12 ++++++++++++ tools/patman/control.py | 6 +++++- tools/patman/cseries.py | 34 +++++++++++++++++++++++++++------- 3 files changed, 44 insertions(+), 8 deletions(-) diff --git a/tools/patman/cmdline.py b/tools/patman/cmdline.py index 3fef37e738d..6141f106fde 100644 --- a/tools/patman/cmdline.py +++ b/tools/patman/cmdline.py @@ -423,6 +423,18 @@ def add_upstream_subparser(subparsers): '-p', '--patchwork-url', help='URL of patchwork server for this upstream, e.g. ' "'https://patchwork.ozlabs.org'") + uadd.add_argument( + '-I', '--identity', + help="Git sendemail identity to use, e.g. 'chromium'") + uadd.add_argument( + '-t', '--series-to', + help="Patman alias for the To address, e.g. 'u-boot'") + uadd.add_argument( + '-m', '--no-maintainers', action='store_true', default=False, + help='Skip get_maintainer.pl for this upstream') + uadd.add_argument( + '--no-tags', action='store_true', default=False, + help='Skip subject-tag alias processing for this upstream') 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 352c76bd14e..689fd732dec 100644 --- a/tools/patman/control.py +++ b/tools/patman/control.py @@ -252,7 +252,11 @@ def upstream(args, test_db=None): if args.subcmd == 'add': cser.upstream_add(args.remote_name, args.url, args.project_name, - patchwork_url=args.patchwork_url) + patchwork_url=args.patchwork_url, + identity=args.identity, + series_to=args.series_to, + no_maintainers=args.no_maintainers, + no_tags=args.no_tags) 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 871e14458a8..36f0b432073 100644 --- a/tools/patman/cseries.py +++ b/tools/patman/cseries.py @@ -932,14 +932,21 @@ class Cseries(cser_helper.CseriesHelper): if not ser.idnum: raise ValueError(f"Series '{ser.name}' not found in database") - if not getattr(args, 'identity', None): - ups = self.get_series_upstream(name) - if ups: - identity = self.db.upstream_get_identity(ups) - if identity: + ups = self.get_series_upstream(name) + if ups: + settings = self.db.upstream_get_send_settings(ups) + if settings: + 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: + args.add_maintainers = False + if no_tags: + args.process_tags = False args.branch = self._get_branch_name(ser.name, version) likely_sent = send.send(args, git_dir=self.gitdir, cwd=self.topdir) @@ -1159,7 +1166,8 @@ class Cseries(cser_helper.CseriesHelper): tout.info('Dry run completed') def upstream_add(self, name, url, project=None, pwork=None, - patchwork_url=None, identity=None): + patchwork_url=None, identity=None, series_to=None, + no_maintainers=False, no_tags=False): """Add a new upstream tree Args: @@ -1171,8 +1179,14 @@ class Cseries(cser_helper.CseriesHelper): patchwork_url (str or None): URL of the patchwork server for this upstream identity (str or None): Git sendemail identity to use + series_to (str or None): Patman alias for the To address + no_maintainers (bool): True to skip get_maintainer.pl + no_tags (bool): True to skip subject-tag alias processing """ - self.db.upstream_add(name, url, patchwork_url, identity=identity) + self.db.upstream_add(name, url, patchwork_url, identity=identity, + series_to=series_to, + no_maintainers=no_maintainers, + no_tags=no_tags) if project: if not pwork: if not patchwork_url: @@ -1186,6 +1200,12 @@ class Cseries(cser_helper.CseriesHelper): msg += f" patchwork '{patchwork_url}'" if identity: msg += f" identity '{identity}'" + if series_to: + msg += f" to '{series_to}'" + if no_maintainers: + msg += ' no-maintainers' + if no_tags: + msg += ' no-tags' if project: msg += f" project '{project}'" tout.notice(msg) -- 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> Use tout.notice() instead of tout.info() so that migration messages are visible at the default verbosity level. Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/patman/database.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/patman/database.py b/tools/patman/database.py index dcd39ea2c69..a5952c6d2b6 100644 --- a/tools/patman/database.py +++ b/tools/patman/database.py @@ -214,7 +214,7 @@ class Database: # pylint:disable=R0904 tools.read_file(self.db_path)) version += 1 - tout.info(f'Update database to v{version}') + tout.notice(f'Update database to v{version}') self.open_it() if version == 1: self.create_v1() -- 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> When adding a new version to an existing series, the description (cover letter subject) is not updated, so it gets stale if the title changes between versions. Update it each time a new version is added. Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/patman/cser_helper.py | 4 ++-- tools/patman/cseries.py | 8 +++++--- tools/patman/database.py | 27 ++++++++++++++++++++------- 3 files changed, 27 insertions(+), 12 deletions(-) diff --git a/tools/patman/cser_helper.py b/tools/patman/cser_helper.py index a60e02878a5..dd654738c06 100644 --- a/tools/patman/cser_helper.py +++ b/tools/patman/cser_helper.py @@ -1369,10 +1369,10 @@ class CseriesHelper: updated += 1 if cover: info = SerVer(svid, None, None, None, cover.id, - cover.num_comments, cover.name, None) + cover.num_comments, cover.name, None, None) else: info = SerVer(svid, None, None, None, None, None, patches[0].name, - None) + None, None) self.db.ser_ver_set_info(info) return updated, 1 if cover else 0 diff --git a/tools/patman/cseries.py b/tools/patman/cseries.py index 47da57acc0f..0f1766f5539 100644 --- a/tools/patman/cseries.py +++ b/tools/patman/cseries.py @@ -84,11 +84,13 @@ class Cseries(cser_helper.CseriesHelper): series_id = self.db.series_add(ser.name, desc, ups=ups) added = True msg += f" series '{ser.name}'" - elif ups: - self.db.series_set_upstream(series_id, ups) + else: + if ups: + self.db.series_set_upstream(series_id, ups) + self.db.series_set_desc(series_id, desc) if version not in self._get_version_list(series_id): - svid = self.db.ser_ver_add(series_id, version, link) + svid = self.db.ser_ver_add(series_id, version, link, desc) msg += f" v{version}" if not added: msg += f" to existing series '{ser.name}'" diff --git a/tools/patman/database.py b/tools/patman/database.py index a5952c6d2b6..e1ec0dc00e2 100644 --- a/tools/patman/database.py +++ b/tools/patman/database.py @@ -25,7 +25,7 @@ LATEST = 5 SerVer = namedtuple( 'SER_VER', 'idnum,series_id,version,link,cover_id,cover_num_comments,name,' - 'archive_tag') + 'archive_tag,desc') # Record from the pcommit table: # idnum (int): record ID @@ -493,6 +493,17 @@ class Database: # pylint:disable=R0904 self.execute( 'UPDATE series SET name = ? WHERE id = ?', (name, series_idnum)) + def series_set_desc(self, series_idnum, desc): + """Update description for a series + + Args: + series_idnum (int): ID num of the series + desc (str): New description + """ + self.execute( + 'UPDATE series SET desc = ? WHERE id = ?', + (desc, series_idnum)) + def series_set_upstream(self, series_idnum, ups): """Update upstream for a series @@ -617,7 +628,7 @@ class Database: # pylint:disable=R0904 if self.rowcount() != 1: raise ValueError(f'No ser_ver updated (svid {svid})') - def ser_ver_add(self, series_idnum, version, link=None): + def ser_ver_add(self, series_idnum, version, link=None, desc=None): """Add a new ser_ver record Args: @@ -625,13 +636,15 @@ class Database: # pylint:disable=R0904 version version (int): Version number to add link (str): Patchwork link, or None if not known + desc (str or None): Series description for this version Return: int: ID num of the new ser_ver record """ self.execute( - 'INSERT INTO ser_ver (series_id, version, link) VALUES (?, ?, ?)', - (series_idnum, version, link)) + 'INSERT INTO ser_ver (series_id, version, link, desc) ' + 'VALUES (?, ?, ?, ?)', + (series_idnum, version, link, desc)) return self.lastrowid() def ser_ver_get_for_series(self, series_idnum, version=None): @@ -648,8 +661,8 @@ class Database: # pylint:disable=R0904 ValueError: There is no matching idnum/version """ base = ('SELECT id, series_id, version, link, cover_id, ' - 'cover_num_comments, name, archive_tag FROM ser_ver ' - 'WHERE series_id = ?') + 'cover_num_comments, name, archive_tag, desc ' + 'FROM ser_ver WHERE series_id = ?') if version: res = self.execute(base + ' AND version = ?', (series_idnum, version)) @@ -690,7 +703,7 @@ class Database: # pylint:disable=R0904 """ res = self.execute( 'SELECT id, series_id, version, link, cover_id, ' - 'cover_num_comments, name, archive_tag FROM ser_ver') + 'cover_num_comments, name, archive_tag, desc FROM ser_ver') items = res.fetchall() return [SerVer(*x) for x in items] -- 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> Use tout.progress() to show an updating status line while waiting for a series to appear on patchwork, instead of printing the full match table every iteration. Only show matches when they change. Back off the sleep interval from 5s up to 30s to reduce server load. On failure, print the patman autolink command to use later. Remove the noisy "Sleeping for N seconds" message from sleep(). Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/patman/cser_helper.py | 1 - tools/patman/cseries.py | 43 +++++++++++++++++++++++++++++++----- tools/patman/test_common.py | 1 + tools/patman/test_cseries.py | 28 +++++++++++++++-------- 4 files changed, 57 insertions(+), 16 deletions(-) diff --git a/tools/patman/cser_helper.py b/tools/patman/cser_helper.py index dd654738c06..ec3956db4d5 100644 --- a/tools/patman/cser_helper.py +++ b/tools/patman/cser_helper.py @@ -205,7 +205,6 @@ class CseriesHelper: Args: time_s (float): Amount of seconds to sleep for """ - print(f'Sleeping for {time_s} seconds') if self._fake_time is not None: self._fake_sleep(time_s) else: diff --git a/tools/patman/cseries.py b/tools/patman/cseries.py index 81a8712b3de..0b4ce91b024 100644 --- a/tools/patman/cseries.py +++ b/tools/patman/cseries.py @@ -284,27 +284,58 @@ class Cseries(cser_helper.CseriesHelper): start = self.get_time() stop = start + wait_s sleep_time = 5 + last_options = None while True: pws, options, name, version, desc = self.link_search( pwork, series, version) if pws: + tout.clear_progress() if wait_s: tout.notice('Link completed after ' f'{self.get_time() - start} seconds') break - print(f"Possible matches for '{name}' v{version} desc '{desc}':") - print(' Link Version Description') - for opt in options: - print(f"{opt['id']:6} {opt['version']:7} {opt['name']}") if not wait_s or self.get_time() > stop: + tout.clear_progress() + if options != last_options: + self._show_autolink_matches(name, version, desc, + options) delay = f' after {wait_s} seconds' if wait_s else '' - raise ValueError(f"Cannot find series '{desc}{delay}'") - + raise ValueError( + f"Cannot find series '{desc}'{delay}; " + 'to try again later:\n' + f" patman series autolink -s {name} -V {version}") + + if options != last_options: + tout.clear_progress() + self._show_autolink_matches(name, version, desc, options) + last_options = options + + elapsed = int(self.get_time() - start) + tout.progress( + f'Waiting for series on patchwork ({elapsed}s)') self.sleep(sleep_time) + sleep_time = min(sleep_time + 5, 30) self.link_set(name, version, pws, update_commit) + def _show_autolink_matches(self, name, version, desc, options): + """Show possible autolink matches + + Args: + name (str): Series name + version (int): Series version + desc (str): Series description + options (list of dict): Possible matches from patchwork + """ + print(f"Possible matches for '{name}' v{version} desc '{desc}':") + if options: + print(' Link Version Description') + for opt in options: + print(f"{opt['id']:6} {opt['version']:7} {opt['name']}") + else: + print(' (none)') + def link_auto_all(self, pwork, update_commit, link_all_versions, replace_existing, dry_run, show_summary=True): """Automatically find a series link by looking in patchwork diff --git a/tools/patman/test_common.py b/tools/patman/test_common.py index 7da995dda22..6cf007cc1f2 100644 --- a/tools/patman/test_common.py +++ b/tools/patman/test_common.py @@ -66,6 +66,7 @@ class TestCommon: self.gitdir = os.path.join(self.tmpdir, '.git') tout.init(tout.DEBUG if self.verbosity else tout.INFO, allow_colour=False) + tout.stdout_is_tty = False def tearDown(self): """Delete the temporary dir""" diff --git a/tools/patman/test_cseries.py b/tools/patman/test_cseries.py index 3d475956ff9..c0beb128265 100644 --- a/tools/patman/test_cseries.py +++ b/tools/patman/test_cseries.py @@ -3750,15 +3750,25 @@ Date: .* with terminal.capture() as (out, _): cser.link_auto(pwork, 'second3', 3, True, 50) itr = iter(out.getvalue().splitlines()) - for i in range(7): - self.assertEqual( - "Possible matches for 'second' v3 desc 'Series for my board':", - next(itr), f'failed at i={i}') - self.assertEqual(' Link Version Description', next(itr)) - self.assertEqual(' 456 1 Series for my board', next(itr)) - self.assertEqual(' 457 2 Series for my board', next(itr)) - self.assertEqual('Sleeping for 5 seconds', next(itr)) - self.assertEqual('Link completed after 35 seconds', next(itr)) + + # Matches shown only once (they don't change between retries) + self.assertEqual( + "Possible matches for 'second' v3 desc 'Series for my board':", + next(itr)) + self.assertEqual(' Link Version Description', next(itr)) + self.assertEqual(' 456 1 Series for my board', next(itr)) + self.assertEqual(' 457 2 Series for my board', next(itr)) + + # Progress messages with backoff (5, 10, 15, 20s sleeps) + self.assertEqual( + 'Waiting for series on patchwork (0s)...', next(itr)) + self.assertEqual( + 'Waiting for series on patchwork (5s)...', next(itr)) + self.assertEqual( + 'Waiting for series on patchwork (15s)...', next(itr)) + self.assertEqual( + 'Waiting for series on patchwork (30s)...', next(itr)) + self.assertEqual('Link completed after 50 seconds', next(itr)) self.assertRegex( next(itr), 'Checking out upstream commit refs/heads/base: .*') self.assertEqual( -- 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 <sjg@chromium.org> When scanning a branch, check if the cover letter title has changed and update the series description in the database. This keeps the description in sync when the cover letter is edited between scans. Signed-off-by: Simon Glass <sjg@chromium.org> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/patman/cseries.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tools/patman/cseries.py b/tools/patman/cseries.py index 0b4ce91b024..716d3c7aa88 100644 --- a/tools/patman/cseries.py +++ b/tools/patman/cseries.py @@ -932,6 +932,13 @@ class Cseries(cser_helper.CseriesHelper): self.db.pcommit_delete(svid) self._add_series_commits(ser, svid) + + # Update series description if the cover letter has changed + branch_desc = ser.cover[0] if ser.cover else None # pylint: disable=E1136 + if branch_desc and branch_desc != ser.desc: + self.db.series_set_desc(ser.idnum, branch_desc) + tout.notice(f"Updated description to '{branch_desc}'") + if not dry_run: self.commit() seq = len(ser.commits) -- 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
participants (1)
-
Simon Glass