[Concept,1/4] pickman: Fix ambiguous checkout in subtree update

Message ID 20260305145452.909661-2-sjg@u-boot.org
State New
Headers
Series pickman: Improve subtree-update and error handling |

Commit Message

Simon Glass March 5, 2026, 2:54 p.m. UTC
  From: Simon Glass <sjg@chromium.org>

When multiple remotes have a branch with the same name (e.g.
ci/master, me/master, us/master), a bare 'git checkout master' fails
because git cannot determine which remote to track.

Fall back to 'git checkout -b <target> <remote>/<target>' when the
bare checkout fails, creating a local branch from the specific remote.
This mirrors the existing pattern used for MR branch creation.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 tools/pickman/control.py | 13 +++++++----
 tools/pickman/ftest.py   | 50 ++++++++++++++++++++++++++++++++++++----
 2 files changed, 55 insertions(+), 8 deletions(-)
  

Patch

diff --git a/tools/pickman/control.py b/tools/pickman/control.py
index a42927f991d..815ac574093 100644
--- a/tools/pickman/control.py
+++ b/tools/pickman/control.py
@@ -1524,7 +1524,7 @@  def _subtree_run_update(name, tag):
         int: 0 on success, 1 on failure
     """
     try:
-        result = command.run(
+        result = command.run_one(
             './tools/update-subtree.sh', 'pull', name, tag,
             capture=True, raise_on_error=False)
         if result.stdout:
@@ -1595,9 +1595,14 @@  def apply_subtree_update(dbs, source, name, tag, merge_hash, args):  # pylint: d
 
     try:
         run_git(['checkout', target])
-    except Exception:  # pylint: disable=broad-except
-        tout.error(f'Could not checkout {target}')
-        return 1
+    except command.CommandExc:
+        # Bare name may be ambiguous when multiple remotes have it
+        try:
+            run_git(['checkout', '-b', target,
+                     f'{args.remote}/{target}'])
+        except command.CommandExc:
+            tout.error(f'Could not checkout {target}')
+            return 1
 
     ret = _subtree_run_update(name, tag)
     if ret:
diff --git a/tools/pickman/ftest.py b/tools/pickman/ftest.py
index 5a4d0c433dc..766cc714ed7 100644
--- a/tools/pickman/ftest.py
+++ b/tools/pickman/ftest.py
@@ -4022,7 +4022,7 @@  class TestApplySubtreeUpdate(unittest.TestCase):
             with mock.patch.object(control, 'run_git',
                                    side_effect=run_git_handler):
                 with mock.patch.object(
-                        control.command, 'run',
+                        control.command, 'run_one',
                         return_value=mock_result):
                     ret = control.apply_subtree_update(
                         dbs, 'us/next', 'dts', 'v6.15-dts',
@@ -4070,7 +4070,7 @@  class TestApplySubtreeUpdate(unittest.TestCase):
             with mock.patch.object(control, 'run_git',
                                    side_effect=run_git_handler):
                 with mock.patch.object(
-                        control.command, 'run',
+                        control.command, 'run_one',
                         return_value=mock_result):
                     with mock.patch.object(
                             control.gitlab_api, 'push_branch',
@@ -4112,6 +4112,48 @@  class TestApplySubtreeUpdate(unittest.TestCase):
             self.assertEqual(dbs.source_get('us/next'), 'base')
             dbs.close()
 
+    def test_apply_checkout_fallback(self):
+        """Test apply_subtree_update falls back to -b when checkout fails."""
+        with terminal.capture():
+            dbs = database.Database(self.db_path)
+            dbs.start()
+            dbs.source_set('us/next', 'base')
+            dbs.commit()
+
+            args = argparse.Namespace(push=False, remote='ci',
+                                      target='master')
+
+            checkout_calls = []
+
+            def run_git_handler(git_args):
+                if 'rev-parse' in git_args:
+                    return 'first_parent\nsquash_hash'
+                if 'checkout' in git_args:
+                    checkout_calls.append(list(git_args))
+                    if '-b' not in git_args:
+                        raise command.CommandExc(
+                            'ambiguous checkout', None)
+                    return ''
+                if 'log' in git_args:
+                    return 'subject|author'
+                return ''
+
+            with mock.patch.object(control, 'run_git',
+                                   side_effect=run_git_handler):
+                with mock.patch.object(control, '_subtree_run_update',
+                                       return_value=0):
+                    ret = control.apply_subtree_update(
+                        dbs, 'us/next', 'dts', 'v6.15-dts',
+                        'merge_hash', args)
+
+            self.assertEqual(ret, 0)
+            # Should have tried bare checkout, then fallback
+            self.assertEqual(len(checkout_calls), 2)
+            self.assertEqual(checkout_calls[0], ['checkout', 'master'])
+            self.assertEqual(checkout_calls[1],
+                             ['checkout', '-b', 'master', 'ci/master'])
+            dbs.close()
+
     def test_apply_no_second_parent(self):
         """Test apply_subtree_update returns 1 when merge has no 2nd parent."""
         with terminal.capture():
@@ -4154,7 +4196,7 @@  class TestApplySubtreeUpdate(unittest.TestCase):
             with mock.patch.object(control, 'run_git',
                                    side_effect=run_git_handler):
                 with mock.patch.object(
-                        control.command, 'run',
+                        control.command, 'run_one',
                         side_effect=Exception('script failed')):
                     ret = control.apply_subtree_update(
                         dbs, 'us/next', 'dts', 'v6.15-dts',
@@ -4193,7 +4235,7 @@  class TestApplySubtreeUpdate(unittest.TestCase):
             with mock.patch.object(control, 'run_git',
                                    side_effect=run_git_handler):
                 with mock.patch.object(
-                        control.command, 'run',
+                        control.command, 'run_one',
                         return_value=mock_result):
                     ret = control.apply_subtree_update(
                         dbs, 'us/next', 'dts', 'v6.15-dts',