[Concept,04/17] pickman: Add add-source command to track source branches

Message ID 20251217022611.389379-5-sjg@u-boot.org
State New
Headers
Series pickman: Add a manager for cherry-picks |

Commit Message

Simon Glass Dec. 17, 2025, 2:25 a.m. UTC
  From: Simon Glass <simon.glass@canonical.com>

Add a command to register a source branch in the database. This finds
the merge-base commit between master and the source branch and stores
it as the starting point for cherry-picking.

Usage: ./tools/pickman/pickman add-source <branch>

Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com>
Signed-off-by: Simon Glass <simon.glass@canonical.com>
---

 tools/pickman/README.rst  | 27 +++++++++++++---
 tools/pickman/__main__.py |  4 +++
 tools/pickman/control.py  | 63 ++++++++++++++++++++++++++++++++-----
 tools/pickman/ftest.py    | 66 ++++++++++++++++++++++++++++++++++++++-
 4 files changed, 148 insertions(+), 12 deletions(-)
  

Patch

diff --git a/tools/pickman/README.rst b/tools/pickman/README.rst
index 299f2cac699..aab0642d374 100644
--- a/tools/pickman/README.rst
+++ b/tools/pickman/README.rst
@@ -11,9 +11,17 @@  Pickman is a tool to help manage cherry-picking commits between branches.
 Usage
 -----
 
+To add a source branch to track::
+
+    ./tools/pickman/pickman add-source us/next
+
+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 compare branches and show commits that need to be cherry-picked::
 
-    ./tools/pickman/pickman
+    ./tools/pickman/pickman compare
 
 This shows:
 
@@ -21,10 +29,18 @@  This shows:
   master branch (ci/master)
 - The last common commit between the two branches
 
+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
+
 Configuration
 -------------
 
-The branches to compare are configured as constants at the top of the script:
+The branches to compare are configured as constants in control.py:
 
 - ``BRANCH_MASTER``: The main branch to compare against (default: ci/master)
 - ``BRANCH_SOURCE``: The source branch with commits to cherry-pick
@@ -35,5 +51,8 @@  Testing
 
 To run the functional tests::
 
-    cd tools/pickman
-    python3 -m pytest ftest.py -v
+    ./tools/pickman/pickman test
+
+Or using pytest::
+
+    python3 -m pytest tools/pickman/ftest.py -v
diff --git a/tools/pickman/__main__.py b/tools/pickman/__main__.py
index 0984c62d3e6..7263f4c5fb0 100755
--- a/tools/pickman/__main__.py
+++ b/tools/pickman/__main__.py
@@ -30,6 +30,10 @@  def parse_args(argv):
     parser = argparse.ArgumentParser(description='Check commit differences')
     subparsers = parser.add_subparsers(dest='cmd', required=True)
 
+    add_source = subparsers.add_parser('add-source',
+                                        help='Add a source branch to track')
+    add_source.add_argument('source', help='Source branch name')
+
     subparsers.add_parser('compare', help='Compare branches')
     subparsers.add_parser('test', help='Run tests')
 
diff --git a/tools/pickman/control.py b/tools/pickman/control.py
index 0ed54dd724c..fa53a26b6ad 100644
--- a/tools/pickman/control.py
+++ b/tools/pickman/control.py
@@ -15,10 +15,14 @@  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 database
 from pickman import ftest
 from u_boot_pylib import command
 from u_boot_pylib import tout
 
+# Default database filename
+DB_FNAME = '.pickman.db'
+
 # Branch names to compare
 BRANCH_MASTER = 'ci/master'
 BRANCH_SOURCE = 'us/next'
@@ -56,11 +60,44 @@  def compare_branches(master, source):
     return count, Commit(full_hash, short_hash, subject, date)
 
 
-def do_compare(args):  # pylint: disable=unused-argument
+def do_add_source(args, dbs):
+    """Add a source branch to the database
+
+    Finds the merge-base commit between master and source and stores it.
+
+    Args:
+        args (Namespace): Parsed arguments with 'source' attribute
+        dbs (Database): Database instance
+
+    Returns:
+        int: 0 on success
+    """
+    source = args.source
+
+    # Find the merge base commit
+    base_hash = run_git(['merge-base', BRANCH_MASTER, source])
+
+    # Get commit details for display
+    info = run_git(['log', '-1', '--format=%h%n%s', base_hash])
+    short_hash, subject = info.split('\n')
+
+    # Store in database
+    dbs.source_set(source, base_hash)
+    dbs.commit()
+
+    tout.info(f"Added source '{source}' with base commit:")
+    tout.info(f'  Hash:    {short_hash}')
+    tout.info(f'  Subject: {subject}')
+
+    return 0
+
+
+def do_compare(args, dbs):  # pylint: disable=unused-argument
     """Compare branches and print results.
 
     Args:
         args (Namespace): Parsed arguments
+        dbs (Database): Database instance
     """
     count, base = compare_branches(BRANCH_MASTER, BRANCH_SOURCE)
 
@@ -74,11 +111,12 @@  def do_compare(args):  # pylint: disable=unused-argument
     return 0
 
 
