From patchwork Fri May 1 11:00:19 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 2270 Return-Path: X-Original-To: u-boot-concept@u-boot.org Delivered-To: u-boot-concept@u-boot.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1777633340; bh=jlWFDyo/omKqnUp7cvFMY3+PFbJBlt6dekpvWsmMLFE=; h=From:To:Date:In-Reply-To:References:CC:Subject:List-Id: List-Archive:List-Help:List-Owner:List-Post:List-Subscribe: List-Unsubscribe:From; b=MkCCfRm9mVyXyLlR7f0CdgArhisDQcMvX6enpin9N2agKde8SSuSyRHMzsk8VG0Zw h5elVSm9qLX+he/0DiB0gLCl36eAgQ/eczkWKF76n1/UW8nH7hcvwhrjQTPKS53+WS +oUPFkrxZqTKNDYAf48kMvwgR8lISOIvbYMV2FzQ= Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 0BA606A833 for ; Fri, 1 May 2026 05:02:20 -0600 (MDT) X-Virus-Scanned: Debian amavis at Received: from mail.u-boot.org ([127.0.0.1]) by localhost (mail.u-boot.org [127.0.0.1]) (amavis, port 10024) with ESMTP id eZTQQGNkO-L1 for ; Fri, 1 May 2026 05:02:19 -0600 (MDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1777633338; bh=jlWFDyo/omKqnUp7cvFMY3+PFbJBlt6dekpvWsmMLFE=; h=From:To:Date:In-Reply-To:References:CC:Subject:List-Id: List-Archive:List-Help:List-Owner:List-Post:List-Subscribe: List-Unsubscribe:From; b=q6UDYSX0K8+xI7gd+cZ7mRv54xbW73ntHrjFJYMyba1XAP7FUfoGjf88E5OqUSdIe uyIORrcFhm3sCnVirn7b98aRoJtL3qu8Aes+11wXKqKpB59dHRJt1qLvTiOfsqV6TV ZfGmOO5olBDah0K35u1zDtXZoOMyHvH4jfd/MTY0= Received: from mail.u-boot.org (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 82CCF6A837 for ; Fri, 1 May 2026 05:02:18 -0600 (MDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1777633335; bh=tIdVh/z4bDxLfzcw9AXqnnt7zfhoMdLu0TVP4EyjPXQ=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=VAuZejgQy0yeSOPUVxjZk5tPJd3bDlv3S9bYLj51La9SnRxjPUbrXW72C48ECRs22 yJJ9gZK447Ulqm3y+WpgovjNgKeePwlfpTMSYEgUjnta12Q63bL3SoUviM/h/p0JvQ rw9VJucpQ2TsX9uVdYYaE4VvCalOuz7QsShL//Hc= Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 56E496A7AF; Fri, 1 May 2026 05:02:15 -0600 (MDT) X-Virus-Scanned: Debian amavis at Received: from mail.u-boot.org ([127.0.0.1]) by localhost (mail.u-boot.org [127.0.0.1]) (amavis, port 10026) with ESMTP id Yw05uvzqYOqm; Fri, 1 May 2026 05:02:15 -0600 (MDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1777633332; bh=96fveYdtdD7/wWzD+bSUpc45+mYDD7xT1BbFaajPWWw=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=TW4nztz9k7AL22KPA+EyDMXpFUwPnvwy0Xeg+eLFmFkL4x4K3qbeKJGBheSfc4EVJ zRq3rJxxjwmN8A6V/uQ/no9fjJ1QfdsZn1g63gtm8+4WzdoACJJz3K2yuFLH1C5Op1 06JS4hQTcn7gtniRFDtsekW4UocLuVJYTwnLC7Ik= Received: from u-boot.org (unknown [174.51.25.52]) by mail.u-boot.org (Postfix) with ESMTPSA id 619946A833; Fri, 1 May 2026 05:02:12 -0600 (MDT) From: Simon Glass To: U-Boot Concept Date: Fri, 1 May 2026 05:00:19 -0600 Message-ID: <20260501110040.1874719-28-sjg@u-boot.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20260501110040.1874719-1-sjg@u-boot.org> References: <20260501110040.1874719-1-sjg@u-boot.org> MIME-Version: 1.0 Message-ID-Hash: 5KZCUNLZ3XSUJG43RMR6WQJOSTUSJKPQ X-Message-ID-Hash: 5KZCUNLZ3XSUJG43RMR6WQJOSTUSJKPQ X-MailFrom: sjg@u-boot.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: Simon Glass X-Mailman-Version: 3.3.10 Precedence: list Subject: [Concept] [PATCH 27/29] patman: Allow choosing the review base branch List-Id: Discussion and patches related to U-Boot Concept Archived-At: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: From: Simon Glass The review command currently picks '/next' as its base branch whenever that ref exists, falling back to '/master' only if next is missing. That works during a release cycle, but it mishandles the period right after a release: the next branch has just been merged into master and contains no exclusive commits, yet patman still applies onto it. The mirror-image case occurs when a reviewer wants to apply onto master even though next has diverged. Honour a new -b/--base-branch flag so the user can override the choice explicitly. When the flag is unset, decide automatically by asking gitutil.count_revs() for the count of commits in '/master../next'. If next is ahead of master, use it; otherwise fall back to master. After a release, master contains everything next had, so count_revs() returns zero and patman picks master until next diverges again at the next cycle's RC1. Signed-off-by: Simon Glass --- tools/patman/cmdline.py | 5 +++ tools/patman/patman.rst | 12 +++++++ tools/patman/review.py | 24 +++++++++---- tools/patman/test_cseries.py | 69 ++++++++++++++++++++++++++++++++++++ 4 files changed, 103 insertions(+), 7 deletions(-) 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()))