From: Simon Glass <simon.glass@canonical.com> Add a tool to check the number of commits in a source branch (us/next) that are not in the master branch (ci/master), and find the last common commit between them. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- doc/develop/index.rst | 1 + doc/develop/pickman.rst | 1 + tools/pickman/README.rst | 39 +++++++++++++++ tools/pickman/__init__.py | 4 ++ tools/pickman/__main__.py | 26 ++++++++++ tools/pickman/control.py | 74 ++++++++++++++++++++++++++++ tools/pickman/ftest.py | 101 ++++++++++++++++++++++++++++++++++++++ tools/pickman/pickman | 1 + 8 files changed, 247 insertions(+) create mode 120000 doc/develop/pickman.rst create mode 100644 tools/pickman/README.rst create mode 100644 tools/pickman/__init__.py create mode 100755 tools/pickman/__main__.py create mode 100644 tools/pickman/control.py create mode 100644 tools/pickman/ftest.py create mode 120000 tools/pickman/pickman diff --git a/doc/develop/index.rst b/doc/develop/index.rst index c40ada5899f..4b55d65de75 100644 --- a/doc/develop/index.rst +++ b/doc/develop/index.rst @@ -16,6 +16,7 @@ General kconfig memory patman + pickman process release_cycle concept_cycle diff --git a/doc/develop/pickman.rst b/doc/develop/pickman.rst new file mode 120000 index 00000000000..84816e57626 --- /dev/null +++ b/doc/develop/pickman.rst @@ -0,0 +1 @@ +../../tools/pickman/README.rst \ No newline at end of file diff --git a/tools/pickman/README.rst b/tools/pickman/README.rst new file mode 100644 index 00000000000..299f2cac699 --- /dev/null +++ b/tools/pickman/README.rst @@ -0,0 +1,39 @@ +.. SPDX-License-Identifier: GPL-2.0+ +.. +.. Copyright 2025 Canonical Ltd. +.. Written by Simon Glass <simon.glass@canonical.com> + +Pickman - Cherry-pick Manager +============================= + +Pickman is a tool to help manage cherry-picking commits between branches. + +Usage +----- + +To compare branches and show commits that need to be cherry-picked:: + + ./tools/pickman/pickman + +This shows: + +- The number of commits in the source branch (us/next) that are not in the + master branch (ci/master) +- The last common commit between the two branches + +Configuration +------------- + +The branches to compare are configured as constants at the top of the script: + +- ``BRANCH_MASTER``: The main branch to compare against (default: ci/master) +- ``BRANCH_SOURCE``: The source branch with commits to cherry-pick + (default: us/next) + +Testing +------- + +To run the functional tests:: + + cd tools/pickman + python3 -m pytest ftest.py -v diff --git a/tools/pickman/__init__.py b/tools/pickman/__init__.py new file mode 100644 index 00000000000..96e553681aa --- /dev/null +++ b/tools/pickman/__init__.py @@ -0,0 +1,4 @@ +# SPDX-License-Identifier: GPL-2.0+ +# +# Copyright 2025 Canonical Ltd. +# Written by Simon Glass <simon.glass@canonical.com> diff --git a/tools/pickman/__main__.py b/tools/pickman/__main__.py new file mode 100755 index 00000000000..eb0d6e226cc --- /dev/null +++ b/tools/pickman/__main__.py @@ -0,0 +1,26 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: GPL-2.0+ +# +# Copyright 2025 Canonical Ltd. +# Written by Simon Glass <simon.glass@canonical.com> +# +"""Entry point for pickman - dispatches to control module.""" + +import os +import sys + +# Allow 'from pickman import xxx' to work via symlink +our_path = os.path.dirname(os.path.realpath(__file__)) +sys.path.insert(0, os.path.join(our_path, '..')) + +# pylint: disable=wrong-import-position,import-error +from pickman import control + + +def main(): + """Main function.""" + return control.do_pickman() + + +if __name__ == '__main__': + sys.exit(main()) diff --git a/tools/pickman/control.py b/tools/pickman/control.py new file mode 100644 index 00000000000..990fa1b0729 --- /dev/null +++ b/tools/pickman/control.py @@ -0,0 +1,74 @@ +# SPDX-License-Identifier: GPL-2.0+ +# +# Copyright 2025 Canonical Ltd. +# Written by Simon Glass <simon.glass@canonical.com> +# +"""Control module for pickman - handles the main logic.""" + +from collections import namedtuple +import os +import sys + +# Allow 'from pickman import xxx' to work via symlink +our_path = os.path.dirname(os.path.realpath(__file__)) +sys.path.insert(0, os.path.join(our_path, '..')) + +# pylint: disable=wrong-import-position,import-error +from u_boot_pylib import command +from u_boot_pylib import tout + +# Branch names to compare +BRANCH_MASTER = 'ci/master' +BRANCH_SOURCE = 'us/next' + +# Named tuple for commit info +Commit = namedtuple('Commit', ['hash', 'short_hash', 'subject', 'date']) + + +def run_git(args): + """Run a git command and return output.""" + return command.output('git', *args).strip() + + +def compare_branches(master, source): + """Compare two branches and return commit difference info. + + Args: + master (str): Main branch to compare against + source (str): Source branch to check for unique commits + + Returns: + tuple: (count, Commit) where count is number of commits and Commit + is the last common commit + """ + # Find commits in source that are not in master + count = int(run_git(['rev-list', '--count', f'{master}..{source}'])) + + # Find the merge base (last common commit) + base = run_git(['merge-base', master, source]) + + # Get details about the merge-base commit + info = run_git(['log', '-1', '--format=%H%n%h%n%s%n%ci', base]) + full_hash, short_hash, subject, date = info.split('\n') + + return count, Commit(full_hash, short_hash, subject, date) + + +def do_pickman(): + """Main entry point for pickman. + + Returns: + int: 0 on success + """ + tout.init(tout.INFO) + + count, base = compare_branches(BRANCH_MASTER, BRANCH_SOURCE) + + tout.info(f'Commits in {BRANCH_SOURCE} not in {BRANCH_MASTER}: {count}') + tout.info('') + tout.info('Last common commit:') + tout.info(f' Hash: {base.short_hash}') + tout.info(f' Subject: {base.subject}') + tout.info(f' Date: {base.date}') + + return 0 diff --git a/tools/pickman/ftest.py b/tools/pickman/ftest.py new file mode 100644 index 00000000000..7b34a260659 --- /dev/null +++ b/tools/pickman/ftest.py @@ -0,0 +1,101 @@ +# SPDX-License-Identifier: GPL-2.0+ +# +# Copyright 2025 Canonical Ltd. +# Written by Simon Glass <simon.glass@canonical.com> +# +"""Tests for pickman.""" + +import os +import sys +import unittest + +# Allow 'from pickman import xxx' to work via symlink +our_path = os.path.dirname(os.path.realpath(__file__)) +sys.path.insert(0, os.path.join(our_path, '..')) + +# pylint: disable=wrong-import-position,import-error +from u_boot_pylib import command + +from pickman import control + + +class TestCommit(unittest.TestCase): + """Tests for the Commit namedtuple.""" + + def test_commit_fields(self): + """Test Commit namedtuple has correct fields.""" + commit = control.Commit( + 'abc123def456', + 'abc123d', + 'Test commit subject', + '2024-01-15 10:30:00 -0600' + ) + self.assertEqual(commit.hash, 'abc123def456') + self.assertEqual(commit.short_hash, 'abc123d') + self.assertEqual(commit.subject, 'Test commit subject') + self.assertEqual(commit.date, '2024-01-15 10:30:00 -0600') + + +class TestRunGit(unittest.TestCase): + """Tests for run_git function.""" + + def test_run_git(self): + """Test run_git returns stripped output.""" + result = command.CommandResult(stdout=' output with spaces \n') + command.TEST_RESULT = result + try: + out = control.run_git(['status']) + self.assertEqual(out, 'output with spaces') + finally: + command.TEST_RESULT = None + + +class TestCompareBranches(unittest.TestCase): + """Tests for compare_branches function.""" + + def test_compare_branches(self): + """Test compare_branches returns correct count and commit.""" + results = iter([ + '42', # rev-list --count + 'abc123def456789', # merge-base + 'abc123def456789\nabc123d\nTest subject\n2024-01-15 10:30:00 -0600', + ]) + + def handle_command(**_): + return command.CommandResult(stdout=next(results)) + + command.TEST_RESULT = handle_command + try: + count, commit = control.compare_branches('master', 'source') + + self.assertEqual(count, 42) + self.assertEqual(commit.hash, 'abc123def456789') + self.assertEqual(commit.short_hash, 'abc123d') + self.assertEqual(commit.subject, 'Test subject') + self.assertEqual(commit.date, '2024-01-15 10:30:00 -0600') + finally: + command.TEST_RESULT = None + + def test_compare_branches_zero_commits(self): + """Test compare_branches with zero commit difference.""" + results = iter([ + '0', + 'def456abc789', + 'def456abc789\ndef456a\nMerge commit\n2024-02-20 14:00:00 -0500', + ]) + + def handle_command(**_): + return command.CommandResult(stdout=next(results)) + + command.TEST_RESULT = handle_command + try: + count, commit = control.compare_branches('branch1', 'branch2') + + self.assertEqual(count, 0) + self.assertEqual(commit.short_hash, 'def456a') + finally: + command.TEST_RESULT = None + + +if __name__ == '__main__': + unittest.main() diff --git a/tools/pickman/pickman b/tools/pickman/pickman new file mode 120000 index 00000000000..5a427d19424 --- /dev/null +++ b/tools/pickman/pickman @@ -0,0 +1 @@ +__main__.py \ No newline at end of file -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> Add an sqlite3 database module to track the state of cherry-picking commits between branches. The database uses .pickman.db and includes: - source table: tracks source branches and their last cherry-picked commit into master - Schema versioning for future migrations The database code is mostly lifted from patman Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/pickman/database.py | 193 ++++++++++++++++++++++++++++++++++++++ tools/pickman/ftest.py | 72 ++++++++++++++ 2 files changed, 265 insertions(+) create mode 100644 tools/pickman/database.py diff --git a/tools/pickman/database.py b/tools/pickman/database.py new file mode 100644 index 00000000000..436734fe1f7 --- /dev/null +++ b/tools/pickman/database.py @@ -0,0 +1,193 @@ +# SPDX-License-Identifier: GPL-2.0+ +# +# Copyright 2025 Canonical Ltd. +# Written by Simon Glass <simon.glass@canonical.com> +# +"""Database for pickman - tracks cherry-pick state. + +This uses sqlite3 with a local file (.pickman.db). + +To adjust the schema, increment LATEST, create a _migrate_to_v<x>() function +and add code in migrate_to() to call it. +""" + +import os +import sqlite3 + +from u_boot_pylib import tools +from u_boot_pylib import tout + +# Schema version (version 0 means there is no database yet) +LATEST = 1 + +# Default database filename +DB_FNAME = '.pickman.db' + + +class Database: + """Database of cherry-pick state used by pickman""" + + # dict of databases: + # key: filename + # value: Database object + instances = {} + + def __init__(self, db_path): + """Set up a new database object + + Args: + db_path (str): Path to the database + """ + if db_path in Database.instances: + raise ValueError(f"There is already a database for '{db_path}'") + self.con = None + self.cur = None + self.db_path = db_path + self.is_open = False + Database.instances[db_path] = self + + @staticmethod + def get_instance(db_path): + """Get the database instance for a path + + Args: + db_path (str): Path to the database + + Return: + tuple: + Database: Database instance, created if necessary + bool: True if newly created + """ + dbs = Database.instances.get(db_path) + if dbs: + return dbs, False + return Database(db_path), True + + def start(self): + """Open the database ready for use, migrate to latest schema""" + self.open_it() + self.migrate_to(LATEST) + + def open_it(self): + """Open the database, creating it if necessary""" + if self.is_open: + raise ValueError('Already open') + if not os.path.exists(self.db_path): + tout.warning(f'Creating new database {self.db_path}') + self.con = sqlite3.connect(self.db_path) + self.cur = self.con.cursor() + self.is_open = True + Database.instances[self.db_path] = self + + def close(self): + """Close the database""" + if not self.is_open: + raise ValueError('Already closed') + self.con.close() + self.cur = None + self.con = None + self.is_open = False + Database.instances.pop(self.db_path, None) + + def _create_v1(self): + """Create a database with the v1 schema""" + # Table for tracking source branches and their last cherry-picked commit + self.cur.execute( + 'CREATE TABLE source (' + 'id INTEGER PRIMARY KEY AUTOINCREMENT, ' + 'name TEXT UNIQUE, ' + 'last_commit TEXT)') + + # Schema version table + self.cur.execute('CREATE TABLE schema_version (version INTEGER)') + + def migrate_to(self, dest_version): + """Migrate the database to the selected version + + Args: + dest_version (int): Version to migrate to + """ + while True: + version = self.get_schema_version() + if version >= dest_version: + break + + self.close() + tools.write_file(f'{self.db_path}old.v{version}', + tools.read_file(self.db_path)) + + version += 1 + tout.info(f'Update database to v{version}') + self.open_it() + if version == 1: + self._create_v1() + + self.cur.execute('DELETE FROM schema_version') + self.cur.execute( + 'INSERT INTO schema_version (version) VALUES (?)', + (version,)) + self.commit() + + def get_schema_version(self): + """Get the version of the database's schema + + Return: + int: Database version, 0 means there is no data + """ + try: + self.cur.execute('SELECT version FROM schema_version') + return self.cur.fetchone()[0] + except sqlite3.OperationalError: + return 0 + + def execute(self, query, parameters=()): + """Execute a database query + + Args: + query (str): Query string + parameters (tuple): Parameters to pass + + Return: + Cursor result + """ + return self.cur.execute(query, parameters) + + def commit(self): + """Commit changes to the database""" + self.con.commit() + + def rollback(self): + """Roll back changes to the database""" + self.con.rollback() + + # source functions + + def source_get(self, name): + """Get the last cherry-picked commit for a source branch + + Args: + name (str): Source branch name + + Return: + str: Commit hash, or None if not found + """ + res = self.execute( + 'SELECT last_commit FROM source WHERE name = ?', (name,)) + rec = res.fetchone() + if rec: + return rec[0] + return None + + def source_set(self, name, commit): + """Set the last cherry-picked commit for a source branch + + Args: + name (str): Source branch name + commit (str): Commit hash + """ + self.execute( + 'UPDATE source SET last_commit = ? WHERE name = ?', (commit, name)) + if self.cur.rowcount == 0: + self.execute( + 'INSERT INTO source (name, last_commit) VALUES (?, ?)', + (name, commit)) diff --git a/tools/pickman/ftest.py b/tools/pickman/ftest.py index eeb19926f76..b975b9c6a2b 100644 --- a/tools/pickman/ftest.py +++ b/tools/pickman/ftest.py @@ -7,6 +7,7 @@ import os import sys +import tempfile import unittest # Allow 'from pickman import xxx' to work via symlink @@ -19,6 +20,7 @@ from u_boot_pylib import terminal from pickman import __main__ as pickman from pickman import control +from pickman import database class TestCommit(unittest.TestCase): @@ -152,5 +154,75 @@ class TestMain(unittest.TestCase): command.TEST_RESULT = None +class TestDatabase(unittest.TestCase): + """Tests for Database class.""" + + def setUp(self): + """Set up test fixtures.""" + fd, self.db_path = tempfile.mkstemp(suffix='.db') + os.close(fd) + os.unlink(self.db_path) # Remove so database creates it fresh + database.Database.instances.clear() + + def tearDown(self): + """Clean up test fixtures.""" + if os.path.exists(self.db_path): + os.unlink(self.db_path) + database.Database.instances.clear() + + def test_create_database(self): + """Test creating a new database.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + self.assertTrue(dbs.is_open) + self.assertEqual(dbs.get_schema_version(), database.LATEST) + dbs.close() + + def test_source_get_empty(self): + """Test getting source from empty database.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + result = dbs.source_get('us/next') + self.assertIsNone(result) + dbs.close() + + def test_source_set_and_get(self): + """Test setting and getting source commit.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + dbs.source_set('us/next', 'abc123def456') + dbs.commit() + result = dbs.source_get('us/next') + self.assertEqual(result, 'abc123def456') + dbs.close() + + def test_source_update(self): + """Test updating source commit.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + dbs.source_set('us/next', 'abc123') + dbs.commit() + dbs.source_set('us/next', 'def456') + dbs.commit() + result = dbs.source_get('us/next') + self.assertEqual(result, 'def456') + dbs.close() + + def test_get_instance(self): + """Test get_instance returns same database.""" + with terminal.capture(): + dbs1, created1 = database.Database.get_instance(self.db_path) + dbs1.start() + dbs2, created2 = database.Database.get_instance(self.db_path) + self.assertTrue(created1) + self.assertFalse(created2) + self.assertIs(dbs1, dbs2) + dbs1.close() + + if __name__ == '__main__': unittest.main() -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> Add a command to list all tracked source branches and their last cherry-picked commits from the database. Usage: ./tools/pickman/pickman list-sources Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/pickman/README.rst | 4 +++ tools/pickman/__main__.py | 1 + tools/pickman/control.py | 23 +++++++++++++ tools/pickman/database.py | 9 +++++ tools/pickman/ftest.py | 70 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 107 insertions(+) diff --git a/tools/pickman/README.rst b/tools/pickman/README.rst index aab0642d374..70f53a6e212 100644 --- a/tools/pickman/README.rst +++ b/tools/pickman/README.rst @@ -19,6 +19,10 @@ This finds the merge-base commit between the master branch (ci/master) and the source branch, and stores it in the database as the starting point for cherry-picking. +To list all tracked source branches:: + + ./tools/pickman/pickman list-sources + To compare branches and show commits that need to be cherry-picked:: ./tools/pickman/pickman compare diff --git a/tools/pickman/__main__.py b/tools/pickman/__main__.py index 7263f4c5fb0..63930953ebb 100755 --- a/tools/pickman/__main__.py +++ b/tools/pickman/__main__.py @@ -35,6 +35,7 @@ def parse_args(argv): add_source.add_argument('source', help='Source branch name') subparsers.add_parser('compare', help='Compare branches') + subparsers.add_parser('list-sources', help='List tracked source branches') subparsers.add_parser('test', help='Run tests') return parser.parse_args(argv) diff --git a/tools/pickman/control.py b/tools/pickman/control.py index fa53a26b6ad..5780703bfba 100644 --- a/tools/pickman/control.py +++ b/tools/pickman/control.py @@ -92,6 +92,28 @@ def do_add_source(args, dbs): return 0 +def do_list_sources(args, dbs): # pylint: disable=unused-argument + """List all tracked source branches + + Args: + args (Namespace): Parsed arguments + dbs (Database): Database instance + + Returns: + int: 0 on success + """ + sources = dbs.source_get_all() + + if not sources: + tout.info('No source branches tracked') + else: + tout.info('Tracked source branches:') + for name, last_commit in sources: + tout.info(f' {name}: {last_commit[:12]}') + + return 0 + + def do_compare(args, dbs): # pylint: disable=unused-argument """Compare branches and print results. @@ -133,6 +155,7 @@ def do_test(args, dbs): # pylint: disable=unused-argument COMMANDS = { 'add-source': do_add_source, 'compare': do_compare, + 'list-sources': do_list_sources, 'test': do_test, } diff --git a/tools/pickman/database.py b/tools/pickman/database.py index 436734fe1f7..46b8556945e 100644 --- a/tools/pickman/database.py +++ b/tools/pickman/database.py @@ -178,6 +178,15 @@ class Database: return rec[0] return None + def source_get_all(self): + """Get all source branches and their last commits + + Return: + list of tuple: (name, last_commit) pairs + """ + res = self.execute('SELECT name, last_commit FROM source ORDER BY name') + return res.fetchall() + def source_set(self, name, commit): """Set the last cherry-picked commit for a source branch diff --git a/tools/pickman/ftest.py b/tools/pickman/ftest.py index 632cd56793f..91a003b649c 100644 --- a/tools/pickman/ftest.py +++ b/tools/pickman/ftest.py @@ -287,6 +287,76 @@ class TestDatabase(unittest.TestCase): self.assertIs(dbs1, dbs2) dbs1.close() + def test_source_get_all(self): + """Test getting all sources.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + + # Empty initially + self.assertEqual(dbs.source_get_all(), []) + + # Add some sources + dbs.source_set('branch-a', 'abc123') + dbs.source_set('branch-b', 'def456') + dbs.commit() + + # Should be sorted by name + sources = dbs.source_get_all() + self.assertEqual(len(sources), 2) + self.assertEqual(sources[0], ('branch-a', 'abc123')) + self.assertEqual(sources[1], ('branch-b', 'def456')) + dbs.close() + + +class TestListSources(unittest.TestCase): + """Tests for list-sources command.""" + + def setUp(self): + """Set up test fixtures.""" + fd, self.db_path = tempfile.mkstemp(suffix='.db') + os.close(fd) + os.unlink(self.db_path) + self.old_db_fname = control.DB_FNAME + control.DB_FNAME = self.db_path + database.Database.instances.clear() + + def tearDown(self): + """Clean up test fixtures.""" + control.DB_FNAME = self.old_db_fname + if os.path.exists(self.db_path): + os.unlink(self.db_path) + database.Database.instances.clear() + + def test_list_sources_empty(self): + """Test list-sources with no sources""" + args = argparse.Namespace(cmd='list-sources') + with terminal.capture() as (stdout, _): + ret = control.do_pickman(args) + self.assertEqual(ret, 0) + self.assertIn('No source branches tracked', stdout.getvalue()) + + def test_list_sources(self): + """Test list-sources with sources""" + # Add some sources first + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + dbs.source_set('us/next', 'abc123def456') + dbs.source_set('other/branch', 'def456abc789') + dbs.commit() + dbs.close() + + database.Database.instances.clear() + args = argparse.Namespace(cmd='list-sources') + with terminal.capture() as (stdout, _): + ret = control.do_pickman(args) + self.assertEqual(ret, 0) + output = stdout.getvalue() + self.assertIn('Tracked source branches:', output) + self.assertIn('other/branch: def456abc789', output) + self.assertIn('us/next: abc123def456', output) + if __name__ == '__main__': unittest.main() -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> Add new database tables to track individual commits and merge requests: - pcommit: tracks commits being cherry-picked with status, author, subject, and optional link to merge request - mergereq: tracks GitLab merge requests with branch name, MR ID, status, and URL Also add helper functions for both tables and update control.py to write commit status to the database during apply operations. Update README.rst with documentation for all database tables. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/pickman/README.rst | 40 ++++- tools/pickman/database.py | 213 +++++++++++++++++++++++- tools/pickman/ftest.py | 337 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 585 insertions(+), 5 deletions(-) diff --git a/tools/pickman/README.rst b/tools/pickman/README.rst index 5cb4f51df5c..d8ab2ff6cf3 100644 --- a/tools/pickman/README.rst +++ b/tools/pickman/README.rst @@ -44,10 +44,42 @@ represents a logical grouping of commits (e.g., a pull request). Database -------- -Pickman uses a sqlite3 database (``.pickman.db``) to track state: - -- **source table**: Tracks source branches and the last commit that was - cherry-picked into master +Pickman uses a sqlite3 database (``.pickman.db``) to track state. The schema +version is stored in the ``schema_version`` table and migrations are applied +automatically when the database is opened. + +Tables +~~~~~~ + +**source** + Tracks source branches and their cherry-pick progress. + + - ``id``: Primary key + - ``name``: Branch name (e.g., 'us/next') + - ``last_commit``: Hash of the last commit cherry-picked from this branch + +**pcommit** + Tracks individual commits being cherry-picked. + + - ``id``: Primary key + - ``chash``: Original commit hash + - ``source_id``: Foreign key to source table + - ``mergereq_id``: Foreign key to mergereq table (optional) + - ``subject``: Commit subject line + - ``author``: Commit author + - ``status``: One of 'pending', 'applied', 'skipped', 'conflict' + - ``cherry_hash``: Hash of the cherry-picked commit (if applied) + +**mergereq** + Tracks merge requests created for cherry-picked commits. + + - ``id``: Primary key + - ``source_id``: Foreign key to source table + - ``branch_name``: Git branch name for this MR + - ``mr_id``: GitLab merge request ID + - ``status``: One of 'open', 'merged', 'closed' + - ``url``: URL to the merge request + - ``created_at``: Timestamp when the MR was created Configuration ------------- diff --git a/tools/pickman/database.py b/tools/pickman/database.py index 46b8556945e..118ac5536fa 100644 --- a/tools/pickman/database.py +++ b/tools/pickman/database.py @@ -18,7 +18,7 @@ from u_boot_pylib import tools from u_boot_pylib import tout # Schema version (version 0 means there is no database yet) -LATEST = 1 +LATEST = 2 # Default database filename DB_FNAME = '.pickman.db' @@ -101,6 +101,34 @@ class Database: # Schema version table self.cur.execute('CREATE TABLE schema_version (version INTEGER)') + def _create_v2(self): + """Migrate database to v2 schema - add commit and mergereq tables""" + # Table for tracking individual commits + self.cur.execute( + 'CREATE TABLE pcommit (' + 'id INTEGER PRIMARY KEY AUTOINCREMENT, ' + 'chash TEXT UNIQUE, ' + 'source_id INTEGER, ' + 'mergereq_id INTEGER, ' + 'subject TEXT, ' + 'author TEXT, ' + 'status TEXT, ' + 'cherry_hash TEXT, ' + 'FOREIGN KEY (source_id) REFERENCES source(id), ' + 'FOREIGN KEY (mergereq_id) REFERENCES mergereq(id))') + + # Table for tracking merge requests + self.cur.execute( + 'CREATE TABLE mergereq (' + 'id INTEGER PRIMARY KEY AUTOINCREMENT, ' + 'source_id INTEGER, ' + 'branch_name TEXT, ' + 'mr_id INTEGER, ' + 'status TEXT, ' + 'url TEXT, ' + 'created_at TEXT, ' + 'FOREIGN KEY (source_id) REFERENCES source(id))') + def migrate_to(self, dest_version): """Migrate the database to the selected version @@ -121,6 +149,8 @@ class Database: self.open_it() if version == 1: self._create_v1() + elif version == 2: + self._create_v2() self.cur.execute('DELETE FROM schema_version') self.cur.execute( @@ -200,3 +230,184 @@ class Database: self.execute( 'INSERT INTO source (name, last_commit) VALUES (?, ?)', (name, commit)) + + def source_get_id(self, name): + """Get the id for a source branch + + Args: + name (str): Source branch name + + Return: + int: Source id, or None if not found + """ + res = self.execute('SELECT id FROM source WHERE name = ?', (name,)) + rec = res.fetchone() + if rec: + return rec[0] + return None + + # commit functions + + def commit_add(self, chash, source_id, subject, author, status='pending', + mergereq_id=None): + """Add a commit to the database + + Args: + chash (str): Commit hash + source_id (int): Source branch id + subject (str): Commit subject line + author (str): Commit author + status (str): Status (pending, applied, skipped, conflict) + mergereq_id (int): Merge request id (optional) + """ + self.execute( + 'INSERT OR REPLACE INTO pcommit ' + '(chash, source_id, mergereq_id, subject, author, status) ' + 'VALUES (?, ?, ?, ?, ?, ?)', + (chash, source_id, mergereq_id, subject, author, status)) + + def commit_get(self, chash): + """Get a commit by hash + + Args: + chash (str): Commit hash + + Return: + tuple: (id, chash, source_id, mergereq_id, subject, author, status, + cherry_hash) or None if not found + """ + res = self.execute( + 'SELECT id, chash, source_id, mergereq_id, subject, author, status, ' + 'cherry_hash FROM pcommit WHERE chash = ?', (chash,)) + return res.fetchone() + + def commit_get_by_source(self, source_id, status=None): + """Get all commits for a source branch + + Args: + source_id (int): Source branch id + status (str): Optional status filter + + Return: + list of tuple: Commit records + """ + if status: + res = self.execute( + 'SELECT id, chash, source_id, mergereq_id, subject, author, ' + 'status, cherry_hash FROM pcommit ' + 'WHERE source_id = ? AND status = ?', + (source_id, status)) + else: + res = self.execute( + 'SELECT id, chash, source_id, mergereq_id, subject, author, ' + 'status, cherry_hash FROM pcommit WHERE source_id = ?', + (source_id,)) + return res.fetchall() + + def commit_get_by_mergereq(self, mergereq_id): + """Get all commits for a merge request + + Args: + mergereq_id (int): Merge request id + + Return: + list of tuple: Commit records + """ + res = self.execute( + 'SELECT id, chash, source_id, mergereq_id, subject, author, ' + 'status, cherry_hash FROM pcommit WHERE mergereq_id = ?', + (mergereq_id,)) + return res.fetchall() + + def commit_set_status(self, chash, status, cherry_hash=None): + """Update the status of a commit + + Args: + chash (str): Commit hash + status (str): New status + cherry_hash (str): Hash of cherry-picked commit (optional) + """ + if cherry_hash: + self.execute( + 'UPDATE pcommit SET status = ?, cherry_hash = ? WHERE chash = ?', + (status, cherry_hash, chash)) + else: + self.execute( + 'UPDATE pcommit SET status = ? WHERE chash = ?', (status, chash)) + + def commit_set_mergereq(self, chash, mergereq_id): + """Set the merge request for a commit + + Args: + chash (str): Commit hash + mergereq_id (int): Merge request id + """ + self.execute( + 'UPDATE pcommit SET mergereq_id = ? WHERE chash = ?', + (mergereq_id, chash)) + + # mergereq functions + + def mergereq_add(self, source_id, branch_name, mr_id, status, url, + created_at): + """Add a merge request to the database + + Args: + source_id (int): Source branch id + branch_name (str): Branch name for the MR + mr_id (int): GitLab MR id + status (str): Status (open, merged, closed) + url (str): URL to the MR + created_at (str): Creation timestamp + """ + self.execute( + 'INSERT INTO mergereq ' + '(source_id, branch_name, mr_id, status, url, created_at) ' + 'VALUES (?, ?, ?, ?, ?, ?)', + (source_id, branch_name, mr_id, status, url, created_at)) + + def mergereq_get(self, mr_id): + """Get a merge request by GitLab MR id + + Args: + mr_id (int): GitLab MR id + + Return: + tuple: (id, source_id, branch_name, mr_id, status, url, created_at) + or None if not found + """ + res = self.execute( + 'SELECT id, source_id, branch_name, mr_id, status, url, created_at ' + 'FROM mergereq WHERE mr_id = ?', (mr_id,)) + return res.fetchone() + + def mergereq_get_by_source(self, source_id, status=None): + """Get all merge requests for a source branch + + Args: + source_id (int): Source branch id + status (str): Optional status filter + + Return: + list of tuple: Merge request records + """ + if status: + res = self.execute( + 'SELECT id, source_id, branch_name, mr_id, status, url, ' + 'created_at FROM mergereq WHERE source_id = ? AND status = ?', + (source_id, status)) + else: + res = self.execute( + 'SELECT id, source_id, branch_name, mr_id, status, url, ' + 'created_at FROM mergereq WHERE source_id = ?', (source_id,)) + return res.fetchall() + + def mergereq_set_status(self, mr_id, status): + """Update the status of a merge request + + Args: + mr_id (int): GitLab MR id + status (str): New status + """ + self.execute( + 'UPDATE mergereq SET status = ? WHERE mr_id = ?', (status, mr_id)) diff --git a/tools/pickman/ftest.py b/tools/pickman/ftest.py index a6331d21c5f..2c9e5b1d780 100644 --- a/tools/pickman/ftest.py +++ b/tools/pickman/ftest.py @@ -309,6 +309,343 @@ class TestDatabase(unittest.TestCase): dbs.close() +class TestDatabaseCommit(unittest.TestCase): + """Tests for Database commit functions.""" + + def setUp(self): + """Set up test fixtures.""" + fd, self.db_path = tempfile.mkstemp(suffix='.db') + os.close(fd) + os.unlink(self.db_path) + database.Database.instances.clear() + + def tearDown(self): + """Clean up test fixtures.""" + if os.path.exists(self.db_path): + os.unlink(self.db_path) + database.Database.instances.clear() + + def test_commit_add_and_get(self): + """Test adding and getting a commit.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + + # First add a source + dbs.source_set('us/next', 'base123') + dbs.commit() + source_id = dbs.source_get_id('us/next') + + # Add a commit + dbs.commit_add('abc123def456', source_id, 'Test subject', + 'Author Name') + dbs.commit() + + # Get the commit + result = dbs.commit_get('abc123def456') + self.assertIsNotNone(result) + self.assertEqual(result[1], 'abc123def456') # chash + self.assertEqual(result[2], source_id) # source_id + self.assertIsNone(result[3]) # mergereq_id + self.assertEqual(result[4], 'Test subject') # subject + self.assertEqual(result[5], 'Author Name') # author + self.assertEqual(result[6], 'pending') # status + dbs.close() + + def test_commit_get_not_found(self): + """Test getting a non-existent commit.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + result = dbs.commit_get('nonexistent') + self.assertIsNone(result) + dbs.close() + + def test_commit_get_by_source(self): + """Test getting commits by source.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + + # Add a source + dbs.source_set('us/next', 'base123') + dbs.commit() + source_id = dbs.source_get_id('us/next') + + # Add commits + dbs.commit_add('commit1', source_id, 'Subject 1', 'Author 1') + dbs.commit_add('commit2', source_id, 'Subject 2', 'Author 2', + status='applied') + dbs.commit_add('commit3', source_id, 'Subject 3', 'Author 3') + dbs.commit() + + # Get all commits for source + commits = dbs.commit_get_by_source(source_id) + self.assertEqual(len(commits), 3) + + # Get only pending commits + pending = dbs.commit_get_by_source(source_id, status='pending') + self.assertEqual(len(pending), 2) + + # Get only applied commits + applied = dbs.commit_get_by_source(source_id, status='applied') + self.assertEqual(len(applied), 1) + self.assertEqual(applied[0][1], 'commit2') + dbs.close() + + def test_commit_set_status(self): + """Test updating commit status.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + + dbs.source_set('us/next', 'base123') + dbs.commit() + source_id = dbs.source_get_id('us/next') + + dbs.commit_add('abc123', source_id, 'Subject', 'Author') + dbs.commit() + + # Update status + dbs.commit_set_status('abc123', 'applied') + dbs.commit() + + result = dbs.commit_get('abc123') + self.assertEqual(result[6], 'applied') + dbs.close() + + def test_commit_set_status_with_cherry_hash(self): + """Test updating commit status with cherry hash.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + + dbs.source_set('us/next', 'base123') + dbs.commit() + source_id = dbs.source_get_id('us/next') + + dbs.commit_add('abc123', source_id, 'Subject', 'Author') + dbs.commit() + + # Update status with cherry hash + dbs.commit_set_status('abc123', 'applied', cherry_hash='xyz789') + dbs.commit() + + result = dbs.commit_get('abc123') + self.assertEqual(result[6], 'applied') + self.assertEqual(result[7], 'xyz789') # cherry_hash + dbs.close() + + def test_source_get_id(self): + """Test getting source id by name.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + + # Not found initially + self.assertIsNone(dbs.source_get_id('us/next')) + + # Add source and get id + dbs.source_set('us/next', 'abc123') + dbs.commit() + + source_id = dbs.source_get_id('us/next') + self.assertIsNotNone(source_id) + self.assertIsInstance(source_id, int) + dbs.close() + + +class TestDatabaseMergereq(unittest.TestCase): + """Tests for Database mergereq functions.""" + + def setUp(self): + """Set up test fixtures.""" + fd, self.db_path = tempfile.mkstemp(suffix='.db') + os.close(fd) + os.unlink(self.db_path) + database.Database.instances.clear() + + def tearDown(self): + """Clean up test fixtures.""" + if os.path.exists(self.db_path): + os.unlink(self.db_path) + database.Database.instances.clear() + + def test_mergereq_add_and_get(self): + """Test adding and getting a merge request.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + + # Add a source + dbs.source_set('us/next', 'base123') + dbs.commit() + source_id = dbs.source_get_id('us/next') + + # Add a merge request + dbs.mergereq_add(source_id, 'cherry-abc123', 42, 'open', + 'https://gitlab.com/mr/42', '2025-01-15') + dbs.commit() + + # Get the merge request + result = dbs.mergereq_get(42) + self.assertIsNotNone(result) + self.assertEqual(result[1], source_id) # source_id + self.assertEqual(result[2], 'cherry-abc123') # branch_name + self.assertEqual(result[3], 42) # mr_id + self.assertEqual(result[4], 'open') # status + self.assertEqual(result[5], 'https://gitlab.com/mr/42') # url + self.assertEqual(result[6], '2025-01-15') # created_at + dbs.close() + + def test_mergereq_get_not_found(self): + """Test getting a non-existent merge request.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + result = dbs.mergereq_get(999) + self.assertIsNone(result) + dbs.close() + + def test_mergereq_get_by_source(self): + """Test getting merge requests by source.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + + # Add a source + dbs.source_set('us/next', 'base123') + dbs.commit() + source_id = dbs.source_get_id('us/next') + + # Add merge requests + dbs.mergereq_add(source_id, 'branch-1', 1, 'open', + 'https://gitlab.com/mr/1', '2025-01-01') + dbs.mergereq_add(source_id, 'branch-2', 2, 'merged', + 'https://gitlab.com/mr/2', '2025-01-02') + dbs.mergereq_add(source_id, 'branch-3', 3, 'open', + 'https://gitlab.com/mr/3', '2025-01-03') + dbs.commit() + + # Get all merge requests for source + mrs = dbs.mergereq_get_by_source(source_id) + self.assertEqual(len(mrs), 3) + + # Get only open merge requests + open_mrs = dbs.mergereq_get_by_source(source_id, status='open') + self.assertEqual(len(open_mrs), 2) + + # Get only merged + merged = dbs.mergereq_get_by_source(source_id, status='merged') + self.assertEqual(len(merged), 1) + self.assertEqual(merged[0][3], 2) # mr_id + dbs.close() + + def test_mergereq_set_status(self): + """Test updating merge request status.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + + dbs.source_set('us/next', 'base123') + dbs.commit() + source_id = dbs.source_get_id('us/next') + + dbs.mergereq_add(source_id, 'branch-1', 42, 'open', + 'https://gitlab.com/mr/42', '2025-01-15') + dbs.commit() + + # Update status + dbs.mergereq_set_status(42, 'merged') + dbs.commit() + + result = dbs.mergereq_get(42) + self.assertEqual(result[4], 'merged') + dbs.close() + + +class TestDatabaseCommitMergereq(unittest.TestCase): + """Tests for commit-mergereq relationship.""" + + def setUp(self): + """Set up test fixtures.""" + fd, self.db_path = tempfile.mkstemp(suffix='.db') + os.close(fd) + os.unlink(self.db_path) + database.Database.instances.clear() + + def tearDown(self): + """Clean up test fixtures.""" + if os.path.exists(self.db_path): + os.unlink(self.db_path) + database.Database.instances.clear() + + def test_commit_set_mergereq(self): + """Test setting merge request for a commit.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + + # Add source + dbs.source_set('us/next', 'base123') + dbs.commit() + source_id = dbs.source_get_id('us/next') + + # Add merge request + dbs.mergereq_add(source_id, 'branch-1', 42, 'open', + 'https://gitlab.com/mr/42', '2025-01-15') + dbs.commit() + mr = dbs.mergereq_get(42) + mr_id = mr[0] # id field + + # Add commit without mergereq + dbs.commit_add('abc123', source_id, 'Subject', 'Author') + dbs.commit() + + # Set mergereq + dbs.commit_set_mergereq('abc123', mr_id) + dbs.commit() + + result = dbs.commit_get('abc123') + self.assertEqual(result[3], mr_id) # mergereq_id + dbs.close() + + def test_commit_get_by_mergereq(self): + """Test getting commits by merge request.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + + # Add source + dbs.source_set('us/next', 'base123') + dbs.commit() + source_id = dbs.source_get_id('us/next') + + # Add merge request + dbs.mergereq_add(source_id, 'branch-1', 42, 'open', + 'https://gitlab.com/mr/42', '2025-01-15') + dbs.commit() + mr = dbs.mergereq_get(42) + mr_id = mr[0] + + # Add commits with mergereq_id + dbs.commit_add('commit1', source_id, 'Subject 1', 'Author 1', + mergereq_id=mr_id) + dbs.commit_add('commit2', source_id, 'Subject 2', 'Author 2', + mergereq_id=mr_id) + dbs.commit_add('commit3', source_id, 'Subject 3', 'Author 3') + dbs.commit() + + # Get commits for merge request + commits = dbs.commit_get_by_mergereq(mr_id) + self.assertEqual(len(commits), 2) + hashes = [c[1] for c in commits] + self.assertIn('commit1', hashes) + self.assertIn('commit2', hashes) + self.assertNotIn('commit3', hashes) + dbs.close() + + class TestListSources(unittest.TestCase): """Tests for list-sources command.""" -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> Add a command that automates cherry-picking commits using the agent module. The apply command: - Gets the next set of commits (same as next-set) - Creates a new branch for the cherry-picks (-b/--branch option) - Runs the agent to execute the cherry-picks - Returns to the original branch after completion The database is not updated automatically; use 'commit-source' to update after reviewing the cherry-picked branch. Also refactor control.py to extract get_next_commits() for reuse and use a dispatch table for commands. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/pickman/README.rst | 23 +++++++ tools/pickman/__main__.py | 5 ++ tools/pickman/control.py | 74 +++++++++++++++++++++++ tools/pickman/ftest.py | 124 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 226 insertions(+) diff --git a/tools/pickman/README.rst b/tools/pickman/README.rst index d8ab2ff6cf3..0ad634516ce 100644 --- a/tools/pickman/README.rst +++ b/tools/pickman/README.rst @@ -41,6 +41,29 @@ This finds commits between the last cherry-picked commit and the next merge commit in the source branch. It stops at the merge commit since that typically represents a logical grouping of commits (e.g., a pull request). +To apply the next set of commits using a Claude agent:: + + ./tools/pickman/pickman apply us/next + +This uses the Claude Agent SDK to automate the cherry-pick process. The agent +will: + +- Run git status to check the repository state +- Cherry-pick each commit in order +- Handle simple conflicts automatically +- Report status after completion +- Update the database with the last successfully applied commit + +Requirements +------------ + +To use the ``apply`` command, install the Claude Agent SDK:: + + pip install claude-agent-sdk + +You will also need an Anthropic API key set in the ``ANTHROPIC_API_KEY`` +environment variable. + Database -------- diff --git a/tools/pickman/__main__.py b/tools/pickman/__main__.py index 26886f1fe1b..0ac7bfacf70 100755 --- a/tools/pickman/__main__.py +++ b/tools/pickman/__main__.py @@ -34,6 +34,11 @@ def parse_args(argv): help='Add a source branch to track') add_source.add_argument('source', help='Source branch name') + apply_cmd = subparsers.add_parser('apply', + help='Apply next commits using Claude') + apply_cmd.add_argument('source', help='Source branch name') + apply_cmd.add_argument('-b', '--branch', help='Branch name to create') + subparsers.add_parser('compare', help='Compare branches') subparsers.add_parser('list-sources', help='List tracked source branches') diff --git a/tools/pickman/control.py b/tools/pickman/control.py index 24453b6dd14..6974bbeeb7c 100644 --- a/tools/pickman/control.py +++ b/tools/pickman/control.py @@ -15,6 +15,7 @@ our_path = os.path.dirname(os.path.realpath(__file__)) sys.path.insert(0, os.path.join(our_path, '..')) # pylint: disable=wrong-import-position,import-error +from pickman import agent from pickman import database from pickman import ftest from u_boot_pylib import command @@ -226,6 +227,78 @@ def do_next_set(args, dbs): return 0 +def do_apply(args, dbs): + """Apply the next set of commits using Claude agent + + Args: + args (Namespace): Parsed arguments with 'source' and 'branch' attributes + dbs (Database): Database instance + + Returns: + int: 0 on success, 1 on failure + """ + source = args.source + commits, merge_found, error = get_next_commits(dbs, source) + + if error: + tout.error(error) + return 1 + + if not commits: + tout.info('No new commits to cherry-pick') + return 0 + + # Save current branch to return to later + original_branch = run_git(['rev-parse', '--abbrev-ref', 'HEAD']) + + # Generate branch name if not provided + branch_name = args.branch + if not branch_name: + # Use first commit's short hash as part of branch name + branch_name = f'cherry-{commits[0].short_hash}' + + if merge_found: + tout.info(f'Applying next set from {source} ({len(commits)} commits):') + else: + tout.info(f'Applying remaining commits from {source} ' + f'({len(commits)} commits, no merge found):') + + tout.info(f' Branch: {branch_name}') + for commit in commits: + tout.info(f' {commit.short_hash} {commit.subject}') + tout.info('') + + # Add commits to database with 'pending' status + source_id = dbs.source_get_id(source) + for commit in commits: + dbs.commit_add(commit.hash, source_id, commit.subject, commit.author, + status='pending') + dbs.commit() + + # Convert CommitInfo to tuple format expected by agent + commit_tuples = [(c.hash, c.short_hash, c.subject) for c in commits] + success = agent.cherry_pick_commits(commit_tuples, source, branch_name) + + # Update commit status based on result + status = 'applied' if success else 'conflict' + for commit in commits: + dbs.commit_set_status(commit.hash, status) + dbs.commit() + + # Return to original branch + current_branch = run_git(['rev-parse', '--abbrev-ref', 'HEAD']) + if current_branch != original_branch: + tout.info(f'Returning to {original_branch}') + run_git(['checkout', original_branch]) + + if success: + tout.info(f"Use 'pickman commit-source {source} {commits[-1].short_hash}' " + 'to update the database') + + return 0 if success else 1 + + + def do_test(args, dbs): # pylint: disable=unused-argument """Run tests for this module. @@ -247,6 +320,7 @@ def do_test(args, dbs): # pylint: disable=unused-argument # Command dispatch table COMMANDS = { 'add-source': do_add_source, + 'apply': do_apply, 'compare': do_compare, 'list-sources': do_list_sources, 'next-set': do_next_set, diff --git a/tools/pickman/ftest.py b/tools/pickman/ftest.py index 2c9e5b1d780..41c0caec1e2 100644 --- a/tools/pickman/ftest.py +++ b/tools/pickman/ftest.py @@ -111,6 +111,20 @@ class TestParseArgs(unittest.TestCase): self.assertEqual(args.cmd, 'add-source') self.assertEqual(args.source, 'us/next') + def test_parse_apply(self): + """Test parsing apply command.""" + args = pickman.parse_args(['apply', 'us/next']) + self.assertEqual(args.cmd, 'apply') + self.assertEqual(args.source, 'us/next') + self.assertIsNone(args.branch) + + def test_parse_apply_with_branch(self): + """Test parsing apply command with branch.""" + args = pickman.parse_args(['apply', 'us/next', '-b', 'my-branch']) + self.assertEqual(args.cmd, 'apply') + self.assertEqual(args.source, 'us/next') + self.assertEqual(args.branch, 'my-branch') + def test_parse_compare(self): """Test parsing compare command.""" args = pickman.parse_args(['compare']) @@ -816,5 +830,115 @@ class TestNextSet(unittest.TestCase): self.assertIn('bbb222b Second commit', output) +class TestGetNextCommits(unittest.TestCase): + """Tests for get_next_commits function.""" + + def setUp(self): + """Set up test fixtures.""" + fd, self.db_path = tempfile.mkstemp(suffix='.db') + os.close(fd) + os.unlink(self.db_path) + self.old_db_fname = control.DB_FNAME + control.DB_FNAME = self.db_path + database.Database.instances.clear() + + def tearDown(self): + """Clean up test fixtures.""" + control.DB_FNAME = self.old_db_fname + if os.path.exists(self.db_path): + os.unlink(self.db_path) + database.Database.instances.clear() + command.TEST_RESULT = None + + def test_get_next_commits_source_not_found(self): + """Test get_next_commits with unknown source""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + commits, merge_found, error = control.get_next_commits(dbs, + 'unknown') + self.assertIsNone(commits) + self.assertFalse(merge_found) + self.assertIn('not found', error) + dbs.close() + + def test_get_next_commits_with_merge(self): + """Test get_next_commits finding commits up to merge""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + dbs.source_set('us/next', 'abc123') + dbs.commit() + + log_output = ( + 'aaa111|aaa111a|Author 1|First commit|abc123\n' + 'bbb222|bbb222b|Author 2|Merge branch|aaa111 ccc333\n' + ) + command.TEST_RESULT = command.CommandResult(stdout=log_output) + + commits, merge_found, error = control.get_next_commits(dbs, + 'us/next') + self.assertIsNone(error) + self.assertTrue(merge_found) + self.assertEqual(len(commits), 2) + self.assertEqual(commits[0].short_hash, 'aaa111a') + self.assertEqual(commits[1].short_hash, 'bbb222b') + dbs.close() + + +class TestApply(unittest.TestCase): + """Tests for apply command.""" + + def setUp(self): + """Set up test fixtures.""" + fd, self.db_path = tempfile.mkstemp(suffix='.db') + os.close(fd) + os.unlink(self.db_path) + self.old_db_fname = control.DB_FNAME + control.DB_FNAME = self.db_path + database.Database.instances.clear() + + def tearDown(self): + """Clean up test fixtures.""" + control.DB_FNAME = self.old_db_fname + if os.path.exists(self.db_path): + os.unlink(self.db_path) + database.Database.instances.clear() + command.TEST_RESULT = None + + def test_apply_source_not_found(self): + """Test apply with unknown source""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + dbs.close() + + database.Database.instances.clear() + + args = argparse.Namespace(cmd='apply', source='unknown') + with terminal.capture() as (_, stderr): + ret = control.do_pickman(args) + self.assertEqual(ret, 1) + self.assertIn("Source 'unknown' not found", stderr.getvalue()) + + def test_apply_no_commits(self): + """Test apply with no new commits""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + dbs.source_set('us/next', 'abc123') + dbs.commit() + dbs.close() + + database.Database.instances.clear() + command.TEST_RESULT = command.CommandResult(stdout='') + + args = argparse.Namespace(cmd='apply', source='us/next') + with terminal.capture() as (stdout, _): + ret = control.do_pickman(args) + self.assertEqual(ret, 0) + self.assertIn('No new commits to cherry-pick', stdout.getvalue()) + + if __name__ == '__main__': unittest.main() -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> Add -p/--push option to the apply command to push the cherry-pick branch to GitLab and create a merge request. Uses the python-gitlab library. Options: -p, --push Push branch and create GitLab MR -r, --remote Git remote for push (default: ci) -t, --target Target branch for MR (default: master) Requires GITLAB_TOKEN environment variable to be set. Also record cherry-pick history in .pickman-history file on successful apply. Each entry includes the date, source branch, commits, and the agent's conversation log. This file is committed automatically and included in the MR description when using -p. Name the module gitlab_api.py to avoid shadowing the python-gitlab library. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- .gitignore | 1 + tools/pickman/README.rst | 32 ++++++++++- tools/pickman/__main__.py | 6 +++ tools/pickman/control.py | 111 ++++++++++++++++++++++++++++++++++++-- tools/pickman/ftest.py | 24 +++++++++ 5 files changed, 169 insertions(+), 5 deletions(-) diff --git a/.gitignore b/.gitignore index 986ab7ffda3..2084bb16aeb 100644 --- a/.gitignore +++ b/.gitignore @@ -8,6 +8,7 @@ .* !.claude !.checkpatch.conf +!.pickman-history *.a *.asn1.[ch] *.bin diff --git a/tools/pickman/README.rst b/tools/pickman/README.rst index 0ad634516ce..ab37763a918 100644 --- a/tools/pickman/README.rst +++ b/tools/pickman/README.rst @@ -52,7 +52,26 @@ will: - Cherry-pick each commit in order - Handle simple conflicts automatically - Report status after completion -- Update the database with the last successfully applied commit + +To push the branch and create a GitLab merge request:: + + ./tools/pickman/pickman apply us/next -p + +Options for the apply command: + +- ``-b, --branch``: Branch name to create (default: cherry-<hash>) +- ``-p, --push``: Push branch and create GitLab MR +- ``-r, --remote``: Git remote for push (default: ci) +- ``-t, --target``: Target branch for MR (default: master) + +On successful cherry-pick, an entry is appended to ``.pickman-history`` with: + +- Date and source branch +- Branch name and list of commits +- The agent's conversation log + +This file is committed automatically and included in the MR description when +using ``-p``. Requirements ------------ @@ -64,6 +83,17 @@ To use the ``apply`` command, install the Claude Agent SDK:: You will also need an Anthropic API key set in the ``ANTHROPIC_API_KEY`` environment variable. +To use the ``-p`` (push) option for GitLab integration, install python-gitlab:: + + pip install python-gitlab + +You will also need a GitLab API token set in the ``GITLAB_TOKEN`` environment +variable. See `GitLab Personal Access Tokens`_ for instructions on creating one. +The token needs ``api`` scope. + +.. _GitLab Personal Access Tokens: + https://docs.gitlab.com/ee/user/profile/personal_access_tokens.html + Database -------- diff --git a/tools/pickman/__main__.py b/tools/pickman/__main__.py index 0ac7bfacf70..ac029a38382 100755 --- a/tools/pickman/__main__.py +++ b/tools/pickman/__main__.py @@ -38,6 +38,12 @@ def parse_args(argv): help='Apply next commits using Claude') apply_cmd.add_argument('source', help='Source branch name') apply_cmd.add_argument('-b', '--branch', help='Branch name to create') + apply_cmd.add_argument('-p', '--push', action='store_true', + help='Push branch and create GitLab MR') + apply_cmd.add_argument('-r', '--remote', default='ci', + help='Git remote for push (default: ci)') + apply_cmd.add_argument('-t', '--target', default='master', + help='Target branch for MR (default: master)') subparsers.add_parser('compare', help='Compare branches') subparsers.add_parser('list-sources', help='List tracked source branches') diff --git a/tools/pickman/control.py b/tools/pickman/control.py index 6974bbeeb7c..a482de85b00 100644 --- a/tools/pickman/control.py +++ b/tools/pickman/control.py @@ -18,6 +18,7 @@ sys.path.insert(0, os.path.join(our_path, '..')) from pickman import agent from pickman import database from pickman import ftest +from pickman import gitlab_api from u_boot_pylib import command from u_boot_pylib import tout @@ -227,7 +228,80 @@ def do_next_set(args, dbs): return 0 -def do_apply(args, dbs): +HISTORY_FILE = '.pickman-history' + + +def format_history_summary(source, commits, branch_name): + """Format a summary of the cherry-pick operation + + Args: + source (str): Source branch name + commits (list): list of CommitInfo tuples + branch_name (str): Name of the cherry-pick branch + + Returns: + str: Formatted summary text + """ + from datetime import date + + commit_list = '\n'.join( + f'- {c.short_hash} {c.subject}' + for c in commits + ) + + return f"""## {date.today()}: {source} + +Branch: {branch_name} + +Commits: +{commit_list}""" + + +def write_history(source, commits, branch_name, conversation_log): + """Write an entry to the pickman history file + + Args: + source (str): Source branch name + commits (list): list of CommitInfo tuples + branch_name (str): Name of the cherry-pick branch + conversation_log (str): The agent's conversation output + """ + import os + import re + + summary = format_history_summary(source, commits, branch_name) + entry = f"""{summary} + +### Conversation log +{conversation_log} + +--- + +""" + + # Read existing content and remove any entry for this branch + existing = '' + if os.path.exists(HISTORY_FILE): + with open(HISTORY_FILE, 'r', encoding='utf-8') as fhandle: + existing = fhandle.read() + # Remove existing entry for this branch (from ## header to ---) + pattern = rf'## [^\n]+\n\nBranch: {re.escape(branch_name)}\n.*?---\n\n' + existing = re.sub(pattern, '', existing, flags=re.DOTALL) + + # Write updated history file + with open(HISTORY_FILE, 'w', encoding='utf-8') as fhandle: + fhandle.write(existing + entry) + + # Commit the history file (use -f in case .gitignore patterns match) + run_git(['add', '-f', HISTORY_FILE]) + msg = f'pickman: Record cherry-pick of {len(commits)} commits from {source}\n\n' + msg += '\n'.join(f'- {c.short_hash} {c.subject}' for c in commits) + run_git(['commit', '-m', msg]) + + tout.info(f'Updated {HISTORY_FILE}') + + +def do_apply(args, dbs): # pylint: disable=too-many-locals """Apply the next set of commits using Claude agent Args: @@ -257,6 +331,14 @@ def do_apply(args, dbs): # Use first commit's short hash as part of branch name branch_name = f'cherry-{commits[0].short_hash}' + # Delete branch if it already exists + try: + run_git(['rev-parse', '--verify', branch_name]) + tout.info(f'Deleting existing branch {branch_name}') + run_git(['branch', '-D', branch_name]) + except Exception: # pylint: disable=broad-except + pass # Branch doesn't exist, which is fine + if merge_found: tout.info(f'Applying next set from {source} ({len(commits)} commits):') else: @@ -277,7 +359,8 @@ def do_apply(args, dbs): # Convert CommitInfo to tuple format expected by agent commit_tuples = [(c.hash, c.short_hash, c.subject) for c in commits] - success = agent.cherry_pick_commits(commit_tuples, source, branch_name) + success, conversation_log = agent.cherry_pick_commits(commit_tuples, source, + branch_name) # Update commit status based on result status = 'applied' if success else 'conflict' @@ -285,6 +368,10 @@ def do_apply(args, dbs): dbs.commit_set_status(commit.hash, status) dbs.commit() + # Write history file if successful + if success: + write_history(source, commits, branch_name, conversation_log) + # Return to original branch current_branch = run_git(['rev-parse', '--abbrev-ref', 'HEAD']) if current_branch != original_branch: @@ -292,8 +379,24 @@ def do_apply(args, dbs): run_git(['checkout', original_branch]) if success: - tout.info(f"Use 'pickman commit-source {source} {commits[-1].short_hash}' " - 'to update the database') + # Push and create MR if requested + if args.push: + remote = args.remote + target = args.target + # Use merge commit subject as title (last commit is the merge) + title = f'[pickman] {commits[-1].subject}' + # Description matches .pickman-history entry (summary + conversation) + summary = format_history_summary(source, commits, branch_name) + description = f'{summary}\n\n### Conversation log\n{conversation_log}' + + mr_url = gitlab_api.push_and_create_mr( + remote, branch_name, target, title, description + ) + if not mr_url: + return 1 + else: + tout.info(f"Use 'pickman commit-source {source} " + f"{commits[-1].short_hash}' to update the database") return 0 if success else 1 diff --git a/tools/pickman/ftest.py b/tools/pickman/ftest.py index 74bf305ab96..4f5c90980c6 100644 --- a/tools/pickman/ftest.py +++ b/tools/pickman/ftest.py @@ -1016,5 +1016,29 @@ class TestCheckAvailable(unittest.TestCase): self.assertTrue(result) +class TestParseApplyWithPush(unittest.TestCase): + """Tests for apply command with push options.""" + + def test_parse_apply_with_push(self): + """Test parsing apply command with push option.""" + args = pickman.parse_args(['apply', 'us/next', '-p']) + self.assertEqual(args.cmd, 'apply') + self.assertEqual(args.source, 'us/next') + self.assertTrue(args.push) + self.assertEqual(args.remote, 'ci') + self.assertEqual(args.target, 'master') + + def test_parse_apply_with_push_options(self): + """Test parsing apply command with all push options.""" + args = pickman.parse_args([ + 'apply', 'us/next', '-p', + '-r', 'origin', '-t', 'main' + ]) + self.assertEqual(args.cmd, 'apply') + self.assertTrue(args.push) + self.assertEqual(args.remote, 'origin') + self.assertEqual(args.target, 'main') + + if __name__ == '__main__': unittest.main() -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> Add do_review() command that lists open pickman MRs and checks each for human comments. Uses a Claude agent to address actionable comments. Also add supporting infrastructure: - gitlab_api: get_open_pickman_mrs(), get_mr_comments(), reply_to_mr() - agent: run_review_agent(), handle_mr_comments() - History file support for recording cherry-pick conversations - Return conversation_log from cherry_pick agent for MR descriptions Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/pickman/README.rst | 12 ++++ tools/pickman/__main__.py | 5 ++ tools/pickman/agent.py | 87 ++++++++++++++++++++++++ tools/pickman/control.py | 77 +++++++++++++++++++++ tools/pickman/ftest.py | 38 +++++++++++ tools/pickman/gitlab_api.py | 130 ++++++++++++++++++++++++++++++++++++ 6 files changed, 349 insertions(+) diff --git a/tools/pickman/README.rst b/tools/pickman/README.rst index d15b15f3331..f17ab734580 100644 --- a/tools/pickman/README.rst +++ b/tools/pickman/README.rst @@ -80,6 +80,18 @@ After successfully applying commits, update the database to record progress:: This updates the last cherry-picked commit for the source branch, so subsequent ``next-set`` and ``apply`` commands will start from the new position. +To check open MRs for comments and address them:: + + ./tools/pickman/pickman review + +This lists open pickman MRs (those with ``[pickman]`` in the title), checks each +for unresolved comments, and uses a Claude agent to address them. The agent will +make code changes based on the feedback and push an updated branch. + +Options for the review command: + +- ``-r, --remote``: Git remote (default: ci) + Requirements ------------ diff --git a/tools/pickman/__main__.py b/tools/pickman/__main__.py index 1693af991ac..17f7d72a619 100755 --- a/tools/pickman/__main__.py +++ b/tools/pickman/__main__.py @@ -57,6 +57,11 @@ def parse_args(argv): help='Show next set of commits to cherry-pick') next_set.add_argument('source', help='Source branch name') + review_cmd = subparsers.add_parser('review', + help='Check open MRs and handle comments') + review_cmd.add_argument('-r', '--remote', default='ci', + help='Git remote (default: ci)') + subparsers.add_parser('test', help='Run tests') return parser.parse_args(argv) diff --git a/tools/pickman/agent.py b/tools/pickman/agent.py index 3b4b96838b7..76cf8f98ec3 100644 --- a/tools/pickman/agent.py +++ b/tools/pickman/agent.py @@ -132,3 +132,90 @@ def cherry_pick_commits(commits, source, branch_name, repo_path=None): """ return asyncio.run(run(commits, source, branch_name, repo_path)) + + +async def run_review_agent(mr_iid, branch_name, comments, remote, repo_path=None): + """Run the Claude agent to handle MR comments + + Args: + mr_iid (int): Merge request IID + branch_name (str): Source branch name + comments (list): List of comment dicts with 'author', 'body' keys + remote (str): Git remote name + repo_path (str): Path to repository (defaults to current directory) + + Returns: + bool: True on success, False on failure + """ + if not check_available(): + return False + + if repo_path is None: + repo_path = os.getcwd() + + # Format comments for the prompt + comment_text = '\n'.join( + f"- [{c['author']}]: {c['body']}" + for c in comments + ) + + prompt = f"""Review comments on merge request !{mr_iid} (branch: {branch_name}): + +{comment_text} + +Steps to follow: +1. Checkout the branch: git checkout {branch_name} +2. Read and understand each comment +3. For each actionable comment: + - Make the requested changes to the code + - Amend the relevant commit or create a fixup commit +4. Run 'buildman -L --board sandbox -w -o /tmp/pickman' to verify the build +5. Create a new branch with suffix '-v2' (or increment existing version) +6. Push the new branch: git push {remote} <new-branch-name> +7. Report what changes were made and what reply should be posted to the MR + +Important: +- Keep changes minimal and focused on addressing the comments +- If a comment is unclear or cannot be addressed, note this in your report +- Do not force push to the original branch +- The new branch name should be: {branch_name}-v2 (or -v3, -v4 etc if needed) +""" + + options = ClaudeAgentOptions( + allowed_tools=['Bash', 'Read', 'Grep', 'Edit', 'Write'], + cwd=repo_path, + ) + + tout.info(f'Starting Claude agent to handle {len(comments)} comment(s)...') + tout.info('') + + conversation_log = [] + try: + async for message in query(prompt=prompt, options=options): + if hasattr(message, 'content'): + for block in message.content: + if hasattr(block, 'text'): + print(block.text) + conversation_log.append(block.text) + return True, '\n\n'.join(conversation_log) + except (RuntimeError, ValueError, OSError) as exc: + tout.error(f'Agent failed: {exc}') + return False, '\n\n'.join(conversation_log) + + +def handle_mr_comments(mr_iid, branch_name, comments, remote, repo_path=None): + """Synchronous wrapper for running the review agent + + Args: + mr_iid (int): Merge request IID + branch_name (str): Source branch name + comments (list): List of comment dicts + remote (str): Git remote name + repo_path (str): Path to repository (defaults to current directory) + + Returns: + tuple: (success, conversation_log) where success is bool and + conversation_log is the agent's output text + """ + return asyncio.run(run_review_agent(mr_iid, branch_name, comments, remote, + repo_path)) diff --git a/tools/pickman/control.py b/tools/pickman/control.py index fe840ee2740..1216b27bc9b 100644 --- a/tools/pickman/control.py +++ b/tools/pickman/control.py @@ -436,6 +436,82 @@ def do_commit_source(args, dbs): return 0 +def process_mr_reviews(remote, mrs): + """Process review comments on open MRs + + Checks each MR for unresolved comments and uses Claude agent to address + them. + + Args: + remote (str): Remote name + mrs (list): List of MR dicts from get_open_pickman_mrs() + + Returns: + int: Number of MRs with comments processed + """ + processed = 0 + + for merge_req in mrs: + comments = gitlab_api.get_mr_comments(remote, merge_req['iid']) + if comments is None: + continue + + # Filter to unresolved comments + unresolved = [c for c in comments if not c.get('resolved', True)] + if not unresolved: + continue + + tout.info('') + tout.info(f"MR !{merge_req['iid']} has {len(unresolved)} comment(s):") + for comment in unresolved: + tout.info(f" [{comment['author']}]: {comment['body'][:80]}...") + + # Run agent to handle comments + success, _ = agent.handle_mr_comments( + merge_req['iid'], + merge_req['source_branch'], + unresolved, + remote, + ) + if not success: + tout.error(f"Failed to handle comments for MR !{merge_req['iid']}") + processed += 1 + + return processed + + +def do_review(args, dbs): # pylint: disable=unused-argument + """Check open pickman MRs and handle comments + + Lists open MRs created by pickman, checks for human comments, and uses + Claude agent to address them. + + Args: + args (Namespace): Parsed arguments with 'remote' attribute + dbs (Database): Database instance + + Returns: + int: 0 on success, 1 on failure + """ + remote = args.remote + + # Get open pickman MRs + mrs = gitlab_api.get_open_pickman_mrs(remote) + if mrs is None: + return 1 + + if not mrs: + tout.info('No open pickman MRs found') + return 0 + + tout.info(f'Found {len(mrs)} open pickman MR(s):') + for merge_req in mrs: + tout.info(f" !{merge_req['iid']}: {merge_req['title']}") + + process_mr_reviews(remote, mrs) + + return 0 + def do_test(args, dbs): # pylint: disable=unused-argument """Run tests for this module. @@ -463,6 +539,7 @@ COMMANDS = { 'compare': do_compare, 'list-sources': do_list_sources, 'next-set': do_next_set, + 'review': do_review, 'test': do_test, } diff --git a/tools/pickman/ftest.py b/tools/pickman/ftest.py index 9b445173b3b..0e722bff48c 100644 --- a/tools/pickman/ftest.py +++ b/tools/pickman/ftest.py @@ -1105,5 +1105,43 @@ class TestParseApplyWithPush(unittest.TestCase): self.assertEqual(args.target, 'main') +class TestParseReview(unittest.TestCase): + """Tests for review command argument parsing.""" + + def test_parse_review_defaults(self): + """Test parsing review command with defaults.""" + args = pickman.parse_args(['review']) + self.assertEqual(args.cmd, 'review') + self.assertEqual(args.remote, 'ci') + + def test_parse_review_with_remote(self): + """Test parsing review command with custom remote.""" + args = pickman.parse_args(['review', '-r', 'origin']) + self.assertEqual(args.cmd, 'review') + self.assertEqual(args.remote, 'origin') + + +class TestReview(unittest.TestCase): + """Tests for review command.""" + + def test_review_no_mrs(self): + """Test review when no open MRs found.""" + with mock.patch.object(gitlab_api, 'get_open_pickman_mrs', + return_value=[]): + args = argparse.Namespace(cmd='review', remote='ci') + ret = control.do_review(args, None) + + self.assertEqual(ret, 0) + + def test_review_gitlab_error(self): + """Test review when GitLab API returns error.""" + with mock.patch.object(gitlab_api, 'get_open_pickman_mrs', + return_value=None): + args = argparse.Namespace(cmd='review', remote='ci') + ret = control.do_review(args, None) + + self.assertEqual(ret, 1) + + if __name__ == '__main__': unittest.main() diff --git a/tools/pickman/gitlab_api.py b/tools/pickman/gitlab_api.py index 6cd9c26f3de..ca239c41271 100644 --- a/tools/pickman/gitlab_api.py +++ b/tools/pickman/gitlab_api.py @@ -152,6 +152,136 @@ def create_mr(host, proj_path, source, target, title, desc=''): return None +def get_open_pickman_mrs(remote): + """Get open merge requests created by pickman + + Args: + remote (str): Remote name + + Returns: + list: List of dicts with 'iid', 'title', 'web_url', 'source_branch' keys, + or None on failure + """ + if not check_available(): + return None + + token = get_token() + if not token: + tout.error('GITLAB_TOKEN environment variable not set') + return None + + remote_url = get_remote_url(remote) + host, proj_path = parse_url(remote_url) + + if not host or not proj_path: + tout.error(f"Could not parse GitLab URL from remote '{remote}'") + return None + + try: + glab = gitlab.Gitlab(f'https://{host}', private_token=token) + project = glab.projects.get(proj_path) + + mrs = project.mergerequests.list(state='opened', get_all=True) + pickman_mrs = [] + for merge_req in mrs: + if '[pickman]' in merge_req.title: + pickman_mrs.append({ + 'iid': merge_req.iid, + 'title': merge_req.title, + 'web_url': merge_req.web_url, + 'source_branch': merge_req.source_branch, + }) + return pickman_mrs + except gitlab.exceptions.GitlabError as exc: + tout.error(f'GitLab API error: {exc}') + return None + + +def get_mr_comments(remote, mr_iid): + """Get human comments on a merge request (excluding bot/system notes) + + Args: + remote (str): Remote name + mr_iid (int): Merge request IID + + Returns: + list: List of dicts with 'id', 'author', 'body', 'created_at', + 'resolvable', 'resolved' keys, or None on failure + """ + if not check_available(): + return None + + token = get_token() + if not token: + tout.error('GITLAB_TOKEN environment variable not set') + return None + + remote_url = get_remote_url(remote) + host, proj_path = parse_url(remote_url) + + if not host or not proj_path: + return None + + try: + glab = gitlab.Gitlab(f'https://{host}', private_token=token) + project = glab.projects.get(proj_path) + merge_req = project.mergerequests.get(mr_iid) + + comments = [] + for note in merge_req.notes.list(get_all=True): + # Skip system notes (merge status, etc.) + if note.system: + continue + comments.append({ + 'id': note.id, + 'author': note.author['username'], + 'body': note.body, + 'created_at': note.created_at, + 'resolvable': getattr(note, 'resolvable', False), + 'resolved': getattr(note, 'resolved', False), + }) + return comments + except gitlab.exceptions.GitlabError as exc: + tout.error(f'GitLab API error: {exc}') + return None + + +def reply_to_mr(remote, mr_iid, message): + """Post a reply to a merge request + + Args: + remote (str): Remote name + mr_iid (int): Merge request IID + message (str): Reply message + + Returns: + bool: True on success + """ + if not check_available(): + return False + + token = get_token() + if not token: + tout.error('GITLAB_TOKEN environment variable not set') + return False + + remote_url = get_remote_url(remote) + host, proj_path = parse_url(remote_url) + + if not host or not proj_path: + return False + + try: + glab = gitlab.Gitlab(f'https://{host}', private_token=token) + project = glab.projects.get(proj_path) + merge_req = project.mergerequests.get(mr_iid) + merge_req.notes.create({'body': message}) + return True + except gitlab.exceptions.GitlabError as exc: + tout.error(f'GitLab API error: {exc}') + return False + + def push_and_create_mr(remote, branch, target, title, desc=''): """Push a branch and create a merge request -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> Add a workflow section at the top of the README explaining the typical usage pattern for pickman: setup with add-source, cherry-pick with apply -p, review/merge, update with commit-source, and repeat. Also mention the step and review commands for automated workflows. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/pickman/README.rst | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tools/pickman/README.rst b/tools/pickman/README.rst index 0d2c55e762a..7a33378a32b 100644 --- a/tools/pickman/README.rst +++ b/tools/pickman/README.rst @@ -8,6 +8,27 @@ Pickman - Cherry-pick Manager Pickman is a tool to help manage cherry-picking commits between branches. +Workflow +-------- + +The typical workflow for using pickman is: + +1. **Setup**: Add a source branch to track with ``add-source``. This records the + starting point (merge-base) for cherry-picking. + +2. **Cherry-pick**: Run ``apply -p`` to cherry-pick the next set of commits (up + to the next merge commit) and create a GitLab MR. A Claude agent handles the + cherry-picks automatically, resolving simple conflicts. + +3. **Review**: Once the MR is reviewed and merged, run ``commit-source`` to + update the database with the last processed commit. + +4. **Repeat**: Go back to step 2 until all commits are cherry-picked. + +For automated workflows, use ``step`` instead of ``apply -p``. It checks for +open pickman MRs first and only creates a new one if none are pending. Use +``review`` to have the agent address MR comments. + Usage ----- -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> Have the step command also process review comments when there are open MRs. This integrates the review functionality into step, so a single poll loop can handle the full workflow: process merged MRs, handle review comments, and create new MRs when ready. Refactor process_mr_reviews() as a helper function used by both do_review() and do_step(). Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/pickman/README.rst | 18 +++++++++++++----- tools/pickman/control.py | 8 ++++++-- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/tools/pickman/README.rst b/tools/pickman/README.rst index 07b9408afa0..b469470138b 100644 --- a/tools/pickman/README.rst +++ b/tools/pickman/README.rst @@ -25,9 +25,15 @@ The typical workflow for using pickman is: 4. **Repeat**: Go back to step 2 until all commits are cherry-picked. -For automated workflows, use ``step`` instead of ``apply -p``. It checks for -open pickman MRs first and only creates a new one if none are pending. Use -``review`` to have the agent address MR comments. +For fully automated workflows, use ``poll`` which runs ``step`` in a loop. The +``step`` command handles the complete cycle automatically: + +- Detects merged MRs and updates the database (no manual ``commit-source``) +- Processes review comments on open MRs using Claude agent +- Creates new MRs when none are pending + +This allows hands-off operation: just run ``poll`` and approve/merge MRs in +GitLab as they come in. Usage ----- @@ -122,11 +128,13 @@ This command performs the following: 1. Checks for merged pickman MRs and updates the database with the last cherry-picked commit from each merged MR 2. Checks for open pickman MRs (those with ``[pickman]`` in the title) -3. If no open MRs exist, runs ``apply`` with ``--push`` to create a new one +3. If open MRs exist, processes any review comments using Claude agent +4. If no open MRs exist, runs ``apply`` with ``--push`` to create a new one This is useful for automated workflows where only one MR should be active at a time. The automatic database update on merge means you don't need to manually -run ``commit-source`` after each MR is merged. +run ``commit-source`` after each MR is merged, and review comments are handled +automatically. Options for the step command: diff --git a/tools/pickman/control.py b/tools/pickman/control.py index aaed06c1e35..86ecc5574e7 100644 --- a/tools/pickman/control.py +++ b/tools/pickman/control.py @@ -600,8 +600,8 @@ def do_step(args, dbs): """Create an MR if none is pending Checks for merged pickman MRs and updates the database, then checks for - open pickman MRs and if none exist, runs apply with push to create a new - one. + open pickman MRs. If open MRs exist, processes any review comments. If no + open MRs exist, runs apply with push to create a new one. Args: args (Namespace): Parsed arguments with 'source', 'remote', 'target' @@ -627,6 +627,10 @@ def do_step(args, dbs): tout.info(f'Found {len(mrs)} open pickman MR(s):') for merge_req in mrs: tout.info(f" !{merge_req['iid']}: {merge_req['title']}") + + # Process any review comments on open MRs + process_mr_reviews(remote, mrs) + tout.info('') tout.info('Not creating new MR while others are pending') return 0 -- 2.43.0