diff --git a/tools/patman/cmdline.py b/tools/patman/cmdline.py index dbd0309090d..10723120c4e 100644 --- a/tools/patman/cmdline.py +++ b/tools/patman/cmdline.py @@ -589,6 +589,11 @@ def add_review_subparser(subparsers): review.add_argument( '-U', '--upstream', type=str, default=None, help='Upstream name (for patchwork URL lookup)') + review.add_argument( + '-b', '--base-branch', type=str, default=None, + help="Base branch to apply review patches onto (e.g. 'us/master', " + "'us/next'). If unset, picks the upstream's '/next' branch " + "when it has commits ahead of '/master', otherwise '/master'.") review.add_argument( '--apply-only', action='store_true', help='Only download and apply patches, skip AI review') diff --git a/tools/patman/patman.rst b/tools/patman/patman.rst index 718078cb882..aba6d8905fb 100644 --- a/tools/patman/patman.rst +++ b/tools/patman/patman.rst @@ -1297,6 +1297,18 @@ without calling the Gmail API. Use ``--apply-only`` to download and apply patches without running the AI review -- useful for checking that patches apply cleanly. +By default the review branch is created from ``/next`` when +that branch has commits ahead of ``/master``, and from +``/master`` otherwise. This tracks U-Boot's release cadence: +right after a release ``next`` is merged into ``master`` and stays +empty until the next cycle's RC1, so reviews land on ``master`` during +that window and switch back to ``next`` automatically once it +diverges. Use ``-b`` / ``--base-branch`` to override the choice for a +particular run:: + + patman review -s 497923 -U us -b us/master \ + --reviewer 'Your Name ' + Use ``-f`` / ``--force`` to re-review a series that has already been reviewed. This deletes the old review records and runs the review again:: diff --git a/tools/patman/review.py b/tools/patman/review.py index 054d18c6d80..524a06df3a0 100644 --- a/tools/patman/review.py +++ b/tools/patman/review.py @@ -1741,25 +1741,35 @@ def _draft_stored_reviews(args, reviews, series_data, pwork, cser): def _get_upstream_branch(args, cser): - """Determine the upstream branch for applying patches + """Determine the base branch for applying patches - Prefers 'next' over 'master' if it exists. + Honours --base-branch if the user supplied one. Otherwise prefers + '/next' when it has commits ahead of '/master', + falling back to '/master' when next is empty (e.g. just + after a release, when next has been merged into master and has not + yet been reopened for the next cycle). Args: args (Namespace): Command-line arguments cser (Cseries): Open cseries instance Returns: - str: Upstream branch ref, e.g. 'us/next' + str: Base branch ref, e.g. 'us/next' or 'us/master' """ + if args.base_branch: + return args.base_branch ups = args.upstream if not ups: ups = cser.db.upstream_get_default() if ups: - branch = f'{ups}/next' - if not gitutil.ref_exists(branch): - branch = f'{ups}/master' - return branch + next_branch = f'{ups}/next' + master_branch = f'{ups}/master' + if gitutil.ref_exists(next_branch): + ahead = gitutil.count_revs( + None, f'{master_branch}..{next_branch}') + if ahead: + return next_branch + return master_branch return 'origin/master' diff --git a/tools/patman/test_cseries.py b/tools/patman/test_cseries.py index cb5724631ad..c267b048fc5 100644 --- a/tools/patman/test_cseries.py +++ b/tools/patman/test_cseries.py @@ -21,6 +21,7 @@ from u_boot_pylib import tools from patman import cmdline from patman import control from patman import cser_helper +from patman import review from patman import cseries from patman.database import Pcommit from patman import database @@ -5223,3 +5224,71 @@ VERDICT: changes_needed""" self.assertEqual(2, len(notes)) self.assertEqual(1, notes[0][0]) self.assertEqual(3, notes[1][0]) + + +class TestGetUpstreamBranch(unittest.TestCase): + """Tests for review._get_upstream_branch() base-branch selection.""" + + def _cser(self, default_upstream=None): + cser = mock.Mock() + cser.db.upstream_get_default.return_value = default_upstream + return cser + + def test_explicit_base_branch_wins(self): + """An explicit -b/--base-branch overrides every other check.""" + args = Namespace(base_branch='custom/branch', upstream='us') + with mock.patch('patman.review.gitutil') as gu: + self.assertEqual( + 'custom/branch', + review._get_upstream_branch(args, self._cser())) + gu.ref_exists.assert_not_called() + gu.count_revs.assert_not_called() + + def test_next_ahead_of_master_picks_next(self): + """When next has commits ahead of master, next is chosen.""" + args = Namespace(base_branch=None, upstream='us') + with mock.patch('patman.review.gitutil.ref_exists', + return_value=True), \ + mock.patch('patman.review.gitutil.count_revs', + return_value=3): + self.assertEqual( + 'us/next', + review._get_upstream_branch(args, self._cser())) + + def test_next_empty_falls_back_to_master(self): + """When next exists but has no commits ahead, master is chosen.""" + args = Namespace(base_branch=None, upstream='us') + with mock.patch('patman.review.gitutil.ref_exists', + return_value=True), \ + mock.patch('patman.review.gitutil.count_revs', + return_value=0): + self.assertEqual( + 'us/master', + review._get_upstream_branch(args, self._cser())) + + def test_next_missing_falls_back_to_master(self): + """When next does not exist at all, master is chosen.""" + args = Namespace(base_branch=None, upstream='us') + with mock.patch('patman.review.gitutil.ref_exists', + return_value=False): + self.assertEqual( + 'us/master', + review._get_upstream_branch(args, self._cser())) + + def test_default_upstream_used_when_unset(self): + """When args.upstream is unset, the cser default is consulted.""" + args = Namespace(base_branch=None, upstream=None) + cser = self._cser(default_upstream='us') + with mock.patch('patman.review.gitutil.ref_exists', + return_value=True), \ + mock.patch('patman.review.gitutil.count_revs', + return_value=5): + self.assertEqual( + 'us/next', review._get_upstream_branch(args, cser)) + + def test_no_upstream_returns_origin_master(self): + """With no upstream configured anywhere, return 'origin/master'.""" + args = Namespace(base_branch=None, upstream=None) + self.assertEqual( + 'origin/master', + review._get_upstream_branch(args, self._cser()))