From patchwork Thu Feb 12 21:16:14 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 1839 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=1770931004; bh=CsMmyK6vTlIrG4GOvY1bGbtcTOvif/IyQRbN8dWDXEI=; 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=X4lrBb/tJL7ECe5HUfagTsVuo+pjkuH4ZO2WDtPLTWnL+gN9a16KM24QH62QOoV95 WuwakSPORg6/QuT7/vpGDmbgHqSuVVZSOF+C+sVls59Rq43CFCPGKYS8x+yVa/H51C 40ZQqhDK/a0UQzF9SvW8JTkMoA4dh0f6tijjx0Zmi722bD48ajS4JfcxmYqOWaxcfF dksU7IFh3YFt1x4FxVa5FoPxmhgqz4DTOeT1iCP7OufxueIeuhbVHf0FvuT3xmGKHW Jid9w1IFb557bI7Q4jALxgNoLnVCYT4o25/ok3WUHWKmTCMsHBU/fgLeC/GxAsqFtd Gkhffnkdbtc/Q== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 9B7B469ABB for ; Thu, 12 Feb 2026 14:16:44 -0700 (MST) 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 reMtc-6IOSXL for ; Thu, 12 Feb 2026 14:16:44 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1770931004; bh=CsMmyK6vTlIrG4GOvY1bGbtcTOvif/IyQRbN8dWDXEI=; 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=X4lrBb/tJL7ECe5HUfagTsVuo+pjkuH4ZO2WDtPLTWnL+gN9a16KM24QH62QOoV95 WuwakSPORg6/QuT7/vpGDmbgHqSuVVZSOF+C+sVls59Rq43CFCPGKYS8x+yVa/H51C 40ZQqhDK/a0UQzF9SvW8JTkMoA4dh0f6tijjx0Zmi722bD48ajS4JfcxmYqOWaxcfF dksU7IFh3YFt1x4FxVa5FoPxmhgqz4DTOeT1iCP7OufxueIeuhbVHf0FvuT3xmGKHW Jid9w1IFb557bI7Q4jALxgNoLnVCYT4o25/ok3WUHWKmTCMsHBU/fgLeC/GxAsqFtd Gkhffnkdbtc/Q== Received: from mail.u-boot.org (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 8161969ABF for ; Thu, 12 Feb 2026 14:16:44 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1770931003; bh=ItS3Kv+BF1IW8NSp0Xf5TDuKHMT/oYKLdMzbo1Gk3FE=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=eJPZEfhBgVVzI6N47a3nds9rVrKEQVjrcFXsYhTe+XMkYkPTTO+n12xjAw05/DPQC hBd3IwqLurcjOSGZX81CVclcfqwg28rZ2zttMlWp7NjrodP1fadHce7MfpB00C3HYu HDOdjafqbAOjWuEPgxqznfZsF9JUhg6flx2p5kskvXswRkTlKr7H11VwQnxcs/Ifon TuxXavbptzMMFDLmaJWI+ueMU0G9c27KgJ+jrXLnE6JSSOmT/j+C4cCprZGA9AXV3O MWL7DVrTTKrmFPTHSbLfGVqMEIw07QSHbWc5R7FYut977IB2u10kGwu+iz/calg2gb xMktmL7Gh5tYA== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 116AA69ABB; Thu, 12 Feb 2026 14:16:43 -0700 (MST) 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 13i3DUeppVDX; Thu, 12 Feb 2026 14:16:42 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1770930998; bh=50WXCtSrGdiQmQvmvYCdt17QCaARC35Z38VEEQwzdlQ=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=W3IIOnen99aCL/qutErXOxlz7ZiVqpudqPgEx1TLHiLawqFRDksJckndrhGgWkORS Ien9BXrPde+iLo1lgyGpkx2TMNvleYXiYFF/mY1jscBDAjugv+tAnIxgq7kXqGBpqU 4qAWiBghnKJfMaV2h+HNg/2KnRogobok0Kq0gZwk9sU8k03USC474e+wqUB3dKghTw Hr9n5wX0O7fl8egZYcpp6FfekdyjKUkkx8/4K2WRoPLUvhvM+aZhTFMqTO+8BsHzO2 tZV6ZUHyLtEG+lwYZPX0RAl7hNqi3fh7rplVX/4fDcLkdx77rjTItOwSemgDzBJEMy vKfhcdNuf2dfA== Received: from u-boot.org (unknown [73.34.74.121]) by mail.u-boot.org (Postfix) with ESMTPSA id 7C6746994D; Thu, 12 Feb 2026 14:16:38 -0700 (MST) From: Simon Glass To: U-Boot Concept Date: Thu, 12 Feb 2026 14:16:14 -0700 Message-ID: <20260212211626.167191-2-sjg@u-boot.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20260212211626.167191-1-sjg@u-boot.org> References: <20260212211626.167191-1-sjg@u-boot.org> MIME-Version: 1.0 Message-ID-Hash: Q76QWCZPNE4ZWJEF2A322Q6GKWGV6OW3 X-Message-ID-Hash: Q76QWCZPNE4ZWJEF2A322Q6GKWGV6OW3 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 , "Claude Opus 4 . 6" X-Mailman-Version: 3.3.10 Precedence: list Subject: [Concept] [PATCH 1/9] pickman: Fix line-too-long pylint warnings in ftest 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 Break three lines that exceed the 80-column limit. Co-developed-by: Claude Opus 4.6 Signed-off-by: Simon Glass --- tools/pickman/agent.py | 3 ++- tools/pickman/ftest.py | 13 ++++++++----- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/tools/pickman/agent.py b/tools/pickman/agent.py index 589d61b727a..129743bb329 100644 --- a/tools/pickman/agent.py +++ b/tools/pickman/agent.py @@ -25,7 +25,8 @@ SIGNAL_APPLIED = 'already_applied' SIGNAL_CONFLICT = 'conflict' # Commits that need special handling (regenerate instead of cherry-pick) -# These run savedefconfig on all boards and depend on target branch Kconfig state +# These run savedefconfig on all boards and depend on target branch +# Kconfig state QCONFIG_SUBJECTS = [ 'configs: Resync with savedefconfig', ] diff --git a/tools/pickman/ftest.py b/tools/pickman/ftest.py index d63cfbc6758..049725475f6 100644 --- a/tools/pickman/ftest.py +++ b/tools/pickman/ftest.py @@ -3195,7 +3195,8 @@ class TestProcessMergedMrs(unittest.TestCase): with terminal.capture(): dbs = database.Database(self.db_path) dbs.start() - dbs.source_set('us/next', 'bbb222bbb222bbb222bbb222bbb222bbb222bbb2') + dbs.source_set('us/next', + 'bbb222bbb222bbb222bbb222bbb222bbb222bbb2') dbs.commit() merged_mrs = [gitlab.PickmanMr( @@ -3282,7 +3283,8 @@ Date: Mon Jan 15 10:30:00 2024 -0600 def test_calc_ratio_different_files(self): """Test delta ratio calculation for different files.""" orig_stats = control.GitStat(2, 15, 3, {'file1.c', 'file2.h'}) - cherry_stats = control.GitStat(3, 15, 3, {'file1.c', 'file2.h', 'file3.c'}) + cherry_stats = control.GitStat( + 3, 15, 3, {'file1.c', 'file2.h', 'file3.c'}) ratio = control.calc_ratio(orig_stats, cherry_stats) self.assertGreater(ratio, 0.0) @@ -3862,8 +3864,9 @@ class TestDoPick(unittest.TestCase): return_value=(commits, None)): with mock.patch.object(control, 'run_git', return_value='main'): - with mock.patch.object(control.agent, 'cherry_pick_commits', - return_value=(True, 'log')) as mock_agent: + with mock.patch.object( + control.agent, 'cherry_pick_commits', + return_value=(True, 'log')) as mock_agent: ret = control.do_pick(args, dbs) # Verify agent was called with correct branch name @@ -3931,7 +3934,7 @@ class TestDoPick(unittest.TestCase): def run_git_handler(args): if '--verify' in args: - raise Exception('branch not found') + raise ValueError('branch not found') return 'main' with mock.patch.object(control, 'get_commits_for_pick', From patchwork Thu Feb 12 21:16:15 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 1840 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=1770931009; bh=C1L8ZUy/QFvDqttZkgFS3benvTNpvGhBNCBs902NEyY=; 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=APypzLHJU+rKHn/TlH2FIleBQmhewm9SIwy4cTQzHPzbKceknlLaKFDtJ/t6QtGke o85zZ6K/rM1hOc1Q6DdmOZIrSBPmZqnyc5TbtN8hueCqYNv94Gk4oToBdxkyDYQm1J wYlBtg5CrvYJG2yv8tt1NQE0fnVM+4CWGEN092s4D75Bzf3WpOttV0oOfP0CLBykXV mfk7lC7612y6I1Nx8LrYGKGmSgrfjVmhHs53fKycRdtq85gYYP8q2ktUwJLkAIAKRU K2FSpf1u0RjS7bQ9EwGviMCC6K1dGpPi+wh3AWq6hj9GQCkas3Rmv2VOPMmhtT3Rbd PDeEm2LcM18fg== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 24D1B69AE6 for ; Thu, 12 Feb 2026 14:16:49 -0700 (MST) 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 7i1NyPclARLg for ; Thu, 12 Feb 2026 14:16:49 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1770931009; bh=C1L8ZUy/QFvDqttZkgFS3benvTNpvGhBNCBs902NEyY=; 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=APypzLHJU+rKHn/TlH2FIleBQmhewm9SIwy4cTQzHPzbKceknlLaKFDtJ/t6QtGke o85zZ6K/rM1hOc1Q6DdmOZIrSBPmZqnyc5TbtN8hueCqYNv94Gk4oToBdxkyDYQm1J wYlBtg5CrvYJG2yv8tt1NQE0fnVM+4CWGEN092s4D75Bzf3WpOttV0oOfP0CLBykXV mfk7lC7612y6I1Nx8LrYGKGmSgrfjVmhHs53fKycRdtq85gYYP8q2ktUwJLkAIAKRU K2FSpf1u0RjS7bQ9EwGviMCC6K1dGpPi+wh3AWq6hj9GQCkas3Rmv2VOPMmhtT3Rbd PDeEm2LcM18fg== Received: from mail.u-boot.org (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 12DF569ABF for ; Thu, 12 Feb 2026 14:16:49 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1770931007; bh=Awb5MFo/RDwou8JmrkQP2zxElLRCEpV3jyEKDmSizgU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=lf5ybY6weWTU+4bb9LxuIGIoK6tiqcwcnjy1mRWmtaMVSYFoMXdbtzeb1AnttJIUC QOz2UUBYnSBxPMMUAtwuubtoyD1Fi5sWbEelvRhj9pt5jNVVx02nPtDXTb8S//nN7O OXiE1FEYMZnVo6UffyyVoq4XZ2a0/5o640UafmISkN+U38rIVYat4ETQ2n+O/ofBuV R/XY0L7wYyV4w9tpT8WEeux4D6bxPTtLXv9cvHlnboLImuGsTHcz2ebPQD+VITei0N KIRYicFdC9ZdgHCYrquTXffvrxfXMqqV49yv2cpo+6nT4IgPz7sAw7qv25rX0xP/0Y Pgi1lhhVKx3BQ== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id CAECD69ABB; Thu, 12 Feb 2026 14:16:47 -0700 (MST) 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 i-iLw2xNlQZs; Thu, 12 Feb 2026 14:16:47 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1770931003; bh=rbFrN4xextnJF+EcExHYrCgG8fBtJOhWXhe6WMXjmkE=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=tfpRaZ4IgYsPNTkSqryfGcpg2ytnDrwC8+Lbvq5rlOYjBwMwX5HyMozUmDVIyfh/s OvZ0Li/9Wyj3MT13oicrYQq6Sfmr21inqMWInswjeo8d+EVMxVNTJGAHvKFSsxcoqQ 18BVUreuHHWxyVVFaPAJKgwK+P5LnPEI+GF5U8LagB/K0moTvTTpqWEV7uuY2GlZrn eySdxXjThy9wwqKpJWj2OcF7MvpAIbBPaEl+C9xbuihEbSgQ6/tyRdkTUQ9XupZ3YB 9Y1N4MagZZum12QaquN3T3AfG1KfTq8sKbTeAPDZPxf1jmpfh4r2LY5g7mA+rpUL2X 1DXfxsy6EXilg== Received: from u-boot.org (unknown [73.34.74.121]) by mail.u-boot.org (Postfix) with ESMTPSA id 493F26994D; Thu, 12 Feb 2026 14:16:43 -0700 (MST) From: Simon Glass To: U-Boot Concept Date: Thu, 12 Feb 2026 14:16:15 -0700 Message-ID: <20260212211626.167191-3-sjg@u-boot.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20260212211626.167191-1-sjg@u-boot.org> References: <20260212211626.167191-1-sjg@u-boot.org> MIME-Version: 1.0 Message-ID-Hash: Z574GEQ6WVVKIFYIS2I3Y74GSL6HFPVA X-Message-ID-Hash: Z574GEQ6WVVKIFYIS2I3Y74GSL6HFPVA 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 , "Claude Opus 4 . 6" X-Mailman-Version: 3.3.10 Precedence: list Subject: [Concept] [PATCH 2/9] pickman: Extract build_applied_map() from execute_apply() 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 Move the creation of the applied-commit mapping into its own function to reduce duplication of the grep logic already in check_already_applied() and simplify execute_apply() Co-developed-by: Claude Opus 4.6 Signed-off-by: Simon Glass --- tools/pickman/control.py | 43 +++++++++++++++++++++++++++------------- 1 file changed, 29 insertions(+), 14 deletions(-) diff --git a/tools/pickman/control.py b/tools/pickman/control.py index f533b00ef60..54e1c568d7b 100644 --- a/tools/pickman/control.py +++ b/tools/pickman/control.py @@ -542,6 +542,34 @@ def check_already_applied(commits, target_branch='ci/master'): return new_commits, applied +def build_applied_map(commits): + """Build a mapping of commit hashes to their applied counterparts + + Checks which commits have already been applied to the target branch + and returns a dict mapping original hashes to the applied hashes. + + Args: + commits (list): List of CommitInfo tuples to check + + Returns: + dict: Mapping of original commit hash to applied commit hash + """ + _, applied = check_already_applied(commits) + + applied_map = {} + if applied: + for c in applied: + escaped_subject = c.subject.replace('"', '\\"') + result = run_git(['log', '--oneline', 'ci/master', + f'--grep={escaped_subject}', '-1']) + if result.strip(): + applied_hash = result.split()[0] + applied_map[c.hash] = applied_hash + tout.info(f'Found {len(applied)} potentially already applied' + ' commit(s)') + return applied_map + + def show_commit_diff(res, no_colour=False): """Show the difference between original and cherry-picked commit patches @@ -1336,20 +1364,7 @@ def execute_apply(dbs, source, commits, branch_name, args): # pylint: disable=t 1 on failure """ # Check for already applied commits before proceeding - _, applied = check_already_applied(commits) - - # Build mapping of applied commits by hash - applied_map = {} - if applied: - for c in applied: - # Get the hash of the applied commit in target branch - escaped_subject = c.subject.replace('"', '\\"') - result = run_git(['log', '--oneline', 'ci/master', - f'--grep={escaped_subject}', '-1']) - if result.strip(): - applied_hash = result.split()[0] - applied_map[c.hash] = applied_hash - tout.info(f'Found {len(applied)} potentially already applied commit(s)') + applied_map = build_applied_map(commits) # Add all commits to database with 'pending' status (agent updates later) source_id = dbs.source_get_id(source) From patchwork Thu Feb 12 21:16:16 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 1841 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=1770931014; bh=K4MzQprB9OXsZy00XyS4I/E1tzShsLiHl/uOn0wLF7M=; 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=ihhzVIUW4OxXRdYi1abiv5CmAl5unSJ6hbD+dzlbpViyi3LmGk3vErncp/HA/0lqc nS9tPJL4djuXzJgzXAG7A3dlp92f05RjU3WCpGluDoMTVTDyUWMN1gM7qo3/+uqdtu eITxWDCZfU3MFITVfD8/s/Rbp6PqFtTyU+TbmGI3dSN8J8RraY1rIMhWND6Hr0lBfT UcKtTwkYjsisncmcKoZKiYP8OtOH27LwF9Qme+RYg8NHRG3FkyV5CPlXz9Av21R+6I mNwuI+CVuhpQ9xwdvVJwWHO4HZhNFjtcZsc9HU/ETyN4+0zZMt7YRnt2tQEWsDpcjF +gIQktL/eHo4A== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 970CA69AE6 for ; Thu, 12 Feb 2026 14:16:54 -0700 (MST) 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 XRVqvm0KNIXm for ; Thu, 12 Feb 2026 14:16:54 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1770931014; bh=K4MzQprB9OXsZy00XyS4I/E1tzShsLiHl/uOn0wLF7M=; 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=ihhzVIUW4OxXRdYi1abiv5CmAl5unSJ6hbD+dzlbpViyi3LmGk3vErncp/HA/0lqc nS9tPJL4djuXzJgzXAG7A3dlp92f05RjU3WCpGluDoMTVTDyUWMN1gM7qo3/+uqdtu eITxWDCZfU3MFITVfD8/s/Rbp6PqFtTyU+TbmGI3dSN8J8RraY1rIMhWND6Hr0lBfT UcKtTwkYjsisncmcKoZKiYP8OtOH27LwF9Qme+RYg8NHRG3FkyV5CPlXz9Av21R+6I mNwuI+CVuhpQ9xwdvVJwWHO4HZhNFjtcZsc9HU/ETyN4+0zZMt7YRnt2tQEWsDpcjF +gIQktL/eHo4A== Received: from mail.u-boot.org (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 8747C69AD8 for ; Thu, 12 Feb 2026 14:16:54 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1770931012; bh=D/gBWz0w3SZOq52pOkshjAF+oUKMjpOpALw+GqIldVA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=iIr/Kbo6MjVl9Hj56xlff/LnscrtnT2Zizgj7JTUva26mpQFQfEGTOqyhfy/+2DT/ rq0IfQYlx7yEAHvL+8lYMNWmyRyUGXAvKzkuoIx24gXT9vhndQJPxrCw2qwfPToZec 15NmaGl0VDUZp/CRwnm52FTybgcvs0Ptl1H5fbup2YkxTKsqRuRwcJbg/yF240w/mQ iW4f0QpNMTT/bvFM1Nw8dPXV1tCW92g+5zEkDuSWdERfwG5DS1GA7kcR5/sRNpqkCM +LGYOOEQn2sH1DwQYSa6+/3de3Ul9Mpm/ieix/zuyLqZT4vW8AZ3e8b9yDKJWg4RdO pJ1WwYoStbKkw== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 45FFB69ABB; Thu, 12 Feb 2026 14:16:52 -0700 (MST) 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 3CRCYXcZGec0; Thu, 12 Feb 2026 14:16:52 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1770931008; bh=SRr0E3PFNJbaEmvIi/uBPBhdVUF//VEFgpKov2L1o2M=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=khgN+nyrZ7Cg0yaf8z/A1GzU+GYr+0FGhTwNueXewLdPB59Ic4WWAY2DaRiwQhF76 fIucb6HbdVcErnJMHLHcORI5y6ul/fwJEw1fNMUTKQ1FyNuwzus5EUTiBnpHkm9H4Z 5DsaAS29MjXZx7cymU0mDkbMv5d2+Np322lBr+oJ0PA3AC5GXtlhS832TqP56zRv7f LfQPu6SGt/DbE7RXcXe8dPR/MFZAAXGM8qPYNsSsXzSzQddIRkQm1c45jg1cIq4L+1 xyQlV1J/R3mTSEMxCiGLVeqQUT8ZPRB7kyoMq7wTdUPhKGpwRdqOGjpeZTLkWCQPQU 7ZUNdF90Ksjgg== Received: from u-boot.org (unknown [73.34.74.121]) by mail.u-boot.org (Postfix) with ESMTPSA id 157E76994D; Thu, 12 Feb 2026 14:16:48 -0700 (MST) From: Simon Glass To: U-Boot Concept Date: Thu, 12 Feb 2026 14:16:16 -0700 Message-ID: <20260212211626.167191-4-sjg@u-boot.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20260212211626.167191-1-sjg@u-boot.org> References: <20260212211626.167191-1-sjg@u-boot.org> MIME-Version: 1.0 Message-ID-Hash: HTEEILJJI4CDY55ALTFFF6UHC5OJX47Y X-Message-ID-Hash: HTEEILJJI4CDY55ALTFFF6UHC5OJX47Y 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 , "Claude Opus 4 . 6" X-Mailman-Version: 3.3.10 Precedence: list Subject: [Concept] [PATCH 3/9] pickman: Group named-tuple declarations together 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 Move AgentCommit and ApplyInfo above parse_log_output() so that all named-tuple declarations are together at the top of the file. Fill out the comment for ApplyInfo Co-developed-by: Claude Opus 4.6 Signed-off-by: Simon Glass --- tools/pickman/control.py | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/tools/pickman/control.py b/tools/pickman/control.py index 54e1c568d7b..c88fa567191 100644 --- a/tools/pickman/control.py +++ b/tools/pickman/control.py @@ -79,6 +79,24 @@ CheckResult = namedtuple('CheckResult', [ CommitInfo = namedtuple('CommitInfo', ['hash', 'chash', 'subject', 'author']) +# Named tuple for simplified commit data passed to agent +# hash: Full SHA-1 commit hash (40 characters) +# chash: Abbreviated commit hash (typically 7-8 characters) +# subject: First line of commit message (commit subject) +# applied_as: Short hash if potentially already applied, None otherwise +AgentCommit = namedtuple('AgentCommit', + ['hash', 'chash', 'subject', 'applied_as']) + +# Named tuple for prepare_apply() result +# +# commits: list of AgentCommit to cherry-pick +# branch_name: name of the branch to create for the MR +# original_branch: branch name before any conflict suffix +# merge_found: True if these commits came from a merge on the source branch +ApplyInfo = namedtuple('ApplyInfo', + ['commits', 'branch_name', 'original_branch', + 'merge_found']) + def parse_log_output(log_output, has_parents=False): """Parse git log output to extract CommitInfo tuples @@ -107,19 +125,6 @@ def parse_log_output(log_output, has_parents=False): commits.append(CommitInfo(commit_hash, chash, subject, author)) return commits -# Named tuple for simplified commit data passed to agent -# hash: Full SHA-1 commit hash (40 characters) -# chash: Abbreviated commit hash (typically 7-8 characters) -# subject: First line of commit message (commit subject) -# applied_as: Short hash if potentially already applied, None otherwise -AgentCommit = namedtuple('AgentCommit', - ['hash', 'chash', 'subject', 'applied_as']) - -# Named tuple for prepare_apply result -ApplyInfo = namedtuple('ApplyInfo', - ['commits', 'branch_name', 'original_branch', - 'merge_found']) - def run_git(args): """Run a git command and return output.""" From patchwork Thu Feb 12 21:16:17 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 1842 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=1770931018; bh=pHsd3Vwbrrul6vWwkRJ+31BL+Vjxd1xSm5Jqs96fhM0=; 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=fZGiqP7rann9ZSQx4rmC26wM2wq/PYBdOOoK2jpuuSGwqJ8AErgRV/R+nf1v2uHVN WiNNshMId/FK+tFPoUqfFX5FLl8UMMCV/zQ5aVJwU1Fa0zjEsZq2OBGPBzX00Nobi9 VNyPGH2IOqVUSY3tWsh98iuWnSxiBVnv+izqYDiEZXXs2zTCAHt8UrdIDx9Mgi/WEj Mn4yLUApB/gxnUn0CbSYP8q3FapgxqSowrKBaLc+caFZx86myqWjzBdWtjBDqq4G7c d1OJr0Yi7p0oT+2SoDRhp6FzXeW0NE+SrTGc1XzUDqarVHHoagLBaNxSWKvYmXvrE5 S6uG0qWsUnxaw== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 5EB3B69AE9 for ; Thu, 12 Feb 2026 14:16:58 -0700 (MST) 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 Pu6o5sD6sFKt for ; Thu, 12 Feb 2026 14:16:58 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1770931018; bh=pHsd3Vwbrrul6vWwkRJ+31BL+Vjxd1xSm5Jqs96fhM0=; 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=fZGiqP7rann9ZSQx4rmC26wM2wq/PYBdOOoK2jpuuSGwqJ8AErgRV/R+nf1v2uHVN WiNNshMId/FK+tFPoUqfFX5FLl8UMMCV/zQ5aVJwU1Fa0zjEsZq2OBGPBzX00Nobi9 VNyPGH2IOqVUSY3tWsh98iuWnSxiBVnv+izqYDiEZXXs2zTCAHt8UrdIDx9Mgi/WEj Mn4yLUApB/gxnUn0CbSYP8q3FapgxqSowrKBaLc+caFZx86myqWjzBdWtjBDqq4G7c d1OJr0Yi7p0oT+2SoDRhp6FzXeW0NE+SrTGc1XzUDqarVHHoagLBaNxSWKvYmXvrE5 S6uG0qWsUnxaw== Received: from mail.u-boot.org (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 43A4569AD8 for ; Thu, 12 Feb 2026 14:16:58 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1770931017; bh=vZuFqTtk8x4edNRnYHerQNDiLCDQFcPQDvL8nuaaSkE=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=A5/bkCndKqhx7FB5WPGDFQMlYIvEyfFPHESjvaX1k+QcSPsGbGZ4JPKfzcmkgdd0e KNC1srD2mR4cwNIknsq6vgO+HbD1HyI2YLx4D3lXkp4tZn3pV5bol3Ep2yAGpnWgUU ySHC3liXYgCbW0EoM411bYzXfc7KsvR/oRBwbRhjPOLFbhKs/eLfp3DDVc1whFeYWb og3uKGKYCpKqME6eIgzC7huFblaR15yU//I8DU7fQi5uDRc1mybUBf2G9FqHOGj+nN R7pJCiXzMv8gBy9zMdoxkbXTZrv00NHGirBM3XwvKdPl5p84n7k3cygodabecPoqMs arXWaJKh9uykQ== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 169D469AD8; Thu, 12 Feb 2026 14:16:57 -0700 (MST) 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 64T6K46ZyfzX; Thu, 12 Feb 2026 14:16:57 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1770931012; bh=/tYdlpE50cSPyzpc9qnqJE0kedzDFeyjhTrHI8tSWko=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=pXxA91vuJHjkoW7h/7PG9c7ZG3f5GH1HbBtp1m2FI8VrwqLmOgTZ4186Q67chas01 1CjaczddsCbI+yFFrnBW3qFYCQBtrQ+LTJ9e4uk/la58ttAmc5gS8qSM811hEgBcwg bKo+4rcsmVAeq838pG6o6lfB5k67hydOpB6yBJ3bQGaZyU6/Tjm1jIOC2zLbdUX1dI Gq4JDcziFp34fzfI4aU8vC4JJmwye76EHdWmjA5O3r+OsqTrlkeNcTyco/sajm0yO3 3J4vkE4EX5QrU5+c8irLdmaXqJLNdIG3JgdOoFFtQn0+jJw6xF5eSFl1F0KYrlxxWO yorzrc2/BH72Q== Received: from u-boot.org (unknown [73.34.74.121]) by mail.u-boot.org (Postfix) with ESMTPSA id 8A3786994D; Thu, 12 Feb 2026 14:16:52 -0700 (MST) From: Simon Glass To: U-Boot Concept Date: Thu, 12 Feb 2026 14:16:17 -0700 Message-ID: <20260212211626.167191-5-sjg@u-boot.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20260212211626.167191-1-sjg@u-boot.org> References: <20260212211626.167191-1-sjg@u-boot.org> MIME-Version: 1.0 Message-ID-Hash: GROAGHKISUDCG43MUV3WU74VPZYAI2V2 X-Message-ID-Hash: GROAGHKISUDCG43MUV3WU74VPZYAI2V2 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 , "Claude Opus 4 . 6" X-Mailman-Version: 3.3.10 Precedence: list Subject: [Concept] [PATCH 4/9] pickman: Rename error to err for local variables 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 Shorten the local variable name from 'error' to 'err' in get_commits_for_pick(), do_next_set(), prepare_apply(), and do_pick(), along with the corresponding test code. Co-developed-by: Claude Opus 4.6 Signed-off-by: Simon Glass --- tools/pickman/control.py | 30 ++++++++++----------- tools/pickman/ftest.py | 56 ++++++++++++++++++++-------------------- 2 files changed, 43 insertions(+), 43 deletions(-) diff --git a/tools/pickman/control.py b/tools/pickman/control.py index c88fa567191..3f034d03d72 100644 --- a/tools/pickman/control.py +++ b/tools/pickman/control.py @@ -835,7 +835,7 @@ def get_commits_for_pick(commit_spec): on success """ commits = None - error = None + err = None if '..' in commit_spec: # Commit range format: hash1..hash2 @@ -847,9 +847,9 @@ def get_commits_for_pick(commit_spec): if log_output: commits = parse_log_output(log_output) else: - commits, error = [], f"No commits found in range: {commit_spec}" + commits, err = [], f"No commits found in range: {commit_spec}" except Exception: # pylint: disable=broad-except - error = f"Invalid commit range: {commit_spec}" + err = f"Invalid commit range: {commit_spec}" else: # Single commit - check if it's a merge try: @@ -872,11 +872,11 @@ def get_commits_for_pick(commit_spec): commits = parse_log_output(log_output) else: commits = [] - error = f"No commits found in merge: {commit_spec}" + err = f"No commits found in merge: {commit_spec}" except Exception: # pylint: disable=broad-except - error = f"Invalid commit: {commit_spec}" + err = f"Invalid commit: {commit_spec}" - return commits, error + return commits, err def do_next_set(args, dbs): @@ -890,10 +890,10 @@ def do_next_set(args, dbs): int: 0 on success, 1 if source not found """ source = args.source - commits, merge_found, error = get_next_commits(dbs, source) + commits, merge_found, err = get_next_commits(dbs, source) - if error: - tout.error(error) + if err: + tout.error(err) return 1 if not commits: @@ -1247,10 +1247,10 @@ def prepare_apply(dbs, source, branch): commits to apply, or None with return_code indicating the result (0 for no commits, 1 for error) """ - commits, merge_found, error = get_next_commits(dbs, source) + commits, merge_found, err = get_next_commits(dbs, source) - if error: - tout.error(error) + if err: + tout.error(err) return None, 1 if not commits: @@ -1479,9 +1479,9 @@ def do_pick(args, dbs): # pylint: disable=unused-argument,too-many-locals commit_spec = args.commits # Get commits to cherry-pick - commits, error = get_commits_for_pick(commit_spec) - if error: - tout.error(error) + commits, err = get_commits_for_pick(commit_spec) + if err: + tout.error(err) return 1 if not commits: diff --git a/tools/pickman/ftest.py b/tools/pickman/ftest.py index 049725475f6..b943804e6f8 100644 --- a/tools/pickman/ftest.py +++ b/tools/pickman/ftest.py @@ -1208,11 +1208,11 @@ class TestGetNextCommits(unittest.TestCase): with terminal.capture(): dbs = database.Database(self.db_path) dbs.start() - commits, merge_found, error = control.get_next_commits(dbs, + commits, merge_found, err = control.get_next_commits(dbs, 'unknown') self.assertIsNone(commits) self.assertFalse(merge_found) - self.assertIn('not found', error) + self.assertIn('not found', err) dbs.close() def test_get_next_commits_with_merge(self): @@ -1243,9 +1243,9 @@ class TestGetNextCommits(unittest.TestCase): command.TEST_RESULT = mock_git - commits, merge_found, error = control.get_next_commits(dbs, + commits, merge_found, err = control.get_next_commits(dbs, 'us/next') - self.assertIsNone(error) + self.assertIsNone(err) self.assertTrue(merge_found) self.assertEqual(len(commits), 2) self.assertEqual(commits[0].chash, 'aaa111a') @@ -2870,9 +2870,9 @@ class TestGetNextCommitsEmptyLine(unittest.TestCase): ) command.TEST_RESULT = command.CommandResult(stdout=log_output) - commits, merge_found, error = control.get_next_commits(dbs, + commits, merge_found, err = control.get_next_commits(dbs, 'us/next') - self.assertIsNone(error) + self.assertIsNone(err) self.assertFalse(merge_found) self.assertEqual(len(commits), 2) dbs.close() @@ -2897,9 +2897,9 @@ class TestGetNextCommitsEmptyLine(unittest.TestCase): ) command.TEST_RESULT = command.CommandResult(stdout=log_output) - commits, merge_found, error = control.get_next_commits(dbs, + commits, merge_found, err = control.get_next_commits(dbs, 'us/next') - self.assertIsNone(error) + self.assertIsNone(err) self.assertFalse(merge_found) # Only second commit should be returned (first is in DB) self.assertEqual(len(commits), 1) @@ -2928,9 +2928,9 @@ class TestGetNextCommitsEmptyLine(unittest.TestCase): ) command.TEST_RESULT = command.CommandResult(stdout=log_output) - commits, merge_found, error = control.get_next_commits(dbs, + commits, merge_found, err = control.get_next_commits(dbs, 'us/next') - self.assertIsNone(error) + self.assertIsNone(err) self.assertFalse(merge_found) # No commits should be returned (all in DB) self.assertEqual(len(commits), 0) @@ -2987,9 +2987,9 @@ class TestGetNextCommitsEmptyLine(unittest.TestCase): command.TEST_RESULT = mock_git - commits, merge_found, error = control.get_next_commits(dbs, + commits, merge_found, err = control.get_next_commits(dbs, 'us/next') - self.assertIsNone(error) + self.assertIsNone(err) self.assertTrue(merge_found) # Should return commits from second merge (first was skipped) self.assertEqual(len(commits), 2) @@ -3641,9 +3641,9 @@ class TestGetCommitsForPick(unittest.TestCase): 'bbb222|bbb222b|Author2|Second commit' ) - commits, error = control.get_commits_for_pick('abc123..def456') + commits, err = control.get_commits_for_pick('abc123..def456') - self.assertIsNone(error) + self.assertIsNone(err) self.assertEqual(len(commits), 2) self.assertEqual(commits[0].hash, 'aaa111') self.assertEqual(commits[0].chash, 'aaa111a') @@ -3658,20 +3658,20 @@ class TestGetCommitsForPick(unittest.TestCase): """Test empty commit range returns error.""" mock_run_git.return_value = '' - commits, error = control.get_commits_for_pick('abc123..abc123') + commits, err = control.get_commits_for_pick('abc123..abc123') self.assertEqual(commits, []) - self.assertIn('No commits found', error) + self.assertIn('No commits found', err) @mock.patch('pickman.control.run_git') def test_commit_range_invalid(self, mock_run_git): """Test invalid commit range returns error.""" mock_run_git.side_effect = Exception('bad revision') - commits, error = control.get_commits_for_pick('invalid..range') + commits, err = control.get_commits_for_pick('invalid..range') self.assertIsNone(commits) - self.assertIn('Invalid commit range', error) + self.assertIn('Invalid commit range', err) @mock.patch('pickman.control.run_git') def test_single_commit_non_merge(self, mock_run_git): @@ -3683,9 +3683,9 @@ class TestGetCommitsForPick(unittest.TestCase): mock_run_git.side_effect = git_handler - commits, error = control.get_commits_for_pick('abc123') + commits, err = control.get_commits_for_pick('abc123') - self.assertIsNone(error) + self.assertIsNone(err) self.assertEqual(len(commits), 1) self.assertEqual(commits[0].hash, 'abc123full') self.assertEqual(commits[0].subject, 'Single commit') @@ -3706,9 +3706,9 @@ class TestGetCommitsForPick(unittest.TestCase): mock_run_git.side_effect = git_handler - commits, error = control.get_commits_for_pick('merge123') + commits, err = control.get_commits_for_pick('merge123') - self.assertIsNone(error) + self.assertIsNone(err) self.assertEqual(len(commits), 2) self.assertEqual(commits[0].hash, 'ccc333') self.assertEqual(commits[1].hash, 'ddd444') @@ -3723,29 +3723,29 @@ class TestGetCommitsForPick(unittest.TestCase): mock_run_git.side_effect = git_handler - commits, error = control.get_commits_for_pick('merge123') + commits, err = control.get_commits_for_pick('merge123') self.assertEqual(commits, []) - self.assertIn('No commits found in merge', error) + self.assertIn('No commits found in merge', err) @mock.patch('pickman.control.run_git') def test_invalid_single_commit(self, mock_run_git): """Test invalid single commit returns error.""" mock_run_git.side_effect = Exception('unknown revision') - commits, error = control.get_commits_for_pick('badcommit') + commits, err = control.get_commits_for_pick('badcommit') self.assertIsNone(commits) - self.assertIn('Invalid commit', error) + self.assertIn('Invalid commit', err) @mock.patch('pickman.control.run_git') def test_subject_with_separator(self, mock_run_git): """Test commit subject containing pipe character.""" mock_run_git.return_value = 'aaa111|aaa111a|Author|Subject|with|pipes' - commits, error = control.get_commits_for_pick('abc..def') + commits, err = control.get_commits_for_pick('abc..def') - self.assertIsNone(error) + self.assertIsNone(err) self.assertEqual(commits[0].subject, 'Subject|with|pipes') From patchwork Thu Feb 12 21:16:18 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 1843 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=1770931023; bh=j7xv1WJJ4j9cPbjZOhduWsPq2t8lBu8CX7WqmscKviQ=; 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=RJRriJ2DQuHFbhhs6zHPp6T88kX4tji08Tyw7S0BGN7ZxwiDGrYOhosMy9xg+0C5H 7O5x+/LBpO1pihgwNm/DeGi5E91SKfWrGx+hPf3YEf6OQDIgYzkrd82p5HW8UArmR+ FhpuKNxnAXq++VMDxTTX3l6MW5fmZfDUlZGShTihN5AIamMBUFvSX+zw2cabNut/z0 4svZ776nI4i2rq1iGNj9owqb0cvdDuvTiyp0xA+H+TZt+lGAh4x4XLXjJnKKlQnx1x PTFriWA609OtDxrPYk8GZYlWnTeFibmIr06nIy2sF/wwqTYvBepOfLGcPZKRLUUjCj m47i6aYn+kJ/w== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id D73DA69AE9 for ; Thu, 12 Feb 2026 14:17:03 -0700 (MST) 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 lf-U_huQCMXO for ; Thu, 12 Feb 2026 14:17:03 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1770931023; bh=j7xv1WJJ4j9cPbjZOhduWsPq2t8lBu8CX7WqmscKviQ=; 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=RJRriJ2DQuHFbhhs6zHPp6T88kX4tji08Tyw7S0BGN7ZxwiDGrYOhosMy9xg+0C5H 7O5x+/LBpO1pihgwNm/DeGi5E91SKfWrGx+hPf3YEf6OQDIgYzkrd82p5HW8UArmR+ FhpuKNxnAXq++VMDxTTX3l6MW5fmZfDUlZGShTihN5AIamMBUFvSX+zw2cabNut/z0 4svZ776nI4i2rq1iGNj9owqb0cvdDuvTiyp0xA+H+TZt+lGAh4x4XLXjJnKKlQnx1x PTFriWA609OtDxrPYk8GZYlWnTeFibmIr06nIy2sF/wwqTYvBepOfLGcPZKRLUUjCj m47i6aYn+kJ/w== Received: from mail.u-boot.org (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id C653869ABF for ; Thu, 12 Feb 2026 14:17:03 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1770931021; bh=wj4M4BqOaZpkgFL65Ybtlc9Sa0NDn4H3mo/DanLYQP4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=hmaVp3oxpDhrZacTg8ycxePa6vd7FEJjcLpVpdqBh2ABHSyuhWbPxtoT1s6PurMpT 8iS3t/GQ4tBdCxhSUjzpNWPk+JBC1A8lKnIgOcXnZ9SuGgEjwZDgOJN4/fyk1y8Oly tPeWyRETjPOx46jfht4SjaRta4KajZ2jvwqyaq6hdGYC43WR0cyLfcVh4MVHtegWN+ 1EnMokgIvq0cCY+crPbcKvh8hBFkAz0ves+XFGPESOndEUjTcuaErRDX0zouMjQtCm GXpL1mBegnYIF1HQcCtl8GDk+XQZ9AG28ZtvZziefjMKieZR+i52+i2TeUx5F2ZUPA 4Vq8/o0rkLMjw== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id CE31969ABB; Thu, 12 Feb 2026 14:17:01 -0700 (MST) 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 1-AI0xBgDgmx; Thu, 12 Feb 2026 14:17:01 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1770931017; bh=fWNq15wv0ZGbJur8NOxIKj6fDNR3QBJD/UpDjTjgApI=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=oDYfo3Uysr6CSCi0lujUN0ZIDhscUQP79BzFvu5lktMA+72KfjkpE9HZJOqnyJhYA wRkiIUckTWsWhnVzaVn2y/hzsnEDWAlr8PPGz0iZU1eYgjCyncOXoPE2hmu0GBi6Rn n7AOf1FdHP2kP86lFT1rHgaGoINKUte1wXRGJkWp0k4Iwq7mHjckJoVw1w1Ik2YutP vuBoKOzVUNAOo7Hxtz/lYZi5DOs0RXCjdU8yJZBlkS9caAzrDKAZnHMyp1Tr2a97A5 bZmQJISA4i+2lK0nSmLGYnHOQoYpskU0Gd47YvLI7LqJRRmcZaqsvO4S0McuhEsEF5 5nEgsNsU7mSnA== Received: from u-boot.org (unknown [73.34.74.121]) by mail.u-boot.org (Postfix) with ESMTPSA id 508316994D; Thu, 12 Feb 2026 14:16:57 -0700 (MST) From: Simon Glass To: U-Boot Concept Date: Thu, 12 Feb 2026 14:16:18 -0700 Message-ID: <20260212211626.167191-6-sjg@u-boot.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20260212211626.167191-1-sjg@u-boot.org> References: <20260212211626.167191-1-sjg@u-boot.org> MIME-Version: 1.0 Message-ID-Hash: SQOKBPHSNG7UTAYUXOWF4D5EDKUPIEKM X-Message-ID-Hash: SQOKBPHSNG7UTAYUXOWF4D5EDKUPIEKM 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 , "Claude Opus 4 . 6" X-Mailman-Version: 3.3.10 Precedence: list Subject: [Concept] [PATCH 5/9] pickman: Return NextCommitsInfo from get_next_commits() 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 Add a NextCommitsInfo named tuple with commits and merge_found fields, and return (NextCommitsInfo, error_msg) from get_next_commits() instead of a bare 3-tuple. Update do_next_set() and prepare_apply() to unpack the new return type, along with the corresponding tests. Co-developed-by: Claude Opus 4.6 Signed-off-by: Simon Glass --- tools/pickman/control.py | 49 ++++++++++++++++++++++---------------- tools/pickman/ftest.py | 51 +++++++++++++++++----------------------- 2 files changed, 51 insertions(+), 49 deletions(-) diff --git a/tools/pickman/control.py b/tools/pickman/control.py index 3f034d03d72..cd138612623 100644 --- a/tools/pickman/control.py +++ b/tools/pickman/control.py @@ -87,6 +87,13 @@ CommitInfo = namedtuple('CommitInfo', AgentCommit = namedtuple('AgentCommit', ['hash', 'chash', 'subject', 'applied_as']) +# Named tuple for get_next_commits() result +# +# commits: list of CommitInfo to cherry-pick +# merge_found: True if these commits came from a merge on the source branch +NextCommitsInfo = namedtuple('NextCommitsInfo', + ['commits', 'merge_found']) + # Named tuple for prepare_apply() result # # commits: list of AgentCommit to cherry-pick @@ -751,16 +758,14 @@ def get_next_commits(dbs, source): source (str): Source branch name Returns: - tuple: (commits, merge_found, error_msg) where: - commits: list of CommitInfo tuples - merge_found: bool, True if stopped at a merge commit - error_msg: str or None, error message if failed + tuple: (NextCommitsInfo, error_msg) where error_msg is None + on success """ # Get the last cherry-picked commit from database last_commit = dbs.source_get(source) if not last_commit: - return None, False, f"Source '{source}' not found in database" + return None, f"Source '{source}' not found in database" # Get all first-parent commits to find merges fp_output = run_git([ @@ -769,7 +774,7 @@ def get_next_commits(dbs, source): ]) if not fp_output: - return [], False, None + return NextCommitsInfo([], False), None # Build list of merge hashes on the first-parent chain merge_hashes = [] @@ -799,7 +804,7 @@ def get_next_commits(dbs, source): commits = [c for c in all_commits if not dbs.commit_get(c.hash)] if commits: - return commits, True, None + return NextCommitsInfo(commits, True), None # All commits in this merge are processed, skip to next prev_commit = merge_hash @@ -811,12 +816,12 @@ def get_next_commits(dbs, source): ]) if not log_output: - return [], False, None + return NextCommitsInfo([], False), None all_commits = parse_log_output(log_output, has_parents=True) commits = [c for c in all_commits if not dbs.commit_get(c.hash)] - return commits, False, None + return NextCommitsInfo(commits, False), None def get_commits_for_pick(commit_spec): @@ -890,23 +895,24 @@ def do_next_set(args, dbs): int: 0 on success, 1 if source not found """ source = args.source - commits, merge_found, err = get_next_commits(dbs, source) + info, err = get_next_commits(dbs, source) if err: tout.error(err) return 1 - if not commits: + if not info.commits: tout.info('No new commits to cherry-pick') return 0 - if merge_found: - tout.info(f'Next set from {source} ({len(commits)} commits):') + if info.merge_found: + tout.info(f'Next set from {source} ' + f'({len(info.commits)} commits):') else: - tout.info(f'Remaining commits from {source} ({len(commits)} commits, ' - 'no merge found):') + tout.info(f'Remaining commits from {source} ' + f'({len(info.commits)} commits, no merge found):') - for commit in commits: + for commit in info.commits: tout.info(f' {commit.chash} {commit.subject}') return 0 @@ -1247,16 +1253,18 @@ def prepare_apply(dbs, source, branch): commits to apply, or None with return_code indicating the result (0 for no commits, 1 for error) """ - commits, merge_found, err = get_next_commits(dbs, source) + info, err = get_next_commits(dbs, source) if err: tout.error(err) return None, 1 - if not commits: + if not info.commits: tout.info('No new commits to cherry-pick') return None, 0 + commits = info.commits + # Save current branch to return to later original_branch = run_git(['rev-parse', '--abbrev-ref', 'HEAD']) @@ -1274,7 +1282,7 @@ def prepare_apply(dbs, source, branch): except Exception: # pylint: disable=broad-except pass # Branch doesn't exist, which is fine - if merge_found: + if info.merge_found: tout.info(f'Applying next set from {source} ({len(commits)} commits):') else: tout.info(f'Applying remaining commits from {source} ' @@ -1285,7 +1293,8 @@ def prepare_apply(dbs, source, branch): tout.info(f' {commit.chash} {commit.subject}') tout.info('') - return ApplyInfo(commits, branch_name, original_branch, merge_found), 0 + return ApplyInfo(commits, branch_name, original_branch, + info.merge_found), 0 # pylint: disable=too-many-arguments diff --git a/tools/pickman/ftest.py b/tools/pickman/ftest.py index b943804e6f8..38e5cef5306 100644 --- a/tools/pickman/ftest.py +++ b/tools/pickman/ftest.py @@ -1208,10 +1208,8 @@ class TestGetNextCommits(unittest.TestCase): with terminal.capture(): dbs = database.Database(self.db_path) dbs.start() - commits, merge_found, err = control.get_next_commits(dbs, - 'unknown') - self.assertIsNone(commits) - self.assertFalse(merge_found) + info, err = control.get_next_commits(dbs, 'unknown') + self.assertIsNone(info) self.assertIn('not found', err) dbs.close() @@ -1243,13 +1241,12 @@ class TestGetNextCommits(unittest.TestCase): command.TEST_RESULT = mock_git - commits, merge_found, err = control.get_next_commits(dbs, - 'us/next') + info, err = control.get_next_commits(dbs, 'us/next') self.assertIsNone(err) - self.assertTrue(merge_found) - self.assertEqual(len(commits), 2) - self.assertEqual(commits[0].chash, 'aaa111a') - self.assertEqual(commits[1].chash, 'bbb222b') + self.assertTrue(info.merge_found) + self.assertEqual(len(info.commits), 2) + self.assertEqual(info.commits[0].chash, 'aaa111a') + self.assertEqual(info.commits[1].chash, 'bbb222b') dbs.close() @@ -2870,11 +2867,10 @@ class TestGetNextCommitsEmptyLine(unittest.TestCase): ) command.TEST_RESULT = command.CommandResult(stdout=log_output) - commits, merge_found, err = control.get_next_commits(dbs, - 'us/next') + info, err = control.get_next_commits(dbs, 'us/next') self.assertIsNone(err) - self.assertFalse(merge_found) - self.assertEqual(len(commits), 2) + self.assertFalse(info.merge_found) + self.assertEqual(len(info.commits), 2) dbs.close() def test_get_next_commits_skips_db_commits(self): @@ -2897,13 +2893,12 @@ class TestGetNextCommitsEmptyLine(unittest.TestCase): ) command.TEST_RESULT = command.CommandResult(stdout=log_output) - commits, merge_found, err = control.get_next_commits(dbs, - 'us/next') + info, err = control.get_next_commits(dbs, 'us/next') self.assertIsNone(err) - self.assertFalse(merge_found) + self.assertFalse(info.merge_found) # Only second commit should be returned (first is in DB) - self.assertEqual(len(commits), 1) - self.assertEqual(commits[0].chash, 'bbb222b') + self.assertEqual(len(info.commits), 1) + self.assertEqual(info.commits[0].chash, 'bbb222b') dbs.close() def test_get_next_commits_all_in_db(self): @@ -2928,12 +2923,11 @@ class TestGetNextCommitsEmptyLine(unittest.TestCase): ) command.TEST_RESULT = command.CommandResult(stdout=log_output) - commits, merge_found, err = control.get_next_commits(dbs, - 'us/next') + info, err = control.get_next_commits(dbs, 'us/next') self.assertIsNone(err) - self.assertFalse(merge_found) + self.assertFalse(info.merge_found) # No commits should be returned (all in DB) - self.assertEqual(len(commits), 0) + self.assertEqual(len(info.commits), 0) dbs.close() def test_get_next_commits_skips_processed_merge(self): @@ -2987,14 +2981,13 @@ class TestGetNextCommitsEmptyLine(unittest.TestCase): command.TEST_RESULT = mock_git - commits, merge_found, err = control.get_next_commits(dbs, - 'us/next') + info, err = control.get_next_commits(dbs, 'us/next') self.assertIsNone(err) - self.assertTrue(merge_found) + self.assertTrue(info.merge_found) # Should return commits from second merge (first was skipped) - self.assertEqual(len(commits), 2) - self.assertEqual(commits[0].chash, 'ccc333c') - self.assertEqual(commits[1].chash, 'merge2m') + self.assertEqual(len(info.commits), 2) + self.assertEqual(info.commits[0].chash, 'ccc333c') + self.assertEqual(info.commits[1].chash, 'merge2m') dbs.close() From patchwork Thu Feb 12 21:16: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: 1844 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=1770931027; bh=QXpei45D/oI6x+3tg5WyiWzGnZO16Ui8u/TkWtojetg=; 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=l6zvKoTLxR3J5TYuTc0ciJeFwzaGTwVD7Gd1GCVOPRJGJTJP07ekHpnJhmO/KgGXg BOt3FR6CL79MiquOa0K/TwP2oiwgKXfn1MDd+UROY0kPGc/kMquQWjRz+l1V/quOn6 PCbnk3nbcEuIE1KxQ6m1te9nFtYtqZkIdrZyfGUOVVfKtPwbqeL5J00dtTb185Dcm2 r81dwPkhfMJQWgfjX9IZSvpWOksoJd2dNfRudmSw0HBYEO+OK44EVci2trsz9QJxZH veQLd8IdIxpjGOoR8WNNDqBfYLqTqPPSzUO5/H+NPivkheTJ7yFqBIj9C+sCnzbYrM igFmB02tmFsAQ== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 58AF369AE9 for ; Thu, 12 Feb 2026 14:17:07 -0700 (MST) 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 o1eApOIqIcbx for ; Thu, 12 Feb 2026 14:17:07 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1770931027; bh=QXpei45D/oI6x+3tg5WyiWzGnZO16Ui8u/TkWtojetg=; 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=l6zvKoTLxR3J5TYuTc0ciJeFwzaGTwVD7Gd1GCVOPRJGJTJP07ekHpnJhmO/KgGXg BOt3FR6CL79MiquOa0K/TwP2oiwgKXfn1MDd+UROY0kPGc/kMquQWjRz+l1V/quOn6 PCbnk3nbcEuIE1KxQ6m1te9nFtYtqZkIdrZyfGUOVVfKtPwbqeL5J00dtTb185Dcm2 r81dwPkhfMJQWgfjX9IZSvpWOksoJd2dNfRudmSw0HBYEO+OK44EVci2trsz9QJxZH veQLd8IdIxpjGOoR8WNNDqBfYLqTqPPSzUO5/H+NPivkheTJ7yFqBIj9C+sCnzbYrM igFmB02tmFsAQ== Received: from mail.u-boot.org (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 4759769ABF for ; Thu, 12 Feb 2026 14:17:07 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1770931026; bh=h0hFa0WhfNf2y33VSmEEJsuKxAjUArCr5DWH7wfjHnM=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=fUCE9mZk/zmbOOCXxoAzud2xpEktJjdo1bim+XubMWZTLAkNROugu4Txew1ii99xZ hAlFtKtaSHObJxSa6/6QZStDxWWCbZnQiBuyoSwlcSOqQLU5umLdKR46qrY8p32Alj fL243+a1WQ9FltCsS9pvbP6HPgKk5noTv/bAHYinph+cyYCtIsXb2siNn5tHC3sti0 XGsBaqW/lg0Ug+qqhWeoRoKFnio7FYa7oHtsfzubXjuiv2wvTPQM0+4LlDKdjaC3Od kBsuKlDmHogab/WwY96MYRVVbPS12rYTEQOvI58uHVoLvDtMiA17U7K6r7LNbzD+Th N6dyaEph3tg5w== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 4A3BC69ABB; Thu, 12 Feb 2026 14:17:06 -0700 (MST) 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 iZisG3UIl0WX; Thu, 12 Feb 2026 14:17:06 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1770931022; bh=Mf1SGI4y877Y45kFdUYiXlo3kjr7iICM2Qz5QN2GGEI=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=hcpUuuHp21s8ZbXelsFB5hjlAcyxwlv6/3fzEoCIkrsjwJSS2uUpO5oGtY2OXPwFQ gQ4YkuxPtVQnPTTr4vUfQrA9D4J/TU/X5LDEzJlaoWehhqgOJLwIfICHFWoAgfTi7z 1fqixRfc7j5Gf+tUZdMMe18mvpq+W71xBGyY++UmzLMf6qhJdyS94Bd/3bsC/i4438 WxVrmgwi6dCXqpLziMk1lR/9cn20mMNSlyyMVlHda0oDSXfBh54ErXgL8f4VJbnZvU /Mm/juN1IVH+1Kdsd1SDLT65k2UPDJMpnjJttKc23kLcDVHEMpNDYatwlehzKsm94B vOACanrrNlEYQ== Received: from u-boot.org (unknown [73.34.74.121]) by mail.u-boot.org (Postfix) with ESMTPSA id 1B05A6994D; Thu, 12 Feb 2026 14:17:02 -0700 (MST) From: Simon Glass To: U-Boot Concept Date: Thu, 12 Feb 2026 14:16:19 -0700 Message-ID: <20260212211626.167191-7-sjg@u-boot.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20260212211626.167191-1-sjg@u-boot.org> References: <20260212211626.167191-1-sjg@u-boot.org> MIME-Version: 1.0 Message-ID-Hash: 24B4EHOYTZG2DRYEQAXZYJLNBCD3ADKX X-Message-ID-Hash: 24B4EHOYTZG2DRYEQAXZYJLNBCD3ADKX 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 , "Claude Opus 4 . 5" X-Mailman-Version: 3.3.10 Precedence: list Subject: [Concept] [PATCH 6/9] pickman: Increase agent SDK buffer size to 10MB 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 default 1MB buffer size can be exceeded when the agent processes large responses, causing a fatal error. Increase max_buffer_size to 10MB for both the cherry-pick and review agents. Co-developed-by: Claude Opus 4.5 Signed-off-by: Simon Glass --- tools/pickman/agent.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tools/pickman/agent.py b/tools/pickman/agent.py index 129743bb329..85f8efee1df 100644 --- a/tools/pickman/agent.py +++ b/tools/pickman/agent.py @@ -24,6 +24,9 @@ SIGNAL_SUCCESS = 'success' SIGNAL_APPLIED = 'already_applied' SIGNAL_CONFLICT = 'conflict' +# Maximum buffer size for agent responses +MAX_BUFFER_SIZE = 10 * 1024 * 1024 # 10MB + # Commits that need special handling (regenerate instead of cherry-pick) # These run savedefconfig on all boards and depend on target branch # Kconfig state @@ -219,6 +222,7 @@ this means the series was already applied via a different path. In this case: options = ClaudeAgentOptions( allowed_tools=['Bash', 'Read', 'Grep'], cwd=repo_path, + max_buffer_size=MAX_BUFFER_SIZE, ) tout.info(f'Starting Claude agent to cherry-pick {len(commits)} commits...') @@ -482,6 +486,7 @@ async def run_review_agent(mr_iid, branch_name, comments, remote, options = ClaudeAgentOptions( allowed_tools=['Bash', 'Read', 'Grep', 'Edit', 'Write'], cwd=repo_path, + max_buffer_size=MAX_BUFFER_SIZE, ) tout.info(f'Starting Claude agent to {task_desc}...') From patchwork Thu Feb 12 21:16:20 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 1845 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=1770931033; bh=KaMBMSS48Y4jbKzMkWA0m53iC0fFIDJtRxUczVbxFIA=; 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=gYFfXCeVwo6W9fVLCTEdqxx1b0/14xzlN41DSQyDpjLlBnWaiktJw5aUvcwrShQt3 VQuc5Ym/rloMYJsdQ4K7MWypI8/MVwmswXzSy5/5BaQ7toGpjMhmkNd6/6WQNmsLPH 3dX0CVFXNo8+d1oA/4wfKsJJ9fF4YApU13ciDuBI0SZrgN4mku6IFwIJgV4YAsm5EN QxtxjCri9u0VEmsAXP+KX6YFfRamfa+xyDsANYaRGUOGtXPo2dW9HBRijUw/Iqf8tU JEY1bAkWpX3PqdRW6jMFttwL1HYVdAht5VsZ+OOYGrPQRgpiz7/Pg+ArB/i9zQLFKm x6ErdYyKV8xhA== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 2AB7D69ABF for ; Thu, 12 Feb 2026 14:17:13 -0700 (MST) 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 t8ayOgi3AkM3 for ; Thu, 12 Feb 2026 14:17:13 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1770931033; bh=KaMBMSS48Y4jbKzMkWA0m53iC0fFIDJtRxUczVbxFIA=; 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=gYFfXCeVwo6W9fVLCTEdqxx1b0/14xzlN41DSQyDpjLlBnWaiktJw5aUvcwrShQt3 VQuc5Ym/rloMYJsdQ4K7MWypI8/MVwmswXzSy5/5BaQ7toGpjMhmkNd6/6WQNmsLPH 3dX0CVFXNo8+d1oA/4wfKsJJ9fF4YApU13ciDuBI0SZrgN4mku6IFwIJgV4YAsm5EN QxtxjCri9u0VEmsAXP+KX6YFfRamfa+xyDsANYaRGUOGtXPo2dW9HBRijUw/Iqf8tU JEY1bAkWpX3PqdRW6jMFttwL1HYVdAht5VsZ+OOYGrPQRgpiz7/Pg+ArB/i9zQLFKm x6ErdYyKV8xhA== Received: from mail.u-boot.org (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 18F8769AD8 for ; Thu, 12 Feb 2026 14:17:13 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1770931030; bh=KvMPtMskO4N2wf8jRYAhkkmXX2bNVfP2LbY5me6xyfU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=nJ53bN7X+4xkfiehVv/R696dcCXAgmik8o89LPh5m/RoCmILtQidL7JvHPCesvKPu f83DZcAjUNVL5Dy+j1jei5Uekb/U1r0A7jQWnuwfAXAqpqPd019iSMG3DpLO1cQCch sASNW0qXDy9A/2E8mV7dm7H3fwnzNJlXdFnod+IQkJMxc4rJ2RCs12P0LwY5JF73b2 r0U3EillEwGuIuxNtg9RmL8juUrZucaOykbfqjQb+zv/6E5PkaBQGK1KPIOOwPWS8t X/DjiED7cni2aW8gmb89YDQ85cEgzK0/BUOLDVGVP2VTaDF1mZCODjwH0OvmLKK9Ja 3le65QqXpfZtA== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id EF3B369ABF; Thu, 12 Feb 2026 14:17:10 -0700 (MST) 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 4lRZVKEmbDdA; Thu, 12 Feb 2026 14:17:10 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1770931026; bh=0EW9cJJMPmhrED97bWFL3ugbqoYnDSL3ynqlkWYsZtI=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=XaKtF3jKZyu/yOdBZOwmu5yxIgnajQ3jw+1aHGMDGYezIUpHml5BtyGRI8y0HLFf3 R2o8UL0lWxA3pFhZlSOA0gs9Tnbwrb2rgG21em+g7Eez7KNV+qMNjoxY969LYuhghg q+sU+Ip1m/eJ2ba2GA3f7GbdwuH1hI9E8vU6sWNYWsBEWdRzF2obcyfT3TD4BiXKk1 pTEte2PxtHTj8amXHgp3fTM8nFRuLFYY2Q/xbCUQXS+2srW+x9e5MzzSaL/x74WDE+ yfJlgp6CRc/IzZLHpx2f/AKYOhMA0utL3+Gvr9LaHhu357WMdXCU1s/VRwD2VhsQPB XU3at4pBjkPdQ== Received: from u-boot.org (unknown [73.34.74.121]) by mail.u-boot.org (Postfix) with ESMTPSA id 82EC16994D; Thu, 12 Feb 2026 14:17:06 -0700 (MST) From: Simon Glass To: U-Boot Concept Date: Thu, 12 Feb 2026 14:16:20 -0700 Message-ID: <20260212211626.167191-8-sjg@u-boot.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20260212211626.167191-1-sjg@u-boot.org> References: <20260212211626.167191-1-sjg@u-boot.org> MIME-Version: 1.0 Message-ID-Hash: 5CLAYXS6DJ4IXKXFUXKYL3YWY664KTV4 X-Message-ID-Hash: 5CLAYXS6DJ4IXKXFUXKYL3YWY664KTV4 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 , "Claude Opus 4 . 6" X-Mailman-Version: 3.3.10 Precedence: list Subject: [Concept] [PATCH 7/9] pickman: Extract find_unprocessed_commits() from get_next_commits() 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 Move the merge-walking loop into its own function to reduce the complexity of get_next_commits() Co-developed-by: Claude Opus 4.6 Signed-off-by: Simon Glass --- tools/pickman/control.py | 91 ++++++++++++++++++++++++---------------- 1 file changed, 55 insertions(+), 36 deletions(-) diff --git a/tools/pickman/control.py b/tools/pickman/control.py index cd138612623..0418fbe2da6 100644 --- a/tools/pickman/control.py +++ b/tools/pickman/control.py @@ -745,6 +745,59 @@ def do_check_gitlab(args, dbs): # pylint: disable=unused-argument return 0 +def find_unprocessed_commits(dbs, last_commit, source, merge_hashes): + """Find the first merge with unprocessed commits + + Walks through the merge hashes in order, looking for one that has + commits not yet tracked in the database. + + Args: + dbs (Database): Database instance + last_commit (str): Hash of the last cherry-picked commit + source (str): Source branch name + merge_hashes (list): List of merge commit hashes to check + + Returns: + NextCommitsInfo: Info about the next commits to process + """ + prev_commit = last_commit + for merge_hash in merge_hashes: + # Get all commits from prev_commit to this merge + log_output = run_git([ + 'log', '--reverse', '--format=%H|%h|%an|%s|%P', + f'{prev_commit}..{merge_hash}' + ]) + + if not log_output: + prev_commit = merge_hash + continue + + # Parse commits, filtering out those already in database + all_commits = parse_log_output(log_output, has_parents=True) + commits = [c for c in all_commits + if not dbs.commit_get(c.hash)] + + if commits: + return NextCommitsInfo(commits, True) + + # All commits in this merge are processed, skip to next + prev_commit = merge_hash + + # No merges with unprocessed commits, check remaining commits + log_output = run_git([ + 'log', '--reverse', '--format=%H|%h|%an|%s|%P', + f'{prev_commit}..{source}' + ]) + + if not log_output: + return NextCommitsInfo([], False) + + all_commits = parse_log_output(log_output, has_parents=True) + commits = [c for c in all_commits if not dbs.commit_get(c.hash)] + + return NextCommitsInfo(commits, False) + + def get_next_commits(dbs, source): """Get the next set of commits to cherry-pick from a source @@ -786,42 +839,8 @@ def get_next_commits(dbs, source): if len(parents) > 1: merge_hashes.append(parts[0]) - # Try each merge in order until we find one with unprocessed commits - prev_commit = last_commit - for merge_hash in merge_hashes: - # Get all commits from prev_commit to this merge - log_output = run_git([ - 'log', '--reverse', '--format=%H|%h|%an|%s|%P', - f'{prev_commit}..{merge_hash}' - ]) - - if not log_output: - prev_commit = merge_hash - continue - - # Parse commits, filtering out those already in database - all_commits = parse_log_output(log_output, has_parents=True) - commits = [c for c in all_commits if not dbs.commit_get(c.hash)] - - if commits: - return NextCommitsInfo(commits, True), None - - # All commits in this merge are processed, skip to next - prev_commit = merge_hash - - # No merges with unprocessed commits, check remaining commits - log_output = run_git([ - 'log', '--reverse', '--format=%H|%h|%an|%s|%P', - f'{prev_commit}..{source}' - ]) - - if not log_output: - return NextCommitsInfo([], False), None - - all_commits = parse_log_output(log_output, has_parents=True) - commits = [c for c in all_commits if not dbs.commit_get(c.hash)] - - return NextCommitsInfo(commits, False), None + return find_unprocessed_commits( + dbs, last_commit, source, merge_hashes), None def get_commits_for_pick(commit_spec): From patchwork Thu Feb 12 21:16:21 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 1846 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=1770931037; bh=+TpLaa6tEVDjb3TQgMt5gn93xJzQ7KGdSOeoM9wOlz0=; 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=SiybWymGlxAS3A/QeyCI6S36/V5gb9AdDzUxhOkxuyhX2QTO/r3FKLoEKEjkOHgdK Di5cSVjzxdGpKrvPcl2pJXDV0AQ4oPmqvUdaf8gbvCBkn8NMc3V4QONi9Xziv6w90F loxHQkkul5sBBuME4BFZA/QzWRUoCWMLVDUlZVp56a0WX8/UctvwOR2ouoGepk2MU4 oDiAW75AjUDJQiNsM/PuAV+po5PJbLssqCOBIcyx3XYGyzWdTSYoh+VSI/O63NwI02 0NzErvUIavU8abfDiAmE7CN8OByFrOusxBpj2abV2E/GCbqdAXdS53KLjHR85jRbXu 5qYQw2sRQTHgg== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id A0E0069AED for ; Thu, 12 Feb 2026 14:17:17 -0700 (MST) 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 WYbR3HwzfHoi for ; Thu, 12 Feb 2026 14:17:17 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1770931037; bh=+TpLaa6tEVDjb3TQgMt5gn93xJzQ7KGdSOeoM9wOlz0=; 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=SiybWymGlxAS3A/QeyCI6S36/V5gb9AdDzUxhOkxuyhX2QTO/r3FKLoEKEjkOHgdK Di5cSVjzxdGpKrvPcl2pJXDV0AQ4oPmqvUdaf8gbvCBkn8NMc3V4QONi9Xziv6w90F loxHQkkul5sBBuME4BFZA/QzWRUoCWMLVDUlZVp56a0WX8/UctvwOR2ouoGepk2MU4 oDiAW75AjUDJQiNsM/PuAV+po5PJbLssqCOBIcyx3XYGyzWdTSYoh+VSI/O63NwI02 0NzErvUIavU8abfDiAmE7CN8OByFrOusxBpj2abV2E/GCbqdAXdS53KLjHR85jRbXu 5qYQw2sRQTHgg== Received: from mail.u-boot.org (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 8F1A369AE8 for ; Thu, 12 Feb 2026 14:17:17 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1770931035; bh=pmH1zbtp0SEGeP7d9naN1tB0iyIrytZ/ChZiGNJBrP0=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=NC3iaFAlTOJWfsEkxzV+NHmCdVVF373v7vOBl2E/ZqtQLCWZHZSvf+XpeZxukdhUx HZMfZzTQfz7K8xYGSsvg1Sw5f9kC8/HskAGN/iFiiorHh0Cgn2Ki7wmucyprQ8qDUc k4KkIP9gNWDTOPuVuNO8XZcmggQ057a+BwwCXZkEBn0DTa/Hs6bjMvdTUEMXzO+n+c p88isWsCgbRnGoOvSz3G5Ma5zF9eOGo13B4uDXd6rGj1NrkoWLIfeeLq5wLS8Rhia3 f8iYjOnIYgsRITngv3/Jbf1oKg6jCs6qWQ4Qbp9zShbUBgz/E/A/mCDED6RlMgoDKz nfa7lsfhkIPug== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id E73CB69AD8; Thu, 12 Feb 2026 14:17:15 -0700 (MST) 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 hiBra0ypEYhR; Thu, 12 Feb 2026 14:17:15 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1770931031; bh=RLKIhH+gZKEp2G9HNx9UsT1ZtYFMeN1H7jZztRNWqQM=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=XzwYkux69ILDwVzD2qPVzfj6tbLkuPagGxKXWybJuqb1jKmSEHDNUEg8qV+6e+JUK qEGuJPXt5GGSTvu2LFGz4v9QXZpbhszFVCsJ7rSr+h4WiLY3W01ZwJTaFwgaDURwV3 B4BypBdbqQs20yez3aDQtuV+ZCgbNOOBtOa/19qtWKLhnA+U+sW1oCanhiKE5GK/Jx O5msSpTtPGRM+TpaqQrPD6Kc0DerWW2glvyJqPdJqegGZVTU4efL7HVMV7c91WlQUC 8DIb4aalObmOol6GcWw/vUmrosJIvFhFLnNz4kBY2Zw+GDNBT0wE7gn9B8Jk00zEqI iI5JA5id0b0vQ== Received: from u-boot.org (unknown [73.34.74.121]) by mail.u-boot.org (Postfix) with ESMTPSA id 42E476994D; Thu, 12 Feb 2026 14:17:11 -0700 (MST) From: Simon Glass To: U-Boot Concept Date: Thu, 12 Feb 2026 14:16:21 -0700 Message-ID: <20260212211626.167191-9-sjg@u-boot.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20260212211626.167191-1-sjg@u-boot.org> References: <20260212211626.167191-1-sjg@u-boot.org> MIME-Version: 1.0 Message-ID-Hash: KA6N6H5BG6FE4RYUIIUGCERDDJKG4ETW X-Message-ID-Hash: KA6N6H5BG6FE4RYUIIUGCERDDJKG4ETW 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 , "Claude Opus 4 . 6" X-Mailman-Version: 3.3.10 Precedence: list Subject: [Concept] [PATCH 8/9] pickman: Decompose mega-merges into sub-merge batches 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 When get_next_commits() encounters a large merge on the first-parent chain (e.g., "Merge branch 'next'"), it currently collects ALL commits from prev_commit..merge_hash at once. For mega-merges containing many sub-merges, this can produce hundreds of commits in a single batch, overwhelming the agent. Add detect_sub_merges() to check if a merge commit contains sub-merges on its second parent's first-parent chain. Add decompose_mega_merge() to return one sub-merge batch at a time across multiple runs, handling three phases: mainline commits, individual sub-merge batches, and remainder commits. Extract find_unprocessed_commits() from get_next_commits() to walk the merge list and find the first with unprocessed commits. Change get_next_commits() to return a 4-tuple with an advance_to field. When advance_to is a hash, the caller advances the source position to that hash. When advance_to is None, the source stays put (sub-merge batch, to be continued next run). Update ApplyInfo, prepare_apply(), execute_apply(), handle_already_applied(), do_apply(), and do_next_set() to thread advance_to through the call chain. Co-developed-by: Claude Opus 4.6 Signed-off-by: Simon Glass --- tools/pickman/control.py | 208 ++++++++++++++++-- tools/pickman/ftest.py | 461 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 640 insertions(+), 29 deletions(-) diff --git a/tools/pickman/control.py b/tools/pickman/control.py index 0418fbe2da6..2e6f49f1816 100644 --- a/tools/pickman/control.py +++ b/tools/pickman/control.py @@ -91,8 +91,9 @@ AgentCommit = namedtuple('AgentCommit', # # commits: list of CommitInfo to cherry-pick # merge_found: True if these commits came from a merge on the source branch +# advance_to: hash to advance the source position to, or None to stay put NextCommitsInfo = namedtuple('NextCommitsInfo', - ['commits', 'merge_found']) + ['commits', 'merge_found', 'advance_to']) # Named tuple for prepare_apply() result # @@ -100,9 +101,10 @@ NextCommitsInfo = namedtuple('NextCommitsInfo', # branch_name: name of the branch to create for the MR # original_branch: branch name before any conflict suffix # merge_found: True if these commits came from a merge on the source branch +# advance_to: hash to advance the source position to, or None to stay put ApplyInfo = namedtuple('ApplyInfo', ['commits', 'branch_name', 'original_branch', - 'merge_found']) + 'merge_found', 'advance_to']) def parse_log_output(log_output, has_parents=False): @@ -749,7 +751,8 @@ def find_unprocessed_commits(dbs, last_commit, source, merge_hashes): """Find the first merge with unprocessed commits Walks through the merge hashes in order, looking for one that has - commits not yet tracked in the database. + commits not yet tracked in the database. Decomposes mega-merges + (merges containing sub-merges) into individual batches. Args: dbs (Database): Database instance @@ -761,7 +764,20 @@ def find_unprocessed_commits(dbs, last_commit, source, merge_hashes): NextCommitsInfo: Info about the next commits to process """ prev_commit = last_commit + skipped_merges = False for merge_hash in merge_hashes: + # Check for mega-merge (contains sub-merges) + sub_merges = detect_sub_merges(merge_hash) + if sub_merges: + commits, advance_to = decompose_mega_merge( + dbs, prev_commit, merge_hash, sub_merges) + if commits: + return NextCommitsInfo(commits, True, advance_to) + # All sub-merges done, skip past this mega-merge + prev_commit = merge_hash + skipped_merges = True + continue + # Get all commits from prev_commit to this merge log_output = run_git([ 'log', '--reverse', '--format=%H|%h|%an|%s|%P', @@ -778,10 +794,11 @@ def find_unprocessed_commits(dbs, last_commit, source, merge_hashes): if not dbs.commit_get(c.hash)] if commits: - return NextCommitsInfo(commits, True) + return NextCommitsInfo(commits, True, merge_hash) # All commits in this merge are processed, skip to next prev_commit = merge_hash + skipped_merges = True # No merges with unprocessed commits, check remaining commits log_output = run_git([ @@ -790,12 +807,14 @@ def find_unprocessed_commits(dbs, last_commit, source, merge_hashes): ]) if not log_output: - return NextCommitsInfo([], False) + # If we skipped merges, advance past them + advance_to = prev_commit if skipped_merges else None + return NextCommitsInfo([], False, advance_to) all_commits = parse_log_output(log_output, has_parents=True) commits = [c for c in all_commits if not dbs.commit_get(c.hash)] - return NextCommitsInfo(commits, False) + return NextCommitsInfo(commits, False, None) def get_next_commits(dbs, source): @@ -804,7 +823,8 @@ def get_next_commits(dbs, source): Finds commits between the last cherry-picked commit and the next merge commit on the first-parent (mainline) chain of the source branch. Skips merges whose commits are already tracked in the database (from - pending MRs). + pending MRs). Decomposes mega-merges (merges containing sub-merges) + into individual sub-merge batches. Args: dbs (Database): Database instance @@ -827,7 +847,7 @@ def get_next_commits(dbs, source): ]) if not fp_output: - return NextCommitsInfo([], False), None + return NextCommitsInfo([], False, None), None # Build list of merge hashes on the first-parent chain merge_hashes = [] @@ -903,6 +923,130 @@ def get_commits_for_pick(commit_spec): return commits, err +def detect_sub_merges(merge_hash): + """Check if a merge commit contains sub-merges + + Examines the second parent's first-parent chain to find merge commits + (sub-merges) within a larger merge. + + Args: + merge_hash (str): Hash of the merge commit to check + + Returns: + list: List of sub-merge hashes in chronological order, or empty + list if not a merge or has no sub-merges + """ + # Get parents of the merge + try: + parents = run_git(['rev-parse', f'{merge_hash}^@']) + except command.CommandExc: + return [] + + parent_list = parents.strip().split('\n') + if len(parent_list) < 2: + return [] + + first_parent = parent_list[0] + second_parent = parent_list[1] + + # Find merges on the second parent's first-parent chain + try: + out = run_git([ + 'log', '--reverse', '--first-parent', '--merges', + '--format=%H', f'^{first_parent}', second_parent + ]) + except command.CommandExc: + return [] + + if not out: + return [] + + return [line for line in out.split('\n') if line] + + +def decompose_mega_merge(dbs, prev_commit, merge_hash, sub_merges): + """Return the next unprocessed batch from a mega-merge + + Handles three phases: + 1. Mainline commits before the merge (prev_commit..merge^1) + 2. Sub-merge batches (one at a time, skipping processed ones) + 3. Remainder commits after the last sub-merge + + Pre-adds the mega-merge commit itself to DB as 'skipped' so it does + not appear as an orphan commit. + + Args: + dbs (Database): Database instance + prev_commit (str): Hash of the last processed commit + merge_hash (str): Hash of the mega-merge commit + sub_merges (list): List of sub-merge hashes in chronological order + + Returns: + tuple: (commits, advance_to) where: + commits: list of CommitInfo tuples for the next batch + advance_to: hash to advance source to, or None to stay put + """ + parents = run_git(['rev-parse', f'{merge_hash}^@']).strip().split('\n') + first_parent = parents[0] + second_parent = parents[1] + + # Pre-add the mega-merge commit itself as skipped + if not dbs.commit_get(merge_hash): + source_id = None + sources = dbs.source_get_all() + if sources: + source_id = dbs.source_get_id(sources[0][0]) + if source_id: + info = run_git(['log', '-1', '--format=%s|%an', merge_hash]) + parts = info.split('|', 1) + subject = parts[0] + author = parts[1] if len(parts) > 1 else '' + dbs.commit_add(merge_hash, source_id, subject, author, + status='skipped') + dbs.commit() + + # Phase 1: mainline commits before the merge + log_output = run_git([ + 'log', '--reverse', '--format=%H|%h|%an|%s|%P', + f'{prev_commit}..{first_parent}' + ]) + if log_output: + all_commits = parse_log_output(log_output, has_parents=True) + commits = [c for c in all_commits if not dbs.commit_get(c.hash)] + if commits: + return commits, first_parent + + # Phase 2: sub-merge batches + prev_sub = first_parent + for sub_hash in sub_merges: + # Get commits for this sub-merge + log_output = run_git([ + 'log', '--reverse', '--format=%H|%h|%an|%s|%P', + f'^{prev_sub}', sub_hash + ]) + if log_output: + all_commits = parse_log_output(log_output, has_parents=True) + commits = [c for c in all_commits if not dbs.commit_get(c.hash)] + if commits: + return commits, None + prev_sub = sub_hash + + # Phase 3: remainder after the last sub-merge + last_sub = sub_merges[-1] if sub_merges else first_parent + log_output = run_git([ + 'log', '--reverse', '--format=%H|%h|%an|%s|%P', + f'^{last_sub}', second_parent + ]) + if log_output: + all_commits = parse_log_output(log_output, has_parents=True) + commits = [c for c in all_commits if not dbs.commit_get(c.hash)] + if commits: + return commits, None + + # All done + return [], None + + def do_next_set(args, dbs): """Show the next set of commits to cherry-pick from a source @@ -1279,6 +1423,14 @@ def prepare_apply(dbs, source, branch): return None, 1 if not info.commits: + # If advance_to is set, advance source past fully-processed merges + if info.advance_to: + dbs.source_set(source, info.advance_to) + dbs.commit() + tout.info(f"Advanced source '{source}' to " + f'{info.advance_to[:12]}') + # Retry with updated position + return prepare_apply(dbs, source, branch) tout.info('No new commits to cherry-pick') return None, 0 @@ -1313,12 +1465,12 @@ def prepare_apply(dbs, source, branch): tout.info('') return ApplyInfo(commits, branch_name, original_branch, - info.merge_found), 0 + info.merge_found, info.advance_to), 0 # pylint: disable=too-many-arguments def handle_already_applied(dbs, source, commits, branch_name, conv_log, args, - signal_commit): + signal_commit, advance_to=None): """Handle the case where commits are already applied to the target branch Creates an MR with [skip] prefix to record the attempt and updates the @@ -1332,6 +1484,9 @@ def handle_already_applied(dbs, source, commits, branch_name, conv_log, args, conv_log (str): Conversation log from the agent args (Namespace): Parsed arguments with 'push', 'remote', 'target' signal_commit (str): Last commit hash from signal file + advance_to (str): Hash to advance source to, or None to use last + commit. If explicitly None (sub-merge batch), source is not + advanced. Returns: int: 0 on success, 1 on failure @@ -1343,11 +1498,20 @@ def handle_already_applied(dbs, source, commits, branch_name, conv_log, args, dbs.commit_set_status(commit.hash, 'skipped') dbs.commit() - # Update source position to the last commit (or signal_commit if provided) - last_hash = signal_commit if signal_commit else commits[-1].hash - dbs.source_set(source, last_hash) - dbs.commit() - tout.info(f"Updated source '{source}' to {last_hash[:12]}") + # Update source position + if advance_to is not None: + dbs.source_set(source, advance_to) + dbs.commit() + tout.info(f"Updated source '{source}' to {advance_to[:12]}") + elif signal_commit: + dbs.source_set(source, signal_commit) + dbs.commit() + tout.info(f"Updated source '{source}' to {signal_commit[:12]}") + else: + last_hash = commits[-1].hash + dbs.source_set(source, last_hash) + dbs.commit() + tout.info(f"Updated source '{source}' to {last_hash[:12]}") # Push and create MR with [skip] prefix if requested if args.push: @@ -1382,7 +1546,7 @@ def handle_already_applied(dbs, source, commits, branch_name, conv_log, args, return 0 -def execute_apply(dbs, source, commits, branch_name, args): # pylint: disable=too-many-locals +def execute_apply(dbs, source, commits, branch_name, args, advance_to=None): # pylint: disable=too-many-locals """Execute the apply operation: run agent, update database, push MR Args: @@ -1391,6 +1555,8 @@ def execute_apply(dbs, source, commits, branch_name, args): # pylint: disable=t commits (list): List of CommitInfo namedtuples branch_name (str): Branch name for cherry-picks args (Namespace): Parsed arguments with 'push', 'remote', 'target' + advance_to (str): Hash to advance source to after success, or None + to skip source advancement (sub-merge batch) Returns: tuple: (ret, success, conv_log) where ret is 0 on success, @@ -1416,7 +1582,8 @@ def execute_apply(dbs, source, commits, branch_name, args): # pylint: disable=t signal_status, signal_commit = agent.read_signal_file() if signal_status == agent.SIGNAL_APPLIED: ret = handle_already_applied(dbs, source, commits, branch_name, - conv_log, args, signal_commit) + conv_log, args, signal_commit, + advance_to) return ret, False, conv_log # Verify the branch actually exists - agent may have aborted and deleted it @@ -1449,8 +1616,8 @@ def execute_apply(dbs, source, commits, branch_name, args): # pylint: disable=t f"{commits[-1].chash}' to update the database") # Update database with the last processed commit if successful - if success: - dbs.source_set(source, commits[-1].hash) + if success and advance_to is not None: + dbs.source_set(source, advance_to) dbs.commit() return ret, success, conv_log @@ -1476,7 +1643,8 @@ def do_apply(args, dbs): original_branch = info.original_branch ret, success, conv_log = execute_apply(dbs, source, commits, - branch_name, args) + branch_name, args, + info.advance_to) # Write history file if successful if success: diff --git a/tools/pickman/ftest.py b/tools/pickman/ftest.py index 38e5cef5306..eb69bde96a4 100644 --- a/tools/pickman/ftest.py +++ b/tools/pickman/ftest.py @@ -951,8 +951,14 @@ class TestNextSet(unittest.TestCase): def mock_git(pipe_list): cmd = pipe_list[0] if pipe_list else [] + if '--first-parent' in cmd and '--merges' in cmd: + # detect_sub_merges: no sub-merges + return command.CommandResult(stdout='') if '--first-parent' in cmd: return command.CommandResult(stdout=fp_log_output) + if 'rev-parse' in cmd: + # detect_sub_merges: return two parents (it's a merge) + return command.CommandResult(stdout='bbb222\nddd444\n') return command.CommandResult(stdout=full_log_output) command.TEST_RESULT = mock_git @@ -1235,8 +1241,14 @@ class TestGetNextCommits(unittest.TestCase): def mock_git(pipe_list): cmd = pipe_list[0] if pipe_list else [] + if '--first-parent' in cmd and '--merges' in cmd: + # detect_sub_merges: no sub-merges + return command.CommandResult(stdout='') if '--first-parent' in cmd: return command.CommandResult(stdout=fp_log_output) + if 'rev-parse' in cmd: + # detect_sub_merges: return parents + return command.CommandResult(stdout='aaa111\nccc333\n') return command.CommandResult(stdout=full_log_output) command.TEST_RESULT = mock_git @@ -2965,18 +2977,22 @@ class TestGetNextCommitsEmptyLine(unittest.TestCase): 'merge2|merge2m|Author 4|Second merge|ccc333 side2\n' ) - call_count = [0] - - # pylint: disable=unused-argument def mock_git(pipe_list): - call_count[0] += 1 - # First call: get first-parent log - if call_count[0] == 1: + cmd = pipe_list[0] if pipe_list else [] + if '--first-parent' in cmd and '--merges' in cmd: + # detect_sub_merges: no sub-merges + return command.CommandResult(stdout='') + if '--first-parent' in cmd: return command.CommandResult(stdout=fp_log) - # Second call: get commits for first merge - if call_count[0] == 2: + if 'rev-parse' in cmd: + # detect_sub_merges: return parents for merges + return command.CommandResult(stdout='aaa111\nside1\n') + # Determine which merge range by checking the cmd args + cmd_str = ' '.join(cmd) + if 'merge1' in cmd_str and 'abc123' in cmd_str: return command.CommandResult(stdout=merge1_log) - # Third call: get commits for second merge + if 'merge2' in cmd_str and 'merge1' in cmd_str: + return command.CommandResult(stdout=merge2_log) return command.CommandResult(stdout=merge2_log) command.TEST_RESULT = mock_git @@ -2991,6 +3007,433 @@ class TestGetNextCommitsEmptyLine(unittest.TestCase): dbs.close() +class TestDetectSubMerges(unittest.TestCase): + """Tests for detect_sub_merges function.""" + + def tearDown(self): + """Clean up test fixtures.""" + command.TEST_RESULT = None + + def test_not_a_merge(self): + """Test detect_sub_merges returns empty for non-merge commit.""" + # Single parent means not a merge + command.TEST_RESULT = command.CommandResult(stdout='abc123\n') + result = control.detect_sub_merges('abc123') + self.assertEqual(result, []) + + def test_no_sub_merges(self): + """Test detect_sub_merges returns empty when no sub-merges exist.""" + call_count = [0] + + def mock_git(pipe_list): # pylint: disable=unused-argument + call_count[0] += 1 + if call_count[0] == 1: + # rev-parse ^@ returns two parents (it's a merge) + return command.CommandResult(stdout='parent1\nparent2\n') + # log --merges returns empty (no sub-merges) + return command.CommandResult(stdout='') + + command.TEST_RESULT = mock_git + result = control.detect_sub_merges('merge123') + self.assertEqual(result, []) + + def test_found_sub_merges(self): + """Test detect_sub_merges finds sub-merges.""" + call_count = [0] + + def mock_git(pipe_list): # pylint: disable=unused-argument + call_count[0] += 1 + if call_count[0] == 1: + # rev-parse ^@ returns two parents + return command.CommandResult(stdout='parent1\nparent2\n') + # log --merges returns sub-merge hashes + return command.CommandResult( + stdout='sub_merge1\nsub_merge2\nsub_merge3\n') + + command.TEST_RESULT = mock_git + result = control.detect_sub_merges('mega_merge') + self.assertEqual(result, ['sub_merge1', 'sub_merge2', 'sub_merge3']) + + def test_error_handling(self): + """Test detect_sub_merges returns empty on git error.""" + def mock_git_fail(**_kwargs): + raise command.CommandExc('git error', command.CommandResult()) + + command.TEST_RESULT = mock_git_fail + result = control.detect_sub_merges('bad_hash') + self.assertEqual(result, []) + + +class TestDecomposeMegaMerge(unittest.TestCase): + """Tests for decompose_mega_merge function.""" + + 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() + command.TEST_RESULT = None + + def test_first_batch_mainline(self): + """Test decompose returns mainline commits first.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + dbs.source_set('us/next', 'base') + dbs.commit() + + call_count = [0] + + def mock_git(pipe_list): # pylint: disable=unused-argument + call_count[0] += 1 + if call_count[0] == 1: + # rev-parse ^@ for mega-merge parents + return command.CommandResult( + stdout='first_parent\nsecond_parent\n') + if call_count[0] == 2: + # log -1 for mega-merge subject/author (pre-add) + return command.CommandResult( + stdout='Mega merge subject|Author\n') + if call_count[0] == 3: + # Mainline commits (prev..first_parent) + return command.CommandResult( + stdout='aaa|aaa1|A|Mainline commit|base\n') + return command.CommandResult(stdout='') + + command.TEST_RESULT = mock_git + + commits, advance_to = control.decompose_mega_merge( + dbs, 'base', 'mega_hash', ['sub1', 'sub2']) + + self.assertEqual(len(commits), 1) + self.assertEqual(commits[0].chash, 'aaa1') + self.assertEqual(advance_to, 'first_parent') + dbs.close() + + def test_sub_merge_batch(self): + """Test decompose returns sub-merge batch when mainline is done.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + dbs.source_set('us/next', 'base') + dbs.commit() + + call_count = [0] + + def mock_git(pipe_list): # pylint: disable=unused-argument + call_count[0] += 1 + if call_count[0] == 1: + # rev-parse ^@ for mega-merge parents + return command.CommandResult( + stdout='first_parent\nsecond_parent\n') + if call_count[0] == 2: + # log -1 for mega-merge subject/author + return command.CommandResult( + stdout='Mega merge|Author\n') + if call_count[0] == 3: + # Mainline commits - empty (already processed) + return command.CommandResult(stdout='') + if call_count[0] == 4: + # Sub-merge 1 commits + return command.CommandResult( + stdout='bbb|bbb1|B|Sub commit 1|first_parent\n' + 'ccc|ccc1|C|Sub commit 2|bbb\n') + return command.CommandResult(stdout='') + + command.TEST_RESULT = mock_git + + commits, advance_to = control.decompose_mega_merge( + dbs, 'base', 'mega_hash', ['sub1', 'sub2']) + + self.assertEqual(len(commits), 2) + self.assertEqual(commits[0].chash, 'bbb1') + self.assertIsNone(advance_to) + dbs.close() + + def test_skips_processed_sub_merge(self): + """Test decompose skips sub-merges already in database.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + dbs.source_set('us/next', 'base') + dbs.commit() + + # Add sub-merge 1 commits to database + source_id = dbs.source_get_id('us/next') + dbs.commit_add('bbb', source_id, 'Sub commit 1', 'B', + status='applied') + dbs.commit() + + call_count = [0] + + def mock_git(pipe_list): # pylint: disable=unused-argument + call_count[0] += 1 + if call_count[0] == 1: + return command.CommandResult( + stdout='first_parent\nsecond_parent\n') + if call_count[0] == 2: + return command.CommandResult( + stdout='Mega merge|Author\n') + if call_count[0] == 3: + # Mainline - empty + return command.CommandResult(stdout='') + if call_count[0] == 4: + # Sub-merge 1 commits (already in DB) + return command.CommandResult( + stdout='bbb|bbb1|B|Sub commit 1|first_parent\n') + if call_count[0] == 5: + # Sub-merge 2 commits (not in DB) + return command.CommandResult( + stdout='ddd|ddd1|D|Sub commit 3|sub1\n') + return command.CommandResult(stdout='') + + command.TEST_RESULT = mock_git + + commits, advance_to = control.decompose_mega_merge( + dbs, 'base', 'mega_hash', ['sub1', 'sub2']) + + self.assertEqual(len(commits), 1) + self.assertEqual(commits[0].chash, 'ddd1') + self.assertIsNone(advance_to) + dbs.close() + + def test_all_done(self): + """Test decompose returns empty when all sub-merges are processed.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + dbs.source_set('us/next', 'base') + dbs.commit() + + # Add all commits to database + source_id = dbs.source_get_id('us/next') + dbs.commit_add('bbb', source_id, 'Sub commit 1', 'B', + status='applied') + dbs.commit_add('ddd', source_id, 'Sub commit 2', 'D', + status='applied') + dbs.commit() + + call_count = [0] + + def mock_git(pipe_list): # pylint: disable=too-many-return-statements,unused-argument + call_count[0] += 1 + if call_count[0] == 1: + return command.CommandResult( + stdout='first_parent\nsecond_parent\n') + if call_count[0] == 2: + return command.CommandResult( + stdout='Mega merge|Author\n') + if call_count[0] == 3: + # Mainline - empty + return command.CommandResult(stdout='') + if call_count[0] == 4: + # Sub-merge 1 + return command.CommandResult( + stdout='bbb|bbb1|B|Sub commit 1|first_parent\n') + if call_count[0] == 5: + # Sub-merge 2 + return command.CommandResult( + stdout='ddd|ddd1|D|Sub commit 2|sub1\n') + if call_count[0] == 6: + # Remainder - empty + return command.CommandResult(stdout='') + return command.CommandResult(stdout='') + + command.TEST_RESULT = mock_git + + commits, advance_to = control.decompose_mega_merge( + dbs, 'base', 'mega_hash', ['sub1', 'sub2']) + + self.assertEqual(len(commits), 0) + self.assertIsNone(advance_to) + dbs.close() + + +class TestGetNextCommitsMegaMerge(unittest.TestCase): + """Tests for get_next_commits with mega-merges.""" + + 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() + command.TEST_RESULT = None + + def test_returns_sub_batch(self): + """Test get_next_commits returns sub-merge batch for mega-merge.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + dbs.source_set('us/next', 'base') + dbs.commit() + + call_count = [0] + + def mock_git(pipe_list): # pylint: disable=too-many-return-statements,unused-argument + call_count[0] += 1 + if call_count[0] == 1: + # First-parent log shows one mega-merge + return command.CommandResult( + stdout='mega|mega1|A|Merge branch next|' + 'base second_parent\n') + if call_count[0] == 2: + # detect_sub_merges: rev-parse ^@ + return command.CommandResult( + stdout='base\nsecond_parent\n') + if call_count[0] == 3: + # detect_sub_merges: log --merges (found sub-merges) + return command.CommandResult(stdout='sub1\n') + if call_count[0] == 4: + # decompose: rev-parse ^@ for mega-merge + return command.CommandResult( + stdout='base\nsecond_parent\n') + if call_count[0] == 5: + # decompose: log -1 for mega-merge info + return command.CommandResult( + stdout='Mega merge|Author\n') + if call_count[0] == 6: + # decompose: mainline commits (empty) + return command.CommandResult(stdout='') + if call_count[0] == 7: + # decompose: sub-merge 1 commits + return command.CommandResult( + stdout='aaa|aaa1|A|Sub commit|base\n') + return command.CommandResult(stdout='') + + command.TEST_RESULT = mock_git + + info, err = control.get_next_commits(dbs, 'us/next') + + self.assertIsNone(err) + self.assertTrue(info.merge_found) + self.assertEqual(len(info.commits), 1) + self.assertEqual(info.commits[0].chash, 'aaa1') + # Sub-merge batch: advance_to should be None + self.assertIsNone(info.advance_to) + dbs.close() + + def test_all_done_advances_past(self): + """Test get_next_commits advances past fully-processed mega-merge.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + dbs.source_set('us/next', 'base') + dbs.commit() + + # Add all sub-merge commits to database + source_id = dbs.source_get_id('us/next') + dbs.commit_add('aaa', source_id, 'Sub commit', 'A', + status='applied') + dbs.commit() + + call_count = [0] + + def mock_git(pipe_list): # pylint: disable=too-many-return-statements,unused-argument + call_count[0] += 1 + if call_count[0] == 1: + # First-parent log shows mega-merge + return command.CommandResult( + stdout='mega|mega1|A|Merge branch next|' + 'base second_parent\n') + if call_count[0] == 2: + # detect_sub_merges: rev-parse ^@ + return command.CommandResult( + stdout='base\nsecond_parent\n') + if call_count[0] == 3: + # detect_sub_merges: log --merges + return command.CommandResult(stdout='sub1\n') + if call_count[0] == 4: + # decompose: rev-parse ^@ + return command.CommandResult( + stdout='base\nsecond_parent\n') + if call_count[0] == 5: + # decompose: log -1 for mega-merge info + return command.CommandResult( + stdout='Mega merge|Author\n') + if call_count[0] == 6: + # decompose: mainline (empty) + return command.CommandResult(stdout='') + if call_count[0] == 7: + # decompose: sub-merge 1 (in DB) + return command.CommandResult( + stdout='aaa|aaa1|A|Sub commit|base\n') + if call_count[0] == 8: + # decompose: remainder (empty) + return command.CommandResult(stdout='') + if call_count[0] == 9: + # Remaining commits after mega-merge (empty) + return command.CommandResult(stdout='') + return command.CommandResult(stdout='') + + command.TEST_RESULT = mock_git + + info, err = control.get_next_commits(dbs, 'us/next') + + self.assertIsNone(err) + self.assertFalse(info.merge_found) + self.assertEqual(len(info.commits), 0) + # Should advance past the mega-merge + self.assertEqual(info.advance_to, 'mega') + dbs.close() + + def test_normal_merge_returns_advance_to(self): + """Test get_next_commits returns advance_to for normal merges.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + dbs.source_set('us/next', 'base') + dbs.commit() + + call_count = [0] + + def mock_git(pipe_list): # pylint: disable=unused-argument + call_count[0] += 1 + if call_count[0] == 1: + # First-parent log shows a normal merge + return command.CommandResult( + stdout='merge1|m1|A|Merge branch feat|' + 'base side1\n') + if call_count[0] == 2: + # detect_sub_merges: rev-parse ^@ + return command.CommandResult( + stdout='base\nside1\n') + if call_count[0] == 3: + # detect_sub_merges: log --merges (no sub-merges) + return command.CommandResult(stdout='') + if call_count[0] == 4: + # Commits for this merge + return command.CommandResult( + stdout='aaa|aaa1|A|Commit 1|base\n' + 'merge1|m1|A|Merge branch feat|' + 'base side1\n') + return command.CommandResult(stdout='') + + command.TEST_RESULT = mock_git + + info, err = control.get_next_commits(dbs, 'us/next') + + self.assertIsNone(err) + self.assertTrue(info.merge_found) + self.assertEqual(len(info.commits), 2) + # Normal merge: advance_to is the merge hash + self.assertEqual(info.advance_to, 'merge1') + dbs.close() + + class TestDoCommitSourceResolveError(unittest.TestCase): """Tests for do_commit_source error handling.""" From patchwork Thu Feb 12 21:16:22 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 1847 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=1770931043; bh=KxYyJPmLIJkRoJGtJzKIMS3dDm24Gj7IHSdX8Gua3rM=; 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=fiJrIH+Xdg27RmJ+2cz2re5TmtrjZTE3BxbPR8fEjDB/yjipoZJE5BXMeD6VbDrA9 O/P8pUEDo5epPZYE1WRPOH6xzW0bYaTW3GwfqLaITg/5mMx+c3ipzdFfzlbieQS1hH PVs4zd+dV9QDUy6hsEoV0VfZtTAn6yFeBP7QJo9nAApVGEEgHj2sExiyXup+zxtIyX fzYm6rBA8tTcAAKxiyk0bZgt/PrmG84cB6gl/ch2Bz7UtcGam20kReHMHVGrA7ckns 0yUxKh3jYeD04Vpug44RwNYc5sMvpwjR3BdG/PGrS7VUZY7fN7siHqw24Gxt98i5N5 7djMlpsZD7Esg== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 2483A69AED for ; Thu, 12 Feb 2026 14:17:23 -0700 (MST) 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 jWTQjATPT6wA for ; Thu, 12 Feb 2026 14:17:23 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1770931043; bh=KxYyJPmLIJkRoJGtJzKIMS3dDm24Gj7IHSdX8Gua3rM=; 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=fiJrIH+Xdg27RmJ+2cz2re5TmtrjZTE3BxbPR8fEjDB/yjipoZJE5BXMeD6VbDrA9 O/P8pUEDo5epPZYE1WRPOH6xzW0bYaTW3GwfqLaITg/5mMx+c3ipzdFfzlbieQS1hH PVs4zd+dV9QDUy6hsEoV0VfZtTAn6yFeBP7QJo9nAApVGEEgHj2sExiyXup+zxtIyX fzYm6rBA8tTcAAKxiyk0bZgt/PrmG84cB6gl/ch2Bz7UtcGam20kReHMHVGrA7ckns 0yUxKh3jYeD04Vpug44RwNYc5sMvpwjR3BdG/PGrS7VUZY7fN7siHqw24Gxt98i5N5 7djMlpsZD7Esg== Received: from mail.u-boot.org (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 1117169AD8 for ; Thu, 12 Feb 2026 14:17:23 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1770931040; bh=+qzFMSF7QFQIxl3Jwfz9xGVN70//HIyZ1y6DhRzFDe4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Bz/GEKjLh6F3mrm2R23egJ9kRUhonP/Sx5S5kO2p4SmtnGR4zpqMGa0GeIBg08ixZ jhOAJETfzuQPCwuXBfx5/5Fo8qczDUUx6RCvZ53cQ/6ovlVtxn2My9GsYw88ul9s0Z /kgFGWjlPUaNNhl9RltPzM3vgh0l8rqIbe/lEsm/xh8cRSi+wpCCQcWTMO2r+mdAAC MeKkjF7ng+jZ2ciCLwX7v6GzRM9mJ2nPpf4ET+huuEiiYpm3tF1jFUvJqL+cbToYIl CRcwzk6XMWQzlKp2Mz3dotJNsRJYdtnaNgvzSwGd99qBGsTCqyLpAsAHwZcduaI0wl zSL5PkfrvupPA== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id D8DA069AD8; Thu, 12 Feb 2026 14:17:20 -0700 (MST) 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 8CdcDoVTYpNQ; Thu, 12 Feb 2026 14:17:20 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1770931036; bh=wLpEobOoGWhoQ78GJSkMRvgYFoQMR/0H0UC9NJZAyuc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=OOQimi2EOKZ56bqXLb2lE3Jl+DrFGyxUWStBrw9aRaU0FbTWwbCTZRilNpGtUUQGj 9GjCgm1WIUOHiTz+uQm183LMOziwTw8RPgWdEXAVCD6enuT1cXbJAr7ThM93J4vPs6 f6mm8DAzHbguZ0vrAux5DVKHaSIQSaOAgw6rmmU2hGNubCW0tF7GvgBp08cC7ivcCL UR0cGbGSl2WG/XSOAxnczIoLuwO0E2ohcmRZfxpnBLzPOEl0QQ2AaXbnMXMpVBuYSk s89lRmF6T1TIJYT6W6aQ0hHDBGWwXPZO3LfNbA9fcnUrf8p2WrgnjKL1KvESW749DU xF3bwabNPHyPg== Received: from u-boot.org (unknown [73.34.74.121]) by mail.u-boot.org (Postfix) with ESMTPSA id 29BFA6994D; Thu, 12 Feb 2026 14:17:16 -0700 (MST) From: Simon Glass To: U-Boot Concept Date: Thu, 12 Feb 2026 14:16:22 -0700 Message-ID: <20260212211626.167191-10-sjg@u-boot.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20260212211626.167191-1-sjg@u-boot.org> References: <20260212211626.167191-1-sjg@u-boot.org> MIME-Version: 1.0 Message-ID-Hash: O7UPAOSQDQAFLJ5F6EMS5VOAXSFFZD2C X-Message-ID-Hash: O7UPAOSQDQAFLJ5F6EMS5VOAXSFFZD2C 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 , "Claude Opus 4 . 6" X-Mailman-Version: 3.3.10 Precedence: list Subject: [Concept] [PATCH 9/9] pickman: Add rewind command for stepping backwards 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 Add a 'rewind' command that moves the source position back by N merges, deleting the corresponding commits from the database. This is useful when a cherry-pick run needs to be retried from an earlier point. By default the command performs a dry run, showing what would happen without modifying the database. Use --force to actually execute the rewind. The command also identifies cherry-pick branches on the remote that correspond to the rewound range, so the user knows which MRs to delete on GitLab. Co-developed-by: Claude Opus 4.6 Signed-off-by: Simon Glass --- tools/pickman/__main__.py | 10 + tools/pickman/control.py | 214 ++++++++++++++++-- tools/pickman/database.py | 8 + tools/pickman/ftest.py | 425 ++++++++++++++++++++++++++++++++++-- tools/pickman/gitlab_api.py | 22 +- 5 files changed, 643 insertions(+), 36 deletions(-) diff --git a/tools/pickman/__main__.py b/tools/pickman/__main__.py index effb74a1db2..7814fd0fedc 100755 --- a/tools/pickman/__main__.py +++ b/tools/pickman/__main__.py @@ -100,6 +100,16 @@ def add_main_commands(subparsers): review_cmd.add_argument('-r', '--remote', default='ci', help='Git remote (default: ci)') + rewind_cmd = subparsers.add_parser( + 'rewind', help='Rewind source position back by N merges') + rewind_cmd.add_argument('source', help='Source branch name') + rewind_cmd.add_argument('-c', '--count', type=int, default=1, + help='Number of merges to rewind (default: 1)') + rewind_cmd.add_argument('-f', '--force', action='store_true', + help='Actually execute (default is dry run)') + rewind_cmd.add_argument('-r', '--remote', default='ci', + help='Git remote for MR lookup (default: ci)') + step_cmd = subparsers.add_parser('step', help='Create MR if none pending') step_cmd.add_argument('source', help='Source branch name') diff --git a/tools/pickman/control.py b/tools/pickman/control.py index 2e6f49f1816..8f7568e47bf 100644 --- a/tools/pickman/control.py +++ b/tools/pickman/control.py @@ -1124,9 +1124,46 @@ def do_next_merges(args, dbs): if len(merges) >= count: break - tout.info(f'Next {len(merges)} merges from {source}:') - for i, (_, chash, subject) in enumerate(merges, 1): - tout.info(f' {i}. {chash} {subject}') + # Build display list, expanding mega-merges into sub-merges + # Each entry is (chash, subject, is_mega, sub_list) where sub_list + # is a list of (chash, subject) for mega-merge sub-merges + display = [] + total_sub = 0 + for commit_hash, chash, subject in merges: + sub_merges = detect_sub_merges(commit_hash) + if sub_merges: + sub_list = [] + for sub_hash in sub_merges: + try: + info = run_git( + ['log', '-1', '--format=%h|%s', sub_hash]) + parts = info.strip().split('|', 1) + sub_chash = parts[0] + sub_subject = parts[1] if len(parts) > 1 else '' + except Exception: # pylint: disable=broad-except + sub_chash = sub_hash[:11] + sub_subject = '(unknown)' + sub_list.append((sub_chash, sub_subject)) + display.append((chash, subject, True, sub_list)) + total_sub += len(sub_list) + else: + display.append((chash, subject, False, None)) + + n_items = total_sub + len(merges) - len( + [d for d in display if d[2]]) + tout.info(f'Next merges from {source} ' + f'({n_items} from {len(merges)} first-parent):') + idx = 1 + for chash, subject, is_mega, sub_list in display: + if is_mega: + tout.info(f' {chash} {subject} ' + f'({len(sub_list)} sub-merges):') + for sub_chash, sub_subject in sub_list: + tout.info(f' {idx}. {sub_chash} {sub_subject}') + idx += 1 + else: + tout.info(f' {idx}. {chash} {subject}') + idx += 1 return 0 @@ -1446,12 +1483,9 @@ def prepare_apply(dbs, source, branch): branch_name = f'cherry-{commits[0].chash}' # Delete branch if it already exists - try: - run_git(['rev-parse', '--verify', branch_name]) + if run_git(['branch', '--list', branch_name]).strip(): 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 info.merge_found: tout.info(f'Applying next set from {source} ({len(commits)} commits):') @@ -1589,8 +1623,10 @@ def execute_apply(dbs, source, commits, branch_name, args, advance_to=None): # # Verify the branch actually exists - agent may have aborted and deleted it if success: try: - run_git(['rev-parse', '--verify', branch_name]) + exists = run_git(['branch', '--list', branch_name]).strip() except Exception: # pylint: disable=broad-except + exists = '' + if not exists: tout.warning(f'Branch {branch_name} does not exist - ' 'agent may have aborted') success = False @@ -1693,12 +1729,9 @@ def do_pick(args, dbs): # pylint: disable=unused-argument,too-many-locals branch_name = f'pick-{commits[0].chash}' # Delete branch if it already exists - try: - run_git(['rev-parse', '--verify', branch_name]) + if run_git(['branch', '--list', branch_name]).strip(): 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 tout.info(f'Cherry-picking {len(commits)} commit(s):') tout.info(f' Branch: {branch_name}') @@ -1717,8 +1750,10 @@ def do_pick(args, dbs): # pylint: disable=unused-argument,too-many-locals # Verify the branch actually exists - agent may have aborted and deleted it if success: try: - run_git(['rev-parse', '--verify', branch_name]) + exists = run_git(['branch', '--list', branch_name]).strip() except Exception: # pylint: disable=broad-except + exists = '' + if not exists: tout.warning(f'Branch {branch_name} does not exist - ' 'agent may have aborted') success = False @@ -1801,6 +1836,154 @@ def do_commit_source(args, dbs): return 0 +def do_rewind(args, dbs): + """Rewind the source position back by N merges + + By default performs a dry run, showing what would happen. Use --force + to actually execute the rewind. + + Walks back N merges on the first-parent chain from the current source + position, deletes the commits in that range from the database, and + resets the source to the earlier position. + + Args: + args (Namespace): Parsed arguments with 'source', 'count', 'force' + dbs (Database): Database instance + + Returns: + int: 0 on success, 1 on failure + """ + source = args.source + count = args.count + force = args.force + + current = dbs.source_get(source) + if not current: + tout.error(f"Source '{source}' not found in database") + return 1 + + # We need to find merges *before* current. Use ancestry instead. + try: + out = run_git([ + 'log', '--first-parent', '--merges', '--format=%H|%h|%s', + f'-{count + 1}', current + ]) + except Exception: # pylint: disable=broad-except + tout.error(f'Could not read merge history for {current[:12]}') + return 1 + + if not out: + tout.error('No merges found in history') + return 1 + + # Parse merges - first line is current (or nearest merge), last is target + merges = [] + for line in out.strip().split('\n'): + if not line: + continue + parts = line.split('|', 2) + merges.append((parts[0], parts[1], parts[2] if len(parts) > 2 else '')) + + if len(merges) < 2: + tout.error(f'Not enough merges to rewind by {count}') + return 1 + + # The target is count merges back from the first entry + target_idx = min(count, len(merges) - 1) + target_hash = merges[target_idx][0] + target_chash = merges[target_idx][1] + target_subject = merges[target_idx][2] + + # Get all commits in the range target..current + try: + range_hashes = run_git([ + 'rev-list', f'{target_hash}..{current}' + ]) + except Exception: # pylint: disable=broad-except + tout.error(f'Could not list commits in range ' + f'{target_hash[:12]}..{current[:12]}') + return 1 + + # Count commits that exist in the database + db_commits = [] + if range_hashes: + for chash in range_hashes.strip().split('\n'): + if chash and dbs.commit_get(chash): + db_commits.append(chash) + + # Find cherry-pick branches that match commits in the range. + # List all ci/cherry-* remote branches, then check if the hash in + # the branch name matches any commit in the rewound range. + mr_branches = [] + if range_hashes: + hash_set = set(range_hashes.strip().split('\n')) + try: + branch_out = run_git( + ['branch', '-r', '--list', f'{args.remote}/cherry-*']) + except Exception: # pylint: disable=broad-except + branch_out = '' + remote_prefix = f'{args.remote}/' + for line in branch_out.strip().split('\n'): + branch = line.strip() + if not branch: + continue + # Branch is like 'ci/cherry-abc1234'; extract the hash part + short = branch.removeprefix(f'{remote_prefix}cherry-') + # Check if any commit in the range starts with this hash + for chash in hash_set: + if chash.startswith(short): + mr_branches.append( + branch.removeprefix(remote_prefix)) + break + + # Look up MR details from GitLab for matching branches + matched_mrs = [] + if mr_branches: + mrs = gitlab_api.get_open_pickman_mrs(args.remote) + if mrs: + branch_set = set(mr_branches) + for merge_req in mrs: + if merge_req.source_branch in branch_set: + matched_mrs.append(merge_req) + + # Show what would happen (or what is happening) + current_short = current[:12] + prefix = '' if force else '[dry run] ' + tout.info(f"{prefix}Rewind '{source}': " + f'{current_short} -> {target_chash}') + tout.info(f' Target: {target_chash} {target_subject}') + tout.info(f' Merges being rewound:') + for i in range(target_idx): + tout.info(f' {merges[i][1]} {merges[i][2]}') + tout.info(f' Commits to delete from database: {len(db_commits)}') + + if matched_mrs: + tout.info(f' MRs to delete on GitLab:') + for merge_req in matched_mrs: + tout.info(f' !{merge_req.iid}: {merge_req.title}') + tout.info(f' {merge_req.web_url}') + elif mr_branches: + tout.info(f' Branches to check for MRs:') + for branch in mr_branches: + tout.info(f' {branch}') + + if not force: + tout.info('Use --force to execute this rewind') + return 0 + + # Delete commits from database + for chash in db_commits: + dbs.commit_delete(chash) + + # Update source position + dbs.source_set(source, target_hash) + dbs.commit() + + tout.info(f' Deleted {len(db_commits)} commit(s) from database') + + return 0 + + # pylint: disable=too-many-locals,too-many-branches,too-many-statements def process_single_mr(remote, merge_req, dbs, target): """Process review comments on a single MR @@ -2071,6 +2254,10 @@ def process_merged_mrs(remote, source, dbs): f"MR !{merge_req.iid}") continue + # Skip if already at this position + if full_hash == current: + continue + # Check if this commit is newer than current (current is ancestor of it) try: # Is current an ancestor of last_hash? (meaning last_hash is newer) @@ -2225,6 +2412,7 @@ COMMANDS = { 'poll': do_poll, 'push-branch': do_push_branch, 'review': do_review, + 'rewind': do_rewind, 'step': do_step, 'test': do_test, } diff --git a/tools/pickman/database.py b/tools/pickman/database.py index f584fe14c21..92bff7a5702 100644 --- a/tools/pickman/database.py +++ b/tools/pickman/database.py @@ -364,6 +364,14 @@ class Database: # pylint: disable=too-many-public-methods 'UPDATE pcommit SET mergereq_id = ? WHERE chash = ?', (mergereq_id, chash)) + def commit_delete(self, chash): + """Delete a commit from the database + + Args: + chash (str): Commit hash to delete + """ + self.execute('DELETE FROM pcommit WHERE chash = ?', (chash,)) + # mergereq functions # pylint: disable-next=too-many-arguments diff --git a/tools/pickman/ftest.py b/tools/pickman/ftest.py index eb69bde96a4..f07e4ecb0db 100644 --- a/tools/pickman/ftest.py +++ b/tools/pickman/ftest.py @@ -1025,6 +1025,23 @@ class TestNextMerges(unittest.TestCase): database.Database.instances.clear() command.TEST_RESULT = None + def _make_simple_merge_mock(self, log_output): + """Create a mock handler for merges with no sub-merges""" + def mock_git(pipe_list): + cmd = pipe_list[0] if pipe_list else [] + # Initial merge listing + if '--reverse' in cmd and '--format=%H|%h|%s' in cmd: + return command.CommandResult(stdout=log_output) + # Sub-merge detection: no sub-merges + if '--first-parent' in cmd and '--merges' in cmd: + return command.CommandResult(stdout='') + # Parent lookup for detect_sub_merges + if 'rev-parse' in cmd: + return command.CommandResult( + stdout='parent1\nparent2\n') + return command.CommandResult(stdout='') + return mock_git + def test_next_merges(self): """Test next-merges shows upcoming merges""" # Add source to database @@ -1037,20 +1054,19 @@ class TestNextMerges(unittest.TestCase): database.Database.instances.clear() - # Mock git log with merge commits log_output = ( 'aaa111|aaa111a|Merge branch feature-1\n' 'bbb222|bbb222b|Merge branch feature-2\n' 'ccc333|ccc333c|Merge branch feature-3\n' ) - command.TEST_RESULT = command.CommandResult(stdout=log_output) + command.TEST_RESULT = self._make_simple_merge_mock(log_output) args = argparse.Namespace(cmd='next-merges', source='us/next', count=10) with terminal.capture() as (stdout, _): ret = control.do_pickman(args) self.assertEqual(ret, 0) output = stdout.getvalue() - self.assertIn('Next 3 merges from us/next:', output) + self.assertIn('3 from 3 first-parent', output) self.assertIn('1. aaa111a Merge branch feature-1', output) self.assertIn('2. bbb222b Merge branch feature-2', output) self.assertIn('3. ccc333c Merge branch feature-3', output) @@ -1067,24 +1083,73 @@ class TestNextMerges(unittest.TestCase): database.Database.instances.clear() - # Mock git log with merge commits log_output = ( 'aaa111|aaa111a|Merge branch feature-1\n' 'bbb222|bbb222b|Merge branch feature-2\n' 'ccc333|ccc333c|Merge branch feature-3\n' ) - command.TEST_RESULT = command.CommandResult(stdout=log_output) + command.TEST_RESULT = self._make_simple_merge_mock(log_output) args = argparse.Namespace(cmd='next-merges', source='us/next', count=2) with terminal.capture() as (stdout, _): ret = control.do_pickman(args) self.assertEqual(ret, 0) output = stdout.getvalue() - self.assertIn('Next 2 merges from us/next:', output) + self.assertIn('2 from 2 first-parent', output) self.assertIn('1. aaa111a', output) self.assertIn('2. bbb222b', output) self.assertNotIn('3. ccc333c', output) + def test_next_merges_expands_mega_merge(self): + """Test next-merges expands mega-merges into sub-merges""" + 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() + + def mock_git(pipe_list): + cmd = pipe_list[0] if pipe_list else [] + cmd_str = ' '.join(cmd) + # Initial merge listing - one mega-merge + if '--reverse' in cmd and '--format=%H|%h|%s' in cmd: + return command.CommandResult( + stdout='mega111|mega111a|Merge branch next\n') + # Parent lookup + if 'rev-parse' in cmd and '^@' in cmd_str: + return command.CommandResult( + stdout='first_parent\nsecond_parent\n') + # Sub-merge detection on second parent chain + if ('--first-parent' in cmd and '--merges' in cmd + and '--format=%H' in cmd): + return command.CommandResult( + stdout='sub_aaa\nsub_bbb\n') + # Sub-merge detail lookup + if 'log' in cmd and '-1' in cmd and '--format=%h|%s' in cmd: + if 'sub_aaa' in cmd_str: + return command.CommandResult( + stdout='sub_aaa1|Merge feature-A\n') + if 'sub_bbb' in cmd_str: + return command.CommandResult( + stdout='sub_bbb1|Merge feature-B\n') + return command.CommandResult(stdout='') + + command.TEST_RESULT = mock_git + + args = argparse.Namespace(cmd='next-merges', source='us/next', count=10) + with terminal.capture() as (stdout, _): + ret = control.do_pickman(args) + self.assertEqual(ret, 0) + output = stdout.getvalue() + self.assertIn('2 from 1 first-parent', output) + self.assertIn('mega111a Merge branch next', output) + self.assertIn('2 sub-merges', output) + self.assertIn('1. sub_aaa1 Merge feature-A', output) + self.assertIn('2. sub_bbb1 Merge feature-B', output) + def test_next_merges_no_merges(self): """Test next-merges with no merges remaining""" # Add source to database @@ -1496,16 +1561,24 @@ class TestPushBranch(unittest.TestCase): with mock.patch.object(gitlab, 'get_push_url', return_value=TEST_SHORT_OAUTH_URL): with mock.patch.object(command, 'output') as mock_output: + mock_output.side_effect = [ + None, # fetch succeeds + 'abc123def\n', # rev-parse returns OID + None, # push succeeds + ] result = gitlab.push_branch('ci', 'test-branch', force=True) self.assertTrue(result) - # Should fetch first, then push with --force-with-lease + # Should fetch, rev-parse, then push with --force-with-lease calls = mock_output.call_args_list - self.assertEqual(len(calls), 2) - self.assertEqual(calls[0], mock.call('git', 'fetch', 'ci', - 'test-branch')) - push_args = calls[1][0] - self.assertIn('--force-with-lease=refs/remotes/ci/test-branch', + self.assertEqual(len(calls), 3) + self.assertEqual(calls[0], mock.call( + 'git', 'fetch', 'ci', + '+refs/heads/test-branch:refs/remotes/ci/test-branch')) + self.assertEqual(calls[1], mock.call( + 'git', 'rev-parse', 'refs/remotes/ci/test-branch')) + push_args = calls[2][0] + self.assertIn('--force-with-lease=test-branch:abc123def', push_args) def test_push_branch_force_no_remote_ref(self): @@ -3477,6 +3550,330 @@ class TestDoCommitSourceResolveError(unittest.TestCase): self.assertIn('Could not resolve', stderr.getvalue()) +class TestRewind(unittest.TestCase): + """Tests for rewind 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_rewind_source_not_found(self): + """Test rewind with unknown source.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + dbs.close() + + database.Database.instances.clear() + + args = argparse.Namespace(cmd='rewind', source='unknown', count=1, + force=True, remote='ci') + with terminal.capture() as (_, stderr): + ret = control.do_pickman(args) + self.assertEqual(ret, 1) + self.assertIn("Source 'unknown' not found", stderr.getvalue()) + + def test_rewind_dry_run(self): + """Test rewind dry run shows what would happen without executing.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + dbs.source_set('us/next', 'current_hash') + source_id = dbs.source_get_id('us/next') + dbs.commit_add('commit_a', source_id, 'Commit A', 'Author') + dbs.commit_add('commit_b', source_id, 'Commit B', 'Author') + dbs.commit() + dbs.close() + + database.Database.instances.clear() + + def mock_git(pipe_list): + cmd = pipe_list[0] if pipe_list else [] + if '--merges' in cmd: + return command.CommandResult( + stdout='current_hash|current1|Current merge\n' + 'prev_hash|prev1234|Previous merge\n') + if 'rev-list' in cmd: + return command.CommandResult( + stdout='commit_a\ncommit_b\n') + if 'branch' in cmd and '--list' in cmd: + return command.CommandResult(stdout='') + return command.CommandResult(stdout='') + + command.TEST_RESULT = mock_git + + args = argparse.Namespace(cmd='rewind', source='us/next', count=1, + force=False, remote='ci') + with terminal.capture() as (stdout, _): + ret = control.do_pickman(args) + self.assertEqual(ret, 0) + output = stdout.getvalue() + self.assertIn('dry run', output) + self.assertIn('prev1234', output) + self.assertIn('2', output) # 2 commits to delete + self.assertIn('--force', output) + + # Verify database was NOT modified + database.Database.instances.clear() + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + self.assertEqual(dbs.source_get('us/next'), 'current_hash') + self.assertIsNotNone(dbs.commit_get('commit_a')) + self.assertIsNotNone(dbs.commit_get('commit_b')) + dbs.close() + + def test_rewind_one_merge(self): + """Test rewinding by one merge with --force.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + dbs.source_set('us/next', 'current_hash') + source_id = dbs.source_get_id('us/next') + # Add some commits that should be deleted + dbs.commit_add('commit_a', source_id, 'Commit A', 'Author') + dbs.commit_add('commit_b', source_id, 'Commit B', 'Author') + dbs.commit() + dbs.close() + + database.Database.instances.clear() + + def mock_git(pipe_list): + cmd = pipe_list[0] if pipe_list else [] + if '--merges' in cmd: + return command.CommandResult( + stdout='current_hash|current1|Current merge\n' + 'prev_hash|prev1234|Previous merge\n') + if 'rev-list' in cmd: + return command.CommandResult( + stdout='commit_a\ncommit_b\n') + if 'branch' in cmd and '--list' in cmd: + return command.CommandResult(stdout='') + return command.CommandResult(stdout='') + + command.TEST_RESULT = mock_git + + args = argparse.Namespace(cmd='rewind', source='us/next', count=1, + force=True, remote='ci') + with terminal.capture() as (stdout, _): + ret = control.do_pickman(args) + self.assertEqual(ret, 0) + output = stdout.getvalue() + self.assertIn('prev1234', output) + self.assertIn('Deleted 2 commit(s)', output) + + # Verify source was updated + database.Database.instances.clear() + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + self.assertEqual(dbs.source_get('us/next'), 'prev_hash') + # Commits should be deleted + self.assertIsNone(dbs.commit_get('commit_a')) + self.assertIsNone(dbs.commit_get('commit_b')) + dbs.close() + + def test_rewind_shows_mr_details(self): + """Test rewind shows MR numbers, titles and URLs.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + dbs.source_set('us/next', 'current_hash') + dbs.commit() + dbs.close() + + database.Database.instances.clear() + + def mock_git(pipe_list): + cmd = pipe_list[0] if pipe_list else [] + if '--merges' in cmd: + return command.CommandResult( + stdout='current_hash|current1|Current merge\n' + 'prev_hash|prev1234|Previous merge\n') + if 'rev-list' in cmd: + return command.CommandResult( + stdout='abc1234ffffff\ndef5678aaaaaa\n') + if 'branch' in cmd and '--list' in cmd: + return command.CommandResult( + stdout=' ci/cherry-abc1234f\n' + ' ci/cherry-other99\n') + return command.CommandResult(stdout='') + + command.TEST_RESULT = mock_git + + mock_mrs = [ + gitlab.PickmanMr( + iid=541, title='[pickman] Some cherry-pick', + web_url='https://gitlab.com/proj/-/merge_requests/541', + source_branch='cherry-abc1234f', + description='desc'), + gitlab.PickmanMr( + iid=540, title='[pickman] Unrelated MR', + web_url='https://gitlab.com/proj/-/merge_requests/540', + source_branch='cherry-zzz9999', + description='desc'), + ] + + args = argparse.Namespace(cmd='rewind', source='us/next', count=1, + force=False, remote='ci') + with mock.patch.object(gitlab, 'get_open_pickman_mrs', + return_value=mock_mrs): + with terminal.capture() as (stdout, _): + ret = control.do_pickman(args) + self.assertEqual(ret, 0) + output = stdout.getvalue() + self.assertIn('!541', output) + self.assertIn('[pickman] Some cherry-pick', output) + self.assertIn('merge_requests/541', output) + # Unrelated MR should not appear + self.assertNotIn('!540', output) + self.assertIn('MRs to delete', output) + + def test_rewind_shows_branches_when_api_unavailable(self): + """Test rewind falls back to branch names when GitLab unavailable.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + dbs.source_set('us/next', 'current_hash') + dbs.commit() + dbs.close() + + database.Database.instances.clear() + + def mock_git(pipe_list): + cmd = pipe_list[0] if pipe_list else [] + if '--merges' in cmd: + return command.CommandResult( + stdout='current_hash|current1|Current merge\n' + 'prev_hash|prev1234|Previous merge\n') + if 'rev-list' in cmd: + return command.CommandResult( + stdout='abc1234ffffff\n') + if 'branch' in cmd and '--list' in cmd: + return command.CommandResult( + stdout=' ci/cherry-abc1234f\n') + return command.CommandResult(stdout='') + + command.TEST_RESULT = mock_git + + args = argparse.Namespace(cmd='rewind', source='us/next', count=1, + force=False, remote='ci') + # GitLab API returns None (unavailable) + with mock.patch.object(gitlab, 'get_open_pickman_mrs', + return_value=None): + with terminal.capture() as (stdout, _): + ret = control.do_pickman(args) + self.assertEqual(ret, 0) + output = stdout.getvalue() + self.assertIn('cherry-abc1234f', output) + self.assertIn('Branches to check', output) + + def test_rewind_two_merges(self): + """Test rewinding by two merges with --force.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + dbs.source_set('us/next', 'current_hash') + dbs.commit() + dbs.close() + + database.Database.instances.clear() + + def mock_git(pipe_list): + cmd = pipe_list[0] if pipe_list else [] + if '--merges' in cmd: + return command.CommandResult( + stdout='current_hash|current1|Current merge\n' + 'mid_hash|mid12345|Middle merge\n' + 'old_hash|old12345|Old merge\n') + if 'rev-list' in cmd: + return command.CommandResult(stdout='') + if 'branch' in cmd and '--list' in cmd: + return command.CommandResult(stdout='') + return command.CommandResult(stdout='') + + command.TEST_RESULT = mock_git + + args = argparse.Namespace(cmd='rewind', source='us/next', count=2, + force=True, remote='ci') + with terminal.capture() as (stdout, _): + ret = control.do_pickman(args) + self.assertEqual(ret, 0) + self.assertIn('old12345', stdout.getvalue()) + + # Verify source was updated to old_hash + database.Database.instances.clear() + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + self.assertEqual(dbs.source_get('us/next'), 'old_hash') + dbs.close() + + def test_rewind_not_enough_merges(self): + """Test rewind fails when not enough merges in history.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + dbs.source_set('us/next', 'current_hash') + dbs.commit() + dbs.close() + + database.Database.instances.clear() + + def mock_git(pipe_list): + cmd = pipe_list[0] if pipe_list else [] + if '--merges' in cmd: + # Only one merge (the current position) + return command.CommandResult( + stdout='current_hash|current1|Current merge\n') + return command.CommandResult(stdout='') + + command.TEST_RESULT = mock_git + + args = argparse.Namespace(cmd='rewind', source='us/next', count=1, + force=True, remote='ci') + with terminal.capture() as (_, stderr): + ret = control.do_pickman(args) + self.assertEqual(ret, 1) + self.assertIn('Not enough merges', stderr.getvalue()) + + def test_parse_rewind(self): + """Test parsing rewind command.""" + args = pickman.parse_args(['rewind', 'us/next']) + self.assertEqual(args.cmd, 'rewind') + self.assertEqual(args.source, 'us/next') + self.assertEqual(args.count, 1) + self.assertFalse(args.force) + self.assertEqual(args.remote, 'ci') + + def test_parse_rewind_with_count(self): + """Test parsing rewind command with count.""" + args = pickman.parse_args(['rewind', 'us/next', '-c', '3']) + self.assertEqual(args.cmd, 'rewind') + self.assertEqual(args.source, 'us/next') + self.assertEqual(args.count, 3) + + def test_parse_rewind_with_force(self): + """Test parsing rewind command with --force.""" + args = pickman.parse_args(['rewind', 'us/next', '-c', '2', '-f']) + self.assertEqual(args.cmd, 'rewind') + self.assertEqual(args.count, 2) + self.assertTrue(args.force) + + class TestDoPushBranch(unittest.TestCase): """Tests for do_push_branch command.""" @@ -4369,8 +4766,8 @@ class TestDoPick(unittest.TestCase): push=False) def run_git_handler(args): - if '--verify' in args: - raise ValueError('branch not found') + if 'branch' in args and '--list' in args: + return '' # Branch doesn't exist return 'main' with mock.patch.object(control, 'get_commits_for_pick', diff --git a/tools/pickman/gitlab_api.py b/tools/pickman/gitlab_api.py index bfaf828edae..9262ac15a0d 100644 --- a/tools/pickman/gitlab_api.py +++ b/tools/pickman/gitlab_api.py @@ -196,24 +196,28 @@ def push_branch(remote, branch, force=False, skip_ci=True): push_target = push_url if push_url else remote # When using --force-with-lease with an HTTPS URL (not remote name), - # git can't find tracking refs automatically. Try to fetch first to - # update the tracking ref. If fetch fails (branch doesn't exist on - # remote yet), use regular --force instead of --force-with-lease. - have_remote_ref = False + # git can't find tracking refs automatically. Fetch and update the + # tracking ref explicitly so we can pass the expected value. + remote_oid = None if force and push_url: try: - command.output('git', 'fetch', remote, branch) - have_remote_ref = True + command.output( + 'git', 'fetch', remote, + f'+refs/heads/{branch}:' + f'refs/remotes/{remote}/{branch}') + remote_oid = command.output( + 'git', 'rev-parse', + f'refs/remotes/{remote}/{branch}').strip() except command.CommandExc: - pass # Branch doesn't exist on remote, will use --force + pass # Branch doesn't exist on remote args = ['git', 'push', '-u'] if skip_ci: args.extend(['-o', 'ci.skip']) if force: - if have_remote_ref: + if remote_oid: args.append( - f'--force-with-lease=refs/remotes/{remote}/{branch}') + f'--force-with-lease={branch}:{remote_oid}') else: args.append('--force') args.extend([push_target, f'HEAD:{branch}'])