From patchwork Thu Mar 5 14:54:44 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 1970 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=1772722520; bh=L3s61dGwgGnS/fv25M917FYEMS2/BXEyj7PcNwwnHeQ=; 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=cMausGOqf+Bq9SehxS7ntBC0P4bU2dX1b0FxVlAPEN1KIhf23Fi0iFbinnNeuwD0u pDbHn2ikrAjoIHpGzsR1QTFe+iyKSi/svd+1NStCCaRaQ+2IADcb+2G7cRtYbAsk5H FbEMC2uO//bQK7YRrCTdEwkpCaJ3WSWlfm8A5SpS2B37RoQMxq2MtlhlUIRDmzQJAN CCbMbVWoC7e9/73+prApXHORkNz/mp0MnODEKxW4S73SNeC+VAH26FwYMqESaBrnws Vf4qHh+kt421NuLdKc1UY2TFt54BQuXxm9SwACZ3Q/mKtWH0y9liKXIuym7ukD/s4i XCGWXovb+yf6Q== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id E4B3269F25 for ; Thu, 5 Mar 2026 07:55: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 10024) with ESMTP id 1ij4fW_rJQmF for ; Thu, 5 Mar 2026 07:55:20 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1772722520; bh=L3s61dGwgGnS/fv25M917FYEMS2/BXEyj7PcNwwnHeQ=; 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=cMausGOqf+Bq9SehxS7ntBC0P4bU2dX1b0FxVlAPEN1KIhf23Fi0iFbinnNeuwD0u pDbHn2ikrAjoIHpGzsR1QTFe+iyKSi/svd+1NStCCaRaQ+2IADcb+2G7cRtYbAsk5H FbEMC2uO//bQK7YRrCTdEwkpCaJ3WSWlfm8A5SpS2B37RoQMxq2MtlhlUIRDmzQJAN CCbMbVWoC7e9/73+prApXHORkNz/mp0MnODEKxW4S73SNeC+VAH26FwYMqESaBrnws Vf4qHh+kt421NuLdKc1UY2TFt54BQuXxm9SwACZ3Q/mKtWH0y9liKXIuym7ukD/s4i XCGWXovb+yf6Q== Received: from mail.u-boot.org (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id D458069F0F for ; Thu, 5 Mar 2026 07:55:20 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1772722518; bh=1jcoGqhbNffVEPP5eJWLNGoS8Ig1w+igjnRhsEjRVrk=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=isiHnTxUiGJBXN3Eas8LvbwC+LcRlKizz2pS7o0T95EtG0j7s8FPgDCcxyuN0nyOr XOnkCkErZ7B+Y415c/J8AdwkhKa5Wn57Zj3Hd/j17rr18dSKMasn+59DxBQK1O6dSZ I4N/gXSPWAjQto+TWwafxd5/C/vXGW+x1LGXqyJd/FROcqUAvqXArxm0mbIlfzsT7K rwCU3XNgYUyxYiuKzb1nbxSGCH6SrHx6b2SUNWo8UZc95to3QIx7Ef3uFbDRCWQV43 +OsV0TtOmP1/CNfePr0K7IZOxxH6dyvAu4k2NoE39cryQisIXw3urUzgN47NZWWK9Q 4RLYlF0ab0k+w== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 1E51469EC0; Thu, 5 Mar 2026 07:55:18 -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 ZuUkGXTzIvz9; Thu, 5 Mar 2026 07:55:18 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1772722512; bh=7wj4B68c7RAYm57/yQJZVOQa3nS8TmY2zVM2Z7T1708=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=k8MTgcR1gWpWnCMVhSGg8D03lFUWL9vRYXmyXcXDcySIAsntlSMCBmKwMenUK+B1y 4eAUwI/OYP3k6BsiDBIll60TPtlWoMV62rKeJct2WclVoALrtAivkDgqyFaNlHJY6P kRs2fsNl5Gy0Xj1wyIauQ5+cLDnVV2ZbAnG++hwEJXjjIcqU8dFq4h5wkVYkzQmBXj me19/+Pn5Zp48pS4f9OLRSq3i/PUgBlHeeknBf9VqZnoO8wIiFH3pwAAuJ2ezgnu0r w9pMzfiGP1ziF4vgtT/S8m7VwbOEfgY2weQArVJTlVaGs7Ghxu6Bf5xP8VrjGy212e tEgxjdt66BCpA== Received: from u-boot.org (unknown [73.34.74.121]) by mail.u-boot.org (Postfix) with ESMTPSA id 8244B69D92; Thu, 5 Mar 2026 07:55:12 -0700 (MST) From: Simon Glass To: U-Boot Concept Date: Thu, 5 Mar 2026 07:54:44 -0700 Message-ID: <20260305145452.909661-2-sjg@u-boot.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20260305145452.909661-1-sjg@u-boot.org> References: <20260305145452.909661-1-sjg@u-boot.org> MIME-Version: 1.0 Message-ID-Hash: 2OVDCJBBJXG3Q5IXH4ARRZCEAXQBFIJO X-Message-ID-Hash: 2OVDCJBBJXG3Q5IXH4ARRZCEAXQBFIJO X-MailFrom: sjg@u-boot.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: Simon Glass X-Mailman-Version: 3.3.10 Precedence: list Subject: [Concept] [PATCH 1/4] pickman: Fix ambiguous checkout in subtree update 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 multiple remotes have a branch with the same name (e.g. ci/master, me/master, us/master), a bare 'git checkout master' fails because git cannot determine which remote to track. Fall back to 'git checkout -b /' when the bare checkout fails, creating a local branch from the specific remote. This mirrors the existing pattern used for MR branch creation. Signed-off-by: Simon Glass --- tools/pickman/control.py | 13 +++++++---- tools/pickman/ftest.py | 50 ++++++++++++++++++++++++++++++++++++---- 2 files changed, 55 insertions(+), 8 deletions(-) diff --git a/tools/pickman/control.py b/tools/pickman/control.py index a42927f991d..815ac574093 100644 --- a/tools/pickman/control.py +++ b/tools/pickman/control.py @@ -1524,7 +1524,7 @@ def _subtree_run_update(name, tag): int: 0 on success, 1 on failure """ try: - result = command.run( + result = command.run_one( './tools/update-subtree.sh', 'pull', name, tag, capture=True, raise_on_error=False) if result.stdout: @@ -1595,9 +1595,14 @@ def apply_subtree_update(dbs, source, name, tag, merge_hash, args): # pylint: d try: run_git(['checkout', target]) - except Exception: # pylint: disable=broad-except - tout.error(f'Could not checkout {target}') - return 1 + except command.CommandExc: + # Bare name may be ambiguous when multiple remotes have it + try: + run_git(['checkout', '-b', target, + f'{args.remote}/{target}']) + except command.CommandExc: + tout.error(f'Could not checkout {target}') + return 1 ret = _subtree_run_update(name, tag) if ret: diff --git a/tools/pickman/ftest.py b/tools/pickman/ftest.py index 5a4d0c433dc..766cc714ed7 100644 --- a/tools/pickman/ftest.py +++ b/tools/pickman/ftest.py @@ -4022,7 +4022,7 @@ class TestApplySubtreeUpdate(unittest.TestCase): with mock.patch.object(control, 'run_git', side_effect=run_git_handler): with mock.patch.object( - control.command, 'run', + control.command, 'run_one', return_value=mock_result): ret = control.apply_subtree_update( dbs, 'us/next', 'dts', 'v6.15-dts', @@ -4070,7 +4070,7 @@ class TestApplySubtreeUpdate(unittest.TestCase): with mock.patch.object(control, 'run_git', side_effect=run_git_handler): with mock.patch.object( - control.command, 'run', + control.command, 'run_one', return_value=mock_result): with mock.patch.object( control.gitlab_api, 'push_branch', @@ -4112,6 +4112,48 @@ class TestApplySubtreeUpdate(unittest.TestCase): self.assertEqual(dbs.source_get('us/next'), 'base') dbs.close() + def test_apply_checkout_fallback(self): + """Test apply_subtree_update falls back to -b when checkout fails.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + dbs.source_set('us/next', 'base') + dbs.commit() + + args = argparse.Namespace(push=False, remote='ci', + target='master') + + checkout_calls = [] + + def run_git_handler(git_args): + if 'rev-parse' in git_args: + return 'first_parent\nsquash_hash' + if 'checkout' in git_args: + checkout_calls.append(list(git_args)) + if '-b' not in git_args: + raise command.CommandExc( + 'ambiguous checkout', None) + return '' + if 'log' in git_args: + return 'subject|author' + return '' + + with mock.patch.object(control, 'run_git', + side_effect=run_git_handler): + with mock.patch.object(control, '_subtree_run_update', + return_value=0): + ret = control.apply_subtree_update( + dbs, 'us/next', 'dts', 'v6.15-dts', + 'merge_hash', args) + + self.assertEqual(ret, 0) + # Should have tried bare checkout, then fallback + self.assertEqual(len(checkout_calls), 2) + self.assertEqual(checkout_calls[0], ['checkout', 'master']) + self.assertEqual(checkout_calls[1], + ['checkout', '-b', 'master', 'ci/master']) + dbs.close() + def test_apply_no_second_parent(self): """Test apply_subtree_update returns 1 when merge has no 2nd parent.""" with terminal.capture(): @@ -4154,7 +4196,7 @@ class TestApplySubtreeUpdate(unittest.TestCase): with mock.patch.object(control, 'run_git', side_effect=run_git_handler): with mock.patch.object( - control.command, 'run', + control.command, 'run_one', side_effect=Exception('script failed')): ret = control.apply_subtree_update( dbs, 'us/next', 'dts', 'v6.15-dts', @@ -4193,7 +4235,7 @@ class TestApplySubtreeUpdate(unittest.TestCase): with mock.patch.object(control, 'run_git', side_effect=run_git_handler): with mock.patch.object( - control.command, 'run', + control.command, 'run_one', return_value=mock_result): ret = control.apply_subtree_update( dbs, 'us/next', 'dts', 'v6.15-dts', From patchwork Thu Mar 5 14:54:45 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 1971 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=1772722525; bh=sJWoV1d+Izp92jrH6J8pLbjhIM/pWYAOq48R3Bwu2Zk=; 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=rBknQfJQpOZVNa1GrhzIDqe9eOy6jmRiLWJmoy0e17bMI9+42BE4Wp2q5m58CPiEs p6miV9xNsXQ6MB+GqluZyZVjLklnlq7oIgCgfWbAsZUwq2NSILoxd+CN4qYOjjDBKA bfmpAct+ImsFdKzO+hEv9UYlskm1zd28gEgFHwLIOcKCVrOlXKtNxr1TbXycPSmqaB 4CLpkGDrV4GPClQGVFsCHhJg6R4F267gHhcY8koUYMJG7Rqay3m//+lfarkYp1UFcH KNfhF1X6FtOc54LoTo0A7+fJDO2YfMU85KF0vt2MHKRLDGYIyX7mc4OLPbtoakkjLz cTRfbFx8yY3OQ== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 9079269D92 for ; Thu, 5 Mar 2026 07:55:25 -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 x5mKvWu1Ov4c for ; Thu, 5 Mar 2026 07:55:25 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1772722525; bh=sJWoV1d+Izp92jrH6J8pLbjhIM/pWYAOq48R3Bwu2Zk=; 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=rBknQfJQpOZVNa1GrhzIDqe9eOy6jmRiLWJmoy0e17bMI9+42BE4Wp2q5m58CPiEs p6miV9xNsXQ6MB+GqluZyZVjLklnlq7oIgCgfWbAsZUwq2NSILoxd+CN4qYOjjDBKA bfmpAct+ImsFdKzO+hEv9UYlskm1zd28gEgFHwLIOcKCVrOlXKtNxr1TbXycPSmqaB 4CLpkGDrV4GPClQGVFsCHhJg6R4F267gHhcY8koUYMJG7Rqay3m//+lfarkYp1UFcH KNfhF1X6FtOc54LoTo0A7+fJDO2YfMU85KF0vt2MHKRLDGYIyX7mc4OLPbtoakkjLz cTRfbFx8yY3OQ== Received: from mail.u-boot.org (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 7F16C69F0F for ; Thu, 5 Mar 2026 07:55:25 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1772722523; bh=6YXaH3iZ37uBM/cLYaKCESPuT/I/9qEzhWZH02NqPY0=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=HJfML1jZnWHzjIQfJHVpgi40WZEraoFq3qvU7PIs2/DYR2YiFQQoLRaJKcH4Tjel/ 5+PDgdajd3JFUXcHCgREUTesPGkPpxcgOZM8sf3VogtUdqDBjeGpt793uOkRK80wk2 jMcMStz+tS9AovP4Flw3/0nNmftO8vQlsIKGBBUFvkzAWro6YWRsTarhOLC/VSHBpK 9NN05XiJ3cvMWKIK9ozA3+MHjn6d+/dUJORV/zlZbMejcpXVdDtUEHxGSDtLMcUVh3 hyJWSQHS9EeVRkrlCQtrWwRPYugT801f+TpiJLRnFYASG6S1WB+Rsle0+LcIAp98Eg 1SVDxCUnHtprA== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 6054169F0F; Thu, 5 Mar 2026 07:55: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 10026) with ESMTP id QxRMFVb0e7ob; Thu, 5 Mar 2026 07:55:23 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1772722518; bh=r37XD0qbgQUWc3fgRDqCBxal/CgPkslg6m7HonZvzao=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=nyLwDdTaTpPiSET5NLJsuew9lGa2Tu9UYmZAF0BeIYRcg2UslRrT8Nn86po9PMz1W WD7att5FE+OdywzQ/m+tS3sDt+Mz2tx1wU10f2eck1YjiUN9AOUDpnlkQcAExD43+q xah2l13uWHnPryGdD+vvop9o7BDmT/I8GfsqVzTQB8CP6N5/q67OMkFiynXNopTq4j zt8kJxe26nPXpoTGhsrbJULFfQEcEGzW5AvTFxXBgsj3f3dZYa+rjBtV3wIWwFjkrT 9qzFLVo4wPRUmL6a3bOIN1XSjKLlZ8+LONRLJxV05pr43Qb2PpTwSMFxOvcWeNtzBl 10gpRdz2HGN1A== Received: from u-boot.org (unknown [73.34.74.121]) by mail.u-boot.org (Postfix) with ESMTPSA id 5B4C169D92; Thu, 5 Mar 2026 07:55:18 -0700 (MST) From: Simon Glass To: U-Boot Concept Date: Thu, 5 Mar 2026 07:54:45 -0700 Message-ID: <20260305145452.909661-3-sjg@u-boot.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20260305145452.909661-1-sjg@u-boot.org> References: <20260305145452.909661-1-sjg@u-boot.org> MIME-Version: 1.0 Message-ID-Hash: 7HTFQKBPGK55APSNRWSVTODF3HGMXSQA X-Message-ID-Hash: 7HTFQKBPGK55APSNRWSVTODF3HGMXSQA X-MailFrom: sjg@u-boot.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: Simon Glass X-Mailman-Version: 3.3.10 Precedence: list Subject: [Concept] [PATCH 2/4] pickman: Add agent-based subtree merge conflict resolution 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 pickman processes a subtree update via update-subtree.sh, merge conflicts are common for large DTS updates. Previously, any failure caused an immediate merge abort and error return. Detect whether a failed subtree pull left a merge in progress by checking for MERGE_HEAD. When conflicts exist, invoke the Claude agent to resolve them (preferring the upstream version), following the same pattern used for cherry-pick conflict resolution. If the agent cannot resolve the conflicts, abort the merge and return failure as before. Add SUBTREE_OK/FAIL/CONFLICT return codes to _subtree_run_update() so the caller can distinguish between merge conflicts and other failures. Add build_subtree_conflict_prompt(), run_subtree_conflict_agent(), and resolve_subtree_conflicts() to agent.py for the actual resolution. Signed-off-by: Simon Glass --- tools/pickman/agent.py | 93 ++++++++++++ tools/pickman/control.py | 100 +++++++++---- tools/pickman/ftest.py | 298 ++++++++++++++++++++++++++++++++++++--- 3 files changed, 444 insertions(+), 47 deletions(-) diff --git a/tools/pickman/agent.py b/tools/pickman/agent.py index ec5c1b0df75..023b65b5d36 100644 --- a/tools/pickman/agent.py +++ b/tools/pickman/agent.py @@ -713,3 +713,96 @@ def fix_pipeline(mr_iid, branch_name, failed_jobs, remote, target='master', return asyncio.run(run_pipeline_fix_agent( mr_iid, branch_name, failed_jobs, remote, target, mr_description, attempt, repo_path)) + + +def build_subtree_conflict_prompt(name, tag, subtree_path): + """Build a prompt for resolving subtree merge conflicts + + Args: + name (str): Subtree name ('dts', 'mbedtls', 'lwip') + tag (str): Tag being pulled (e.g. 'v6.15-dts') + subtree_path (str): Path to the subtree (e.g. 'dts/upstream') + + Returns: + str: The prompt for the agent + """ + return f"""Resolve merge conflicts from a subtree pull. + +Context: A 'git subtree pull --prefix {subtree_path}' for the '{name}' \ +subtree (tag {tag}) has produced merge conflicts. Git is currently in a \ +merge state with MERGE_HEAD set. + +Resolution strategy - prefer the upstream version: +1. Run 'git status' to see conflicted files +2. For each conflicted file: + - If the file exists in MERGE_HEAD (content conflict or add/add): + git checkout MERGE_HEAD -- + - If the file is a modify/delete conflict (deleted upstream): + git rm +3. After resolving all conflicts, stage everything: + git add -A +4. Complete the merge: + git commit --no-edit +5. Verify with 'git status' that the working tree is clean + +If you cannot resolve the conflicts, abort with: + git merge --abort + +Important: +- Always prefer the upstream (MERGE_HEAD) version of conflicted files +- The subtree path is '{subtree_path}' - most conflicts will be there +- Do NOT modify files outside the subtree path +- Do NOT use 'git merge --continue'; use 'git commit --no-edit' +""" + + +async def run_subtree_conflict_agent(name, tag, subtree_path, + repo_path=None): + """Run the Claude agent to resolve subtree merge conflicts + + Args: + name (str): Subtree name ('dts', 'mbedtls', 'lwip') + tag (str): Tag being pulled (e.g. 'v6.15-dts') + subtree_path (str): Path to the subtree (e.g. 'dts/upstream') + repo_path (str): Path to repository (defaults to current directory) + + Returns: + tuple: (success, conversation_log) where success is bool and + conversation_log is the agent's output text + """ + if not check_available(): + return False, '' + + if repo_path is None: + repo_path = os.getcwd() + + prompt = build_subtree_conflict_prompt(name, tag, subtree_path) + + options = ClaudeAgentOptions( + allowed_tools=['Bash', 'Read', 'Grep'], + cwd=repo_path, + max_buffer_size=MAX_BUFFER_SIZE, + ) + + tout.info(f'Starting Claude agent to resolve {name} subtree ' + f'conflicts...') + tout.info('') + + return await run_agent_collect(prompt, options) + + +def resolve_subtree_conflicts(name, tag, subtree_path, repo_path=None): + """Synchronous wrapper for running the subtree conflict agent + + Args: + name (str): Subtree name ('dts', 'mbedtls', 'lwip') + tag (str): Tag being pulled (e.g. 'v6.15-dts') + subtree_path (str): Path to the subtree (e.g. 'dts/upstream') + repo_path (str): Path to repository (defaults to current directory) + + Returns: + tuple: (success, conversation_log) where success is bool and + conversation_log is the agent's output text + """ + return asyncio.run( + run_subtree_conflict_agent(name, tag, subtree_path, repo_path)) diff --git a/tools/pickman/control.py b/tools/pickman/control.py index 815ac574093..ed6825c2333 100644 --- a/tools/pickman/control.py +++ b/tools/pickman/control.py @@ -58,6 +58,11 @@ SUBTREE_NAMES = { 'lib/lwip/lwip': 'lwip', } +# Return codes for _subtree_run_update() +SUBTREE_OK = 0 +SUBTREE_FAIL = 1 +SUBTREE_CONFLICT = 2 + # Named tuple for commit info Commit = namedtuple('Commit', ['hash', 'chash', 'subject', 'date']) @@ -1514,14 +1519,30 @@ def push_mr(args, branch_name, title, description): return bool(mr_url) +def _is_merge_in_progress(): + """Check if a git merge is currently in progress. + + Returns: + bool: True if MERGE_HEAD exists (merge in progress), False otherwise + """ + try: + run_git(['rev-parse', '--verify', 'MERGE_HEAD']) + return True + except Exception: # pylint: disable=broad-except + return False + + def _subtree_run_update(name, tag): """Run update-subtree.sh to pull a subtree update. - On failure, aborts any in-progress merge to clean up the working - tree. + On failure, checks whether a merge is in progress (indicating + conflicts). If so, returns SUBTREE_CONFLICT with the merge state + intact so the caller can invoke the agent. Otherwise aborts any + in-progress merge and returns SUBTREE_FAIL. Returns: - int: 0 on success, 1 on failure + int: SUBTREE_OK on success, SUBTREE_CONFLICT on merge conflicts, + SUBTREE_FAIL on other failures """ try: result = command.run_one( @@ -1534,16 +1555,18 @@ def _subtree_run_update(name, tag): f'{result.return_code})') if result.stderr: tout.error(result.stderr) + if _is_merge_in_progress(): + return SUBTREE_CONFLICT try: run_git(['merge', '--abort']) except Exception: # pylint: disable=broad-except pass - return 1 + return SUBTREE_FAIL except Exception as exc: # pylint: disable=broad-except tout.error(f'Subtree update failed: {exc}') - return 1 + return SUBTREE_FAIL - return 0 + return SUBTREE_OK def _subtree_record(dbs, source, squash_hash, merge_hash): @@ -1565,7 +1588,8 @@ def _subtree_record(dbs, source, squash_hash, merge_hash): f'{merge_hash[:12]}') -def apply_subtree_update(dbs, source, name, tag, merge_hash, args): # pylint: disable=too-many-arguments +def apply_subtree_update(dbs, source, name, tag, merge_hash, remote, # pylint: disable=too-many-arguments + target, push=True): """Apply a subtree update on the target branch Runs tools/update-subtree.sh to pull the subtree update, then @@ -1577,13 +1601,13 @@ def apply_subtree_update(dbs, source, name, tag, merge_hash, args): # pylint: d name (str): Subtree name ('dts', 'mbedtls', 'lwip') tag (str): Tag to pull (e.g. 'v6.15-dts') merge_hash (str): Hash of the subtree merge commit to advance past - args (Namespace): Parsed arguments with 'push', 'remote', 'target' + remote (str): Git remote name (e.g. 'ci') + target (str): Target branch name (e.g. 'master') + push (bool): Whether to push the result to the remote Returns: int: 0 on success, 1 on failure """ - target = args.target - tout.info(f'Applying subtree update: {name} -> {tag}') # Get the squash commit (second parent of the merge) @@ -1599,20 +1623,34 @@ def apply_subtree_update(dbs, source, name, tag, merge_hash, args): # pylint: d # Bare name may be ambiguous when multiple remotes have it try: run_git(['checkout', '-b', target, - f'{args.remote}/{target}']) + f'{remote}/{target}']) except command.CommandExc: tout.error(f'Could not checkout {target}') return 1 ret = _subtree_run_update(name, tag) - if ret: - return ret + if ret == SUBTREE_FAIL: + return 1 + if ret == SUBTREE_CONFLICT: + # Resolve via reverse lookup of subtree path + subtree_path = next( + (p for p, n in SUBTREE_NAMES.items() if n == name), None) + tout.info('Merge conflicts detected, invoking agent...') + success, _ = agent.resolve_subtree_conflicts( + name, tag, subtree_path) + if not success: + tout.error('Agent could not resolve subtree conflicts') + try: + run_git(['merge', '--abort']) + except command.CommandExc: + pass + return 1 - if args.push: + if push: try: - gitlab_api.push_branch(args.remote, target, skip_ci=True) - tout.info(f'Pushed {target} to {args.remote}') - except Exception as exc: # pylint: disable=broad-except + gitlab_api.push_branch(remote, target, skip_ci=True) + tout.info(f'Pushed {target} to {remote}') + except command.CommandExc as exc: tout.error(f'Failed to push {target}: {exc}') return 1 @@ -1621,7 +1659,7 @@ def apply_subtree_update(dbs, source, name, tag, merge_hash, args): # pylint: d return 0 -def _prepare_get_commits(dbs, source, args): +def _prepare_get_commits(dbs, source, remote, target): """Get the next commits to apply, handling subtrees and skips. Fetch the next batch of commits from the source. If a subtree @@ -1631,7 +1669,9 @@ def _prepare_get_commits(dbs, source, args): Args: dbs (Database): Database instance source (str): Source branch name - args (Namespace): Parsed arguments (needed for subtree updates) + remote (str): Git remote name (e.g. 'ci'), or None to skip + subtree updates + target (str): Target branch name (e.g. 'master') Returns: tuple: (NextCommitsInfo, return_code) where return_code is None @@ -1646,11 +1686,12 @@ def _prepare_get_commits(dbs, source, args): if info.subtree_update: name, tag = info.subtree_update tout.info(f'Subtree update needed: {name} -> {tag}') - if not args: - tout.error('Cannot apply subtree update without args') + if not remote: + tout.error('Cannot apply subtree update without remote') return None, 1 ret = apply_subtree_update(dbs, source, name, tag, - info.advance_to, args) + info.advance_to, remote, + target) if ret: return None, ret continue @@ -1668,7 +1709,8 @@ def _prepare_get_commits(dbs, source, args): return info, None -def prepare_apply(dbs, source, branch, args=None, info=None): +def prepare_apply(dbs, source, branch, remote=None, target=None, # pylint: disable=too-many-arguments + info=None): """Prepare for applying commits from a source branch Get the next commits, set up the branch name and prints info about @@ -1679,8 +1721,9 @@ def prepare_apply(dbs, source, branch, args=None, info=None): dbs (Database): Database instance source (str): Source branch name branch (str): Branch name to use, or None to auto-generate - args (Namespace): Parsed arguments with 'push', 'remote', 'target' - (needed for subtree updates) + remote (str): Git remote name (e.g. 'ci'), or None to skip + subtree updates + target (str): Target branch name (e.g. 'master') info (NextCommitsInfo): Pre-fetched commit info from _prepare_get_commits(), or None to fetch it here @@ -1690,7 +1733,7 @@ def prepare_apply(dbs, source, branch, args=None, info=None): (0 for no commits, 1 for error) """ if info is None: - info, ret = _prepare_get_commits(dbs, source, args) + info, ret = _prepare_get_commits(dbs, source, remote, target) if ret is not None: return None, ret @@ -1910,7 +1953,8 @@ def do_apply(args, dbs, info=None): int: 0 on success, 1 on failure """ source = args.source - info, ret = prepare_apply(dbs, source, args.branch, args, info=info) + info, ret = prepare_apply(dbs, source, args.branch, args.remote, + args.target, info=info) if not info: return ret @@ -2863,7 +2907,7 @@ def do_step(args, dbs): # Process subtree updates and advance past fully-processed merges # regardless of MR count, since these don't create MRs - info, ret = _prepare_get_commits(dbs, source, args) + info, ret = _prepare_get_commits(dbs, source, remote, args.target) if ret is not None: if ret: return ret diff --git a/tools/pickman/ftest.py b/tools/pickman/ftest.py index 766cc714ed7..590677d6b5b 100644 --- a/tools/pickman/ftest.py +++ b/tools/pickman/ftest.py @@ -1442,7 +1442,8 @@ class TestApply(unittest.TestCase): database.Database.instances.clear() - args = argparse.Namespace(cmd='apply', source='unknown', branch=None) + args = argparse.Namespace(cmd='apply', source='unknown', branch=None, + remote='ci', target='master') with terminal.capture() as (_, stderr): ret = control.do_pickman(args) self.assertEqual(ret, 1) @@ -1460,7 +1461,8 @@ class TestApply(unittest.TestCase): database.Database.instances.clear() command.TEST_RESULT = command.CommandResult(stdout='') - args = argparse.Namespace(cmd='apply', source='us/next', branch=None) + args = argparse.Namespace(cmd='apply', source='us/next', branch=None, + remote='ci', target='master') with terminal.capture() as (stdout, _): ret = control.do_pickman(args) self.assertEqual(ret, 0) @@ -4026,7 +4028,8 @@ class TestApplySubtreeUpdate(unittest.TestCase): return_value=mock_result): ret = control.apply_subtree_update( dbs, 'us/next', 'dts', 'v6.15-dts', - 'merge_hash', args) + 'merge_hash', 'ci', 'master', + push=args.push) self.assertEqual(ret, 0) @@ -4077,7 +4080,48 @@ class TestApplySubtreeUpdate(unittest.TestCase): side_effect=mock_push): ret = control.apply_subtree_update( dbs, 'us/next', 'dts', 'v6.15-dts', - 'merge_hash', args) + 'merge_hash', 'ci', 'master', + push=args.push) + + self.assertEqual(ret, 0) + self.assertTrue(pushed[0]) + dbs.close() + + def test_apply_push_defaults_to_true(self): + """Test apply_subtree_update pushes when push is not specified.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + dbs.source_set('us/next', 'base') + dbs.commit() + + def run_git_handler(git_args): + if 'rev-parse' in git_args: + return 'first_parent\nsquash_hash' + if 'checkout' in git_args: + return '' + if '--format=%s|%an' in git_args: + return 'Commit subject|Author' + return '' + + pushed = [False] + + def mock_push(remote, branch, skip_ci=False): + pushed[0] = True + return True + + mock_result = command.CommandResult('ok', '', '', 0) + with mock.patch.object(control, 'run_git', + side_effect=run_git_handler): + with mock.patch.object( + control.command, 'run_one', + return_value=mock_result): + with mock.patch.object( + control.gitlab_api, 'push_branch', + side_effect=mock_push): + ret = control.apply_subtree_update( + dbs, 'us/next', 'dts', 'v6.15-dts', + 'merge_hash', 'ci', 'master') self.assertEqual(ret, 0) self.assertTrue(pushed[0]) @@ -4105,7 +4149,8 @@ class TestApplySubtreeUpdate(unittest.TestCase): side_effect=run_git_handler): ret = control.apply_subtree_update( dbs, 'us/next', 'dts', 'v6.15-dts', - 'merge_hash', args) + 'merge_hash', 'ci', 'master', + push=args.push) self.assertEqual(ret, 1) # Source should not be advanced @@ -4144,7 +4189,8 @@ class TestApplySubtreeUpdate(unittest.TestCase): return_value=0): ret = control.apply_subtree_update( dbs, 'us/next', 'dts', 'v6.15-dts', - 'merge_hash', args) + 'merge_hash', 'ci', 'master', + push=args.push) self.assertEqual(ret, 0) # Should have tried bare checkout, then fallback @@ -4170,7 +4216,8 @@ class TestApplySubtreeUpdate(unittest.TestCase): return_value='single_parent'): ret = control.apply_subtree_update( dbs, 'us/next', 'dts', 'v6.15-dts', - 'merge_hash', args) + 'merge_hash', 'ci', 'master', + push=args.push) self.assertEqual(ret, 1) dbs.close() @@ -4200,7 +4247,8 @@ class TestApplySubtreeUpdate(unittest.TestCase): side_effect=Exception('script failed')): ret = control.apply_subtree_update( dbs, 'us/next', 'dts', 'v6.15-dts', - 'merge_hash', args) + 'merge_hash', 'ci', 'master', + push=args.push) self.assertEqual(ret, 1) # Source should not be advanced @@ -4208,7 +4256,7 @@ class TestApplySubtreeUpdate(unittest.TestCase): dbs.close() def test_apply_merge_conflict(self): - """Test apply_subtree_update aborts merge on non-zero exit.""" + """Test apply_subtree_update aborts merge on non-conflict failure.""" with terminal.capture(): dbs = database.Database(self.db_path) dbs.start() @@ -4222,6 +4270,8 @@ class TestApplySubtreeUpdate(unittest.TestCase): def run_git_handler(git_args): if 'rev-parse' in git_args: + if '--verify' in git_args: + raise Exception('no MERGE_HEAD') return 'first_parent\nsquash_hash' if 'checkout' in git_args: return '' @@ -4239,7 +4289,96 @@ class TestApplySubtreeUpdate(unittest.TestCase): return_value=mock_result): ret = control.apply_subtree_update( dbs, 'us/next', 'dts', 'v6.15-dts', - 'merge_hash', args) + 'merge_hash', 'ci', 'master', + push=args.push) + + self.assertEqual(ret, 1) + self.assertTrue(merge_aborted[0]) + # Source should not be advanced + self.assertEqual(dbs.source_get('us/next'), 'base') + dbs.close() + + def test_apply_merge_conflict_agent_resolves(self): + """Test apply_subtree_update invokes agent on conflict and succeeds.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + dbs.source_set('us/next', 'base') + dbs.commit() + + args = argparse.Namespace(push=False, remote='ci', + target='master') + + def run_git_handler(git_args): + if 'rev-parse' in git_args: + if '--verify' in git_args: + return 'abc123' + return 'first_parent\nsquash_hash' + if 'checkout' in git_args: + return '' + if '--format=%s|%an' in git_args: + return 'subject|author' + return '' + + mock_result = command.CommandResult( + '', 'CONFLICT (content): Merge conflict', '', 1) + with mock.patch.object(control, 'run_git', + side_effect=run_git_handler): + with mock.patch.object( + control.command, 'run_one', + return_value=mock_result): + with mock.patch.object( + agent, 'resolve_subtree_conflicts', + return_value=(True, 'resolved ok')): + ret = control.apply_subtree_update( + dbs, 'us/next', 'dts', 'v6.15-dts', + 'merge_hash', 'ci', 'master', + push=args.push) + + self.assertEqual(ret, 0) + # Source should be advanced + self.assertEqual(dbs.source_get('us/next'), 'merge_hash') + dbs.close() + + def test_apply_merge_conflict_agent_fails(self): + """Test apply_subtree_update aborts when agent fails to resolve.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + dbs.source_set('us/next', 'base') + dbs.commit() + + args = argparse.Namespace(push=False, remote='ci', + target='master') + + merge_aborted = [False] + + def run_git_handler(git_args): + if 'rev-parse' in git_args: + if '--verify' in git_args: + return 'abc123' + return 'first_parent\nsquash_hash' + if 'checkout' in git_args: + return '' + if 'merge' in git_args and '--abort' in git_args: + merge_aborted[0] = True + return '' + return '' + + mock_result = command.CommandResult( + '', 'CONFLICT (content): Merge conflict', '', 1) + with mock.patch.object(control, 'run_git', + side_effect=run_git_handler): + with mock.patch.object( + control.command, 'run_one', + return_value=mock_result): + with mock.patch.object( + agent, 'resolve_subtree_conflicts', + return_value=(False, 'failed')): + ret = control.apply_subtree_update( + dbs, 'us/next', 'dts', 'v6.15-dts', + 'merge_hash', 'ci', 'master', + push=args.push) self.assertEqual(ret, 1) self.assertTrue(merge_aborted[0]) @@ -4276,8 +4415,6 @@ class TestPrepareApplySubtreeUpdate(unittest.TestCase): dbs.source_set('us/next', 'base') dbs.commit() - args = argparse.Namespace(push=False, remote='ci', - target='master') subtree_info = control.NextCommitsInfo( [], True, 'merge1', ('dts', 'v6.15-dts')) normal_info = control.NextCommitsInfo([], False, None) @@ -4296,11 +4433,12 @@ class TestPrepareApplySubtreeUpdate(unittest.TestCase): control, 'apply_subtree_update', return_value=0) as mock_apply: info, ret = control.prepare_apply( - dbs, 'us/next', None, args) + dbs, 'us/next', None, 'ci', 'master') # Should have called apply_subtree_update mock_apply.assert_called_once_with( - dbs, 'us/next', 'dts', 'v6.15-dts', 'merge1', args) + dbs, 'us/next', 'dts', 'v6.15-dts', 'merge1', + 'ci', 'master') # No commits after retry, so returns None/0 self.assertIsNone(info) self.assertEqual(ret, 0) @@ -4314,8 +4452,6 @@ class TestPrepareApplySubtreeUpdate(unittest.TestCase): dbs.source_set('us/next', 'base') dbs.commit() - args = argparse.Namespace(push=False, remote='ci', - target='master') subtree_info = control.NextCommitsInfo( [], True, 'merge1', ('dts', 'v6.15-dts')) @@ -4325,14 +4461,14 @@ class TestPrepareApplySubtreeUpdate(unittest.TestCase): control, 'apply_subtree_update', return_value=1): info, ret = control.prepare_apply( - dbs, 'us/next', None, args) + dbs, 'us/next', None, 'ci', 'master') self.assertIsNone(info) self.assertEqual(ret, 1) dbs.close() - def test_prepare_apply_subtree_without_args(self): - """Test prepare_apply returns error when subtree needs args=None.""" + def test_prepare_apply_subtree_without_remote(self): + """Test prepare_apply returns error when subtree needs remote=None.""" with terminal.capture(): dbs = database.Database(self.db_path) dbs.start() @@ -6265,5 +6401,129 @@ class TestStepFixRetries(unittest.TestCase): self.assertEqual(args.fix_retries, 1) +class TestIsMergeInProgress(unittest.TestCase): + """Tests for _is_merge_in_progress helper.""" + + def test_merge_in_progress(self): + """Test returns True when MERGE_HEAD exists.""" + with mock.patch.object(control, 'run_git', return_value='abc123'): + self.assertTrue(control._is_merge_in_progress()) + + def test_no_merge_in_progress(self): + """Test returns False when MERGE_HEAD does not exist.""" + with mock.patch.object(control, 'run_git', + side_effect=Exception('not found')): + self.assertFalse(control._is_merge_in_progress()) + + +class TestSubtreeRunUpdateReturnValues(unittest.TestCase): + """Tests for _subtree_run_update return values.""" + + def test_returns_ok_on_success(self): + """Test returns SUBTREE_OK on successful update.""" + mock_result = command.CommandResult('done', '', '', 0) + with terminal.capture(): + with mock.patch.object(control.command, 'run_one', + return_value=mock_result): + ret = control._subtree_run_update('dts', 'v6.15-dts') + self.assertEqual(ret, control.SUBTREE_OK) + + def test_returns_conflict_when_merge_in_progress(self): + """Test returns SUBTREE_CONFLICT when merge state exists.""" + mock_result = command.CommandResult( + '', 'CONFLICT', '', 1) + with terminal.capture(): + with mock.patch.object(control.command, 'run_one', + return_value=mock_result): + with mock.patch.object(control, '_is_merge_in_progress', + return_value=True): + ret = control._subtree_run_update('dts', 'v6.15-dts') + self.assertEqual(ret, control.SUBTREE_CONFLICT) + + def test_returns_fail_when_no_merge(self): + """Test returns SUBTREE_FAIL when script fails without merge.""" + mock_result = command.CommandResult( + '', 'error', '', 1) + with terminal.capture(): + with mock.patch.object(control.command, 'run_one', + return_value=mock_result): + with mock.patch.object(control, '_is_merge_in_progress', + return_value=False): + with mock.patch.object(control, 'run_git'): + ret = control._subtree_run_update( + 'dts', 'v6.15-dts') + self.assertEqual(ret, control.SUBTREE_FAIL) + + def test_returns_fail_on_exception(self): + """Test returns SUBTREE_FAIL when script raises exception.""" + with terminal.capture(): + with mock.patch.object(control.command, 'run_one', + side_effect=Exception('boom')): + ret = control._subtree_run_update('dts', 'v6.15-dts') + self.assertEqual(ret, control.SUBTREE_FAIL) + + +class TestSubtreeConflictPrompt(unittest.TestCase): + """Tests for build_subtree_conflict_prompt.""" + + def test_dts_prompt_content(self): + """Test prompt contains correct details for dts subtree.""" + prompt = agent.build_subtree_conflict_prompt( + 'dts', 'v6.15-dts', 'dts/upstream') + self.assertIn('dts/upstream', prompt) + self.assertIn('v6.15-dts', prompt) + self.assertIn('MERGE_HEAD', prompt) + self.assertIn('git commit --no-edit', prompt) + self.assertIn('git merge --abort', prompt) + + def test_mbedtls_prompt_content(self): + """Test prompt contains correct details for mbedtls subtree.""" + prompt = agent.build_subtree_conflict_prompt( + 'mbedtls', 'v3.6.0', 'lib/mbedtls/external/mbedtls') + self.assertIn('lib/mbedtls/external/mbedtls', prompt) + self.assertIn('v3.6.0', prompt) + self.assertIn("'mbedtls'", prompt) + + +class TestResolveSubtreeConflicts(unittest.TestCase): + """Tests for resolve_subtree_conflicts.""" + + def test_success(self): + """Test successful conflict resolution.""" + mock_collect = mock.AsyncMock(return_value=(True, 'resolved')) + with terminal.capture(): + with mock.patch.object(agent, 'AGENT_AVAILABLE', True): + with mock.patch.object(agent, 'run_agent_collect', + mock_collect): + with mock.patch.object(agent, 'ClaudeAgentOptions'): + success, log = agent.resolve_subtree_conflicts( + 'dts', 'v6.15-dts', 'dts/upstream', + '/tmp/test') + self.assertTrue(success) + self.assertEqual(log, 'resolved') + + def test_failure(self): + """Test failed conflict resolution.""" + mock_collect = mock.AsyncMock(return_value=(False, 'failed')) + with terminal.capture(): + with mock.patch.object(agent, 'AGENT_AVAILABLE', True): + with mock.patch.object(agent, 'run_agent_collect', + mock_collect): + with mock.patch.object(agent, 'ClaudeAgentOptions'): + success, log = agent.resolve_subtree_conflicts( + 'dts', 'v6.15-dts', 'dts/upstream', + '/tmp/test') + self.assertFalse(success) + + def test_sdk_unavailable(self): + """Test returns failure when SDK is not available.""" + with terminal.capture(): + with mock.patch.object(agent, 'AGENT_AVAILABLE', False): + success, log = agent.resolve_subtree_conflicts( + 'dts', 'v6.15-dts', 'dts/upstream', '/tmp/test') + self.assertFalse(success) + self.assertEqual(log, '') + + if __name__ == '__main__': unittest.main() From patchwork Thu Mar 5 14:54:46 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 1972 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=1772722530; bh=HnrBvlmZLu8eiVTT7Hmi1vtQN4zeKfW7nTplhWq4rd4=; 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=OTjVccPpgYNMV5VsHzE/UxgcWmipkYSUtHkhik6WP1VJcQWQE39fi6WE1qR/9nk2S xGHi2TDECadKlGMDWJLOPe55dEzJLjF9m4/bM1Q/2NRirr2rxYG0O319XPtI2m2PRR xXf7PIU0idKt7E0p2mPPx450Kme1pDsdUfa5d+M7Z20aJOwg91F98oQyRpGnHb+jEt IAfsy/EuK34u0D5lhaqWkOxd2X++cJk31Y68q7CNy5vaOasjYQFq/fR3XE8sdEJ1Oy zdn79IDSYVHZmzUOjwqL5hwhq4cBRuR6zm2exLb2NgGT/wDwgG68e7k+kCdRme0hxj 8rYvavxVsQaQg== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 28B6A69F25 for ; Thu, 5 Mar 2026 07:55:30 -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 JWAD-jdvq11E for ; Thu, 5 Mar 2026 07:55:30 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1772722530; bh=HnrBvlmZLu8eiVTT7Hmi1vtQN4zeKfW7nTplhWq4rd4=; 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=OTjVccPpgYNMV5VsHzE/UxgcWmipkYSUtHkhik6WP1VJcQWQE39fi6WE1qR/9nk2S xGHi2TDECadKlGMDWJLOPe55dEzJLjF9m4/bM1Q/2NRirr2rxYG0O319XPtI2m2PRR xXf7PIU0idKt7E0p2mPPx450Kme1pDsdUfa5d+M7Z20aJOwg91F98oQyRpGnHb+jEt IAfsy/EuK34u0D5lhaqWkOxd2X++cJk31Y68q7CNy5vaOasjYQFq/fR3XE8sdEJ1Oy zdn79IDSYVHZmzUOjwqL5hwhq4cBRuR6zm2exLb2NgGT/wDwgG68e7k+kCdRme0hxj 8rYvavxVsQaQg== Received: from mail.u-boot.org (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 14B7469F0F for ; Thu, 5 Mar 2026 07:55:30 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1772722527; bh=VbxFIymALPYOhNZdt53Oin+CdAzYHma6kPAIddpGmtA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=oFRmL8vl9YV7PXzv1YRKQzi21XkofrzlRWxqVmCnlcvsbj/RpekI7W0Y0wYaLOhNU h7oB3tLeHhMBxh6FsuG+vdtA39DxqKwKsR96bRkAxmyuCjUgrBI6F98SDBQX8UWVn0 L35MIoWVTXREZdJknISdIU6QHXXCUTNwrtfAS2Va996g2r54qtWkNf12DpkvaGfjv0 YfqtT4TN7BJnSh9PYJPnfQQ8q0Vmjx3lojbrJrq/UzWJMkelT1+SbMjT/ueNe0Py9l YKgTMgzt+sooMQR8PbDby5cNwVqaASVwkfbyN19SyUs2UxNpMDXXJKBSyhw1WLCJUp lyBZ+NLUQe7+w== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 9BD3669D92; Thu, 5 Mar 2026 07:55:27 -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 CWlFz5-wZxWp; Thu, 5 Mar 2026 07:55:27 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1772722523; bh=lCN1fa3+7sAzha/LQjYp2RvakRkehkuoGkgyUJhVE/U=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=fHjownYsVze0gGIax4BsvWhz7Tn1C0TpjE/8iGGvfaiYlV9gVOgrU267tF+86hdPP iLt2Ym99vv2lrHJ1La3tt+yIOMuhdDM5n7QXtcPJEl7wiuH0mcL9osSCRguW9rEbY2 npvUl04Hi81/fz4wLahxGMUC4axItLoIQz87rzKAmBEw2BC4n/ZmFvnX+jCiWF9NKa ERlxbZgSX3YdNhCgDXmPp+DBFHxXFO/VXPYAU3SYQqqrG/mFuAAWcilNyj7svQXHPE 2iEAtw9fk98SNWZFv5tlCVlaELUWO4hNO/8MD/E+74xtRvOW0L0W8WLTh5OYDsgC+T j2fnErUhJORYA== Received: from u-boot.org (unknown [73.34.74.121]) by mail.u-boot.org (Postfix) with ESMTPSA id 3E46C69EC0; Thu, 5 Mar 2026 07:55:23 -0700 (MST) From: Simon Glass To: U-Boot Concept Date: Thu, 5 Mar 2026 07:54:46 -0700 Message-ID: <20260305145452.909661-4-sjg@u-boot.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20260305145452.909661-1-sjg@u-boot.org> References: <20260305145452.909661-1-sjg@u-boot.org> MIME-Version: 1.0 Message-ID-Hash: HKWOFEUN3X2EHV2MVZB3KLDRVOGUKDB5 X-Message-ID-Hash: HKWOFEUN3X2EHV2MVZB3KLDRVOGUKDB5 X-MailFrom: sjg@u-boot.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: Simon Glass X-Mailman-Version: 3.3.10 Precedence: list Subject: [Concept] [PATCH 3/4] pickman: Fix push_branch() to raise on failure 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 push_branch() catches CommandExc and returns False on failure, but callers like apply_subtree_update() use try/except to detect push failures rather than checking the return value. This means push failures are silently ignored and the source is advanced even though the branch was not pushed. Change push_branch() to re-raise the CommandExc instead of returning False, and update all callers to use exception handling consistently. This ensures apply_subtree_update() properly aborts when a push fails. Signed-off-by: Simon Glass --- tools/pickman/control.py | 9 ++++++--- tools/pickman/ftest.py | 6 ++++-- tools/pickman/gitlab_api.py | 9 +++++++-- 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/tools/pickman/control.py b/tools/pickman/control.py index ed6825c2333..6d787d02163 100644 --- a/tools/pickman/control.py +++ b/tools/pickman/control.py @@ -2080,9 +2080,12 @@ def do_push_branch(args, dbs): # pylint: disable=unused-argument int: 0 on success, 1 on failure """ skip_ci = not args.run_ci - success = gitlab_api.push_branch(args.remote, args.branch, args.force, - skip_ci=skip_ci) - return 0 if success else 1 + try: + gitlab_api.push_branch(args.remote, args.branch, args.force, + skip_ci=skip_ci) + except command.CommandExc: + return 1 + return 0 def do_commit_source(args, dbs): diff --git a/tools/pickman/ftest.py b/tools/pickman/ftest.py index 590677d6b5b..95053e78d21 100644 --- a/tools/pickman/ftest.py +++ b/tools/pickman/ftest.py @@ -4956,8 +4956,10 @@ class TestDoPushBranch(unittest.TestCase): tout.init(tout.INFO) args = argparse.Namespace(cmd='push-branch', branch='test-branch', remote='ci', force=False, run_ci=False) - with mock.patch.object(gitlab, 'push_branch', - return_value=False): + with mock.patch.object( + gitlab, 'push_branch', + side_effect=command.CommandExc( + 'push failed', command.CommandResult())): with terminal.capture(): ret = control.do_push_branch(args, None) self.assertEqual(ret, 1) diff --git a/tools/pickman/gitlab_api.py b/tools/pickman/gitlab_api.py index 81918c80c3c..4d90d622dbc 100644 --- a/tools/pickman/gitlab_api.py +++ b/tools/pickman/gitlab_api.py @@ -196,6 +196,9 @@ def push_branch(remote, branch, force=False, skip_ci=True): Returns: bool: True on success + + Raises: + command.CommandExc: If the push fails """ try: # Use token-authenticated URL if available @@ -232,7 +235,7 @@ def push_branch(remote, branch, force=False, skip_ci=True): return True except command.CommandExc as exc: tout.error(f'Failed to push branch: {exc}') - return False + raise # pylint: disable=too-many-arguments @@ -630,7 +633,9 @@ def push_and_create_mr(remote, branch, target, title, desc=''): return None tout.info(f'Pushing {branch} to {remote}...') - if not push_branch(remote, branch, force=True): + try: + push_branch(remote, branch, force=True) + except command.CommandExc: return None tout.info(f'Creating merge request to {target}...') From patchwork Thu Mar 5 14:54:47 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 1973 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=1772722534; bh=WKGNEuwavI7KZZhFNnIxT/Ja8pTLzkH912bRtGArchQ=; 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=kSPfMfCQxvaxm3uPSeLU4Q0OAKPccnhqDhkY2Rntfy77rdJ6CPpFfBz80c4UvHAWI 1vzNxJJSWZ5GXt6H3QBty7/XnD5jU7akK3rHGNA/+hOLbOuxFVJzDmU3pJB1KWcGsa Z1tiv2apjLKIjkz6Xjk2Xok0Hdl5oZmRRfmgGYWcv20hFIo60tXrcFK4jA6Fc8e9+c Pulqm80w2ZfXheTnNIcw8+XBNNUy8V5Eb27SoxvtSFaYgw8SfCB2e7YIkpWTtnHT+T Fl9j5VwDLzwfhWwg8TCNOeXCI+DM54ZIh/BPmx9o7hlnnHfIryNWvWhvWX6u8jDaX+ pQjFghBL/4dWw== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id AB5FC69F22 for ; Thu, 5 Mar 2026 07:55:34 -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 NgH896IZItSX for ; Thu, 5 Mar 2026 07:55:34 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1772722534; bh=WKGNEuwavI7KZZhFNnIxT/Ja8pTLzkH912bRtGArchQ=; 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=kSPfMfCQxvaxm3uPSeLU4Q0OAKPccnhqDhkY2Rntfy77rdJ6CPpFfBz80c4UvHAWI 1vzNxJJSWZ5GXt6H3QBty7/XnD5jU7akK3rHGNA/+hOLbOuxFVJzDmU3pJB1KWcGsa Z1tiv2apjLKIjkz6Xjk2Xok0Hdl5oZmRRfmgGYWcv20hFIo60tXrcFK4jA6Fc8e9+c Pulqm80w2ZfXheTnNIcw8+XBNNUy8V5Eb27SoxvtSFaYgw8SfCB2e7YIkpWTtnHT+T Fl9j5VwDLzwfhWwg8TCNOeXCI+DM54ZIh/BPmx9o7hlnnHfIryNWvWhvWX6u8jDaX+ pQjFghBL/4dWw== Received: from mail.u-boot.org (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 9AB6269EC0 for ; Thu, 5 Mar 2026 07:55:34 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1772722532; bh=V//o/oyxeLQzuTDcP7vtuSdushef3ic6SLRscMGnrkQ=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=pjuxanMFvGLxFZd4VJWuXnxuDwYqbzOAp8FjdObS1+fDMi/WSFz/U8K+icoh9ItV2 mr3+Z23k8ODWPhPPd+hLCdSXv1x9hHLaJ8/iW7x0HqrycZK41TgcLQChfJL30v2FmG GGHOvNZjRUUDUnyEqXJ9ZTAIsfkMXaQcPLbPGvYNsKSvw+9/4jHicWCyx2pp12D2DL tCcttQ3QomDm8eYyL3nwNeZVQIfloFZMYcBVoE4BAiPUKRhHXM4KzyU6905qIOBYgr fh+g/zuvtv208NtLBirYuLd06jcxS+x2rpEoodj1lGvvY2tl7fyxcvak3ryIBiT4RE Ua5rQCsb/YQBw== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 9B37F69F0F; Thu, 5 Mar 2026 07:55:32 -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 H_BZpeozKrJM; Thu, 5 Mar 2026 07:55:32 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1772722528; bh=8ONThggmGdPKfrdvuTwwrRgkUXpQMRjX1ml/KUzgIdA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=egb2pwnrnnKN86UbJXTV6ZCT1ZXz0eJEOnLA4loixj0tWQiSxXOylYnJ2gn/djsdZ ebvixczum6IEMXfVAPJKGhQiFd+XWIrgPdI5XsImZZGQ5QF3e1S4IjvacZ0+qqb7Eh qwkWMokvD+afodAAECuPQBv5UxJ7HarHenaBhrQRx9+ts2UFh8kQUPU4Uf/1AYI9JM lnUPFcbjrfkypPM6EXwEER8P6HrWT+byDFVnGL6h3KZt/PLsqgJhaWUni6doeNUoYN k0HGjhhQDvIRpDWsudNbKNC/XW5uge0sm9XIzSeuP6+EMRf4IPrv7DgO2KRpx7U5Fx TT+20gpyzUvEg== Received: from u-boot.org (unknown [73.34.74.121]) by mail.u-boot.org (Postfix) with ESMTPSA id DE2C669EC0; Thu, 5 Mar 2026 07:55:27 -0700 (MST) From: Simon Glass To: U-Boot Concept Date: Thu, 5 Mar 2026 07:54:47 -0700 Message-ID: <20260305145452.909661-5-sjg@u-boot.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20260305145452.909661-1-sjg@u-boot.org> References: <20260305145452.909661-1-sjg@u-boot.org> MIME-Version: 1.0 Message-ID-Hash: NUIBC645AE4GITJZO5CURU5SRHQDMTA2 X-Message-ID-Hash: NUIBC645AE4GITJZO5CURU5SRHQDMTA2 X-MailFrom: sjg@u-boot.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: Simon Glass X-Mailman-Version: 3.3.10 Precedence: list Subject: [Concept] [PATCH 4/4] pickman: Handle connection errors gracefully in do_step() 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 the GitLab server drops the connection, do_step() crashes with an unhandled requests.exceptions.ConnectionError. Catch this specific exception in do_step(), report it, and return failure. This means 'poll' continues to the next iteration after the sleep, while 'step' reports the error and aborts cleanly. Signed-off-by: Simon Glass --- tools/pickman/control.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tools/pickman/control.py b/tools/pickman/control.py index 6d787d02163..817ae02d900 100644 --- a/tools/pickman/control.py +++ b/tools/pickman/control.py @@ -15,6 +15,8 @@ import tempfile import time import unittest +import requests + # Allow 'from pickman import xxx' to work via symlink our_path = os.path.dirname(os.path.realpath(__file__)) sys.path.insert(0, os.path.join(our_path, '..')) @@ -2872,6 +2874,15 @@ def do_step(args, dbs): Returns: int: 0 on success, 1 on failure """ + try: + return _do_step(args, dbs) + except requests.exceptions.ConnectionError as exc: + tout.error(f'step failed with connection error: {exc}') + return 1 + + +def _do_step(args, dbs): + """Internal implementation of do_step""" remote = args.remote source = args.source