-def do_test(args):  # pylint: disable=unused-argument
+def do_test(args, dbs):  # pylint: disable=unused-argument
     """Run tests for this module.
 
     Args:
         args (Namespace): Parsed arguments
+        dbs (Database): Database instance
 
     Returns:
         int: 0 if tests passed, 1 otherwise
@@ -91,6 +129,14 @@  def do_test(args):  # pylint: disable=unused-argument
     return 0 if result.wasSuccessful() else 1
 
 
+# Command dispatch table
+COMMANDS = {
+    'add-source': do_add_source,
+    'compare': do_compare,
+    'test': do_test,
+}
+
+
 def do_pickman(args):
     """Main entry point for pickman commands.
 
@@ -102,9 +148,12 @@  def do_pickman(args):
     """
     tout.init(tout.INFO)
 
-    if args.cmd == 'compare':
-        return do_compare(args)
-    if args.cmd == 'test':
-        return do_test(args)
-
+    handler = COMMANDS.get(args.cmd)
+    if handler:
+        dbs = database.Database(DB_FNAME)
+        dbs.start()
+        try:
+            return handler(args, dbs)
+        finally:
+            dbs.close()
     return 1
diff --git a/tools/pickman/ftest.py b/tools/pickman/ftest.py
index b975b9c6a2b..632cd56793f 100644
--- a/tools/pickman/ftest.py
+++ b/tools/pickman/ftest.py
@@ -5,6 +5,7 @@ 
 #
 """Tests for pickman."""
 
+import argparse
 import os
 import sys
 import tempfile
@@ -104,6 +105,12 @@  class TestCompareBranches(unittest.TestCase):
 class TestParseArgs(unittest.TestCase):
     """Tests for parse_args function."""
 
+    def test_parse_add_source(self):
+        """Test parsing add-source command."""
+        args = pickman.parse_args(['add-source', 'us/next'])
+        self.assertEqual(args.cmd, 'add-source')
+        self.assertEqual(args.source, 'us/next')
+
     def test_parse_compare(self):
         """Test parsing compare command."""
         args = pickman.parse_args(['compare'])
@@ -124,6 +131,48 @@  class TestParseArgs(unittest.TestCase):
 class TestMain(unittest.TestCase):
     """Tests for main function."""
 
+    def test_add_source(self):
+        """Test add-source command"""
+        results = iter([
+            'abc123def456',  # merge-base
+            'abc123d\nTest subject',  # log
+        ])
+
+        def handle_command(**_):
+            return command.CommandResult(stdout=next(results))
+
+        # Use a temp database file
+        fd, db_path = tempfile.mkstemp(suffix='.db')
+        os.close(fd)
+        os.unlink(db_path)
+        old_db_fname = control.DB_FNAME
+        control.DB_FNAME = db_path
+        database.Database.instances.clear()
+
+        command.TEST_RESULT = handle_command
+        try:
+            args = argparse.Namespace(cmd='add-source', source='us/next')
+            with terminal.capture() as (stdout, _):
+                ret = control.do_pickman(args)
+            self.assertEqual(ret, 0)
+            output = stdout.getvalue()
+            self.assertIn("Added source 'us/next' with base commit:", output)
+            self.assertIn('Hash:    abc123d', output)
+            self.assertIn('Subject: Test subject', output)
+
+            # Verify database was updated
+            database.Database.instances.clear()
+            dbs = database.Database(db_path)
+            dbs.start()
+            self.assertEqual(dbs.source_get('us/next'), 'abc123def456')
+            dbs.close()
+        finally:
+            command.TEST_RESULT = None
+            control.DB_FNAME = old_db_fname
+            if os.path.exists(db_path):
+                os.unlink(db_path)
+            database.Database.instances.clear()
+
     def test_main_compare(self):
         """Test main with compare command."""
         results = iter([
@@ -135,12 +184,23 @@  class TestMain(unittest.TestCase):
         def handle_command(**_):
             return command.CommandResult(stdout=next(results))
 
+        # Use a temp database file
+        fd, db_path = tempfile.mkstemp(suffix='.db')
+        os.close(fd)
+        os.unlink(db_path)
+        old_db_fname = control.DB_FNAME
+        control.DB_FNAME = db_path
+        database.Database.instances.clear()
+
         command.TEST_RESULT = handle_command
         try:
             with terminal.capture() as (stdout, _):
                 ret = pickman.main(['compare'])
             self.assertEqual(ret, 0)
-            lines = iter(stdout.getvalue().splitlines())
+            # Filter out database migration messages
+            output_lines = [l for l in stdout.getvalue().splitlines()
+                            if not l.startswith(('Update database', 'Creating'))]
+            lines = iter(output_lines)
             self.assertEqual('Commits in us/next not in ci/master: 10',
                              next(lines))
             self.assertEqual('', next(lines))
@@ -152,6 +212,10 @@  class TestMain(unittest.TestCase):
             self.assertRaises(StopIteration, next, lines)
         finally:
             command.TEST_RESULT = None
+            control.DB_FNAME = old_db_fname
+            if os.path.exists(db_path):
+                os.unlink(db_path)
+            database.Database.instances.clear()
 
 
 class TestDatabase(unittest.TestCase):