From patchwork Sun Feb 22 15:42:41 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 1922 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=1771775009; bh=qYtJJT7j8y379ST+UiPU8ApyaR/fh1PsEY+9O61bPcI=; 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=b1Z73pTTjisGTDf8A2BbgzcnlBb5rG01w0Ov/pOsMnPhGlQBA19MYi/7+bEmaFqW5 4Cgh8vX8yvWqxG1iUm4A5ymAVWgZThGUd0/ho+jJxDsNO3Ave5i3M4Y71hTUoYgMyY kwRtiPVQgBl6qqXm2m6mtMnJt34WTKhMK3839iKCfmTOqO00hDs5m9eCAhR4xbf6QH 7qklj6OQtI6yN5fhZ1iv4njsfNs0sRA8QVoTyC6jycUJdO1BRyQmGfFOyLEKHmYRXn HJGFRIvCrDDGzj6Q9FfZ+Gm9szRLoqfNK3J2zqj/fKsJ4uLA+z2HXM6tKMrwoeoGWq xdBDFMvTi+gFw== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id D010B69D36 for ; Sun, 22 Feb 2026 08:43:29 -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 jgovUS_mHWGz for ; Sun, 22 Feb 2026 08:43:29 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1771775009; bh=qYtJJT7j8y379ST+UiPU8ApyaR/fh1PsEY+9O61bPcI=; 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=b1Z73pTTjisGTDf8A2BbgzcnlBb5rG01w0Ov/pOsMnPhGlQBA19MYi/7+bEmaFqW5 4Cgh8vX8yvWqxG1iUm4A5ymAVWgZThGUd0/ho+jJxDsNO3Ave5i3M4Y71hTUoYgMyY kwRtiPVQgBl6qqXm2m6mtMnJt34WTKhMK3839iKCfmTOqO00hDs5m9eCAhR4xbf6QH 7qklj6OQtI6yN5fhZ1iv4njsfNs0sRA8QVoTyC6jycUJdO1BRyQmGfFOyLEKHmYRXn HJGFRIvCrDDGzj6Q9FfZ+Gm9szRLoqfNK3J2zqj/fKsJ4uLA+z2HXM6tKMrwoeoGWq xdBDFMvTi+gFw== Received: from mail.u-boot.org (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id BD3B669D3F for ; Sun, 22 Feb 2026 08:43:29 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1771775006; bh=pQP3l2EatCmavDijLEm+RB1jF8njO1dSbzNAW5HxTqE=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=nPclFjrZALfQ+zLn7unJFmGIvNmtYtv8t+APyv0VKHK8+HE9JrlejxpIum+qZc5At ce9UyUlRY/I8HsLMsMGIzkUHFY2X60s83fWlvod6Jv0QRUXC99ju/iAL43EY8EtxYr zdDW6RRhsGrVvqfq+BiWzqWIhMRP+5cEVf3Dh81JZQRChGAbNW9j/EaBHyRktJW19O 6TTHK3ybbsmEHAq0RKtRq5sVz3m7xP1U4us7HaWT2GI2PCsPsTUhU0h5NmGkPWMYBu TWrIivKA+Ug00y6jagGPusIHZP91vofICx9RXKtwjlAfiUAr7h4S+WLi9q59FavfEw 61mTs9cLtRESQ== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 9674069C65; Sun, 22 Feb 2026 08:43:26 -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 AG0gMr9JiaFJ; Sun, 22 Feb 2026 08:43:26 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1771775002; bh=mh8EhJkEvDSXNj9JgKYberS0R74ilHX9+K6JSLooNcs=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=a4kwZy+yd3phu0e1mtF4yR8/PQmW/TL6AA0pnSkQ4CEMDAYMSWojREu9j5UrifFOz L5JFKyACN8DgMA/Uaop8RtCWmaTirCh0tswXkDy90dgSxIlUZOL89AS2F9ENg3bBzO RYTsm7wEfROp5xZcjF+xH9J2ZQdFsx0TXGx1aNMwHmZP6I+0VzBkrvopQrl/8s4OwZ oUUYk8JVq4Ngw6Gn8f27CDWXwqqHjNbtZd4O97EfYjq7ufbE86OcwuYapFLtN+/921 dAMEMXiWN0AunbjflI5jj4mgqeyiqYDUokXFBthwxi4qlSZBD1B2zqHH80oHjyTAxU 1ZGky7QuT02WA== Received: from u-boot.org (unknown [73.34.74.121]) by mail.u-boot.org (Postfix) with ESMTPSA id 1077E69C5C; Sun, 22 Feb 2026 08:43:22 -0700 (MST) From: Simon Glass To: U-Boot Concept Date: Sun, 22 Feb 2026 08:42:41 -0700 Message-ID: <20260222154303.2851319-2-sjg@u-boot.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20260222154303.2851319-1-sjg@u-boot.org> References: <20260222154303.2851319-1-sjg@u-boot.org> MIME-Version: 1.0 Message-ID-Hash: YTO5HGP7NSO6HG2AWN72SQ6PRRKH667Y X-Message-ID-Hash: YTO5HGP7NSO6HG2AWN72SQ6PRRKH667Y 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 01/16] pickman: Drop unnecessary f-string prefixes in do_rewind() 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 Three tout.info() calls in do_rewind() use f-strings that contain no interpolated variables, triggering pylint W1309. Remove the f prefix from these plain strings. Co-developed-by: Claude Opus 4.6 Signed-off-by: Simon Glass --- tools/pickman/control.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/pickman/control.py b/tools/pickman/control.py index 8f7568e47bf..40bcaaef120 100644 --- a/tools/pickman/control.py +++ b/tools/pickman/control.py @@ -1952,18 +1952,18 @@ def do_rewind(args, dbs): 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:') + tout.info(' 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:') + tout.info(' 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:') + tout.info(' Branches to check for MRs:') for branch in mr_branches: tout.info(f' {branch}') From patchwork Sun Feb 22 15:42:42 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 1923 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=1771775014; bh=Z4pDHZbm209lAY2fLdfIY5UhWj4WJ4X9Zy5eNeUJggM=; 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=sRIUauN0ZHLGxRGRT2P35bgBvFCK1HqeMWHhoDe+Svl1D8xj5G6Rk99Ems6K3UBwm hPfUvzgVRa1ppNz6Hq+jAuP1YQVAKVjhF3sE1CNX4fI8SEtSD7Lo3aaUpRProzPnxo /1Vnhe/4w7F5drC9U8G/ufcpT6dpnUQ7iTizeQb978feVeCpv29tYAAWkqQrV9AGhk V63QSIBAzFMzrNrdQRIzdLcAyI4VpkGvouxnjBqUpWztinOJ/5mHF4zKuGreUVrurs Q5KM5h8NfjL0ILL0KL0oMlRSYcLFDvWZ0btBKsJHoAvvMpIKhoNYbExKFOzwXfMvum jwgHYRuXBmnxQ== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 728E469C5C for ; Sun, 22 Feb 2026 08:43: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 lUR0AUxXZAVL for ; Sun, 22 Feb 2026 08:43:34 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1771775014; bh=Z4pDHZbm209lAY2fLdfIY5UhWj4WJ4X9Zy5eNeUJggM=; 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=sRIUauN0ZHLGxRGRT2P35bgBvFCK1HqeMWHhoDe+Svl1D8xj5G6Rk99Ems6K3UBwm hPfUvzgVRa1ppNz6Hq+jAuP1YQVAKVjhF3sE1CNX4fI8SEtSD7Lo3aaUpRProzPnxo /1Vnhe/4w7F5drC9U8G/ufcpT6dpnUQ7iTizeQb978feVeCpv29tYAAWkqQrV9AGhk V63QSIBAzFMzrNrdQRIzdLcAyI4VpkGvouxnjBqUpWztinOJ/5mHF4zKuGreUVrurs Q5KM5h8NfjL0ILL0KL0oMlRSYcLFDvWZ0btBKsJHoAvvMpIKhoNYbExKFOzwXfMvum jwgHYRuXBmnxQ== Received: from mail.u-boot.org (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 5EA9769D36 for ; Sun, 22 Feb 2026 08:43:34 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1771775012; bh=1n/JEyJB54ZmnZhw8dsCP3QCUF/RNCUN665pKuZb2qs=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=dNLSnthnMR/pgABBRHER4GH30Dz3wdlcsv9PVcD96q78D3Zu2nl1M5IfJgq4/xOiQ 8lnPTyqvvtdZbICa35PfVnUxyKmUjf+MKKGo2b5J2cliH5MPEBOCfrX6Q2k37PXvHr CCkIKYCtPcG+3gzp2x2WJ3ZKovRc1J6Qj+2bUvoJZPDytrda7n3TCnocNhHBBMQtCc 56tsGvGehzQwpEMRC/ndltFOpBpIIV1g7lvFgr3CexMNnmBYEnoKLkfbq1Jb9PU/8K D17JzQfJk6hcUctEJy2Gt6oXXKlmVSkKh1gtmqEG9qMmEq9Ga4iCZ3LSIXTxSxhBnh gW+E7U5YMpOTw== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 5F65069D36; Sun, 22 Feb 2026 08:43: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 vwWfzvRBkT3d; Sun, 22 Feb 2026 08:43:32 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1771775007; bh=YaJjL0EN8oGkPTbvM3WPnhYLd7wiCT0sp6JyU9i4YU8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=RNKc0nXjToP4SQW+3M0vZaeaDBVJWx9at8mfi3U0YWuYBJzVwGNsXSW8Ew1D/t1wb sZ2WkykvBt9fd6seiBQ2hxABn/Fr1eH7q9vvrnSHLcuJCyhzyOMRRYX4lpemMq1Cvu GoCGffhwAyWgNgAvgEB36Qi3w440q0Yu8BDRI9Mo5hreeO7pmbz+9K8KbtfAhBjmdD Z6rwhY76CjiAyGc4o/jKDbve7mVL6VWSqTLkqMx4ZGqC6cAhQoZ+NxvIZfdGOuwCpm D68bFUGPPYXfjpKFJu8gxst2dNH+XCS4EkI9xySuzSYJRIYHk3sNZ9ysP3XcxPa9zh DQfIa48X1XhUg== Received: from u-boot.org (unknown [73.34.74.121]) by mail.u-boot.org (Postfix) with ESMTPSA id D062E69C5C; Sun, 22 Feb 2026 08:43:26 -0700 (MST) From: Simon Glass To: U-Boot Concept Date: Sun, 22 Feb 2026 08:42:42 -0700 Message-ID: <20260222154303.2851319-3-sjg@u-boot.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20260222154303.2851319-1-sjg@u-boot.org> References: <20260222154303.2851319-1-sjg@u-boot.org> MIME-Version: 1.0 Message-ID-Hash: QYS5KY2FRJTHUBFWQF267WBGL4OTH7GN X-Message-ID-Hash: QYS5KY2FRJTHUBFWQF267WBGL4OTH7GN 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 02/16] pickman: Refactor do_rewind() into smaller helpers 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 do_rewind() is ~145 lines with 8 distinct sequential phases, making it hard to follow. Extract five private helpers that each handle one phase: - _rewind_fetch_merges(): fetch merge history and locate the target - _rewind_get_range_commits(): list commits in the range and filter to those in the database - _rewind_find_branches(): match cherry-pick branches against the range - _rewind_find_mrs(): look up MR details for matching branches - _rewind_show_summary(): display the dry-run / execution summary This reduces do_rewind() to ~40 lines of sequential calls with clear control flow, while each helper is independently understandable. Co-developed-by: Claude Opus 4.6 Signed-off-by: Simon Glass --- tools/pickman/control.py | 196 +++++++++++++++++++++++++-------------- 1 file changed, 124 insertions(+), 72 deletions(-) diff --git a/tools/pickman/control.py b/tools/pickman/control.py index 40bcaaef120..868e03df8c9 100644 --- a/tools/pickman/control.py +++ b/tools/pickman/control.py @@ -1836,33 +1836,13 @@ 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 +def _rewind_fetch_merges(current, count): + """Fetch first-parent merges and find target index. Returns: - int: 0 on success, 1 on failure + tuple: (merges, target_idx) where merges is a list of + (hash, short_hash, subject) tuples, or None on error """ - 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', @@ -1870,31 +1850,34 @@ def do_rewind(args, dbs): ]) except Exception: # pylint: disable=broad-except tout.error(f'Could not read merge history for {current[:12]}') - return 1 + return None if not out: tout.error('No merges found in history') - return 1 + return None - # 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 '')) + 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 + return None - # 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] + return merges, target_idx + - # Get all commits in the range target..current +def _rewind_get_range_commits(dbs, target_hash, current): + """Get commits in range and filter to those in database. + + Returns: + tuple: (range_hashes_str, db_commits_list) or None on error + """ try: range_hashes = run_git([ 'rev-list', f'{target_hash}..{current}' @@ -1902,52 +1885,79 @@ def do_rewind(args, dbs): except Exception: # pylint: disable=broad-except tout.error(f'Could not list commits in range ' f'{target_hash[:12]}..{current[:12]}') - return 1 + return None - # 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. + return range_hashes, db_commits + + +def _rewind_find_branches(range_hashes, remote): + """Find cherry-pick branches matching commits in the range. + + Returns: + list: Branch names (without remote prefix) that match + """ + if not range_hashes: + return [] + + hash_set = set(range_hashes.strip().split('\n')) + try: + branch_out = run_git( + ['branch', '-r', '--list', f'{remote}/cherry-*']) + except Exception: # pylint: disable=broad-except + branch_out = '' + 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 + remote_prefix = f'{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 + + return mr_branches + + +def _rewind_find_mrs(mr_branches, remote): + """Look up MR details for matching branches. + + Returns: + list: PickmanMr objects whose source_branch matches + """ + if not mr_branches: + return [] + 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) + mrs = gitlab_api.get_open_pickman_mrs(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) + + return matched_mrs + + +def _rewind_show_summary(source, current, merges, target_idx, + db_commits, matched_mrs, mr_branches, + force): + """Display rewind summary.""" current_short = current[:12] + target_chash = merges[target_idx][1] + target_subject = merges[target_idx][2] + prefix = '' if force else '[dry run] ' tout.info(f"{prefix}Rewind '{source}': " f'{current_short} -> {target_chash}') @@ -1967,15 +1977,57 @@ def do_rewind(args, dbs): for branch in mr_branches: tout.info(f' {branch}') + +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 + + result = _rewind_fetch_merges(current, count) + if not result: + return 1 + merges, target_idx = result + target_hash = merges[target_idx][0] + + result = _rewind_get_range_commits(dbs, target_hash, current) + if not result: + return 1 + range_hashes, db_commits = result + + mr_branches = _rewind_find_branches(range_hashes, args.remote) + matched_mrs = _rewind_find_mrs(mr_branches, args.remote) + + _rewind_show_summary(source, current, merges, target_idx, + db_commits, matched_mrs, mr_branches, force) + 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() From patchwork Sun Feb 22 15:42:43 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 1924 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=1771775018; bh=DpKqgQI1pGBdaV+4C/LzvnUwaKnGJYHy/+UrSJcqSNI=; 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=db1qKLmvXdBu5lAJvdlVwht5fyC7YSRWsFKR36daDdM9giLZy78lxj56rOjT/L6em VoULBq1UdCQ2Z8BeXsSlrauGR/U13PncVPC8bE7tnc1JQ7rqgrzoIz84rts0YjbF9r mIH0eRLIaEmGwynX+8Pjh/3In6TC5rTlOGt0Lq9eQPocrju2K0tuOpGcWPIKZcRdKE d/mwt7/DK89LhosJWqExj1F7q28XirdnQSQs8l5SoW92vauMp0i4jit3GLc21bwU1j f1MryWJXwA7BPmYgFiXgNQk4vrBQlqS2PTJQLcBE5n2M22ibFSF2hYgQZvplPXWPBg 5AIDr3iN1uSQA== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id EDC1E69C78 for ; Sun, 22 Feb 2026 08:43:38 -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 QdTk0v9MR_4H for ; Sun, 22 Feb 2026 08:43:38 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1771775018; bh=DpKqgQI1pGBdaV+4C/LzvnUwaKnGJYHy/+UrSJcqSNI=; 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=db1qKLmvXdBu5lAJvdlVwht5fyC7YSRWsFKR36daDdM9giLZy78lxj56rOjT/L6em VoULBq1UdCQ2Z8BeXsSlrauGR/U13PncVPC8bE7tnc1JQ7rqgrzoIz84rts0YjbF9r mIH0eRLIaEmGwynX+8Pjh/3In6TC5rTlOGt0Lq9eQPocrju2K0tuOpGcWPIKZcRdKE d/mwt7/DK89LhosJWqExj1F7q28XirdnQSQs8l5SoW92vauMp0i4jit3GLc21bwU1j f1MryWJXwA7BPmYgFiXgNQk4vrBQlqS2PTJQLcBE5n2M22ibFSF2hYgQZvplPXWPBg 5AIDr3iN1uSQA== Received: from mail.u-boot.org (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id CFD6369D36 for ; Sun, 22 Feb 2026 08:43:38 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1771775017; bh=MIgOS9leFemDDFvaDo+pnZcm9M8Vi67L8Nb0I/0gEoU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=sRbz4EzxBXTLWt8aD3Svr2fDHYVGy9pICnf+/EWAUOGfRFRT5f+Loz6OFx1mqKpFk 7kCrYcteKG9Q9HQ+jQAjQrWpLRyexKKfT5IFLoXhA7w+ldIDV0A1VJ3jmfQ1fo025d wKvYe/i94rOBhU+hPum9w8GuFZf3uMBMihHpTQI/nUTaxvTP4xS293Hbor9cyB/aNK GOjGFwQ57rSk0+SxwHIUkbFPSm1nUjJqkYW1x4TnJefqkQYYtimHNKFa279z6Elqjw 9Wej/DBnBNWrSKIvB6PTizjIaOuj4eQgYrRbWRZMzQJ3Mt8ebOsLwZOkcvY8IUXNOi pHrw/RSZs3bXQ== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 0346169D36; Sun, 22 Feb 2026 08:43:37 -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 ihWKczMkYt9w; Sun, 22 Feb 2026 08:43:36 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1771775011; bh=s0QbEkr+7U7mnlDxPxO1cRO/q2VTnoS7mr3voGD4SNU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=bdpU7WkzSdiz+3Ve6BJo11/7PZWTux+pnYyV1iDvRhkeVIEVGpEZKZGlakiLjKMoI pYPXGHWlgP8uRebFcXgeJC9MRba0+ymQSCvidv6ivFw6YVv+62dQFhqjqnIHFBmzgj nj2bNlot2PYeE3CAl2YFoQBKixQfK/fkc4HTkTeEhk8ILJ2GmmsGPXdHPN/sJv+cnT s1o9cM9i8qYS+NvohSzpJQwbqtvgglSf4iBDJinm41E2rdLAWVIbfQxTBUyQU4KZZr ug4ZV3bz2PaenLMf8ZCEoomxM22MxR8GppkkpyEk8/xfHxHx4ccrgYHVEa8qmY/Jwq AnAzeR7g0uvAA== Received: from u-boot.org (unknown [73.34.74.121]) by mail.u-boot.org (Postfix) with ESMTPSA id AA43F69C78; Sun, 22 Feb 2026 08:43:31 -0700 (MST) From: Simon Glass To: U-Boot Concept Date: Sun, 22 Feb 2026 08:42:43 -0700 Message-ID: <20260222154303.2851319-4-sjg@u-boot.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20260222154303.2851319-1-sjg@u-boot.org> References: <20260222154303.2851319-1-sjg@u-boot.org> MIME-Version: 1.0 Message-ID-Hash: H4SED5CXOPVO4IEAJ2XZU3BTRRGXAW7O X-Message-ID-Hash: H4SED5CXOPVO4IEAJ2XZU3BTRRGXAW7O 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 03/16] pickman: Refactor do_next_merges() into smaller helpers 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 do_next_merges() mixes fetching, display-building and output in a single function. Extract three private helpers that each handle one phase: - _next_fetch_merges(): fetch and parse merge commits from git log - _next_build_display(): expand mega-merges into sub-merge display entries - _next_show_merges(): print the formatted listing This reduces do_next_merges() to ~20 lines of sequential calls. Co-developed-by: Claude Opus 4.6 Signed-off-by: Simon Glass --- tools/pickman/control.py | 72 +++++++++++++++++++++++++++------------- 1 file changed, 49 insertions(+), 23 deletions(-) diff --git a/tools/pickman/control.py b/tools/pickman/control.py index 868e03df8c9..d3cb8a8fff4 100644 --- a/tools/pickman/control.py +++ b/tools/pickman/control.py @@ -1081,27 +1081,12 @@ def do_next_set(args, dbs): return 0 -def do_next_merges(args, dbs): - """Show the next N merges to be applied from a source - - Args: - args (Namespace): Parsed arguments with 'source' and 'count' attributes - dbs (Database): Database instance +def _next_fetch_merges(last_commit, source, count): + """Fetch the next merge commits from a source. Returns: - int: 0 on success, 1 if source not found + list: (hash, short_hash, subject) tuples, up to count entries """ - source = args.source - count = args.count - - # Get the last cherry-picked commit from database - last_commit = dbs.source_get(source) - - if not last_commit: - tout.error(f"Source '{source}' not found in database") - return 1 - - # Find merge commits on the first-parent chain out = run_git([ 'log', '--reverse', '--first-parent', '--merges', '--format=%H|%h|%s', @@ -1109,8 +1094,7 @@ def do_next_merges(args, dbs): ]) if not out: - tout.info('No merges remaining') - return 0 + return [] merges = [] for line in out.split('\n'): @@ -1124,9 +1108,18 @@ def do_next_merges(args, dbs): if len(merges) >= count: break - # 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 + return merges + + +def _next_build_display(merges): + """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. + + Returns: + tuple: (display_list, total_sub_count) + """ display = [] total_sub = 0 for commit_hash, chash, subject in merges: @@ -1149,6 +1142,11 @@ def do_next_merges(args, dbs): else: display.append((chash, subject, False, None)) + return display, total_sub + + +def _next_show_merges(source, merges, display, total_sub): + """Display the next-merges listing.""" n_items = total_sub + len(merges) - len( [d for d in display if d[2]]) tout.info(f'Next merges from {source} ' @@ -1165,6 +1163,34 @@ def do_next_merges(args, dbs): tout.info(f' {idx}. {chash} {subject}') idx += 1 + +def do_next_merges(args, dbs): + """Show the next N merges to be applied from a source + + Args: + args (Namespace): Parsed arguments with 'source' and 'count' attributes + dbs (Database): Database instance + + Returns: + int: 0 on success, 1 if source not found + """ + source = args.source + count = args.count + + last_commit = dbs.source_get(source) + if not last_commit: + tout.error(f"Source '{source}' not found in database") + return 1 + + merges = _next_fetch_merges(last_commit, source, count) + if not merges: + tout.info('No merges remaining') + return 0 + + display, total_sub = _next_build_display(merges) + + _next_show_merges(source, merges, display, total_sub) + return 0 From patchwork Sun Feb 22 15:42: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: 1925 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=1771775022; bh=5coj6nBF1w90K3fcxlqw0vei9ruyyVojR7fZXzUhaXc=; 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=moHkXUF7Q7NgQjYEx7qKIu1Pz12dCCCXgFYkudBU4bjVbo4YumrvZtCYrXUCLwh81 2FFI0h+6RJkmyZVKM0oW+t9KWkGsM99JSoCPmoU2A4QsA+6HH417ZScWEabIfmQfNX +geLwgrRJchklstQ4+xRYFLxrPC3AcExTtiPzUh4VyfuiSoexHmCniUSCtvO1z9qQ1 8KfNePM8u9xsybNaVHRc7HrQZZcS49YCxGOyRDB7uPSwYNIm/fTlc99brXUgH00iQ1 k8t+BNQiRxvrO6M81xYlj6GcMfPBLNTY9NNuXFtqeFnU7tM+BxVj//5wmA//twqP1Y OwVU+9Hzol/mw== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 36FEF69C78 for ; Sun, 22 Feb 2026 08:43:42 -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 ahC2ijpIJKyS for ; Sun, 22 Feb 2026 08:43:42 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1771775022; bh=5coj6nBF1w90K3fcxlqw0vei9ruyyVojR7fZXzUhaXc=; 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=moHkXUF7Q7NgQjYEx7qKIu1Pz12dCCCXgFYkudBU4bjVbo4YumrvZtCYrXUCLwh81 2FFI0h+6RJkmyZVKM0oW+t9KWkGsM99JSoCPmoU2A4QsA+6HH417ZScWEabIfmQfNX +geLwgrRJchklstQ4+xRYFLxrPC3AcExTtiPzUh4VyfuiSoexHmCniUSCtvO1z9qQ1 8KfNePM8u9xsybNaVHRc7HrQZZcS49YCxGOyRDB7uPSwYNIm/fTlc99brXUgH00iQ1 k8t+BNQiRxvrO6M81xYlj6GcMfPBLNTY9NNuXFtqeFnU7tM+BxVj//5wmA//twqP1Y OwVU+9Hzol/mw== Received: from mail.u-boot.org (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 24CBD69D36 for ; Sun, 22 Feb 2026 08:43:42 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1771775020; bh=xRq+QP98MhqIKl9wSkCc57e3Tkd51FnKWuMYz8DlRNc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=CCROp1YIWInkfp5/E/hGgB8HdlGUwl09MyC3su1zKmX+f70RsPMzStO1zZdF8/TBW PGZMxddAhyYHTjWTMdFctDHw7bWxkMWtqwTs9Ok9MeXU9JoFZYlS0xCSvXSbA+x22I AmkvAztxiviV6YwpMSovZepb5ZZ111cRYdJmyugA/6QuJzVRAT5l2tSGNscO9d/+eu hoK6rFHBleFQfasg3AKPEdmH14ZryBjIyFhV7BSZAMdzmDBdRhs6tblUZyy4nSZb0y mAL2s2G/8LtnH8sbeLjEV3oBzd5h93bFm+rU7LJuEVY/EZPOvX4Us5nKX0sAN2ZwGB VTBLHAvdj3D4A== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id E8FFE69C78; Sun, 22 Feb 2026 08:43:40 -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 R4CQqH_4rZ9N; Sun, 22 Feb 2026 08:43:40 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1771775016; bh=lZY9TH1+Q7GSt3zZN8OK0f0dABuOMvAOyOVdfwS/S3k=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Xp1MP4gJXGEGlpq3cE6C6M7DjC4eGgGg8zd/iyOryo6Jtq7jkXfChgEC+SPHScSVz 9RTl+D+nQCRmZ3T9X1gSap5q9inuv0P1IM3f+MACwJWujTrb8QwC1FxqWa0OjTCebb azieWx7uGOOvKzZVcsFoXFZLT2USCWDOI4Ma3cMm9Nt/ZHT/kGdPGNqghykaw8FrbG kqOcpMKf3G3W8bQMqBCxnVfoMkV+/07LpbBNInUFbWJPqza5xMy2lURH3j0kUixDTX 9AN3Pcy1tc7SlEHSOoIf0GmO2m44ZWof6YCyXoS8VPVVD/3VjBpqmMKGfICrq7w43l 0bT2ucVgo3B0w== Received: from u-boot.org (unknown [73.34.74.121]) by mail.u-boot.org (Postfix) with ESMTPSA id 7414A69C5C; Sun, 22 Feb 2026 08:43:36 -0700 (MST) From: Simon Glass To: U-Boot Concept Date: Sun, 22 Feb 2026 08:42:44 -0700 Message-ID: <20260222154303.2851319-5-sjg@u-boot.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20260222154303.2851319-1-sjg@u-boot.org> References: <20260222154303.2851319-1-sjg@u-boot.org> MIME-Version: 1.0 Message-ID-Hash: YM4MVW6JYE2SFC6OBJCMTCWTS6WKJJ6O X-Message-ID-Hash: YM4MVW6JYE2SFC6OBJCMTCWTS6WKJJ6O 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 04/16] pickman: Refactor decompose_mega_merge() into smaller helpers 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 three phases of decompose_mega_merge() all follow the same pattern: run git log, parse the output, filter out commits already in the database. Extract two helpers to reduce the duplication: - _mega_preadd(): pre-add the mega-merge commit as 'skipped' in the DB - _mega_get_batch(): common git-log/parse/filter pattern shared by all three phases This makes each phase a single call to _mega_get_batch() instead of repeated inline blocks. Co-developed-by: Claude Opus 4.6 Signed-off-by: Simon Glass --- tools/pickman/control.py | 96 ++++++++++++++++++++++------------------ 1 file changed, 53 insertions(+), 43 deletions(-) diff --git a/tools/pickman/control.py b/tools/pickman/control.py index d3cb8a8fff4..19cd2813b64 100644 --- a/tools/pickman/control.py +++ b/tools/pickman/control.py @@ -964,6 +964,49 @@ def detect_sub_merges(merge_hash): return [line for line in out.split('\n') if line] +def _mega_preadd(dbs, merge_hash): + """Pre-add a mega-merge commit to the database as 'skipped'. + + This prevents the mega-merge from appearing as an orphan commit. + Does nothing if the commit already exists in the database. + """ + if dbs.commit_get(merge_hash): + return + + 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() + + +def _mega_get_batch(dbs, exclude_ref, include_ref): + """Fetch a batch of unprocessed commits between two refs. + + Runs git log for the range ^exclude_ref include_ref, parses the + output and filters out commits already in the database. + + Returns: + list: CommitInfo tuples for unprocessed commits, may be empty + """ + log_output = run_git([ + 'log', '--reverse', '--format=%H|%h|%an|%s|%P', + f'^{exclude_ref}', include_ref + ]) + if not log_output: + return [] + + all_commits = parse_log_output(log_output, has_parents=True) + return [c for c in all_commits if not dbs.commit_get(c.hash)] + + def decompose_mega_merge(dbs, prev_commit, merge_hash, sub_merges): """Return the next unprocessed batch from a mega-merge @@ -990,60 +1033,27 @@ def decompose_mega_merge(dbs, prev_commit, merge_hash, sub_merges): 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() + _mega_preadd(dbs, merge_hash) # 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 + commits = _mega_get_batch(dbs, prev_commit, first_parent) + 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 + commits = _mega_get_batch(dbs, prev_sub, sub_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 + commits = _mega_get_batch(dbs, last_sub, second_parent) + if commits: + return commits, None - # All done return [], None From patchwork Sun Feb 22 15:42: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: 1926 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=1771775026; bh=DzEQDKZF+aeJAFkEYa/igqIgJ00QDSuUSZmB+bzCbvU=; 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=AiS9VFn1ujuKVO2PQeiGCVt5yGuyLCPqSCRYMrksM7ik0XIMSjLqEs6VYAxUVxznv tXNKJWRKzrwNHD51ZQqr4qw1lza3MJkrEPBluYSJ6f/pDpTf8auzR5pIZfV0xKSilG oz+KJ8zNZ1SFLZEHmpEK8Ds0bYRRaugnI7pcg4qx4EJzCJ6t8ygssnA69yD6MwnAjR iH3dMTkk2XanWakyqxYGnzZdloJSqNmAkNZS4VAApMLrytXbKUhSOB/VH8/11NFN4P VPdunucm3BfcmZ9TWzTVxn8Zv21HRTnhu3l88XDOLj2LZjSYpU+SrsImaZMaSS5IYz R0bwtJwQHlkQg== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id C602A69C5E for ; Sun, 22 Feb 2026 08:43:46 -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 kRk1wPxuBekt for ; Sun, 22 Feb 2026 08:43:46 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1771775026; bh=DzEQDKZF+aeJAFkEYa/igqIgJ00QDSuUSZmB+bzCbvU=; 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=AiS9VFn1ujuKVO2PQeiGCVt5yGuyLCPqSCRYMrksM7ik0XIMSjLqEs6VYAxUVxznv tXNKJWRKzrwNHD51ZQqr4qw1lza3MJkrEPBluYSJ6f/pDpTf8auzR5pIZfV0xKSilG oz+KJ8zNZ1SFLZEHmpEK8Ds0bYRRaugnI7pcg4qx4EJzCJ6t8ygssnA69yD6MwnAjR iH3dMTkk2XanWakyqxYGnzZdloJSqNmAkNZS4VAApMLrytXbKUhSOB/VH8/11NFN4P VPdunucm3BfcmZ9TWzTVxn8Zv21HRTnhu3l88XDOLj2LZjSYpU+SrsImaZMaSS5IYz R0bwtJwQHlkQg== Received: from mail.u-boot.org (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id B092E69D36 for ; Sun, 22 Feb 2026 08:43:46 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1771775025; bh=Dn3430p5peK7cbOCKMHWpR1l8MM5lflgba7gifwQTEQ=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=qCZ/YPWxNV0YYNOT2P3gGYcW2RY4iYh0Fh7pz1QaLNxouYH//ifIIsipAZcX5BFX2 bb9ElORmayQX/x9T/XrvnO7L9ScXb7FN8E3aZR1Fs3E56C4CJpOqCzcLvrXaa4Tqdg lnPhkACjReTkoAu6la0jA+jhq5r6E8v1NvRWL8bA2qnJBSFuQtwO7mXczPyEPhU+M+ iRvW6/kgh/TkQV1Abzhys0aNp4Jfi6SVZnkF7/RFE3/nZeqRfnNB+DM7qjlqLo8UPK VgPCarcvtj0lhTeHGXCJb4SGjOKRYJcVdQDcBlMjLMr6xaki3WQqW5mBma0YTyf2e4 Xo1AiaOjjrRKw== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id AF01B69C5E; Sun, 22 Feb 2026 08:43:45 -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 2n3Jxw_GzmFj; Sun, 22 Feb 2026 08:43:45 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1771775021; bh=w3VIP24Yj9ygJWv1VTAS4zvIRuj68XYuY58Yd7B4/Vs=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=pXYojOF8dIn+NZsf3aUyMgqlExsCaJABbCFwMj6IM31ZPrRZ6So13oMX52YwB4HAE zlQsM1fQnKuzKuPdxSKhTguz4YeyxLtgyW8fVhaNKwznXQsbmj2kiFjRSmFWdLiTDW wwYHQLw3yggy4VHCTgO15QHo6GLTod03jd8NBAAAja3ej0biSiVmlMfNVQjXKphsQ+ uqxPQDwsqXLx/m+rcS3j0eXfqEsM3/JIzcS09BZoyHMtLbG7CXGaJaRW+ESD2Uh85K oY2dKX+IWkHTDZBY9/KTJS0t6kQuaf0Mv4o5K+kNRCXyvpRhB+HX02BPdifc6cnbTT 4KlYNLzsYUE/w== Received: from u-boot.org (unknown [73.34.74.121]) by mail.u-boot.org (Postfix) with ESMTPSA id 38CB669C5C; Sun, 22 Feb 2026 08:43:41 -0700 (MST) From: Simon Glass To: U-Boot Concept Date: Sun, 22 Feb 2026 08:42:45 -0700 Message-ID: <20260222154303.2851319-6-sjg@u-boot.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20260222154303.2851319-1-sjg@u-boot.org> References: <20260222154303.2851319-1-sjg@u-boot.org> MIME-Version: 1.0 Message-ID-Hash: AFXQGJL6QEIWBLR7247LW7HIEAKAWU7G X-Message-ID-Hash: AFXQGJL6QEIWBLR7247LW7HIEAKAWU7G 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 05/16] pickman: Refactor handle_already_applied() into smaller helpers 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 Extract two helpers from handle_already_applied() to separate its functionality: - _applied_advance_source(): choose and set the new source position from the three possible cases (advance_to, signal_commit, last hash) - _applied_create_skip_mr(): push a skip branch and create the MR This reduces handle_already_applied() to a short sequence of mark, advance, and optionally push. Co-developed-by: Claude Opus 4.6 Signed-off-by: Simon Glass --- tools/pickman/control.py | 101 ++++++++++++++++++++++----------------- 1 file changed, 58 insertions(+), 43 deletions(-) diff --git a/tools/pickman/control.py b/tools/pickman/control.py index 19cd2813b64..39ce57d0895 100644 --- a/tools/pickman/control.py +++ b/tools/pickman/control.py @@ -1539,6 +1539,60 @@ def prepare_apply(dbs, source, branch): # pylint: disable=too-many-arguments +def _applied_advance_source(dbs, source, commits, advance_to, + signal_commit): + """Advance the source position after skipping already-applied commits. + + Chooses the new position from advance_to, signal_commit, or the + last commit hash, in that priority order. + """ + if advance_to is not None: + new_hash = advance_to + elif signal_commit: + new_hash = signal_commit + else: + new_hash = commits[-1].hash + + dbs.source_set(source, new_hash) + dbs.commit() + tout.info(f"Updated source '{source}' to {new_hash[:12]}") + + +def _applied_create_skip_mr(args, source, commits, branch_name, conv_log): + """Push a skip branch and create an MR recording the skip. + + Returns: + int: 0 on success, 1 on failure + """ + remote = args.remote + target = args.target + + try: + run_git(['checkout', '-b', branch_name, f'{remote}/{target}']) + except Exception: # pylint: disable=broad-except + # Branch may already exist from failed attempt + try: + run_git(['checkout', branch_name]) + except Exception: # pylint: disable=broad-except + tout.error(f'Could not create/checkout branch {branch_name}') + return 1 + + title = f'{SKIPPED_TAG} [pickman] {commits[-1].subject}' + summary = format_history(source, commits, branch_name) + description = (f'{summary}\n\n' + f'**Status:** Commits already applied to {target} ' + f'with different hashes.\n\n' + f'### Conversation log\n{conv_log}') + + mr_url = gitlab_api.push_and_create_mr( + remote, branch_name, target, title, description + ) + if not mr_url: + return 1 + + return 0 + + def handle_already_applied(dbs, source, commits, branch_name, conv_log, args, signal_commit, advance_to=None): """Handle the case where commits are already applied to the target branch @@ -1563,55 +1617,16 @@ def handle_already_applied(dbs, source, commits, branch_name, conv_log, args, """ tout.info('Commits already applied to target branch - creating skip MR') - # Mark commits as 'skipped' in database for commit in commits: dbs.commit_set_status(commit.hash, 'skipped') dbs.commit() - # 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]}") + _applied_advance_source(dbs, source, commits, advance_to, + signal_commit) - # Push and create MR with [skip] prefix if requested if args.push: - remote = args.remote - target = args.target - - # Create a skip branch from ci/master (no changes) - try: - run_git(['checkout', '-b', branch_name, f'{remote}/{target}']) - except Exception: # pylint: disable=broad-except - # Branch may already exist from failed attempt - try: - run_git(['checkout', branch_name]) - except Exception: # pylint: disable=broad-except - tout.error(f'Could not create/checkout branch {branch_name}') - return 1 - - # Use merge commit subject as title with [skip] prefix - title = f'{SKIPPED_TAG} [pickman] {commits[-1].subject}' - summary = format_history(source, commits, branch_name) - description = (f'{summary}\n\n' - f'**Status:** Commits already applied to {target} ' - f'with different hashes.\n\n' - f'### Conversation log\n{conv_log}') - - mr_url = gitlab_api.push_and_create_mr( - remote, branch_name, target, title, description - ) - if not mr_url: - return 1 + return _applied_create_skip_mr(args, source, commits, + branch_name, conv_log) return 0 From patchwork Sun Feb 22 15:42: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: 1927 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=1771775032; bh=z77RT0Y1kx2ujYx3EfnWn8usrQEhiGSRlwdVGv1YnGc=; 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=vq03uGhCNG7YVApx8Q3qsmX6to1OO3qtMEWc5U2M9veIf8ms6xby2Fc/MzORxwoVd rhkfGBDADuijqcodPvfjUSKu1xBoK433IESoW2yPLFWuIBMrjHg/3ckafCdsUwP5r4 pfaIPZ9TL6CBGmhRuHLG3wTS572ZaoTyk13B+8I92eMoBcZon2xjx+ML07wVcxHCDU +jmNHzghsGIAB42XzQnA6bvu35cX35qNhLkmwG5HkcKnjpsnEPAi+mz1CwA16Suj3n yYiimKde/koSSb1eiYAebix+zx2dTRXRYEmlwjBvqSxL1XUiUPgdEWI/H4/JSEk3CF ZlcAiOUPMgpyA== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 9238669D42 for ; Sun, 22 Feb 2026 08:43: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 10024) with ESMTP id Fjopv4FugGXR for ; Sun, 22 Feb 2026 08:43:52 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1771775032; bh=z77RT0Y1kx2ujYx3EfnWn8usrQEhiGSRlwdVGv1YnGc=; 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=vq03uGhCNG7YVApx8Q3qsmX6to1OO3qtMEWc5U2M9veIf8ms6xby2Fc/MzORxwoVd rhkfGBDADuijqcodPvfjUSKu1xBoK433IESoW2yPLFWuIBMrjHg/3ckafCdsUwP5r4 pfaIPZ9TL6CBGmhRuHLG3wTS572ZaoTyk13B+8I92eMoBcZon2xjx+ML07wVcxHCDU +jmNHzghsGIAB42XzQnA6bvu35cX35qNhLkmwG5HkcKnjpsnEPAi+mz1CwA16Suj3n yYiimKde/koSSb1eiYAebix+zx2dTRXRYEmlwjBvqSxL1XUiUPgdEWI/H4/JSEk3CF ZlcAiOUPMgpyA== Received: from mail.u-boot.org (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 5E62B69D36 for ; Sun, 22 Feb 2026 08:43:52 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1771775030; bh=yPK4QofDWby54AfsLSwsolkb3Hg4+HPo8bBUqL5N/c4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=DN5jjMxi1p5xlqryKyk4w/kzZkkkmydWOVddfYpswRf6selEcclkQ65IakQP5zrbz /bSPPJGQnVGtAfg/V83WYytaGKFiVjipLfQmRseOTD5HmXZ00La9pPFUEZ9XJkB/nf +MJuSMnPdC8aJPFgzaGVQRJ6X4n8OLxNd+IC1KTK/VteRLflBF7wE8JGHnIzkaIT8Q 5F/ICt/AHqO2KixbvmXzI1gT4mr5JYswgxZhATftGMB9HVWBdfKAiAZ+7L5kcnk79R i82HHNluMV5VI2LDZlxrqzuFX2kfLj4qF9JQ+RmL8YyIo15dCg/4SV8QW0/8j0XB2G fwWIpJiROBwHQ== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 800A369C5E; Sun, 22 Feb 2026 08:43:50 -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 cj9W2wWn6ufG; Sun, 22 Feb 2026 08:43:50 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1771775026; bh=POiqqLTpVIXHwN2JJnLe/uW68YEnuZM/duTOuydtIt8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=tvvN3HZUKc8aPeEMhekVU2B1oIm/oYOJLhvkBYZWx7R0127yLLJQS8Yf5a4Tv3KYQ 0hLwyfVqM3SVGffNQQ3QDw69nNVj9ndKa3XaLTnwxnGAFDp0M3xBHmTiBmYriy1C1l 6fRbHf/3DtjHIAQO1DEcUs2ZLyEWes8K1enTPcXCjv+bT92pGktLZu1pFj4eQV1PWS 4CawJFyFo1VYYvCMi6nVrJPbz9e2O7jiSpFrh4W20lxXctOeckM4lJiVVXxytgM9eR cScZvA775COx1jKuNm7KVlt9gz37CJQE5MfUwF/SzCOS8myZSg/sx8T/6GY146vFh3 ViJIZzX7Eychw== Received: from u-boot.org (unknown [73.34.74.121]) by mail.u-boot.org (Postfix) with ESMTPSA id EFE9B69C5C; Sun, 22 Feb 2026 08:43:45 -0700 (MST) From: Simon Glass To: U-Boot Concept Date: Sun, 22 Feb 2026 08:42:46 -0700 Message-ID: <20260222154303.2851319-7-sjg@u-boot.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20260222154303.2851319-1-sjg@u-boot.org> References: <20260222154303.2851319-1-sjg@u-boot.org> MIME-Version: 1.0 Message-ID-Hash: ROMFOG3JOOPU3PBUDMFBYUUEC2OSOAWJ X-Message-ID-Hash: ROMFOG3JOOPU3PBUDMFBYUUEC2OSOAWJ 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 06/16] pickman: Add tests for prepare_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 Add test coverage for the remaining paths in prepare_apply(): - Existing branch deletion when the target branch already exists - The merge_found=True message and advance_to passthrough - The no-merge-found message for trailing commits Also initialise tout in setUp() so the output-checking tests can capture tout.info() messages. Co-developed-by: Claude Opus 4.6 Signed-off-by: Simon Glass --- tools/pickman/ftest.py | 99 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 99 insertions(+) diff --git a/tools/pickman/ftest.py b/tools/pickman/ftest.py index f07e4ecb0db..a2c19a1159e 100644 --- a/tools/pickman/ftest.py +++ b/tools/pickman/ftest.py @@ -2677,6 +2677,105 @@ class TestPrepareApply(unittest.TestCase): self.assertEqual(info.branch_name, 'my-branch') dbs.close() + def test_prepare_apply_deletes_existing_branch(self): + """Test prepare_apply deletes a branch that already exists.""" + git_cmds = [] + + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + dbs.source_set('us/next', 'abc123') + dbs.commit() + + log_output = 'aaa111|aaa111a|Author 1|First commit|abc123\n' + + def mock_git(pipe_list): + cmd = pipe_list[0] if pipe_list else [] + git_cmds.append(cmd) + if 'log' in cmd: + return command.CommandResult(stdout=log_output) + if 'rev-parse' in cmd: + return command.CommandResult(stdout='master') + if 'branch' in cmd and '--list' in cmd: + return command.CommandResult(stdout='cherry-aaa111a\n') + return command.CommandResult(stdout='') + + command.TEST_RESULT = mock_git + + info, ret = control.prepare_apply(dbs, 'us/next', None) + + self.assertIsNotNone(info) + self.assertEqual(ret, 0) + self.assertTrue( + any('branch' in c and '-D' in c for c in git_cmds)) + dbs.close() + + def test_prepare_apply_merge_found(self): + """Test prepare_apply sets merge_found and advance_to.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + dbs.source_set('us/next', 'abc123') + dbs.commit() + + merge_hash = 'ccc333ccc333ccc333' + + merge_info = control.NextCommitsInfo( + commits=[ + control.CommitInfo('aaa111', 'aaa111a', 'First commit', + 'Author 1'), + control.CommitInfo('bbb222', 'bbb222b', 'Second commit', + 'Author 2'), + ], + merge_found=True, + advance_to=merge_hash, + ) + + def mock_git(pipe_list): + cmd = pipe_list[0] if pipe_list else [] + if 'rev-parse' in cmd: + return command.CommandResult(stdout='master') + return command.CommandResult(stdout='') + + with mock.patch.object(control, 'get_next_commits', + return_value=(merge_info, None)): + command.TEST_RESULT = mock_git + info, ret = control.prepare_apply(dbs, 'us/next', None) + + self.assertIsNotNone(info) + self.assertEqual(ret, 0) + self.assertTrue(info.merge_found) + self.assertEqual(info.advance_to, merge_hash) + self.assertEqual(len(info.commits), 2) + dbs.close() + + def test_prepare_apply_no_merge(self): + """Test prepare_apply reports no merge found.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + dbs.source_set('us/next', 'abc123') + dbs.commit() + + log_output = 'aaa111|aaa111a|Author 1|First commit|abc123\n' + + def mock_git(pipe_list): + cmd = pipe_list[0] if pipe_list else [] + if 'log' in cmd: + return command.CommandResult(stdout=log_output) + if 'rev-parse' in cmd: + return command.CommandResult(stdout='master') + return command.CommandResult(stdout='') + + command.TEST_RESULT = mock_git + + info, ret = control.prepare_apply(dbs, 'us/next', None) + + self.assertIsNotNone(info) + self.assertEqual(ret, 0) + self.assertFalse(info.merge_found) + dbs.close() + class TestExecuteApply(unittest.TestCase): """Tests for execute_apply function.""" From patchwork Sun Feb 22 15:42: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: 1928 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=1771775038; bh=WwX1DcfBuOT5ZFf3oWeYDDdFd3e+aI5UFbhNdGek0qo=; 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=RscykknbHD3ZgS38Cml45ZTVuS0s2sFh9yU96jXUAQh2MqZUsHojAGMcDy37HjogU P4sVNULwrlbprEL3S7/Qfuqg5dMn2ZZkOv63uhUCQbBk/5dZKNUZ/AymLnK1bxvifT Z6L3EJAlfWjIq5oVnjN1yMiCymz5dg+a/ySTefp8kW6DiCVEtWLPhKij/Ii7oyyobC /YZU57fvlc79Ox11rcYGMUUWYHc0sxqXZq0M30WpVOvD1eYjWSKFbtUK0eUv+1W3af bIFIY6NHvDkIYHK/mhJXhIDhzc6Cl3b43qBz5v0eRFSkwPWBFYTlRdXDk6Yuo/EjuA yrBrnU2Vi9+Nw== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 588A969D42 for ; Sun, 22 Feb 2026 08:43: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 7IKmZA2_c6e0 for ; Sun, 22 Feb 2026 08:43:58 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1771775038; bh=WwX1DcfBuOT5ZFf3oWeYDDdFd3e+aI5UFbhNdGek0qo=; 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=RscykknbHD3ZgS38Cml45ZTVuS0s2sFh9yU96jXUAQh2MqZUsHojAGMcDy37HjogU P4sVNULwrlbprEL3S7/Qfuqg5dMn2ZZkOv63uhUCQbBk/5dZKNUZ/AymLnK1bxvifT Z6L3EJAlfWjIq5oVnjN1yMiCymz5dg+a/ySTefp8kW6DiCVEtWLPhKij/Ii7oyyobC /YZU57fvlc79Ox11rcYGMUUWYHc0sxqXZq0M30WpVOvD1eYjWSKFbtUK0eUv+1W3af bIFIY6NHvDkIYHK/mhJXhIDhzc6Cl3b43qBz5v0eRFSkwPWBFYTlRdXDk6Yuo/EjuA yrBrnU2Vi9+Nw== Received: from mail.u-boot.org (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 2162C69C5E for ; Sun, 22 Feb 2026 08:43:58 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1771775035; bh=5qYU+atroB1yOmLH3jheXyJRiLJGOU43LOatkILD0VE=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=fS6wfxroggrODjOxFgb+D138CH6NCzNxNm+SIdk5+nhhBfnTIZ275sYiknd4Nj8us w3XSWCG9hWhXDwQuy5VYrT6gVNW2v8WAEfmv+U9ZflDMQG/k6pvDjDwyVFOtiW4mRc vPHn/lUhW6cEs1KcHx5plzmCKS7XZVdG/HiXRc7drmObYiAL74f+eIP7tzLX8u4WnD o1sKhqjLlKWH/FSg84dl527FGBe1ZoXkNtE4UvdHHuJNyV5w3ZBi0phCErpffzcEYJ X6t4NuI/Wl+cZrF4xf4bQL1n7aXYxVACxahIctxo+5/sLecIRQow+XGbLyzlCJjDTr 2nQc7tvdsxaRA== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id E80AC69C5E; Sun, 22 Feb 2026 08:43:55 -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 qCwzgysB1Zp1; Sun, 22 Feb 2026 08:43:55 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1771775031; bh=VjAPjjgEJNxZ2QcuEK8r3JroBHZNGxovv52fCfs21X8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=WNPIweYEXFSv/JDRhCXxbljfOKUEu8HydHy4rwP1JIFIy999bAEe62nI2Bsvf+Hww F5rcixRfYWCxDBuJn/mMtDRLG8dKXhb/2zxhw5WB9r0k9ksbp45yy6IKwHAsSAbLR2 43wGaDX5uqJodJcHsDi6w40u1sufbtbjXH4DCalgEVKg2xM/u4pxJngdX5cAfgzA1Z 13KyPldFOfzWvFEi3jAbXRZl8d4xyw2oDUlJIx0np/hL+KNKMBzHy1AtF3tE83o/k3 iVeLn5qpEu7vLxqNcJm87oJU7zTsFpNZS8NB2YX5r2f+s03TfHz+mcmuVJGxPP7bjL Qrb2xt6adKTYA== Received: from u-boot.org (unknown [73.34.74.121]) by mail.u-boot.org (Postfix) with ESMTPSA id C0CD569C5C; Sun, 22 Feb 2026 08:43:50 -0700 (MST) From: Simon Glass To: U-Boot Concept Date: Sun, 22 Feb 2026 08:42:47 -0700 Message-ID: <20260222154303.2851319-8-sjg@u-boot.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20260222154303.2851319-1-sjg@u-boot.org> References: <20260222154303.2851319-1-sjg@u-boot.org> MIME-Version: 1.0 Message-ID-Hash: KQ6HHZC2CTKGMDPTA4YUUUDXNGL7ODFD X-Message-ID-Hash: KQ6HHZC2CTKGMDPTA4YUUUDXNGL7ODFD 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 07/16] pickman: Refactor the initial part of prepare_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 Create a new _prepare_get_commits() helper to handle finding the next set of commits to process. This helps to reduce the size of prepare_apply() Signed-off-by: Simon Glass --- tools/pickman/control.py | 43 ++++++++++++++++++++++++++++------------ 1 file changed, 30 insertions(+), 13 deletions(-) diff --git a/tools/pickman/control.py b/tools/pickman/control.py index 39ce57d0895..3e69cba7a9a 100644 --- a/tools/pickman/control.py +++ b/tools/pickman/control.py @@ -1473,40 +1473,57 @@ def push_mr(args, branch_name, title, description): return bool(mr_url) -def prepare_apply(dbs, source, branch): - """Prepare for applying commits from a source branch +def _prepare_get_commits(dbs, source): + """Get the next commits to apply, handling skips. - Gets the next commits, sets up the branch name, and prints info about - what will be applied. + Fetches the next batch of commits from the source. Args: dbs (Database): Database instance source (str): Source branch name - branch (str): Branch name to use, or None to auto-generate Returns: - tuple: (ApplyInfo, return_code) where ApplyInfo is set if there are - commits to apply, or None with return_code indicating the result - (0 for no commits, 1 for error) + tuple: (NextCommitsInfo, return_code) where return_code is None + on success, or an int (0 or 1) if there is nothing to do """ info, err = get_next_commits(dbs, source) - if err: tout.error(err) 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') + else: + tout.info('No new commits to cherry-pick') return None, 0 + return info, None + + +def prepare_apply(dbs, source, branch): + """Prepare for applying commits from a source branch + + Gets the next commits, sets up the branch name, and prints info about + what will be applied. + + Args: + dbs (Database): Database instance + source (str): Source branch name + branch (str): Branch name to use, or None to auto-generate + + Returns: + tuple: (ApplyInfo, return_code) where ApplyInfo is set if there are + commits to apply, or None with return_code indicating the result + (0 for no commits, 1 for error) + """ + info, ret = _prepare_get_commits(dbs, source) + if ret is not None: + return None, ret + commits = info.commits # Save current branch to return to later From patchwork Sun Feb 22 15:42:48 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 1929 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=1771775048; bh=5twfDMogAzsMZ4MVyD9NAsNTpxo+y0ekLrfJ2H6FDNY=; 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=wNncA43xGyIM+m2XeD38MTHFxq6K8tQEeWLGVnzmRtjyLO5haYhIzmqNUA/jO63rS 3Huek9AqIzoLoiAWofngiUnTJx2cG+MJMzT11NtBiaWZ2/C/Mx/XEtVzDe01HH9MAS 5IGAJRusfriw8unwYMXJhWny/lBnwzW0MLYocNdYiGp3dbI5+Xza10XO1UpSGBraob S9CLxZY9wh6uuR+yfpqRhgKo0RJJnlZIjXX/OxOMGkCoo5REVx/SHbOoGx9drTEeD4 Rb1BfalfB9vSFqc/3a9+R51mFIRgv8dszNm0u6ZPrw7yi9zKsVrdDJ8VEjJHa6uiK6 wAZDsJLJ1hxvQ== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id AB28569D48 for ; Sun, 22 Feb 2026 08:44:08 -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 3LvZP_Y5cP6g for ; Sun, 22 Feb 2026 08:44:08 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1771775048; bh=5twfDMogAzsMZ4MVyD9NAsNTpxo+y0ekLrfJ2H6FDNY=; 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=wNncA43xGyIM+m2XeD38MTHFxq6K8tQEeWLGVnzmRtjyLO5haYhIzmqNUA/jO63rS 3Huek9AqIzoLoiAWofngiUnTJx2cG+MJMzT11NtBiaWZ2/C/Mx/XEtVzDe01HH9MAS 5IGAJRusfriw8unwYMXJhWny/lBnwzW0MLYocNdYiGp3dbI5+Xza10XO1UpSGBraob S9CLxZY9wh6uuR+yfpqRhgKo0RJJnlZIjXX/OxOMGkCoo5REVx/SHbOoGx9drTEeD4 Rb1BfalfB9vSFqc/3a9+R51mFIRgv8dszNm0u6ZPrw7yi9zKsVrdDJ8VEjJHa6uiK6 wAZDsJLJ1hxvQ== Received: from mail.u-boot.org (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 7D71C69C5E for ; Sun, 22 Feb 2026 08:44:08 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1771775044; bh=IY/DU0xgenh8ZE2Uj2HQcYdhL5jNSqjuTII4Icexj6c=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=s7wz35uXwKMxbvnP4gpv8w42lxtM13XDGg/WlAgVK3KrBu5Xw+Ofj2XudxD+5fH35 DbKT5jZLjsYwtSJpF3KggYeH1qENpR6KZRWxHRyxTRTscH0KWRxEbkUc7T9Ek4cImJ 5h6k8gEkv6mSgpXcayYAUyKHftBSgXq5HUmjGfRkEUdBbagGqUSZS526LlFG/Aga9H I8kWIqdEVwWJGu9ZqbvTvSMnBvv1XIOiPzoH9HALKk7Xx8G0IeQCthp5tjHyNb0rt3 AhHHchCIvovWPT0DaxfVSK/amU4FL8atN6JMlqzRjme7N9CvEFUbuIBzsWSa0uZvm0 CrIFQ9mE8F06Q== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 0D5CE69C5E; Sun, 22 Feb 2026 08:44:04 -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 fFhArok07lJz; Sun, 22 Feb 2026 08:44:03 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1771775036; bh=GWhejId7rd3J+nDdgs+Yh/986CacSSEXDRMF6vbvarU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=E/jOi9QaTN2hVmEnJKNjGS4hRk71lx6JmYh9m528+eFFH8LjfaLnG1XFQFgTvK2jp L+DetPh1okqwiA1RvVfczBqUNwqD902LSYMPt8iZ7WUPuLH9ri9CxZOIk0yTKDc4Ti xOuA6WyjRoxkEhrqVLe1cWLaqBecyrn2MnD+j4AHjg2mh5crxksiViiYnWeDzp0Buf FtESHtBfDtJ3adj88Fqs4qVgY8KMJ5uqJ6pXsIBVJcZPve0X7xbPduxIiPz8ci19T6 V4EnwWSm3QWOdvXs8whpd4eKPp38pDtYlg0A2rT4BgshcqSPix5pT5gkDhXyd/hSL9 PDUzBJT1Um88g== Received: from u-boot.org (unknown [73.34.74.121]) by mail.u-boot.org (Postfix) with ESMTPSA id EDBAD69D36; Sun, 22 Feb 2026 08:43:55 -0700 (MST) From: Simon Glass To: U-Boot Concept Date: Sun, 22 Feb 2026 08:42:48 -0700 Message-ID: <20260222154303.2851319-9-sjg@u-boot.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20260222154303.2851319-1-sjg@u-boot.org> References: <20260222154303.2851319-1-sjg@u-boot.org> MIME-Version: 1.0 Message-ID-Hash: ZBWOWW5RXHHKSP2J4PYUBYIJKFDPOZGU X-Message-ID-Hash: ZBWOWW5RXHHKSP2J4PYUBYIJKFDPOZGU 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 08/16] pickman: Handle dts/upstream subtree merges automatically 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 cherry-picks a series from us/next, it may encounter commits that depend on a dts/upstream subtree update that is not yet in ci/master. Subtree merges cannot be cherry-picked; they must be applied with ./tools/update-subtree.sh pull dts . Add subtree-merge detection to find_unprocessed_commits(). When a merge subject matches "Subtree merge tag '' of ... into ", pickman checks out the target branch, runs update-subtree.sh, marks both the squash and merge commits as applied, advances the source position, and retries to get the next batch. This handles dts/upstream, lib/mbedtls/external/mbedtls and lib/lwip/lwip subtrees. Co-developed-by: Claude Opus 4.6 Signed-off-by: Simon Glass --- tools/pickman/README.rst | 33 ++ tools/pickman/control.py | 207 +++++++++++-- tools/pickman/ftest.py | 640 ++++++++++++++++++++++++++++++++++++++- 3 files changed, 844 insertions(+), 36 deletions(-) diff --git a/tools/pickman/README.rst b/tools/pickman/README.rst index 5c6121610be..70309032838 100644 --- a/tools/pickman/README.rst +++ b/tools/pickman/README.rst @@ -111,6 +111,39 @@ If there are no merge commits between the last processed commit and the branch tip, pickman includes all remaining commits in a single set. This is noted in the output as "no merge found". +Subtree Merge Handling +---------------------- + +The source branch may contain subtree merges that update vendored trees such as +``dts/upstream``, ``lib/mbedtls/external/mbedtls`` or ``lib/lwip/lwip``. These +appear as a pair of commits on the first-parent chain: + +1. ``Squashed 'dts/upstream/' changes from ..`` (the actual file + changes) +2. ``Subtree merge tag '' of into dts/upstream`` (the merge commit + joining histories) + +These commits cannot be cherry-picked. Pickman detects them automatically by +matching the merge subject against the pattern +``Subtree merge tag '' of ... into ``. When a subtree merge is found, +pickman: + +1. Checks out the target branch (e.g. ``ci/master``) +2. Runs ``./tools/update-subtree.sh pull `` to apply the update +3. Pushes the target branch (if ``--push`` is active) +4. Records both the squash and merge commits as 'applied' in the database +5. Advances the source position past the merge and continues with the next batch + +This is works without manual intervention. The currently supported subtrees are: + +================================= =========== +Path Name +================================= =========== +``dts/upstream`` ``dts`` +``lib/mbedtls/external/mbedtls`` ``mbedtls`` +``lib/lwip/lwip`` ``lwip`` +================================= =========== + Skipping MRs ------------ diff --git a/tools/pickman/control.py b/tools/pickman/control.py index 3e69cba7a9a..48f7ced1617 100644 --- a/tools/pickman/control.py +++ b/tools/pickman/control.py @@ -46,6 +46,17 @@ RE_GIT_STAT_FILE = re.compile(r'^([^|]+)\s*\|') # Extract hash from line like "(cherry picked from commit abc123def)" RE_CHERRY_PICK = re.compile(r'cherry picked from commit ([a-f0-9]+)') +# Detect subtree merge commits on the first-parent chain +RE_SUBTREE_MERGE = re.compile( + r"Subtree merge tag '([^']+)' of .* into (.*)") + +# Map from subtree path to update-subtree.sh name +SUBTREE_NAMES = { + 'dts/upstream': 'dts', + 'lib/mbedtls/external/mbedtls': 'mbedtls', + 'lib/lwip/lwip': 'lwip', +} + # Named tuple for commit info Commit = namedtuple('Commit', ['hash', 'chash', 'subject', 'date']) @@ -92,8 +103,11 @@ 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 +# subtree_update: (name, tag) tuple if a subtree update is needed, else None NextCommitsInfo = namedtuple('NextCommitsInfo', - ['commits', 'merge_found', 'advance_to']) + ['commits', 'merge_found', 'advance_to', + 'subtree_update'], + defaults=[None]) # Named tuple for prepare_apply() result # @@ -747,6 +761,22 @@ def do_check_gitlab(args, dbs): # pylint: disable=unused-argument return 0 +def _check_subtree_merge(merge_hash): + """Check if a merge commit is a subtree merge. + + Returns: + tuple: (name, tag) where name is the subtree name (or None for + unknown paths), or None if not a subtree merge + """ + subject = run_git(['log', '-1', '--format=%s', merge_hash]) + match = RE_SUBTREE_MERGE.match(subject) + if not match: + return None + tag = match.group(1) + path = match.group(2) + return SUBTREE_NAMES.get(path), tag + + def find_unprocessed_commits(dbs, last_commit, source, merge_hashes): """Find the first merge with unprocessed commits @@ -766,6 +796,18 @@ def find_unprocessed_commits(dbs, last_commit, source, merge_hashes): prev_commit = last_commit skipped_merges = False for merge_hash in merge_hashes: + # Check for subtree merge (e.g. dts/upstream update) + result = _check_subtree_merge(merge_hash) + if result is not None: + name, tag = result + if name: + return NextCommitsInfo([], True, merge_hash, + (name, tag)) + # Unknown subtree path - skip past it + prev_commit = merge_hash + skipped_merges = True + continue + # Check for mega-merge (contains sub-merges) sub_merges = detect_sub_merges(merge_hash) if sub_merges: @@ -1473,54 +1515,175 @@ def push_mr(args, branch_name, title, description): return bool(mr_url) -def _prepare_get_commits(dbs, source): - """Get the next commits to apply, handling skips. +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. + + Returns: + int: 0 on success, 1 on failure + """ + try: + result = command.run( + './tools/update-subtree.sh', 'pull', name, tag, + capture=True, raise_on_error=False) + if result.stdout: + tout.info(result.stdout) + if result.return_code: + tout.error(f'Subtree update failed (exit code ' + f'{result.return_code})') + if result.stderr: + tout.error(result.stderr) + try: + run_git(['merge', '--abort']) + except Exception: # pylint: disable=broad-except + pass + return 1 + except Exception as exc: # pylint: disable=broad-except + tout.error(f'Subtree update failed: {exc}') + return 1 + + return 0 + - Fetches the next batch of commits from the source. +def _subtree_record(dbs, source, squash_hash, merge_hash): + """Mark subtree commits as applied and advance the source position.""" + source_id = dbs.source_get_id(source) + for commit_hash in [squash_hash, merge_hash]: + if not dbs.commit_get(commit_hash): + info = run_git(['log', '-1', '--format=%s|%an', commit_hash]) + parts = info.split('|', 1) + subj = parts[0] + auth = parts[1] if len(parts) > 1 else '' + dbs.commit_add(commit_hash, source_id, subj, auth, + status='applied') + dbs.commit() + + dbs.source_set(source, merge_hash) + dbs.commit() + tout.info(f"Advanced source '{source}' past subtree merge " + f'{merge_hash[:12]}') + + +def apply_subtree_update(dbs, source, name, tag, merge_hash, args): # pylint: disable=too-many-arguments + """Apply a subtree update on the target branch + + Runs tools/update-subtree.sh to pull the subtree update, then + optionally pushes the result to the remote target branch. Args: dbs (Database): Database instance source (str): Source branch name + 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' + + 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) + parents = run_git(['rev-parse', f'{merge_hash}^@']).split() + if len(parents) < 2: + tout.error(f'Subtree merge {merge_hash[:12]} has no second parent') + return 1 + squash_hash = parents[1] + + try: + run_git(['checkout', target]) + except Exception: # pylint: disable=broad-except + tout.error(f'Could not checkout {target}') + return 1 + + ret = _subtree_run_update(name, tag) + if ret: + return ret + + if args.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 + tout.error(f'Failed to push {target}: {exc}') + return 1 + + _subtree_record(dbs, source, squash_hash, merge_hash) + + return 0 + + +def _prepare_get_commits(dbs, source, args): + """Get the next commits to apply, handling subtrees and skips. + + Fetch the next batch of commits from the source. If a subtree + update is encountered, apply it and retry. If all commits in a + merge are already processed, advance the source and retry. + + Args: + dbs (Database): Database instance + source (str): Source branch name + args (Namespace): Parsed arguments (needed for subtree updates) Returns: tuple: (NextCommitsInfo, return_code) where return_code is None on success, or an int (0 or 1) if there is nothing to do """ - info, err = get_next_commits(dbs, source) - if err: - tout.error(err) - return None, 1 + while True: + info, err = get_next_commits(dbs, source) + if err: + tout.error(err) + return None, 1 + + 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') + return None, 1 + ret = apply_subtree_update(dbs, source, name, tag, + info.advance_to, args) + if ret: + return None, ret + continue - if not info.commits: - 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]}') - else: + if not info.commits: + 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]}') + continue tout.info('No new commits to cherry-pick') - return None, 0 + return None, 0 - return info, None + return info, None -def prepare_apply(dbs, source, branch): +def prepare_apply(dbs, source, branch, args=None): """Prepare for applying commits from a source branch - Gets the next commits, sets up the branch name, and prints info about - what will be applied. + Get the next commits, set up the branch name and prints info about + what will be applied. When a subtree update is encountered, apply it + automatically and retry. Args: 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) Returns: tuple: (ApplyInfo, return_code) where ApplyInfo is set if there are commits to apply, or None with return_code indicating the result (0 for no commits, 1 for error) """ - info, ret = _prepare_get_commits(dbs, source) + info, ret = _prepare_get_commits(dbs, source, args) if ret is not None: return None, ret @@ -1738,7 +1901,7 @@ def do_apply(args, dbs): int: 0 on success, 1 on failure """ source = args.source - info, ret = prepare_apply(dbs, source, args.branch) + info, ret = prepare_apply(dbs, source, args.branch, args) if not info: return ret diff --git a/tools/pickman/ftest.py b/tools/pickman/ftest.py index a2c19a1159e..de6bce40614 100644 --- a/tools/pickman/ftest.py +++ b/tools/pickman/ftest.py @@ -2706,6 +2706,7 @@ class TestPrepareApply(unittest.TestCase): self.assertIsNotNone(info) self.assertEqual(ret, 0) + # Check that branch -D was called self.assertTrue( any('branch' in c and '-D' in c for c in git_cmds)) dbs.close() @@ -3463,24 +3464,28 @@ class TestGetNextCommitsMegaMerge(unittest.TestCase): stdout='mega|mega1|A|Merge branch next|' 'base second_parent\n') if call_count[0] == 2: + # Subtree check: log -1 --format=%s + return command.CommandResult( + stdout='Merge branch next') + if call_count[0] == 3: # detect_sub_merges: rev-parse ^@ return command.CommandResult( stdout='base\nsecond_parent\n') - if call_count[0] == 3: + if call_count[0] == 4: # detect_sub_merges: log --merges (found sub-merges) return command.CommandResult(stdout='sub1\n') - if call_count[0] == 4: + if call_count[0] == 5: # decompose: rev-parse ^@ for mega-merge return command.CommandResult( stdout='base\nsecond_parent\n') - if call_count[0] == 5: + if call_count[0] == 6: # decompose: log -1 for mega-merge info return command.CommandResult( stdout='Mega merge|Author\n') - if call_count[0] == 6: + if call_count[0] == 7: # decompose: mainline commits (empty) return command.CommandResult(stdout='') - if call_count[0] == 7: + if call_count[0] == 8: # decompose: sub-merge 1 commits return command.CommandResult( stdout='aaa|aaa1|A|Sub commit|base\n') @@ -3522,31 +3527,35 @@ class TestGetNextCommitsMegaMerge(unittest.TestCase): stdout='mega|mega1|A|Merge branch next|' 'base second_parent\n') if call_count[0] == 2: + # Subtree check: log -1 --format=%s + return command.CommandResult( + stdout='Merge branch next') + if call_count[0] == 3: # detect_sub_merges: rev-parse ^@ return command.CommandResult( stdout='base\nsecond_parent\n') - if call_count[0] == 3: + if call_count[0] == 4: # detect_sub_merges: log --merges return command.CommandResult(stdout='sub1\n') - if call_count[0] == 4: + if call_count[0] == 5: # decompose: rev-parse ^@ return command.CommandResult( stdout='base\nsecond_parent\n') - if call_count[0] == 5: + if call_count[0] == 6: # decompose: log -1 for mega-merge info return command.CommandResult( stdout='Mega merge|Author\n') - if call_count[0] == 6: + if call_count[0] == 7: # decompose: mainline (empty) return command.CommandResult(stdout='') - if call_count[0] == 7: + if call_count[0] == 8: # decompose: sub-merge 1 (in DB) return command.CommandResult( stdout='aaa|aaa1|A|Sub commit|base\n') - if call_count[0] == 8: + if call_count[0] == 9: # decompose: remainder (empty) return command.CommandResult(stdout='') - if call_count[0] == 9: + if call_count[0] == 10: # Remaining commits after mega-merge (empty) return command.CommandResult(stdout='') return command.CommandResult(stdout='') @@ -3580,13 +3589,17 @@ class TestGetNextCommitsMegaMerge(unittest.TestCase): stdout='merge1|m1|A|Merge branch feat|' 'base side1\n') if call_count[0] == 2: + # Subtree check: log -1 --format=%s + return command.CommandResult( + stdout='Merge branch feat') + if call_count[0] == 3: # detect_sub_merges: rev-parse ^@ return command.CommandResult( stdout='base\nside1\n') - if call_count[0] == 3: + if call_count[0] == 4: # detect_sub_merges: log --merges (no sub-merges) return command.CommandResult(stdout='') - if call_count[0] == 4: + if call_count[0] == 5: # Commits for this merge return command.CommandResult( stdout='aaa|aaa1|A|Commit 1|base\n' @@ -3606,6 +3619,605 @@ class TestGetNextCommitsMegaMerge(unittest.TestCase): dbs.close() +class TestSubtreeMergeDetection(unittest.TestCase): + """Tests for subtree merge detection in find_unprocessed_commits.""" + + 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_detects_dts_subtree_merge(self): + """Test find_unprocessed_commits detects dts/upstream subtree merge.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + dbs.source_set('us/next', 'base') + dbs.commit() + + command.TEST_RESULT = command.CommandResult( + stdout="Subtree merge tag 'v6.15-dts' of " + "https://example.com/dts.git into dts/upstream") + + info = control.find_unprocessed_commits( + dbs, 'base', 'us/next', ['merge1']) + + self.assertTrue(info.merge_found) + self.assertEqual(info.commits, []) + self.assertEqual(info.advance_to, 'merge1') + self.assertEqual(info.subtree_update, ('dts', 'v6.15-dts')) + dbs.close() + + def test_detects_mbedtls_subtree_merge(self): + """Test find_unprocessed_commits detects mbedtls subtree merge.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + dbs.source_set('us/next', 'base') + dbs.commit() + + command.TEST_RESULT = command.CommandResult( + stdout="Subtree merge tag 'v3.6.2' of " + "https://example.com/mbedtls.git into " + "lib/mbedtls/external/mbedtls") + + info = control.find_unprocessed_commits( + dbs, 'base', 'us/next', ['merge1']) + + self.assertEqual(info.subtree_update, + ('mbedtls', 'v3.6.2')) + dbs.close() + + def test_detects_lwip_subtree_merge(self): + """Test find_unprocessed_commits detects lwip subtree merge.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + dbs.source_set('us/next', 'base') + dbs.commit() + + command.TEST_RESULT = command.CommandResult( + stdout="Subtree merge tag 'STABLE-2_2_0' of " + "https://example.com/lwip.git into lib/lwip/lwip") + + info = control.find_unprocessed_commits( + dbs, 'base', 'us/next', ['merge1']) + + self.assertEqual(info.subtree_update, + ('lwip', 'STABLE-2_2_0')) + dbs.close() + + def test_skips_unknown_subtree_path(self): + """Test find_unprocessed_commits skips unknown subtree paths.""" + 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: + # Subject for merge1: unknown subtree + return command.CommandResult( + stdout="Subtree merge tag 'v1.0' of " + "https://x.com/x.git into lib/unknown") + if call_count[0] == 2: + # Subject for merge2: not a subtree merge + return command.CommandResult( + stdout='Normal merge commit') + if call_count[0] == 3: + # detect_sub_merges: rev-parse ^@ + return command.CommandResult( + stdout='merge1\nside1\n') + if call_count[0] == 4: + # detect_sub_merges: log --merges (no sub-merges) + return command.CommandResult(stdout='') + if call_count[0] == 5: + # Commits for merge2 + return command.CommandResult( + stdout='aaa|aaa1|A|Commit 1|merge1\n') + return command.CommandResult(stdout='') + + command.TEST_RESULT = mock_git + + info = control.find_unprocessed_commits( + dbs, 'base', 'us/next', ['merge1', 'merge2']) + + # Should have skipped merge1 and found commits in merge2 + self.assertIsNone(info.subtree_update) + self.assertTrue(info.merge_found) + self.assertEqual(len(info.commits), 1) + self.assertEqual(info.commits[0].chash, 'aaa1') + dbs.close() + + def test_subtree_merge_via_get_next_commits(self): + """Test get_next_commits returns subtree_update for subtree 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=unused-argument + call_count[0] += 1 + if call_count[0] == 1: + # First-parent log shows one merge + return command.CommandResult( + stdout='merge1|m1|A|Subtree merge tag ' + "'v6.15-dts' of https://x.com/dts.git" + ' into dts/upstream|base second\n') + if call_count[0] == 2: + # find_unprocessed: log -1 --format=%s for merge1 + return command.CommandResult( + stdout="Subtree merge tag 'v6.15-dts' of " + "https://x.com/dts.git into " + "dts/upstream") + return command.CommandResult(stdout='') + + command.TEST_RESULT = mock_git + + info, err = control.get_next_commits(dbs, 'us/next') + + self.assertIsNone(err) + self.assertEqual(info.subtree_update, ('dts', 'v6.15-dts')) + self.assertEqual(info.advance_to, 'merge1') + self.assertEqual(info.commits, []) + dbs.close() + + def test_non_subtree_merge_has_no_subtree_update(self): + """Test normal merges have subtree_update=None.""" + 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: + # Subject: not a subtree merge + return command.CommandResult( + stdout='Merge branch some-feature') + 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 in merge + return command.CommandResult( + stdout='aaa|aaa1|A|Commit 1|base\n') + return command.CommandResult(stdout='') + + command.TEST_RESULT = mock_git + + info = control.find_unprocessed_commits( + dbs, 'base', 'us/next', ['merge1']) + + self.assertIsNone(info.subtree_update) + self.assertTrue(info.merge_found) + self.assertEqual(len(info.commits), 1) + dbs.close() + + +class TestApplySubtreeUpdate(unittest.TestCase): + """Tests for apply_subtree_update 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_apply_success(self): + """Test apply_subtree_update succeeds and updates database.""" + 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: + # Parents of merge: first_parent squash_hash + return 'first_parent\nsquash_hash' + if 'checkout' in git_args: + return '' + if '--format=%s|%an' in git_args: + if 'squash_hash' in git_args: + return "Squashed 'dts/upstream/' changes|Author" + return "Subtree merge tag 'v6.15-dts'|Author" + return '' + + mock_result = command.CommandResult( + 'Subtree updated', '', '', 0) + with mock.patch.object(control, 'run_git', + side_effect=run_git_handler): + with mock.patch.object( + control.command, 'run', + return_value=mock_result): + ret = control.apply_subtree_update( + dbs, 'us/next', 'dts', 'v6.15-dts', + 'merge_hash', args) + + self.assertEqual(ret, 0) + + # Source should be advanced past the merge + self.assertEqual(dbs.source_get('us/next'), 'merge_hash') + + # Both commits should be in the database + squash = dbs.commit_get('squash_hash') + self.assertIsNotNone(squash) + merge = dbs.commit_get('merge_hash') + self.assertIsNotNone(merge) + dbs.close() + + def test_apply_with_push(self): + """Test apply_subtree_update pushes when args.push is True.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + dbs.source_set('us/next', 'base') + dbs.commit() + + args = argparse.Namespace(push=True, remote='ci', + target='master') + + 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', + 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', args) + + self.assertEqual(ret, 0) + self.assertTrue(pushed[0]) + dbs.close() + + def test_apply_checkout_failure(self): + """Test apply_subtree_update returns 1 on checkout failure.""" + 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: + return 'first_parent\nsquash_hash' + if 'checkout' in git_args: + raise Exception('checkout failed') + return '' + + with mock.patch.object(control, 'run_git', + side_effect=run_git_handler): + ret = control.apply_subtree_update( + dbs, 'us/next', 'dts', 'v6.15-dts', + 'merge_hash', args) + + self.assertEqual(ret, 1) + # Source should not be advanced + self.assertEqual(dbs.source_get('us/next'), 'base') + dbs.close() + + def test_apply_no_second_parent(self): + """Test apply_subtree_update returns 1 when merge has no 2nd parent.""" + 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') + + # Only one parent + with mock.patch.object(control, 'run_git', + return_value='single_parent'): + ret = control.apply_subtree_update( + dbs, 'us/next', 'dts', 'v6.15-dts', + 'merge_hash', args) + + self.assertEqual(ret, 1) + dbs.close() + + def test_apply_script_exception(self): + """Test apply_subtree_update returns 1 on script exception.""" + 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: + return 'first_parent\nsquash_hash' + if 'checkout' in git_args: + return '' + return '' + + with mock.patch.object(control, 'run_git', + side_effect=run_git_handler): + with mock.patch.object( + control.command, 'run', + side_effect=Exception('script failed')): + ret = control.apply_subtree_update( + dbs, 'us/next', 'dts', 'v6.15-dts', + 'merge_hash', args) + + self.assertEqual(ret, 1) + # Source should not be advanced + self.assertEqual(dbs.source_get('us/next'), 'base') + dbs.close() + + def test_apply_merge_conflict(self): + """Test apply_subtree_update aborts merge on non-zero exit.""" + 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: + 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', + return_value=mock_result): + ret = control.apply_subtree_update( + dbs, 'us/next', 'dts', 'v6.15-dts', + 'merge_hash', args) + + self.assertEqual(ret, 1) + self.assertTrue(merge_aborted[0]) + # Source should not be advanced + self.assertEqual(dbs.source_get('us/next'), 'base') + dbs.close() + + +class TestPrepareApplySubtreeUpdate(unittest.TestCase): + """Tests for prepare_apply handling of subtree updates.""" + + 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_prepare_apply_calls_subtree_update(self): + """Test prepare_apply applies subtree update and retries.""" + 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') + subtree_info = control.NextCommitsInfo( + [], True, 'merge1', ('dts', 'v6.15-dts')) + normal_info = control.NextCommitsInfo([], False, None) + + call_count = [0] + + def mock_get_next(dbs_arg, source): + call_count[0] += 1 + if call_count[0] == 1: + return subtree_info, None + return normal_info, None + + with mock.patch.object(control, 'get_next_commits', + side_effect=mock_get_next): + with mock.patch.object( + control, 'apply_subtree_update', + return_value=0) as mock_apply: + info, ret = control.prepare_apply( + dbs, 'us/next', None, args) + + # Should have called apply_subtree_update + mock_apply.assert_called_once_with( + dbs, 'us/next', 'dts', 'v6.15-dts', 'merge1', args) + # No commits after retry, so returns None/0 + self.assertIsNone(info) + self.assertEqual(ret, 0) + dbs.close() + + def test_prepare_apply_subtree_update_failure(self): + """Test prepare_apply returns error when subtree update 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') + subtree_info = control.NextCommitsInfo( + [], True, 'merge1', ('dts', 'v6.15-dts')) + + with mock.patch.object(control, 'get_next_commits', + return_value=(subtree_info, None)): + with mock.patch.object( + control, 'apply_subtree_update', + return_value=1): + info, ret = control.prepare_apply( + dbs, 'us/next', None, args) + + 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.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + dbs.source_set('us/next', 'base') + dbs.commit() + + subtree_info = control.NextCommitsInfo( + [], True, 'merge1', ('dts', 'v6.15-dts')) + + with mock.patch.object(control, 'get_next_commits', + return_value=(subtree_info, None)): + info, ret = control.prepare_apply( + dbs, 'us/next', None) + + self.assertIsNone(info) + self.assertEqual(ret, 1) + dbs.close() + + +class TestNextCommitsInfoDefault(unittest.TestCase): + """Tests for NextCommitsInfo subtree_update default value.""" + + def test_default_subtree_update_is_none(self): + """Test NextCommitsInfo defaults subtree_update to None.""" + info = control.NextCommitsInfo([], False, None) + self.assertIsNone(info.subtree_update) + + def test_explicit_subtree_update(self): + """Test NextCommitsInfo accepts explicit subtree_update.""" + info = control.NextCommitsInfo([], True, 'hash1', + ('dts', 'v6.15-dts')) + self.assertEqual(info.subtree_update, ('dts', 'v6.15-dts')) + + def test_explicit_none_subtree_update(self): + """Test NextCommitsInfo accepts explicit None subtree_update.""" + info = control.NextCommitsInfo([], False, None, None) + self.assertIsNone(info.subtree_update) + + +class TestSubtreeMergeRegex(unittest.TestCase): + """Tests for RE_SUBTREE_MERGE regex pattern.""" + + def test_matches_dts_merge(self): + """Test regex matches dts subtree merge subject.""" + subject = ("Subtree merge tag 'v6.15-dts' of " + "https://git.kernel.org/pub/scm/linux/kernel/git/" + "devicetree/devicetree-rebasing.git into dts/upstream") + match = control.RE_SUBTREE_MERGE.match(subject) + self.assertIsNotNone(match) + self.assertEqual(match.group(1), 'v6.15-dts') + self.assertEqual(match.group(2), 'dts/upstream') + + def test_matches_mbedtls_merge(self): + """Test regex matches mbedtls subtree merge subject.""" + subject = ("Subtree merge tag 'v3.6.2' of " + "https://github.com/Mbed-TLS/mbedtls.git into " + "lib/mbedtls/external/mbedtls") + match = control.RE_SUBTREE_MERGE.match(subject) + self.assertIsNotNone(match) + self.assertEqual(match.group(1), 'v3.6.2') + self.assertEqual(match.group(2), 'lib/mbedtls/external/mbedtls') + + def test_matches_lwip_merge(self): + """Test regex matches lwip subtree merge subject.""" + subject = ("Subtree merge tag 'STABLE-2_2_0' of " + "https://git.savannah.gnu.org/git/lwip.git into " + "lib/lwip/lwip") + match = control.RE_SUBTREE_MERGE.match(subject) + self.assertIsNotNone(match) + self.assertEqual(match.group(1), 'STABLE-2_2_0') + self.assertEqual(match.group(2), 'lib/lwip/lwip') + + def test_no_match_normal_merge(self): + """Test regex does not match normal merge subjects.""" + subject = "Merge branch 'feature-xyz' into main" + match = control.RE_SUBTREE_MERGE.match(subject) + self.assertIsNone(match) + + def test_no_match_squash_commit(self): + """Test regex does not match subtree squash commits.""" + subject = ("Squashed 'dts/upstream/' changes from " + "v6.14-dts..v6.15-dts") + match = control.RE_SUBTREE_MERGE.match(subject) + self.assertIsNone(match) + + class TestDoCommitSourceResolveError(unittest.TestCase): """Tests for do_commit_source error handling.""" From patchwork Sun Feb 22 15:42:49 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 1930 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=1771775055; bh=uOtDpFMQU2TR1ZSFWv7f5zRnpJCDl5Zq6IlyOZt/bxY=; 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=rfRR+CUbKbSjt4bCuZ4Oo4Kos6lRc3S7MLUdyLUAITdGiIx3Ar1uyZzJf27JIaMCe pZ0zdA9IuJVzwCTiWuwlRKLZbh9Iqro7OBGVpNP29y6kV/WsvlVKTm6+hQXebMiiyH r8Ix1cq9jNifsV9dQDGuvXL7Eb9Xtn5MPTohYkZh+pBCV+6tz/+vi0Z1Vcw401nGUP 22KjN55/opZqkTrgT7NoZyXSuXp+B2FrQu5P3OUSuQ3yLusvN27TFqztC2zvCYYT3h gGyzl2UUg4xzW8DLqFsuFDewGXJ3RfGD8Hev3z7F3GN9dND47/KBTLFri2izhoR//J i1d0AqX5NnQqA== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 3C57169C5E for ; Sun, 22 Feb 2026 08:44: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 10024) with ESMTP id BFIzruSNUWig for ; Sun, 22 Feb 2026 08:44:15 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1771775055; bh=uOtDpFMQU2TR1ZSFWv7f5zRnpJCDl5Zq6IlyOZt/bxY=; 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=rfRR+CUbKbSjt4bCuZ4Oo4Kos6lRc3S7MLUdyLUAITdGiIx3Ar1uyZzJf27JIaMCe pZ0zdA9IuJVzwCTiWuwlRKLZbh9Iqro7OBGVpNP29y6kV/WsvlVKTm6+hQXebMiiyH r8Ix1cq9jNifsV9dQDGuvXL7Eb9Xtn5MPTohYkZh+pBCV+6tz/+vi0Z1Vcw401nGUP 22KjN55/opZqkTrgT7NoZyXSuXp+B2FrQu5P3OUSuQ3yLusvN27TFqztC2zvCYYT3h gGyzl2UUg4xzW8DLqFsuFDewGXJ3RfGD8Hev3z7F3GN9dND47/KBTLFri2izhoR//J i1d0AqX5NnQqA== Received: from mail.u-boot.org (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 2887769D3F for ; Sun, 22 Feb 2026 08:44:15 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1771775052; bh=MmzpRytZLqKPbiDBv6ZIIx71ZHPdTZz2mo1QK0R7DUc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=JCzXtsNEz93HYGwLT0PJ6GRc+Fm1W2JJTGNvtiDO0EMG4uf9oDxwH+QkMVJ+l4rzG Wo8Xf1hJmXTZNP8aMHP/B36t9jMQZT81MO/KUIZe6a4UmNUHD/nJoN2WJ+6iad7kW8 PTOClepGyGCpDBoUesQXg75IL6VochAqPx9iIJTKoqDCwrMpIzOJd4FkOHFwx8Vnaq MzGRbbxdGqcKJKTeANLnCG5YTbK258nzdjDI+RJsdge8mZjvDo4QPNpOiT+AynIvSm gvh/Mjf6ezLEASWDwmPak7DA24D3OXcXM7YeKCuN3WyXMrBz/8stbfMo0J5qN6F/xB Wp9KGyAqo6bDA== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 52A4269D53; Sun, 22 Feb 2026 08:44:12 -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 rlSpG8RiiiOK; Sun, 22 Feb 2026 08:44:12 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1771775044; bh=KMGs2zsHh5vqTvkqegdVJ7tZLlQ/d/NiXu3qSGJiREI=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=lVrPXIodesdLHBnz0exNVjFSur5GK2FLXME2OcO5T58zHiFETo0sGeH7pARi8wdaX qdPm7EeH/KgL8coOn2qoHtFCaDxQpymmfN/xiD0HdhaIrE3nYnD2uwsFtIWuLRJbOg 6Qg5MeYXUrJj0taPesgk/29044Gqjyai73mXF512wv/JuR6V6TtCwOmkNk5ALUwu0R 7JaTq3eINPsF82ey4XcnN92Camkb8tyoOweGyRd70BEoqyjGYAMXm+iDlAnDLoW8Wl 2sRPUDnEsbZAsvYRwDJkJ/hdowV3qTktSaKS3sCP/QgEovjLcpqpsqFOsJl01EWJng U6OmfSoynsm4g== Received: from u-boot.org (unknown [73.34.74.121]) by mail.u-boot.org (Postfix) with ESMTPSA id 35DA369D3F; Sun, 22 Feb 2026 08:44:04 -0700 (MST) From: Simon Glass To: U-Boot Concept Date: Sun, 22 Feb 2026 08:42:49 -0700 Message-ID: <20260222154303.2851319-10-sjg@u-boot.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20260222154303.2851319-1-sjg@u-boot.org> References: <20260222154303.2851319-1-sjg@u-boot.org> MIME-Version: 1.0 Message-ID-Hash: LQVEZDC4IJLMJFSFRL7BNXCFUUT7FPPV X-Message-ID-Hash: LQVEZDC4IJLMJFSFRL7BNXCFUUT7FPPV 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 09/16] pickman: Create a function to run an agent 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 agent-message-streaming pattern (async iteration, text extraction and conversation-log collection) is duplicated in run() and run_review_agent() Extract it into a shared run_agent_collect() helper. Co-developed-by: Claude Opus 4.6 Signed-off-by: Simon Glass --- tools/pickman/agent.py | 55 ++++++++++++++++++++----------------- tools/pickman/ftest.py | 62 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 25 deletions(-) diff --git a/tools/pickman/agent.py b/tools/pickman/agent.py index 85f8efee1df..63952c1c005 100644 --- a/tools/pickman/agent.py +++ b/tools/pickman/agent.py @@ -55,6 +55,34 @@ def check_available(): return True +async def run_agent_collect(prompt, options): + """Run a Claude agent and collect its conversation log + + Sends the prompt to a Claude agent, streams output to stdout and + collects all text blocks into a conversation log. + + Args: + prompt (str): The prompt to send to the agent + options (ClaudeAgentOptions): Agent configuration + + Returns: + tuple: (success, conversation_log) where success is bool and + conversation_log is the agent's output text + """ + conversation_log = [] + try: + async for message in query(prompt=prompt, options=options): + if hasattr(message, 'content'): + for block in message.content: + if hasattr(block, 'text'): + print(block.text) + conversation_log.append(block.text) + return True, '\n\n'.join(conversation_log) + except (RuntimeError, ValueError, OSError) as exc: + tout.error(f'Agent failed: {exc}') + return False, '\n\n'.join(conversation_log) + + def is_qconfig_commit(subject): """Check if a commit subject indicates a qconfig resync commit @@ -228,19 +256,7 @@ this means the series was already applied via a different path. In this case: tout.info(f'Starting Claude agent to cherry-pick {len(commits)} commits...') tout.info('') - conversation_log = [] - try: - async for message in query(prompt=prompt, options=options): - # Print agent output and capture it - if hasattr(message, 'content'): - for block in message.content: - if hasattr(block, 'text'): - print(block.text) - conversation_log.append(block.text) - return True, '\n\n'.join(conversation_log) - except (RuntimeError, ValueError, OSError) as exc: - tout.error(f'Agent failed: {exc}') - return False, '\n\n'.join(conversation_log) + return await run_agent_collect(prompt, options) def read_signal_file(repo_path=None): @@ -492,18 +508,7 @@ async def run_review_agent(mr_iid, branch_name, comments, remote, tout.info(f'Starting Claude agent to {task_desc}...') tout.info('') - conversation_log = [] - try: - async for message in query(prompt=prompt, options=options): - if hasattr(message, 'content'): - for block in message.content: - if hasattr(block, 'text'): - print(block.text) - conversation_log.append(block.text) - return True, '\n\n'.join(conversation_log) - except (RuntimeError, ValueError, OSError) as exc: - tout.error(f'Agent failed: {exc}') - return False, '\n\n'.join(conversation_log) + return await run_agent_collect(prompt, options) # pylint: disable=too-many-arguments diff --git a/tools/pickman/ftest.py b/tools/pickman/ftest.py index de6bce40614..42ce05962e2 100644 --- a/tools/pickman/ftest.py +++ b/tools/pickman/ftest.py @@ -6,6 +6,7 @@ # pylint: disable=too-many-lines """Tests for pickman.""" +import asyncio import argparse import os import shutil @@ -2971,6 +2972,67 @@ class TestExecuteApply(unittest.TestCase): dbs.close() +class TestRunAgentCollect(unittest.TestCase): + """Tests for run_agent_collect function.""" + + def test_success(self): + """Test successful agent run collects text blocks.""" + block1 = mock.MagicMock() + block1.text = 'hello' + block2 = mock.MagicMock() + block2.text = 'world' + msg = mock.MagicMock() + msg.content = [block1, block2] + + async def fake_query(**kwargs): + yield msg + + with mock.patch.object(agent, 'query', fake_query, create=True): + with terminal.capture(): + opts = mock.MagicMock() + success, log = asyncio.run( + agent.run_agent_collect('prompt', opts)) + + self.assertTrue(success) + self.assertEqual(log, 'hello\n\nworld') + + def test_failure(self): + """Test agent failure returns False with partial log.""" + block = mock.MagicMock() + block.text = 'partial' + msg = mock.MagicMock() + msg.content = [block] + + async def fake_query(**kwargs): + yield msg + raise RuntimeError('agent crashed') + + with mock.patch.object(agent, 'query', fake_query, create=True): + with terminal.capture(): + opts = mock.MagicMock() + success, log = asyncio.run( + agent.run_agent_collect('prompt', opts)) + + self.assertFalse(success) + self.assertEqual(log, 'partial') + + def test_no_content(self): + """Test messages without content are skipped.""" + msg = mock.MagicMock(spec=[]) + + async def fake_query(**kwargs): + yield msg + + with mock.patch.object(agent, 'query', fake_query, create=True): + with terminal.capture(): + opts = mock.MagicMock() + success, log = asyncio.run( + agent.run_agent_collect('prompt', opts)) + + self.assertTrue(success) + self.assertEqual(log, '') + + class TestSignalFile(unittest.TestCase): """Tests for signal file handling.""" From patchwork Sun Feb 22 15:42:50 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 1931 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=1771775058; bh=ImQimOLM7y/PZbyTUjNae/XX0OH52fvhC4NeUcTXiTY=; 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=OBMn42XBOMqmMa2fF9SKxbuAqRl+l1AJrh1tXE4xRLHNMMZA4SVKneiU3pmhEA6nw ufK+Ch4CX3OOp9dQTrUqqkUcpNXIGLjSXFaAIKvbAjitYUgQTnjc67FS98OIa/79PT G3G1WvwBuhPjlFGxCRSveVTGnFZXR0wJuMvJy2d86tildTztFYrE6vUQWSIIrTSQaF trWjF4U3bppcnjTuvz8Rr5DA80qe67VDLLDcSje9Cq6O9CQr6miZlnWHHBRRHkrzwM tfFZLEoaduVrAG6sdWLZOnsW5C0t3vluU5ahR4Lt4aARiT7ZTFYyWzIuavfeAkLQP4 td08bpO9RDU9w== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 6128769D53 for ; Sun, 22 Feb 2026 08:44: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 10024) with ESMTP id En0MC7G9KvQS for ; Sun, 22 Feb 2026 08:44:18 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1771775058; bh=ImQimOLM7y/PZbyTUjNae/XX0OH52fvhC4NeUcTXiTY=; 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=OBMn42XBOMqmMa2fF9SKxbuAqRl+l1AJrh1tXE4xRLHNMMZA4SVKneiU3pmhEA6nw ufK+Ch4CX3OOp9dQTrUqqkUcpNXIGLjSXFaAIKvbAjitYUgQTnjc67FS98OIa/79PT G3G1WvwBuhPjlFGxCRSveVTGnFZXR0wJuMvJy2d86tildTztFYrE6vUQWSIIrTSQaF trWjF4U3bppcnjTuvz8Rr5DA80qe67VDLLDcSje9Cq6O9CQr6miZlnWHHBRRHkrzwM tfFZLEoaduVrAG6sdWLZOnsW5C0t3vluU5ahR4Lt4aARiT7ZTFYyWzIuavfeAkLQP4 td08bpO9RDU9w== Received: from mail.u-boot.org (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 46F5669C5E for ; Sun, 22 Feb 2026 08:44:18 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1771775056; bh=A0mxhAzSDvq3cLc7jFKnFLyMuOY2jQVqJ3pSl3yc3DA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Tkh8a0xodhQnDnsurbThkZugGsWy+g18NNExj4m+G8d4FiUOK7W0IIQJs1w2/FUD6 otsYhxWUG4DHHqHSueF/ul86sJzMtUKI00e91Yk3B7g5cNTnRF1Ifu3lhrZWA4NqoO pqNe2HGTTs9+nopIZ99HpmcwR2BMJKrjMlvEn4q+WNLrnU0xvN0DzrNvjNSS239DLw PNBWaoLoUo6/LfVMam1Dy0xFGkRnuIyf1z5kHkiF3fGK4+Ja4tGM0FvUUX1oENeBp4 92EaNY2Lpw556rdL9B4DVvI6vjHdjKTzd8q+ETCn5Ug+3lUexLL7UQW5DvITNykTW/ DXFDhdsZlFiaA== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 158EC69D58; Sun, 22 Feb 2026 08:44:16 -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 SOd2KDKubLXO; Sun, 22 Feb 2026 08:44:15 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1771775051; bh=R4jDeGia1v1L5rwkZXzSsXI2yJDoWp5Di7HBAWnxQ6k=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=MujNQWVEqjA0a84JWrV3GumYHGF1wEMO5CzlnzUM1Boer9aKFYDrbPOoRfpb6Fik9 OlIgIuk5r+LvcOeo3zXqg23QIokAnR8z9dMTd+kdDNmiKn5MqebK0Xlbp+b5qDXuLt Y7KbEJFnGrJF2a72Bqbm5XNwj76LOocCG1zGrEeQGhz3d22pBbp8A1++I6vBJYYdFF G2yonqbBH7q8D58Rhvi7RP9nkKWsoRo2rF6MjZrTK6jN7cyRF/kawj+uj056RTOrjd MtfwU5eivS5hsfx0E2Zdg/nmLsVwEfdqiivEsyttIrzz8vpo7jJUU4wnpAWbEhh2DZ x1POioljn/43Q== Received: from u-boot.org (unknown [73.34.74.121]) by mail.u-boot.org (Postfix) with ESMTPSA id 6644869C5C; Sun, 22 Feb 2026 08:44:11 -0700 (MST) From: Simon Glass To: U-Boot Concept Date: Sun, 22 Feb 2026 08:42:50 -0700 Message-ID: <20260222154303.2851319-11-sjg@u-boot.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20260222154303.2851319-1-sjg@u-boot.org> References: <20260222154303.2851319-1-sjg@u-boot.org> MIME-Version: 1.0 Message-ID-Hash: J3VMXNJMF2OQHLR4EA3PZX3TC7OFMASQ X-Message-ID-Hash: J3VMXNJMF2OQHLR4EA3PZX3TC7OFMASQ 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 10/16] pickman: Use tools for file I/O 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 Replace manual open() calls with tools.read_file() and tools.write_file() for consistency with the rest of the codebase. Co-developed-by: Claude Opus 4.6 Signed-off-by: Simon Glass --- tools/pickman/agent.py | 5 +++-- tools/pickman/control.py | 13 +++++-------- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/tools/pickman/agent.py b/tools/pickman/agent.py index 63952c1c005..4b914e314e8 100644 --- a/tools/pickman/agent.py +++ b/tools/pickman/agent.py @@ -14,6 +14,7 @@ our_path = os.path.dirname(os.path.realpath(__file__)) sys.path.insert(0, os.path.join(our_path, '..')) # pylint: disable=wrong-import-position,import-error +from u_boot_pylib import tools from u_boot_pylib import tout # Signal file for agent to communicate status back to pickman @@ -278,8 +279,8 @@ def read_signal_file(repo_path=None): return None, None try: - with open(signal_path, 'r', encoding='utf-8') as fhandle: - lines = fhandle.read().strip().split('\n') + data = tools.read_file(signal_path, binary=False) + lines = data.strip().split('\n') status = lines[0] if lines else None last_commit = lines[1] if len(lines) > 1 else None diff --git a/tools/pickman/control.py b/tools/pickman/control.py index 48f7ced1617..057e4400adb 100644 --- a/tools/pickman/control.py +++ b/tools/pickman/control.py @@ -26,6 +26,7 @@ from pickman import ftest from pickman import gitlab_api from u_boot_pylib import command from u_boot_pylib import terminal +from u_boot_pylib import tools from u_boot_pylib import tout # Default database filename @@ -1458,8 +1459,7 @@ def get_history(fname, source, commits, branch_name, conv_log): # Read existing content existing = '' if os.path.exists(fname): - with open(fname, 'r', encoding='utf-8') as fhandle: - existing = fhandle.read() + existing = tools.read_file(fname, binary=False) # Remove existing entry for this branch (from ## header to ---) pattern = rf'## [^\n]+\n\nBranch: {re.escape(branch_name)}\n.*?---\n\n' existing = re.sub(pattern, '', existing, flags=re.DOTALL) @@ -1467,8 +1467,7 @@ def get_history(fname, source, commits, branch_name, conv_log): content = existing + entry # Write updated history file - with open(fname, 'w', encoding='utf-8') as fhandle: - fhandle.write(content) + tools.write_file(fname, content, binary=False) # Generate commit message commit_msg = (f'pickman: Record cherry-pick of {len(commits)} commits ' @@ -2429,11 +2428,9 @@ Comments addressed: # Append to history file existing = '' if os.path.exists(HISTORY_FILE): - with open(HISTORY_FILE, 'r', encoding='utf-8') as fhandle: - existing = fhandle.read() + existing = tools.read_file(HISTORY_FILE, binary=False) - with open(HISTORY_FILE, 'w', encoding='utf-8') as fhandle: - fhandle.write(existing + entry) + tools.write_file(HISTORY_FILE, existing + entry, binary=False) # Commit the history file run_git(['add', '-f', HISTORY_FILE]) From patchwork Sun Feb 22 15:42:51 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 1932 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=1771775063; bh=ShCeZLDV3QwPa3RgG9AJGUVDnNs1zYn6ooeXvipRXo4=; 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=OKMxLwgYKkrceLD9iY9TRjXJNf4iec0JEn2EZTzvrhZz5FPif0nYPgU/nR87XgsuV Lcyi3KHbjim8UEDn+dAjQYxAep1guylMAu8k9OFA/EdSZylYGLPVHGPd92C3coZiEN 5OPrDDnauVkCzG29tAMaiCMx7nk7CpwKgoaBm6TUvKoCltLUIRa6+oT98x7e8CqD2/ 2+hQ6w+GSppvpluaxWwM/bItVxlcOJ07AtL7GZKtOQFc/K/9EJ0ns7QVSpW8zBOwQC oDprPbhxPcckL9C+RqHYrKGHtMK5OcU5+bWIXPdOrn+65swzEyFFwuCNxdSnSZCBLa 3PCFbQNrZHZTw== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 19C5169C5E for ; Sun, 22 Feb 2026 08:44: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 c_qdoSyWoh5o for ; Sun, 22 Feb 2026 08:44:23 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1771775063; bh=ShCeZLDV3QwPa3RgG9AJGUVDnNs1zYn6ooeXvipRXo4=; 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=OKMxLwgYKkrceLD9iY9TRjXJNf4iec0JEn2EZTzvrhZz5FPif0nYPgU/nR87XgsuV Lcyi3KHbjim8UEDn+dAjQYxAep1guylMAu8k9OFA/EdSZylYGLPVHGPd92C3coZiEN 5OPrDDnauVkCzG29tAMaiCMx7nk7CpwKgoaBm6TUvKoCltLUIRa6+oT98x7e8CqD2/ 2+hQ6w+GSppvpluaxWwM/bItVxlcOJ07AtL7GZKtOQFc/K/9EJ0ns7QVSpW8zBOwQC oDprPbhxPcckL9C+RqHYrKGHtMK5OcU5+bWIXPdOrn+65swzEyFFwuCNxdSnSZCBLa 3PCFbQNrZHZTw== Received: from mail.u-boot.org (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 058CB69D3F for ; Sun, 22 Feb 2026 08:44:23 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1771775061; bh=Jne9q+uICiiAc/1mJy3d87ZMaCg+IrPmYMAWX3FerOs=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Rggp44Ol3DyamgCZcHhnSP/Q5UauA0AtG4GzCLIIlv5Iymli0L/7uWAL/WR7bb5y9 QhaH/v+U7MaTh9qVscDIzh31l8NHPZ6H0Y2zm+2R0HPx3H5xUyqCmnrGhl4P/uO0d/ xdQJC55LtZ8lyx2YuwerhKjkpKDxn7sYtId5Y9u4bUAhIK+K/Pn1lDXhoLeG7lbBTO nMY4l2GuNPf4xe5dweTr5oUlWM4hmMPBPZ02Y+5C77LXBEARJns1ERH1Yf9rMS41of 7xNkjJTBpHeTfZ91qGxZN+n68J+gsx4I+cAjru/lHcvjVUExSrLrbdGkBw9TwJg3c2 YVBN1G8RrH5lQ== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 0B14269C5E; Sun, 22 Feb 2026 08:44:21 -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 sNpoOX-BqNAX; Sun, 22 Feb 2026 08:44:20 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1771775056; bh=PB7uhSWlf9/BoBctg3qaF+U+pun+ycHXls0AkdQNdjU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=wul/trDo8F5Z8PVv9wLUvYGy4HnyFIEf29qwM91ZodQxL3P9LPmicN0QCJsdCpWBA w6jUWgSYYcGm8AiVq5dLLWgqCQ8oaXyD8BzLEfIpyqXViOgjDFf9uxVP/7x0+VFEQD wpiEW/o3VyLQ+KOskbdQ6pHEYGPBUytyX98rHCHXE6sdh6Hu2j4Bqc39GHYEocgBZS prhLUkeXS0yHPyOgj2KGAi7O0zlaovbac+7aIj93JFZkCoo+2Qji8jsU4acVDaWQy9 gJm2ZTscmVg92t2jpbXJg/+RiEPg7VGGebUFaLmeS99Vu8N43/T4hI7f2fKNeURuee x6nXN5WjM70qQ== Received: from u-boot.org (unknown [73.34.74.121]) by mail.u-boot.org (Postfix) with ESMTPSA id 3495969D59; Sun, 22 Feb 2026 08:44:16 -0700 (MST) From: Simon Glass To: U-Boot Concept Date: Sun, 22 Feb 2026 08:42:51 -0700 Message-ID: <20260222154303.2851319-12-sjg@u-boot.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20260222154303.2851319-1-sjg@u-boot.org> References: <20260222154303.2851319-1-sjg@u-boot.org> MIME-Version: 1.0 Message-ID-Hash: ABZZHIUMLJBBG3ZBFI4SQVYVP6VFTYSP X-Message-ID-Hash: ABZZHIUMLJBBG3ZBFI4SQVYVP6VFTYSP 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 11/16] pickman: Use tools for file I/O in tests 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 Replace manual open() calls with tools.read_file() and tools.write_file() in the test file for consistency. Co-developed-by: Claude Opus 4.6 Signed-off-by: Simon Glass --- tools/pickman/ftest.py | 41 +++++++++++++++++++---------------------- 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/tools/pickman/ftest.py b/tools/pickman/ftest.py index 42ce05962e2..4a58a9371ce 100644 --- a/tools/pickman/ftest.py +++ b/tools/pickman/ftest.py @@ -23,6 +23,7 @@ sys.path.insert(0, os.path.join(our_path, '..')) # pylint: disable=wrong-import-position,import-error,cyclic-import from u_boot_pylib import command from u_boot_pylib import terminal +from u_boot_pylib import tools from u_boot_pylib import tout from pickman import __main__ as pickman @@ -1634,8 +1635,9 @@ class TestConfigFile(unittest.TestCase): def test_get_token_from_config(self): """Test getting token from config file.""" - with open(self.config_file, 'w', encoding='utf-8') as fhandle: - fhandle.write('[gitlab]\ntoken = test-config-token\n') + tools.write_file(self.config_file, + '[gitlab]\ntoken = test-config-token\n', + binary=False) with mock.patch.object(gitlab, 'CONFIG_FILE', self.config_file): token = gitlab.get_token() @@ -1651,8 +1653,8 @@ class TestConfigFile(unittest.TestCase): def test_get_token_config_missing_section(self): """Test config file without gitlab section.""" - with open(self.config_file, 'w', encoding='utf-8') as fhandle: - fhandle.write('[other]\nkey = value\n') + tools.write_file(self.config_file, '[other]\nkey = value\n', + binary=False) with mock.patch.object(gitlab, 'CONFIG_FILE', self.config_file): with mock.patch.dict(os.environ, {'GITLAB_TOKEN': 'env-token'}): @@ -1661,8 +1663,8 @@ class TestConfigFile(unittest.TestCase): def test_get_config_value(self): """Test get_config_value function.""" - with open(self.config_file, 'w', encoding='utf-8') as fhandle: - fhandle.write('[section1]\nkey1 = value1\n') + tools.write_file(self.config_file, '[section1]\nkey1 = value1\n', + binary=False) with mock.patch.object(gitlab, 'CONFIG_FILE', self.config_file): value = gitlab.get_config_value('section1', 'key1') @@ -2149,8 +2151,7 @@ class TestUpdateHistoryWithReview(unittest.TestCase): # Check history file was created self.assertTrue(os.path.exists(control.HISTORY_FILE)) - with open(control.HISTORY_FILE, 'r', encoding='utf-8') as fhandle: - content = fhandle.read() + content = tools.read_file(control.HISTORY_FILE, binary=False) self.assertIn('### Review:', content) self.assertIn('Branch: cherry-abc123', content) @@ -2161,8 +2162,8 @@ class TestUpdateHistoryWithReview(unittest.TestCase): def test_update_history_appends(self): """Test that review handling appends to existing history.""" # Create existing history - with open(control.HISTORY_FILE, 'w', encoding='utf-8') as fhandle: - fhandle.write('Existing history content\n') + tools.write_file(control.HISTORY_FILE, + 'Existing history content\n', binary=False) subprocess.run(['git', 'add', control.HISTORY_FILE], check=True, capture_output=True) subprocess.run(['git', 'commit', '-m', 'Initial'], @@ -2173,8 +2174,7 @@ class TestUpdateHistoryWithReview(unittest.TestCase): resolved=False)] control.update_history('cherry-xyz', comms, 'Fixed it') - with open(control.HISTORY_FILE, 'r', encoding='utf-8') as fhandle: - content = fhandle.read() + content = tools.read_file(control.HISTORY_FILE, binary=False) self.assertIn('Existing history content', content) self.assertIn('### Review:', content) @@ -2496,15 +2496,14 @@ class TestGetHistory(unittest.TestCase): self.assertIn('- aaa111a First commit', commit_msg) # Verify file was written - with open(self.history_file, 'r', encoding='utf-8') as fhandle: - file_content = fhandle.read() + file_content = tools.read_file(self.history_file, binary=False) self.assertEqual(file_content, content) def test_get_history_with_existing(self): """Test get_history appends to existing content.""" # Create existing file - with open(self.history_file, 'w', encoding='utf-8') as fhandle: - fhandle.write('Previous history content\n') + tools.write_file(self.history_file, + 'Previous history content\n', binary=False) commits = [ control.CommitInfo('bbb222', 'bbb222b', 'New commit', 'Author 2'), @@ -2535,8 +2534,7 @@ Old conversation Other content """ - with open(self.history_file, 'w', encoding='utf-8') as fhandle: - fhandle.write(existing) + tools.write_file(self.history_file, existing, binary=False) commits = [ control.CommitInfo('ccc333', 'ccc333c', 'Updated commit', 'Author'), @@ -3055,8 +3053,8 @@ class TestSignalFile(unittest.TestCase): def test_read_signal_file_already_applied(self): """Test read_signal_file with already_applied status.""" - with open(self.signal_path, 'w', encoding='utf-8') as fhandle: - fhandle.write('already_applied\nabc123def456\n') + tools.write_file(self.signal_path, + 'already_applied\nabc123def456\n', binary=False) status, commit = agent.read_signal_file(self.test_dir) self.assertEqual(status, 'already_applied') @@ -3067,8 +3065,7 @@ class TestSignalFile(unittest.TestCase): def test_read_signal_file_status_only(self): """Test read_signal_file with only status line.""" - with open(self.signal_path, 'w', encoding='utf-8') as fhandle: - fhandle.write('conflict\n') + tools.write_file(self.signal_path, 'conflict\n', binary=False) status, commit = agent.read_signal_file(self.test_dir) self.assertEqual(status, 'conflict') From patchwork Sun Feb 22 15:42:52 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 1933 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=1771775067; bh=rHVCoCvyRhbwxdAEvCBIpQO64th3Dxh3WZVLxidd/1k=; 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=szhG7P9+ET97V44z+5rpSWbyyOF0N56Rqiiz+FrX/lF6IxPFkbNV39UO1ivkwy48r b3utc/hmjIjtImp5COrq7EZmY8qF30N13SSBI3pdy+rF5hQVwci9lsyDFvBGpZVKYm p6It/mkr7AM5ha17OgkOkmOLTIwcO/5TtAY+CKhNgbXwOu8nqa4I5rFK/8t5hNcgVe +Q+vcnA0OFKuT7q8KF8UkbFhL47JaEUqzk1u2LkfyLdBtRDpNfNmAhwN4VRVHswUy4 g7HER114eVaGpntJVlnXDaNlx7w3Ayb5km59/Ks61vYBEAz7depExGAG1nYk/NVF2x KD+i1tmlEcPSA== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 4BACB69D3F for ; Sun, 22 Feb 2026 08:44: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 10024) with ESMTP id DB7hupU2R3Gf for ; Sun, 22 Feb 2026 08:44:27 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1771775067; bh=rHVCoCvyRhbwxdAEvCBIpQO64th3Dxh3WZVLxidd/1k=; 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=szhG7P9+ET97V44z+5rpSWbyyOF0N56Rqiiz+FrX/lF6IxPFkbNV39UO1ivkwy48r b3utc/hmjIjtImp5COrq7EZmY8qF30N13SSBI3pdy+rF5hQVwci9lsyDFvBGpZVKYm p6It/mkr7AM5ha17OgkOkmOLTIwcO/5TtAY+CKhNgbXwOu8nqa4I5rFK/8t5hNcgVe +Q+vcnA0OFKuT7q8KF8UkbFhL47JaEUqzk1u2LkfyLdBtRDpNfNmAhwN4VRVHswUy4 g7HER114eVaGpntJVlnXDaNlx7w3Ayb5km59/Ks61vYBEAz7depExGAG1nYk/NVF2x KD+i1tmlEcPSA== Received: from mail.u-boot.org (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 3982C69D48 for ; Sun, 22 Feb 2026 08:44:27 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1771775065; bh=n/SvtcIz7K/pSCvSl6SL9dkkYYeSnXP6010kMl+MdUw=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=iwkL5CKOqB6pNVR+nzoXpFTRLvnH+PJ03lGTShqAC83BUyGyreJlpZmo/ddYcRiTJ eyRlQ67gYbMRegHUjBqcobS0t7yCL/TxApD5/6vohC/A7VCTCzOkOPIn9G+PQ8pw1R sZNnqnHOUvPPTsjFZEvr+fRjoJBJRnLNT1nNMAUqqnUHnhjLErclUKIpxIZen+/SF1 EKY2Xbr/PtoddcNRDtFaiNkj2MqB3WKX17Q9BHZNrPQGbHb2rb1A+EoWVDWWkYwH4y 3S01dWby8eQKwtQb8k5Yv2I5hEoTEgu9wtrciDbVpzoJnFvw4FB6cxJ0uF7HjDA5ZO JlMD+C0nekkUQ== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 77A6A69D3F; Sun, 22 Feb 2026 08:44: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 10026) with ESMTP id Hlb2-bMD65vD; Sun, 22 Feb 2026 08:44:25 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1771775061; bh=LygppgSnkrwDKLRu6x0bBENcZ3cLDBFH/REvEXM63Xg=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=lx4ikFfsazgm4v0LihjAvvp+14YH/L6U++q0q/Mvqf7Yoma9Akk4y4A0tPHgr8HMC GDJSIcGvwpjZtsFGeEdVPu3UiJV+WPz3O++0/N/A6xh3NQ+eOXtK6i6NXwQ2meBcou uo9qzTFPWAWSWhlJZAkDfW2iemlP+6/N6cal5sO7q09A/DiTV5IibzRCUwzM7cuxBC /ah/oe5/WUOOG8apVLD0VPhTCAyNkNeihpm0zp1070EAfPcB+Vh6WoQIn3seZ47+J3 MBa5tcIqzft+rRQZ4YMUceXC73tf1JaCsjSorEX1ktevRRH/4tyQ7KDocjCme1/c8u sm+8OKVBB2etw== Received: from u-boot.org (unknown [73.34.74.121]) by mail.u-boot.org (Postfix) with ESMTPSA id EEC8169C5C; Sun, 22 Feb 2026 08:44:20 -0700 (MST) From: Simon Glass To: U-Boot Concept Date: Sun, 22 Feb 2026 08:42:52 -0700 Message-ID: <20260222154303.2851319-13-sjg@u-boot.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20260222154303.2851319-1-sjg@u-boot.org> References: <20260222154303.2851319-1-sjg@u-boot.org> MIME-Version: 1.0 Message-ID-Hash: KJJIVHK4HIGT2T3BSAOWITXPXXOX674H X-Message-ID-Hash: KJJIVHK4HIGT2T3BSAOWITXPXXOX674H 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 12/16] pickman: Add a database table to track pipeline fixes 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 pipeline_fix table (schema v4) for tracking pipeline-fix attempts per MR. Each row records the MR IID, pipeline ID, attempt number, status and timestamp. A UNIQUE constraint on (mr_iid, pipeline_id) ensures each pipeline is only processed once. Add pfix_count(), pfix_add() and pfix_has() accessors. Co-developed-by: Claude Opus 4.6 Signed-off-by: Simon Glass --- tools/pickman/database.py | 65 +++++++++++++++++++++++++++++- tools/pickman/ftest.py | 84 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 148 insertions(+), 1 deletion(-) diff --git a/tools/pickman/database.py b/tools/pickman/database.py index 92bff7a5702..317668a979d 100644 --- a/tools/pickman/database.py +++ b/tools/pickman/database.py @@ -19,7 +19,7 @@ from u_boot_pylib import tools from u_boot_pylib import tout # Schema version (version 0 means there is no database yet) -LATEST = 3 +LATEST = 4 # Default database filename DB_FNAME = '.pickman.db' @@ -141,6 +141,19 @@ class Database: # pylint: disable=too-many-public-methods 'processed_at TEXT, ' 'UNIQUE(mr_iid, comment_id))') + def _create_v4(self): + """Migrate database to v4 schema - add pipeline_fix table""" + # Table for tracking pipeline fix attempts per MR + self.cur.execute( + 'CREATE TABLE pipeline_fix (' + 'id INTEGER PRIMARY KEY AUTOINCREMENT, ' + 'mr_iid INTEGER, ' + 'pipeline_id INTEGER, ' + 'attempt INTEGER, ' + 'status TEXT, ' + 'created_at TEXT, ' + 'UNIQUE(mr_iid, pipeline_id))') + def migrate_to(self, dest_version): """Migrate the database to the selected version @@ -165,6 +178,8 @@ class Database: # pylint: disable=too-many-public-methods self._create_v2() elif version == 3: self._create_v3() + elif version == 4: + self._create_v4() self.cur.execute('DELETE FROM schema_version') self.cur.execute( @@ -481,3 +496,51 @@ class Database: # pylint: disable=too-many-public-methods 'SELECT comment_id FROM comment WHERE mr_iid = ?', (mr_iid,)) return [row[0] for row in res.fetchall()] + + # pipeline_fix functions + + def pfix_count(self, mr_iid): + """Count fix attempts for an MR + + Args: + mr_iid (int): Merge request IID + + Return: + int: Number of fix attempts + """ + res = self.execute( + 'SELECT COUNT(*) FROM pipeline_fix WHERE mr_iid = ?', + (mr_iid,)) + return res.fetchone()[0] + + def pfix_add(self, mr_iid, pipeline_id, attempt, status): + """Record a pipeline fix attempt + + Args: + mr_iid (int): Merge request IID + pipeline_id (int): Pipeline ID + attempt (int): Attempt number + status (str): Status ('success' or 'failure') + """ + self.execute( + 'INSERT OR IGNORE INTO pipeline_fix ' + '(mr_iid, pipeline_id, attempt, status, created_at) ' + 'VALUES (?, ?, ?, ?, ?)', + (mr_iid, pipeline_id, attempt, status, + datetime.now().isoformat())) + + def pfix_has(self, mr_iid, pipeline_id): + """Check if a pipeline has already been handled + + Args: + mr_iid (int): Merge request IID + pipeline_id (int): Pipeline ID + + Return: + bool: True if already handled + """ + res = self.execute( + 'SELECT id FROM pipeline_fix ' + 'WHERE mr_iid = ? AND pipeline_id = ?', + (mr_iid, pipeline_id)) + return res.fetchone() is not None diff --git a/tools/pickman/ftest.py b/tools/pickman/ftest.py index 4a58a9371ce..67a7d004ca6 100644 --- a/tools/pickman/ftest.py +++ b/tools/pickman/ftest.py @@ -818,6 +818,90 @@ class TestDatabaseComment(unittest.TestCase): dbs.close() +class TestDatabasePipelineFix(unittest.TestCase): + """Tests for Database pipeline_fix functions.""" + + def setUp(self): + """Set up test fixtures.""" + fd, self.db_path = tempfile.mkstemp(suffix='.db') + os.close(fd) + os.unlink(self.db_path) + + def tearDown(self): + """Clean up test fixtures.""" + if os.path.exists(self.db_path): + os.unlink(self.db_path) + database.Database.instances.clear() + + def test_pfix_add(self): + """Test adding a pipeline fix record""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + + dbs.pfix_add(123, 456, 1, 'success') + dbs.commit() + + self.assertTrue(dbs.pfix_has(123, 456)) + + dbs.close() + + def test_pfix_count(self): + """Test counting pipeline fix attempts""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + + self.assertEqual(dbs.pfix_count(123), 0) + + dbs.pfix_add(123, 100, 1, 'failure') + dbs.pfix_add(123, 200, 2, 'success') + dbs.commit() + + self.assertEqual(dbs.pfix_count(123), 2) + # Different MR should have 0 + self.assertEqual(dbs.pfix_count(999), 0) + + dbs.close() + + def test_pfix_has(self): + """Test checking if a pipeline was already handled""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + + self.assertFalse(dbs.pfix_has(123, 456)) + + dbs.pfix_add(123, 456, 1, 'success') + dbs.commit() + + self.assertTrue(dbs.pfix_has(123, 456)) + # Different pipeline should not be handled + self.assertFalse(dbs.pfix_has(123, 789)) + # Different MR should not be handled + self.assertFalse(dbs.pfix_has(999, 456)) + + dbs.close() + + def test_pfix_unique(self): + """Test that duplicate mr_iid/pipeline_id pairs are ignored""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + + dbs.pfix_add(123, 456, 1, 'failure') + dbs.commit() + + # Adding same pair again should not raise (OR IGNORE) + dbs.pfix_add(123, 456, 2, 'success') + dbs.commit() + + # Count should still be 1 (second insert ignored) + self.assertEqual(dbs.pfix_count(123), 1) + + dbs.close() + + class TestListSources(unittest.TestCase): """Tests for list-sources command.""" From patchwork Sun Feb 22 15:42:53 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 1934 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=1771775072; bh=TLZmzONQdWAZLI26VtaI13n4c/r8z+m2uoJOwefIpXA=; 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=nTSwNK8rTIaZalBga5pZ+kdFLb0mIYLEGxjEDQMuZdEf4NlX51GvcBWFx90TUj9vn wqHUx08SSnt4WxqrbIcd6/D3BboOmJ5yRM3X35Pdi3lzz2+wIs58CAalLDY0W6Oa5a ZeSQn3kLiVeVaTXlnH14JqnOvz086QsdTX3oxLGVlwwB3Che+djJgwjVDrzhxG4NL+ QzvhnbaWGInZQRVLV2ni7kEMVIDDOzBXQ2xalwucRijumcKOZnCbOedK0342VjfzLp VK1JULKW1CEYGDO0WDeLCLnr3WckOJy39En/0V2je5aqlvC91eD/t6PA1FuCizhXcc wTjrIExCS1D8g== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id A279669D3F for ; Sun, 22 Feb 2026 08:44: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 10024) with ESMTP id I24KDf0aiRlO for ; Sun, 22 Feb 2026 08:44:32 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1771775072; bh=TLZmzONQdWAZLI26VtaI13n4c/r8z+m2uoJOwefIpXA=; 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=nTSwNK8rTIaZalBga5pZ+kdFLb0mIYLEGxjEDQMuZdEf4NlX51GvcBWFx90TUj9vn wqHUx08SSnt4WxqrbIcd6/D3BboOmJ5yRM3X35Pdi3lzz2+wIs58CAalLDY0W6Oa5a ZeSQn3kLiVeVaTXlnH14JqnOvz086QsdTX3oxLGVlwwB3Che+djJgwjVDrzhxG4NL+ QzvhnbaWGInZQRVLV2ni7kEMVIDDOzBXQ2xalwucRijumcKOZnCbOedK0342VjfzLp VK1JULKW1CEYGDO0WDeLCLnr3WckOJy39En/0V2je5aqlvC91eD/t6PA1FuCizhXcc wTjrIExCS1D8g== Received: from mail.u-boot.org (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 8BFFA69D48 for ; Sun, 22 Feb 2026 08:44:32 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1771775070; bh=pi/IppJZBKTcUyb15iAkRt9hDQG1FS/x63mUYqfIzmI=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=lW6VnosRcT3/VNVMNYVm5Z7tu40qYHKNR0pyagBTWlMOGyBJdP9afEOun1heFUogt QGQT/6xfkP+Mxc/ImbS34kVtJUYQ58SZed9jyd9lFk/qNBrJxrLLIqWKcBqUxb73ed H+YzJDbyc8R7o/ifQmh10S+smcjPTtiwUMeIr4r/QTukrHeWN9oJ68OnyI0SBQtgrF csY2eYPuv79lgsBk2qAGFwiLL8J9gE0Vl9V/kbKK1SLONSXaonDjZvCbMb8Qbn++tM WEXC8Hc4Zhmi4yIK9f+epABnyZHZG5SJIy10w1D5owJrt+iOfIxu9mWf0cbd3mIOlX 09MYk58+zXxMA== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 1297C69D3F; Sun, 22 Feb 2026 08:44: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 10026) with ESMTP id 5j7z1YeyLVWf; Sun, 22 Feb 2026 08:44:29 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1771775066; bh=ulOHxx/hYtAw46cfcYI6LcXFJFrVnNLam2DClBxShfk=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=cmKPlqa2spEH3DkjpIGWkxRd+6JfF/s4M2anfubpE11cjv/l0LukaAEWEWr+b3qS5 HVElDGcXMiiB++PX96cBr7XWBitxnfh/ssDGMgGFmImdZPoPqorw816542vX6cBUFE Um/6CzZAxM8LCVK3uaL99o4WeIK/aSds1MpO7bsSF225oZEnrRZr5uFRfsrvO/ATI4 atDCGuGJYYPadd0cAlEI1/dT+Yo/67uNxX8YfORVI9agYsrMkf0fjZZ6C+BbZGkBlC RqFJVCJ+lynIECX9R5hswXwlg2yhM9kWHW7nbaWt3ed8uj+Qurt5UxMZ0LknYj1ARQ OtB3roCSlBTeQ== Received: from u-boot.org (unknown [73.34.74.121]) by mail.u-boot.org (Postfix) with ESMTPSA id B4B5469C5C; Sun, 22 Feb 2026 08:44:25 -0700 (MST) From: Simon Glass To: U-Boot Concept Date: Sun, 22 Feb 2026 08:42:53 -0700 Message-ID: <20260222154303.2851319-14-sjg@u-boot.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20260222154303.2851319-1-sjg@u-boot.org> References: <20260222154303.2851319-1-sjg@u-boot.org> MIME-Version: 1.0 Message-ID-Hash: BVVXBF4RTZ64MTBMEL7KKUHPWGVFHCV5 X-Message-ID-Hash: BVVXBF4RTZ64MTBMEL7KKUHPWGVFHCV5 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 13/16] pickman: Add pipeline helpers to gitlab_api 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 PickmanMr pipeline_status/pipeline_id fields (extracted from the head_pipeline attribute), PipelineInfo and FailedJob named tuples, and get_failed_jobs() which fetches the trace logs of failed jobs from a GitLab pipeline. Co-developed-by: Claude Opus 4.6 Signed-off-by: Simon Glass --- tools/pickman/gitlab_api.py | 77 ++++++++++++++++++++++++++++++++++++- 1 file changed, 75 insertions(+), 2 deletions(-) diff --git a/tools/pickman/gitlab_api.py b/tools/pickman/gitlab_api.py index 9262ac15a0d..d9635d392ee 100644 --- a/tools/pickman/gitlab_api.py +++ b/tools/pickman/gitlab_api.py @@ -42,14 +42,21 @@ class MrCreateError(Exception): # Use defaults for new fields so existing code doesn't break PickmanMr = namedtuple('PickmanMr', [ 'iid', 'title', 'web_url', 'source_branch', 'description', - 'has_conflicts', 'needs_rebase' -], defaults=[False, False]) + 'has_conflicts', 'needs_rebase', 'pipeline_status', 'pipeline_id' +], defaults=[False, False, None, None]) # Comment info returned by get_mr_comments() MrComment = namedtuple('MrComment', [ 'id', 'author', 'body', 'created_at', 'resolvable', 'resolved' ]) +# Pipeline info +PipelineInfo = namedtuple('PipelineInfo', ['id', 'status', 'web_url']) + +# Failed job info from a pipeline +FailedJob = namedtuple('FailedJob', + ['id', 'name', 'stage', 'web_url', 'log_tail']) + def check_available(): """Check if the python-gitlab module is available @@ -320,6 +327,9 @@ def get_pickman_mrs(remote, state='opened'): # For open MRs, fetch full details since list() doesn't # include accurate merge status fields + pipeline_status = None + pipeline_id = None + if state == 'opened': full_mr = project.mergerequests.get(merge_req.iid) has_conflicts = getattr(full_mr, 'has_conflicts', False) @@ -333,6 +343,12 @@ def get_pickman_mrs(remote, state='opened'): diverged = getattr(full_mr, 'diverged_commits_count', 0) needs_rebase = diverged and diverged > 0 + # Extract pipeline info from head_pipeline + head_pipeline = getattr(full_mr, 'head_pipeline', None) + if head_pipeline: + pipeline_status = head_pipeline.get('status') + pipeline_id = head_pipeline.get('id') + pickman_mrs.append(PickmanMr( iid=merge_req.iid, title=merge_req.title, @@ -341,6 +357,8 @@ def get_pickman_mrs(remote, state='opened'): description=merge_req.description or '', has_conflicts=has_conflicts, needs_rebase=needs_rebase, + pipeline_status=pipeline_status, + pipeline_id=pipeline_id, )) return pickman_mrs except gitlab.exceptions.GitlabError as exc: @@ -423,6 +441,61 @@ def get_mr_comments(remote, mr_iid): return None +def get_failed_jobs(remote, pipeline_id, max_log_lines=200): + """Get failed jobs from a pipeline + + Args: + remote (str): Remote name + pipeline_id (int): Pipeline ID + max_log_lines (int): Maximum log lines to fetch per job + + Returns: + list: List of FailedJob tuples, or None on failure + """ + if not check_available(): + return None + + token = get_token() + if not token: + tout.error('GITLAB_TOKEN environment variable not set') + return None + + remote_url = get_remote_url(remote) + host, proj_path = parse_url(remote_url) + + if not host or not proj_path: + return None + + try: + glab = gitlab.Gitlab(f'https://{host}', private_token=token) + project = glab.projects.get(proj_path) + pipeline = project.pipelines.get(pipeline_id) + jobs = pipeline.jobs.list(scope='failed', get_all=True) + + failed_jobs = [] + for job in jobs: + # Fetch full job to get trace + full_job = project.jobs.get(job.id) + try: + trace = full_job.trace().decode('utf-8', errors='replace') + lines = trace.splitlines() + log_tail = '\n'.join(lines[-max_log_lines:]) + except (AttributeError, gitlab.exceptions.GitlabError): + log_tail = '' + + failed_jobs.append(FailedJob( + id=job.id, + name=job.name, + stage=job.stage, + web_url=job.web_url, + log_tail=log_tail, + )) + return failed_jobs + except gitlab.exceptions.GitlabError as exc: + tout.error(f'GitLab API error: {exc}') + return None + + def reply_to_mr(remote, mr_iid, message): """Post a reply to a merge request From patchwork Sun Feb 22 15:42:54 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 1935 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=1771775077; bh=GdSk6AZVJ1fwlKBR899vy9NHo1EnoO0ekJfZk/12fag=; 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=q4aRSTkuR15mMv8Cq9ncvF8kAjD6ke88L1/h49DuTj9aiRJxH9HmI21hSt7LYKoVj ikZ1e8e5QiBqGqOtX3qJ30VImrE7WewuU6afUPoVPZicYsb4sUwmgQ5+XDfwSXH9kj 4XvqESrG7BRbbFp//m6n4UQXSlReZ8apmt+U8Wo7m3j86VTJty16dKAINAEsTf4TMm lvF2mfXLIiuQtGhIhnHRE1WyeGcKIou6EepY58jI8C/U05ijS4XAX3SGXTxBjq5Tiv IBzvA4BO0yrVAknMyL7RiHOiyDONH0wWud3KNCK8PajvENUtok5P5/aBXwscsIexJY V1v+sPa71yEoA== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 63A0C69D59 for ; Sun, 22 Feb 2026 08:44:37 -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 KOLO9GqN91sO for ; Sun, 22 Feb 2026 08:44:37 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1771775077; bh=GdSk6AZVJ1fwlKBR899vy9NHo1EnoO0ekJfZk/12fag=; 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=q4aRSTkuR15mMv8Cq9ncvF8kAjD6ke88L1/h49DuTj9aiRJxH9HmI21hSt7LYKoVj ikZ1e8e5QiBqGqOtX3qJ30VImrE7WewuU6afUPoVPZicYsb4sUwmgQ5+XDfwSXH9kj 4XvqESrG7BRbbFp//m6n4UQXSlReZ8apmt+U8Wo7m3j86VTJty16dKAINAEsTf4TMm lvF2mfXLIiuQtGhIhnHRE1WyeGcKIou6EepY58jI8C/U05ijS4XAX3SGXTxBjq5Tiv IBzvA4BO0yrVAknMyL7RiHOiyDONH0wWud3KNCK8PajvENUtok5P5/aBXwscsIexJY V1v+sPa71yEoA== Received: from mail.u-boot.org (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 4F22469D3F for ; Sun, 22 Feb 2026 08:44:37 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1771775075; bh=X6ONWxOljWr/okMmwoxW+EXS5qTse5Yu5Z4aybPJN38=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=SAp/GfefZlTqYR+H56ri/SDBb+6wHUSxcGULbWv9VKt6AKFTTbp8t2VDTplI88+E3 F3BznpFSY34pPg1K+m8yl/A8wiumAXLHGLqYRWmS/5fXmdNKGXF8Ff/mh/ZfAKTcX4 ylgxAvtX0mh+o7GvrbsFSfh0uC5uslQs3xT+u4M0D45n68eVbPErc2uqrChXAGdTac M+pJn17Yr3TqIlFCyjZ5joaIBk2BjxKbrXnqXvTysOiAB47ZZoCzsLgKhzwOGY9LCT JyjGETYmBjZC7R4lnOFZo3SAhnM62l0pWqBhqeugtEhgG+4DR6SRSrPj+5wJraItUi mNQl/wmGnnIDg== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 5A44769C5E; Sun, 22 Feb 2026 08:44:35 -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 1CzMJ-5zzLKp; Sun, 22 Feb 2026 08:44:35 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1771775070; bh=zoNueU9q5PuGF4M+YYELAVnpcY05MlcCWY+zAjbTNI8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=r7iPOtnCKUuCwZvrK7C0QqwQ04WPNRBEHKAO1ijJx8oUGgcYtZ4/mpxLNLQNm07Xg AN+bWv/WZL6NYiKbRoEHHXseplpcAuWqdvB26b3OdnWt+MVllT/ze/5rqjH4/Fmoyi vGc15K1gpie54Vz7qg3Fa5A8aLeWdWEPLHRLUmxidU/tiK3rW0m7RdT8hXAhSmoaNW 8KXyAjjZqLNDrtn7Ev6fp8v930j5VeSK/sBZmjrNavus69Hdr2PRcg+Er08VVBRVe7 LFRLfUZT9CnfVbUwNjdjl59pw3AQo0vqI41wGqQ49+gA9F2wnMGtDzOyMk7O3FhXqi mf/q9ojlaqyPg== Received: from u-boot.org (unknown [73.34.74.121]) by mail.u-boot.org (Postfix) with ESMTPSA id 51CEC69C5C; Sun, 22 Feb 2026 08:44:30 -0700 (MST) From: Simon Glass To: U-Boot Concept Date: Sun, 22 Feb 2026 08:42:54 -0700 Message-ID: <20260222154303.2851319-15-sjg@u-boot.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20260222154303.2851319-1-sjg@u-boot.org> References: <20260222154303.2851319-1-sjg@u-boot.org> MIME-Version: 1.0 Message-ID-Hash: P7BCBKEMANJZWZTMU7ZRZLKIW5YCHWJV X-Message-ID-Hash: P7BCBKEMANJZWZTMU7ZRZLKIW5YCHWJV 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 14/16] pickman: Support automatically fixing pipeline failures 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 After pickman pushes cherry-pick MRs to GitLab, CI pipelines sometimes fail due to build or test errors introduced by the cherry-picks. This currently requires manual intervention. Add a pipeline-fix feature that detects failed pipelines on open MRs and uses a Claude agent to diagnose and fix them. The agent analyses the failed job logs, identifies the responsible commit, amends it via interactive rebase (using uman's rf/rn helpers), verifies the fix with um build and buildman, then leaves the result on a local branch for the caller to push. The feature integrates into the existing do_step()/do_poll() flow, running after review comment processing. A --fix-retries/-F flag (default 3, 0 to disable) controls the maximum attempts per MR. Each attempt is tracked per pipeline ID in the pipeline_fix table so the same failure is never reprocessed, and a new pipeline from a rebase is treated independently. On success, pickman pushes the fix branch, posts an MR comment summarising what was fixed, updates the MR description with the full agent log, and records the fix in .pickman-history. On failure or when the retry limit is reached, a comment is posted requesting manual intervention. Co-developed-by: Claude Opus 4.6 Signed-off-by: Simon Glass --- tools/pickman/README.rst | 48 ++++ tools/pickman/__main__.py | 6 + tools/pickman/agent.py | 177 +++++++++++++ tools/pickman/control.py | 247 ++++++++++++++++++ tools/pickman/ftest.py | 536 +++++++++++++++++++++++++++++++++++++- 5 files changed, 1011 insertions(+), 3 deletions(-) diff --git a/tools/pickman/README.rst b/tools/pickman/README.rst index 70309032838..0c9d2d1521b 100644 --- a/tools/pickman/README.rst +++ b/tools/pickman/README.rst @@ -212,6 +212,39 @@ This ensures: - No manual intervention is required to continue - False positives are minimized by comparing actual patch content +Pipeline Fix +------------ + +When a CI pipeline fails on a pickman MR, the ``step`` and ``poll`` commands +can automatically diagnose and fix the failure using a Claude agent. This is +useful when cherry-picks introduce build or test failures that need minor +adjustments. + +**How it works** + +During each step, after processing review comments, pickman checks active MRs +for failed pipelines. For each failed pipeline: + +1. Pickman fetches the failed job logs from GitLab +2. A Claude agent analyses the logs, diagnoses the root cause, and makes + targeted fixes +3. The fix is pushed to the MR branch, triggering a new pipeline +4. The attempt is recorded in the database to avoid reprocessing + +**Retry behaviour** + +Each MR gets up to ``--fix-retries`` attempts (default: 3). If the limit is +reached, pickman posts a comment on the MR indicating that manual intervention +is required. Set ``--fix-retries 0`` to disable automatic pipeline fixing. + +Each attempt is tracked per pipeline ID, so a new pipeline triggered by a rebase +or comment fix is treated independently. + +**Options** + +- ``-F, --fix-retries``: Maximum pipeline-fix attempts per MR (default: 3, 0 to + disable). Available on both ``step`` and ``poll`` commands. + CI Pipelines ------------ @@ -448,6 +481,7 @@ review comments are handled automatically. Options for the step command: +- ``-F, --fix-retries``: Max pipeline-fix attempts per MR (default: 3, 0 to disable) - ``-m, --max-mrs``: Maximum open MRs allowed (default: 5) - ``-r, --remote``: Git remote for push (default: ci) - ``-t, --target``: Target branch for MR (default: master) @@ -461,6 +495,7 @@ creating new MRs as previous ones are merged. Press Ctrl+C to stop. Options for the poll command: +- ``-F, --fix-retries``: Max pipeline-fix attempts per MR (default: 3, 0 to disable) - ``-i, --interval``: Interval between steps in seconds (default: 300) - ``-m, --max-mrs``: Maximum open MRs allowed (default: 5) - ``-r, --remote``: Git remote for push (default: ci) @@ -563,6 +598,19 @@ Tables This table prevents the same comment from being addressed multiple times when running ``review`` or ``poll`` commands. +**pipeline_fix** + Tracks pipeline fix attempts per MR to avoid reprocessing. + + - ``id``: Primary key + - ``mr_iid``: GitLab merge request IID + - ``pipeline_id``: GitLab pipeline ID + - ``attempt``: Attempt number + - ``status``: Result ('success', 'failure', 'skipped', 'no_jobs') + - ``created_at``: Timestamp when the attempt was made + + The ``(mr_iid, pipeline_id)`` pair is unique, so each pipeline is only + processed once. + Configuration ------------- diff --git a/tools/pickman/__main__.py b/tools/pickman/__main__.py index 7814fd0fedc..1e13646ec52 100755 --- a/tools/pickman/__main__.py +++ b/tools/pickman/__main__.py @@ -113,6 +113,9 @@ def add_main_commands(subparsers): step_cmd = subparsers.add_parser('step', help='Create MR if none pending') step_cmd.add_argument('source', help='Source branch name') + step_cmd.add_argument('-F', '--fix-retries', type=int, default=3, + help='Max pipeline-fix attempts per MR ' + '(0 to disable, default: 3)') step_cmd.add_argument('-m', '--max-mrs', type=int, default=5, help='Max open MRs allowed (default: 5)') step_cmd.add_argument('-r', '--remote', default='ci', @@ -123,6 +126,9 @@ def add_main_commands(subparsers): poll_cmd = subparsers.add_parser('poll', help='Run step repeatedly until stopped') poll_cmd.add_argument('source', help='Source branch name') + poll_cmd.add_argument('-F', '--fix-retries', type=int, default=3, + help='Max pipeline-fix attempts per MR ' + '(0 to disable, default: 3)') poll_cmd.add_argument('-i', '--interval', type=int, default=300, help='Interval between steps in seconds ' '(default: 300)') diff --git a/tools/pickman/agent.py b/tools/pickman/agent.py index 4b914e314e8..ec5c1b0df75 100644 --- a/tools/pickman/agent.py +++ b/tools/pickman/agent.py @@ -536,3 +536,180 @@ def handle_mr_comments(mr_iid, branch_name, comments, remote, target='master', return asyncio.run(run_review_agent(mr_iid, branch_name, comments, remote, target, needs_rebase, has_conflicts, mr_description, repo_path)) + + +# pylint: disable=too-many-arguments +def build_pipeline_fix_prompt(mr_iid, branch_name, failed_jobs, remote, + target, mr_description, attempt): + """Build prompt and task description for the pipeline fix agent + + Args: + mr_iid (int): Merge request IID + branch_name (str): Source branch name + failed_jobs (list): List of FailedJob tuples + remote (str): Git remote name + target (str): Target branch + mr_description (str): MR description with context + attempt (int): Fix attempt number + + Returns: + tuple: (prompt, task_desc) where prompt is the full agent prompt and + task_desc is a short description + """ + task_desc = f'fix {len(failed_jobs)} failed pipeline job(s) (attempt {attempt})' + + # Format failed jobs + job_sections = [] + for job in failed_jobs: + job_sections.append( + f'### Job: {job.name} (stage: {job.stage})\n' + f'URL: {job.web_url}\n' + f'Log tail:\n```\n{job.log_tail}\n```' + ) + jobs_text = '\n\n'.join(job_sections) + + # Include MR description for context + context_section = '' + if mr_description: + context_section = f''' +Context from MR description: + +{mr_description} +''' + + # Extract board names from failed job names for targeted builds. + # CI job names typically contain a board name (e.g. 'build:sandbox', + # 'test:venice_gw7905', 'world build '). Collect unique names + # to pass to buildman so the agent can verify all affected boards. + board_names = set() + for job in failed_jobs: + # Try common CI patterns: 'build:', 'test:', + # or a board name token in the job name + for part in job.name.replace(':', ' ').replace('/', ' ').split(): + # Skip generic tokens that are not board names + if part.lower() in ('build', 'test', 'world', 'check', 'lint', + 'ci', 'job', 'stage'): + continue + board_names.add(part) + + # Always include sandbox for a basic sanity check + board_names.add('sandbox') + boards_csv = ','.join(sorted(board_names)) + + prompt = f"""Fix pipeline failures for merge request !{mr_iid} \ +(branch: {branch_name}, attempt {attempt}). +{context_section} +Failed jobs: + +{jobs_text} + +Steps to follow: +1. Checkout the branch: git checkout {branch_name} +2. Diagnose the root cause from the job logs above +3. Identify which commit introduced the problem: + - Use 'git log --oneline' to list the commits on the branch + - Correlate the failing file/symbol with the commit that touched it + - Use 'git log --oneline -- ' if needed +4. Apply the fix to the appropriate commit: + - If you can identify the responsible commit, use uman's rebase helpers + to amend it: + a) 'rf N' to start an interactive rebase going back N commits from + HEAD, stopping at the oldest (first) commit in the range + b) Make your fix, then amend the commit with a 1-2 line note appended + to the end of the commit message describing the fix, e.g.: + git add + git commit --amend -m "$(git log -1 --format=%B) + + [pickman] Fix " + c) 'rn' to advance to the next commit (or 'git rebase --continue' + to finish) + - If the cause spans multiple commits or cannot be pinpointed, add a new + fixup commit on top of the branch +5. Build and verify: + a) Quick sandbox check: um build sandbox + b) Build all affected boards: \ +buildman -o /tmp/pickman {boards_csv} + Fix any build errors before proceeding. +6. Create a local branch: {branch_name}-fix{attempt} +7. Report what was fixed: which commit was responsible, what the root cause + was, and what change was made. Do NOT push the branch; the caller + handles that. + +Important: +- Keep changes minimal and focused on fixing the failures +- Prefer amending the responsible commit over adding a new commit, so the + MR history stays clean +- If the failure is an infrastructure or transient issue (network timeout, \ +runner problem, etc.), report this without making changes +- Do not modify unrelated code +- Use 'um build sandbox' for sandbox builds (fast, local) +- Use 'buildman -o /tmp/pickman ...' to build multiple + boards in one go +- Leave the result on local branch {branch_name}-fix{attempt} +""" + + return prompt, task_desc + + +async def run_pipeline_fix_agent(mr_iid, branch_name, failed_jobs, remote, + target='master', mr_description='', + attempt=1, repo_path=None): + """Run the Claude agent to fix pipeline failures + + Args: + mr_iid (int): Merge request IID + branch_name (str): Source branch name + failed_jobs (list): List of FailedJob tuples + remote (str): Git remote name + target (str): Target branch + mr_description (str): MR description with context + attempt (int): Fix attempt number + 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, task_desc = build_pipeline_fix_prompt( + mr_iid, branch_name, failed_jobs, remote, target, + mr_description, attempt) + + 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}...') + tout.info('') + + return await run_agent_collect(prompt, options) + + +def fix_pipeline(mr_iid, branch_name, failed_jobs, remote, target='master', + mr_description='', attempt=1, repo_path=None): + """Synchronous wrapper for running the pipeline fix agent + + Args: + mr_iid (int): Merge request IID + branch_name (str): Source branch name + failed_jobs (list): List of FailedJob tuples + remote (str): Git remote name + target (str): Target branch + mr_description (str): MR description with context + attempt (int): Fix attempt number + 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_pipeline_fix_agent( + mr_iid, branch_name, failed_jobs, remote, target, + mr_description, attempt, repo_path)) diff --git a/tools/pickman/control.py b/tools/pickman/control.py index 057e4400adb..f4d4a43c292 100644 --- a/tools/pickman/control.py +++ b/tools/pickman/control.py @@ -2398,6 +2398,248 @@ def process_mr_reviews(remote, mrs, dbs, target='master'): return processed +def _rebase_mr_branch(remote, merge_req, dbs, target): + """Rebase an MR branch onto the target before attempting a pipeline fix + + When a branch needs rebasing, the pipeline failure may be caused by the + stale base rather than by the cherry-picked commits. Rebasing and pushing + triggers a fresh pipeline run. + + Args: + remote (str): Remote name + merge_req (PickmanMr): MR with a failed pipeline + dbs (Database): Database instance for tracking fix attempts + target (str): Target branch + + Returns: + True if the branch was rebased and pushed, False if the rebase + failed (conflicts), or None if no rebase is needed + """ + if not merge_req.needs_rebase and not merge_req.has_conflicts: + return None + + mr_iid = merge_req.iid + branch = merge_req.source_branch + if merge_req.has_conflicts: + tout.info(f'MR !{mr_iid}: has conflicts, rebasing before ' + f'pipeline fix...') + else: + tout.info(f'MR !{mr_iid}: needs rebase, rebasing before ' + f'pipeline fix...') + run_git(['checkout', branch]) + try: + run_git(['rebase', f'{remote}/{target}']) + except command.CommandExc: + tout.warning(f'MR !{mr_iid}: rebase failed, aborting') + try: + run_git(['rebase', '--abort']) + except command.CommandExc: + pass + return False + gitlab_api.push_branch(remote, branch, force=True, skip_ci=False) + dbs.pfix_add(mr_iid, merge_req.pipeline_id, 0, 'rebased') + dbs.commit() + tout.info(f'MR !{mr_iid}: rebased and pushed, waiting for ' + f'new pipeline') + return True + + +def _attempt_pipeline_fix(remote, merge_req, dbs, target, attempt): + """Run the agent to fix a failed pipeline and report the result + + Fetches the failed-job logs, invokes the fix agent, then pushes the + result and updates the MR description and history on success, or posts + a failure comment otherwise. + + Args: + remote (str): Remote name + merge_req (PickmanMr): MR with a failed pipeline + dbs (Database): Database instance for tracking fix attempts + target (str): Target branch + attempt (int): Current fix attempt number + + Returns: + bool: True if the fix was attempted, False if no failed jobs + were found + """ + mr_iid = merge_req.iid + + # Fetch failed jobs + failed_jobs = gitlab_api.get_failed_jobs(remote, merge_req.pipeline_id) + if not failed_jobs: + tout.info(f'MR !{mr_iid}: no failed jobs found') + dbs.pfix_add(mr_iid, merge_req.pipeline_id, attempt, 'no_jobs') + dbs.commit() + return False + + # Run agent to fix the failures + success, conversation_log = agent.fix_pipeline( + mr_iid, + merge_req.source_branch, + failed_jobs, + remote, + target, + mr_description=merge_req.description, + attempt=attempt, + ) + + status = 'success' if success else 'failure' + dbs.pfix_add(mr_iid, merge_req.pipeline_id, attempt, status) + dbs.commit() + + if success: + # Push the fix branch to the original MR branch + branch = merge_req.source_branch + gitlab_api.push_branch(remote, branch, force=True, + skip_ci=False) + + # Update MR description with fix log + old_desc = merge_req.description + job_names = ', '.join(j.name for j in failed_jobs) + new_desc = (f"{old_desc}\n\n### Pipeline fix (attempt {attempt})" + f"\n\n**Failed jobs:** {job_names}\n\n" + f"**Response:**\n{conversation_log}") + gitlab_api.update_mr_desc(remote, mr_iid, new_desc) + + # Post a comment summarising the fix + gitlab_api.reply_to_mr( + remote, mr_iid, + f'Pipeline fix (attempt {attempt}): ' + f'fixed failed job(s) {job_names}.\n\n' + f'{conversation_log[:2000]}') + + # Update .pickman-history + update_history_pipeline_fix(merge_req.source_branch, failed_jobs, + conversation_log, attempt) + + tout.info(f'MR !{mr_iid}: pipeline fix pushed (attempt {attempt})') + else: + gitlab_api.reply_to_mr( + remote, mr_iid, + f'Pipeline fix attempt {attempt} failed. ' + f'Agent output:\n\n{conversation_log[:1000]}') + tout.error(f'MR !{mr_iid}: pipeline fix failed ' + f'(attempt {attempt})') + + return True + + +def process_pipeline_failures(remote, mrs, dbs, target, max_retries): + """Process pipeline failures on open MRs + + Checks each MR for failed pipelines and uses Claude agent to diagnose + and fix them. Tracks attempts in the database to avoid reprocessing. + + Args: + remote (str): Remote name + mrs (list): List of active (non-skipped) PickmanMr tuples + dbs (Database): Database instance for tracking fix attempts + target (str): Target branch + max_retries (int): Maximum fix attempts per MR + + Returns: + int: Number of MRs with pipeline fixes attempted + """ + # Save current branch to restore later + original_branch = run_git(['rev-parse', '--abbrev-ref', 'HEAD']) + + # Fetch to get latest remote state + tout.info(f'Fetching {remote}...') + run_git(['fetch', remote]) + + processed = 0 + for merge_req in mrs: + mr_iid = merge_req.iid + + # Skip if pipeline is not failed or has no pipeline + if merge_req.pipeline_status != 'failed': + continue + if merge_req.pipeline_id is None: + continue + + # Skip if this pipeline was already handled + if dbs.pfix_has(mr_iid, merge_req.pipeline_id): + continue + + rebased = _rebase_mr_branch(remote, merge_req, dbs, target) + if rebased is not None: + if rebased: + processed += 1 + continue + + attempt = dbs.pfix_count(mr_iid) + 1 + + # Check retry limit + if attempt > max_retries: + tout.info(f'MR !{mr_iid}: reached fix retry limit ' + f'({max_retries}), skipping') + gitlab_api.reply_to_mr( + remote, mr_iid, + f'Pipeline fix: reached retry limit ({max_retries} ' + f'attempts). Manual intervention required.') + dbs.pfix_add(mr_iid, merge_req.pipeline_id, attempt, 'skipped') + dbs.commit() + continue + + tout.info('') + tout.info(f'MR !{mr_iid}: pipeline {merge_req.pipeline_id} failed, ' + f'attempting fix (attempt {attempt}/{max_retries})...') + + if _attempt_pipeline_fix(remote, merge_req, dbs, target, attempt): + processed += 1 + + # Restore original branch + if processed: + tout.info(f'Returning to {original_branch}') + run_git(['checkout', original_branch]) + + return processed + + +def update_history_pipeline_fix(branch_name, failed_jobs, conversation_log, + attempt): + """Append pipeline fix handling to .pickman-history + + Args: + branch_name (str): Branch name for the MR + failed_jobs (list): List of FailedJob tuples that were fixed + conversation_log (str): Agent conversation log + attempt (int): Fix attempt number + """ + job_summary = '\n'.join( + f'- {j.name} ({j.stage})' + for j in failed_jobs + ) + + entry = f'''### Pipeline fix: {date.today()} (attempt {attempt}) + +Branch: {branch_name} + +Failed jobs: +{job_summary} + +### Conversation log +{conversation_log} + +--- + +''' + + # Append to history file + existing = '' + if os.path.exists(HISTORY_FILE): + with open(HISTORY_FILE, 'r', encoding='utf-8') as fhandle: + existing = fhandle.read() + + with open(HISTORY_FILE, 'w', encoding='utf-8') as fhandle: + fhandle.write(existing + entry) + + # Commit the history file + run_git(['add', '-f', HISTORY_FILE]) + run_git(['commit', '-m', + f'pickman: Record pipeline fix for {branch_name}']) + + def update_history(branch_name, comments, conversation_log): """Append review handling to .pickman-history @@ -2604,6 +2846,11 @@ def do_step(args, dbs): # in case they have an unskip request) process_mr_reviews(remote, mrs, dbs, args.target) + # Process pipeline failures on active MRs only + if active_mrs and args.fix_retries > 0: + process_pipeline_failures(remote, active_mrs, dbs, + args.target, args.fix_retries) + # Only block new MR creation if we've reached the max allowed open MRs max_mrs = args.max_mrs if len(active_mrs) >= max_mrs: diff --git a/tools/pickman/ftest.py b/tools/pickman/ftest.py index 67a7d004ca6..af26cbb4229 100644 --- a/tools/pickman/ftest.py +++ b/tools/pickman/ftest.py @@ -2068,7 +2068,7 @@ class TestStep(unittest.TestCase): return_value=[mock_mr]): args = argparse.Namespace(cmd='step', source='us/next', remote='ci', target='master', - max_mrs=1) + max_mrs=1, fix_retries=3) with terminal.capture(): ret = control.do_step(args, None) @@ -2118,7 +2118,7 @@ class TestStep(unittest.TestCase): return_value=0) as mock_apply: args = argparse.Namespace(cmd='step', source='us/next', remote='ci', target='master', - max_mrs=5) + max_mrs=5, fix_retries=3) with terminal.capture(): ret = control.do_step(args, None) @@ -2146,7 +2146,7 @@ class TestStep(unittest.TestCase): with mock.patch.object(control, 'do_apply') as mock_apply: args = argparse.Namespace(cmd='step', source='us/next', remote='ci', target='master', - max_mrs=3) + max_mrs=3, fix_retries=3) with terminal.capture(): ret = control.do_step(args, None) @@ -5636,5 +5636,535 @@ class TestDoPick(unittest.TestCase): dbs.close() +class TestPickmanMrPipelineFields(unittest.TestCase): + """Tests for PickmanMr pipeline fields.""" + + def test_defaults_none(self): + """Test that pipeline fields default to None""" + pmr = gitlab.PickmanMr( + iid=1, + title='[pickman] Test', + web_url='https://example.com/mr/1', + source_branch='cherry-test', + description='Test', + ) + self.assertIsNone(pmr.pipeline_status) + self.assertIsNone(pmr.pipeline_id) + + def test_with_pipeline(self): + """Test creating PickmanMr with pipeline fields""" + pmr = gitlab.PickmanMr( + iid=1, + title='[pickman] Test', + web_url='https://example.com/mr/1', + source_branch='cherry-test', + description='Test', + pipeline_status='failed', + pipeline_id=42, + ) + self.assertEqual(pmr.pipeline_status, 'failed') + self.assertEqual(pmr.pipeline_id, 42) + + +class TestGetFailedJobs(unittest.TestCase): + """Tests for get_failed_jobs function.""" + + def _make_mock_job(self, job_id, name, stage, web_url, trace_bytes): + """Helper to create a mock job object""" + job = mock.MagicMock() + job.id = job_id + job.name = name + job.stage = stage + job.web_url = web_url + return job + + @mock.patch.object(gitlab, 'get_remote_url', + return_value=TEST_SSH_URL) + @mock.patch.object(gitlab, 'get_token', return_value='test-token') + @mock.patch.object(gitlab, 'AVAILABLE', True) + def test_success(self, _mock_token, _mock_url): + """Test successful retrieval of failed jobs""" + mock_job = self._make_mock_job( + 1, 'build:sandbox', 'build', 'https://gitlab.com/job/1', + b'line1\nline2\nerror: build failed\n') + + mock_full_job = mock.MagicMock() + mock_full_job.trace.return_value = b'line1\nline2\nerror: build failed\n' + + mock_pipeline = mock.MagicMock() + mock_pipeline.jobs.list.return_value = [mock_job] + + mock_project = mock.MagicMock() + mock_project.pipelines.get.return_value = mock_pipeline + mock_project.jobs.get.return_value = mock_full_job + + mock_glab = mock.MagicMock() + mock_glab.projects.get.return_value = mock_project + + with mock.patch('gitlab.Gitlab', return_value=mock_glab): + with terminal.capture(): + result = gitlab.get_failed_jobs('ci', 100) + + self.assertIsNotNone(result) + self.assertEqual(len(result), 1) + self.assertEqual(result[0].name, 'build:sandbox') + self.assertEqual(result[0].stage, 'build') + self.assertIn('error: build failed', result[0].log_tail) + + @mock.patch.object(gitlab, 'get_remote_url', + return_value=TEST_SSH_URL) + @mock.patch.object(gitlab, 'get_token', return_value='test-token') + @mock.patch.object(gitlab, 'AVAILABLE', True) + def test_empty(self, _mock_token, _mock_url): + """Test when no failed jobs exist""" + mock_pipeline = mock.MagicMock() + mock_pipeline.jobs.list.return_value = [] + + mock_project = mock.MagicMock() + mock_project.pipelines.get.return_value = mock_pipeline + + mock_glab = mock.MagicMock() + mock_glab.projects.get.return_value = mock_project + + with mock.patch('gitlab.Gitlab', return_value=mock_glab): + with terminal.capture(): + result = gitlab.get_failed_jobs('ci', 100) + + self.assertIsNotNone(result) + self.assertEqual(len(result), 0) + + @mock.patch.object(gitlab, 'get_remote_url', + return_value=TEST_SSH_URL) + @mock.patch.object(gitlab, 'get_token', return_value='test-token') + @mock.patch.object(gitlab, 'AVAILABLE', True) + def test_log_truncation(self, _mock_token, _mock_url): + """Test that log output is truncated to max_log_lines""" + # Create a trace with 500 lines + trace_lines = [f'line {i}' for i in range(500)] + trace_bytes = '\n'.join(trace_lines).encode() + + mock_job = self._make_mock_job( + 1, 'test:sandbox', 'test', 'https://gitlab.com/job/1', + trace_bytes) + + mock_full_job = mock.MagicMock() + mock_full_job.trace.return_value = trace_bytes + + mock_pipeline = mock.MagicMock() + mock_pipeline.jobs.list.return_value = [mock_job] + + mock_project = mock.MagicMock() + mock_project.pipelines.get.return_value = mock_pipeline + mock_project.jobs.get.return_value = mock_full_job + + mock_glab = mock.MagicMock() + mock_glab.projects.get.return_value = mock_project + + with mock.patch('gitlab.Gitlab', return_value=mock_glab): + with terminal.capture(): + result = gitlab.get_failed_jobs('ci', 100, max_log_lines=50) + + self.assertEqual(len(result), 1) + # Should only have last 50 lines + log_lines = result[0].log_tail.splitlines() + self.assertEqual(len(log_lines), 50) + self.assertIn('line 499', result[0].log_tail) + + +class TestBuildPipelineFixPrompt(unittest.TestCase): + """Tests for build_pipeline_fix_prompt function.""" + + def test_single_job(self): + """Test prompt with a single failed job""" + failed_jobs = [ + gitlab.FailedJob( + id=1, name='build:sandbox', stage='build', + web_url='https://gitlab.com/job/1', + log_tail='error: undefined reference'), + ] + prompt, task_desc = agent.build_pipeline_fix_prompt( + 42, 'cherry-abc123', failed_jobs, 'ci', 'master', + 'Test MR desc', 1) + + self.assertIn('!42', prompt) + self.assertIn('cherry-abc123', prompt) + self.assertIn('build:sandbox', prompt) + self.assertIn('error: undefined reference', prompt) + self.assertIn('attempt 1', prompt) + self.assertIn('cherry-abc123-fix1', prompt) + self.assertIn('1 failed', task_desc) + + def test_multiple_jobs(self): + """Test prompt with multiple failed jobs""" + failed_jobs = [ + gitlab.FailedJob( + id=1, name='build:sandbox', stage='build', + web_url='https://gitlab.com/job/1', + log_tail='build error'), + gitlab.FailedJob( + id=2, name='test:dm', stage='test', + web_url='https://gitlab.com/job/2', + log_tail='test failure'), + ] + prompt, task_desc = agent.build_pipeline_fix_prompt( + 42, 'cherry-abc123', failed_jobs, 'ci', 'master', '', 1) + + self.assertIn('build:sandbox', prompt) + self.assertIn('test:dm', prompt) + self.assertIn('build error', prompt) + self.assertIn('test failure', prompt) + self.assertIn('2 failed', task_desc) + + def test_attempt_number(self): + """Test that attempt number is reflected in prompt""" + failed_jobs = [ + gitlab.FailedJob( + id=1, name='build', stage='build', + web_url='https://gitlab.com/job/1', + log_tail='error'), + ] + prompt, task_desc = agent.build_pipeline_fix_prompt( + 42, 'cherry-abc123', failed_jobs, 'ci', 'master', '', 3) + + self.assertIn('attempt 3', prompt) + self.assertIn('cherry-abc123-fix3', prompt) + self.assertIn('attempt 3', task_desc) + + def test_uses_um_build(self): + """Test that prompt uses 'um build sandbox' for sandbox""" + failed_jobs = [ + gitlab.FailedJob( + id=1, name='build:sandbox', stage='build', + web_url='https://gitlab.com/job/1', + log_tail='error'), + ] + prompt, _ = agent.build_pipeline_fix_prompt( + 42, 'cherry-abc123', failed_jobs, 'ci', 'master', '', 1) + + self.assertIn('um build sandbox', prompt) + + def test_extracts_board_names(self): + """Test that board names are extracted from job names""" + failed_jobs = [ + gitlab.FailedJob( + id=1, name='build:imx8mm_venice', stage='build', + web_url='https://gitlab.com/job/1', + log_tail='error'), + gitlab.FailedJob( + id=2, name='build:rpi_4', stage='build', + web_url='https://gitlab.com/job/2', + log_tail='error'), + ] + prompt, _ = agent.build_pipeline_fix_prompt( + 42, 'cherry-abc123', failed_jobs, 'ci', 'master', '', 1) + + # Should include both boards plus sandbox in the buildman command + self.assertIn('buildman', prompt) + self.assertIn('imx8mm_venice', prompt) + self.assertIn('rpi_4', prompt) + self.assertIn('sandbox', prompt) + + def test_buildman_for_multiple_boards(self): + """Test that buildman is used for building multiple boards""" + failed_jobs = [ + gitlab.FailedJob( + id=1, name='build:coral', stage='build', + web_url='https://gitlab.com/job/1', + log_tail='error'), + ] + prompt, _ = agent.build_pipeline_fix_prompt( + 42, 'cherry-abc123', failed_jobs, 'ci', 'master', '', 1) + + self.assertIn('buildman -o /tmp/pickman', prompt) + self.assertIn('coral', prompt) + + +class TestProcessPipelineFailures(unittest.TestCase): + """Tests for process_pipeline_failures function.""" + + def setUp(self): + """Set up test fixtures.""" + fd, self.db_path = tempfile.mkstemp(suffix='.db') + os.close(fd) + os.unlink(self.db_path) + + def tearDown(self): + """Clean up test fixtures.""" + if os.path.exists(self.db_path): + os.unlink(self.db_path) + database.Database.instances.clear() + + def _make_mr(self, iid=1, pipeline_status='failed', pipeline_id=100, + needs_rebase=False, has_conflicts=False): + """Helper to create a PickmanMr with pipeline fields""" + return gitlab.PickmanMr( + iid=iid, + title=f'[pickman] Test MR {iid}', + web_url=f'https://gitlab.com/mr/{iid}', + source_branch=f'cherry-test-{iid}', + description='Test description', + has_conflicts=has_conflicts, + needs_rebase=needs_rebase, + pipeline_status=pipeline_status, + pipeline_id=pipeline_id, + ) + + def test_skips_running(self): + """Test that running pipelines are skipped""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + + mrs = [self._make_mr(pipeline_status='running')] + with mock.patch.object(control, 'run_git'): + result = control.process_pipeline_failures( + 'ci', mrs, dbs, 'master', 3) + + self.assertEqual(result, 0) + dbs.close() + + def test_skips_success(self): + """Test that successful pipelines are skipped""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + + mrs = [self._make_mr(pipeline_status='success')] + with mock.patch.object(control, 'run_git'): + result = control.process_pipeline_failures( + 'ci', mrs, dbs, 'master', 3) + + self.assertEqual(result, 0) + dbs.close() + + def test_skips_already_processed(self): + """Test that already-processed pipelines are skipped""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + + # Pre-record this pipeline + dbs.pfix_add(1, 100, 1, 'success') + dbs.commit() + + mrs = [self._make_mr()] + with mock.patch.object(control, 'run_git'): + result = control.process_pipeline_failures( + 'ci', mrs, dbs, 'master', 3) + + self.assertEqual(result, 0) + dbs.close() + + def test_respects_retry_limit(self): + """Test that retry limit is respected""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + + # Pre-record 3 attempts with different pipeline IDs + dbs.pfix_add(1, 10, 1, 'failure') + dbs.pfix_add(1, 20, 2, 'failure') + dbs.pfix_add(1, 30, 3, 'failure') + dbs.commit() + + mrs = [self._make_mr(pipeline_id=40)] + with mock.patch.object(control, 'run_git'): + with mock.patch.object(gitlab, 'reply_to_mr', + return_value=True): + result = control.process_pipeline_failures( + 'ci', mrs, dbs, 'master', 3) + + # Should have been processed (comment posted) but not fixed + self.assertEqual(result, 0) + dbs.close() + + def test_posts_comment_at_limit(self): + """Test that a comment is posted when retry limit is reached""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + + # Pre-record 3 attempts + dbs.pfix_add(1, 10, 1, 'failure') + dbs.pfix_add(1, 20, 2, 'failure') + dbs.pfix_add(1, 30, 3, 'failure') + dbs.commit() + + mrs = [self._make_mr(pipeline_id=40)] + with mock.patch.object(control, 'run_git'): + with mock.patch.object(gitlab, 'reply_to_mr', + return_value=True) as mock_reply: + control.process_pipeline_failures( + 'ci', mrs, dbs, 'master', 3) + + mock_reply.assert_called_once() + call_args = mock_reply.call_args + self.assertIn('retry limit', call_args[0][2]) + dbs.close() + + def test_processes_failed(self): + """Test processing a failed pipeline""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + + failed_jobs = [ + gitlab.FailedJob(id=1, name='build', stage='build', + web_url='https://gitlab.com/job/1', + log_tail='error'), + ] + mrs = [self._make_mr()] + + with mock.patch.object(control, 'run_git'): + with mock.patch.object(gitlab, 'get_failed_jobs', + return_value=failed_jobs): + with mock.patch.object(agent, 'fix_pipeline', + return_value=(True, 'Fixed it')): + with mock.patch.object( + gitlab, 'push_branch', + return_value=True) as mock_push: + with mock.patch.object(gitlab, 'update_mr_desc', + return_value=True): + with mock.patch.object( + gitlab, 'reply_to_mr', + return_value=True) as mock_reply: + with mock.patch.object( + control, + 'update_history_pipeline_fix'): + result = \ + control.process_pipeline_failures( + 'ci', mrs, dbs, 'master', 3) + + self.assertEqual(result, 1) + # Should be recorded in database + self.assertTrue(dbs.pfix_has(1, 100)) + # Should push the branch + mock_push.assert_called_once_with( + 'ci', 'cherry-test-1', force=True, skip_ci=False) + # Should post a comment on the MR + mock_reply.assert_called_once() + reply_msg = mock_reply.call_args[0][2] + self.assertIn('Fixed it', reply_msg) + self.assertIn('build', reply_msg) + dbs.close() + + def test_skips_skipped_mr(self): + """Test that MRs without pipeline_id are skipped""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + + mrs = [self._make_mr(pipeline_id=None)] + with mock.patch.object(control, 'run_git'): + result = control.process_pipeline_failures( + 'ci', mrs, dbs, 'master', 3) + + self.assertEqual(result, 0) + dbs.close() + + def test_rebases_before_fix(self): + """Test that a branch needing rebase is rebased instead of fixed""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + + mrs = [self._make_mr(needs_rebase=True)] + with mock.patch.object(control, 'run_git'): + with mock.patch.object( + gitlab, 'push_branch', + return_value=True) as mock_push: + with mock.patch.object(agent, 'fix_pipeline') as mock_fix: + result = control.process_pipeline_failures( + 'ci', mrs, dbs, 'master', 3) + + self.assertEqual(result, 1) + # Should push the rebased branch, not call fix_pipeline + mock_push.assert_called_once_with( + 'ci', 'cherry-test-1', force=True, skip_ci=False) + mock_fix.assert_not_called() + # Should be recorded as 'rebased' in database + self.assertTrue(dbs.pfix_has(1, 100)) + dbs.close() + + def test_rebase_with_conflicts_skips(self): + """Test that a failed rebase skips the pipeline fix""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + + mrs = [self._make_mr(has_conflicts=True)] + + def mock_run_git_fn(args): + if args[0] == 'rebase': + raise command.CommandExc('conflict', None) + return '' + + with mock.patch.object(control, 'run_git', + side_effect=mock_run_git_fn): + with mock.patch.object(agent, 'fix_pipeline') as mock_fix: + result = control.process_pipeline_failures( + 'ci', mrs, dbs, 'master', 3) + + self.assertEqual(result, 0) + mock_fix.assert_not_called() + dbs.close() + + def test_disabled_with_zero(self): + """Test that fix_retries=0 is handled in do_step (not called)""" + mock_mr = gitlab.PickmanMr( + iid=123, + title='[pickman] Test MR', + web_url='https://gitlab.com/mr/123', + source_branch='cherry-test', + description='Test', + pipeline_status='failed', + pipeline_id=100, + ) + with mock.patch.object(control, 'run_git'): + with mock.patch.object(gitlab, 'get_merged_pickman_mrs', + return_value=[]): + with mock.patch.object(gitlab, 'get_open_pickman_mrs', + return_value=[mock_mr]): + with mock.patch.object( + control, 'process_pipeline_failures') as mock_ppf: + args = argparse.Namespace( + cmd='step', source='us/next', + remote='ci', target='master', + max_mrs=1, fix_retries=0) + with terminal.capture(): + control.do_step(args, None) + + mock_ppf.assert_not_called() + + +class TestStepFixRetries(unittest.TestCase): + """Tests for --fix-retries argument parsing.""" + + def test_default(self): + """Test default fix-retries value for step""" + args = pickman.parse_args(['step', 'us/next']) + self.assertEqual(args.fix_retries, 3) + + def test_custom(self): + """Test custom fix-retries value for step""" + args = pickman.parse_args(['step', 'us/next', '--fix-retries', '5']) + self.assertEqual(args.fix_retries, 5) + + def test_zero_disables(self): + """Test that fix-retries=0 is accepted""" + args = pickman.parse_args(['step', 'us/next', '--fix-retries', '0']) + self.assertEqual(args.fix_retries, 0) + + def test_poll_default(self): + """Test default fix-retries value for poll""" + args = pickman.parse_args(['poll', 'us/next']) + self.assertEqual(args.fix_retries, 3) + + def test_poll_custom(self): + """Test custom fix-retries value for poll""" + args = pickman.parse_args(['poll', 'us/next', '--fix-retries', '1']) + self.assertEqual(args.fix_retries, 1) + + if __name__ == '__main__': unittest.main() From patchwork Sun Feb 22 15:42:55 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 1936 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=1771775082; bh=QZD/4hPK38EvaSeYIZPmUgxPHnE2scxYFSMSN5dqwNA=; 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=J+M3Qgs+dzlFAZzuHOv5Bc9ZDSu/zGxPEUoqM2p9hTjGfwuiz3x1Y8hJYdeRTv4Go 3soMCOqbWnNa/2TRwTwg7f3l3K9YnPLbFd5t3tPbCX7NKouazMjXy9XDLpxbxUJUpT oucoihU4MS+QLcwk2WhlEhicOYDVTOuoZbSMIoBfCMUdXcbSQsWtBBuLgWPuvPVuzH 0pcQ3YKSVYyAA38CNZe2vq70tTqHTR8fqtAhAORU2X2NgZwVrHif+OSP1tc0gxX5is 2NmXEc5O3PvU6UxwCY5cpqsE8mcM1SZAsLaJYGi/s+vXCHRcfrxbYLbEjKawmB7JEr lRLWN0CuE1PKQ== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id CF1CE69C5E for ; Sun, 22 Feb 2026 08:44:42 -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 GAs0nlYRksZs for ; Sun, 22 Feb 2026 08:44:42 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1771775082; bh=QZD/4hPK38EvaSeYIZPmUgxPHnE2scxYFSMSN5dqwNA=; 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=J+M3Qgs+dzlFAZzuHOv5Bc9ZDSu/zGxPEUoqM2p9hTjGfwuiz3x1Y8hJYdeRTv4Go 3soMCOqbWnNa/2TRwTwg7f3l3K9YnPLbFd5t3tPbCX7NKouazMjXy9XDLpxbxUJUpT oucoihU4MS+QLcwk2WhlEhicOYDVTOuoZbSMIoBfCMUdXcbSQsWtBBuLgWPuvPVuzH 0pcQ3YKSVYyAA38CNZe2vq70tTqHTR8fqtAhAORU2X2NgZwVrHif+OSP1tc0gxX5is 2NmXEc5O3PvU6UxwCY5cpqsE8mcM1SZAsLaJYGi/s+vXCHRcfrxbYLbEjKawmB7JEr lRLWN0CuE1PKQ== Received: from mail.u-boot.org (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id BB06F69D57 for ; Sun, 22 Feb 2026 08:44:42 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1771775080; bh=C/zIIzAznz2AayxLVUz5DvoyHVm50REJruJcUz6SYyc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Gk+jThZ02+5lf8wgMZIvWlfQk4D3SdOy7r45RmLjy0P/CGoRdOkm1Ymx963y7sHD7 eNpnf06s4wPm66+Bwou2P1ptK07jhJ5GcJwvSQLduiW27G4Mdm1xSbSZWOTPF0tdvk JXhGdoh9wQaNWcd5K66dlWUTYPufIwXD78KP6SZDHrCCVdtFsoJtCtrZMbApiLZOdp HWX3DJPJxhSUXQ/h2y4yYcXg/nMvIj4an8fzgQfnpSEFfKt9jflJwkNbBlZUhaISch Eanx7ceVnoPnoM5phhwUpV/y+6MKD6fgiUFegb4mTNYnIIC8PlH5PP3bu6VNIA5WQP EZWctJpoKEeKQ== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 0AE0869C5E; Sun, 22 Feb 2026 08:44:40 -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 HGi3rDkwhfKW; Sun, 22 Feb 2026 08:44:39 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1771775075; bh=YJY38XsAse4PEP2gIsXou92mGPhRgP0cfdypH30+zis=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=FYvTgWEgVpGX1UHWhBMAFSmF0nrJVXJve+Qv2Cw2LydRd4AVU5znghYBH36itTPxz 2tUyfUZJ3+i/Wm7RPUXEqdUGGMnbwSmQSRAffi9QW7SUI3+ayQ0YMMNTuqtbX5Hu0b 7mrpJn1dLhTvYmJr2QzDe5N0l61vQxtd0NYBGeAqxNxZ9W99KQt1QWnQZPQywh7d56 Ul64xi6ReFHzZs1PsJhVUErgPZYoZbpbFwG43yuOsARAM1qhk9gSgK/osXyMasDSA1 YCbKpxVTuE3POGFK/4YX9xUC/IZY6yP/R8jankZd9fXOWE9ArNdXY+WkDJMQ/3pYuX GdGSyK9d5Lodw== Received: from u-boot.org (unknown [73.34.74.121]) by mail.u-boot.org (Postfix) with ESMTPSA id 7A5CD69C5C; Sun, 22 Feb 2026 08:44:35 -0700 (MST) From: Simon Glass To: U-Boot Concept Date: Sun, 22 Feb 2026 08:42:55 -0700 Message-ID: <20260222154303.2851319-16-sjg@u-boot.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20260222154303.2851319-1-sjg@u-boot.org> References: <20260222154303.2851319-1-sjg@u-boot.org> MIME-Version: 1.0 Message-ID-Hash: YMGVKVQCMDVTZJUQXP2PJRBU7BHBH5RS X-Message-ID-Hash: YMGVKVQCMDVTZJUQXP2PJRBU7BHBH5RS 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 15/16] pickman: Process subtree updates even at max MRs 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 do_step() finds the number of open MRs is at the maximum, it returns immediately without processing subtree updates or advancing past fully-processed merges. These actions don't create MRs (subtree updates push to the target branch directly), so they should happen regardless. Currently the user must wait for the next poll interval and for an MR slot to open before subtree updates are applied. Move the _prepare_get_commits() call to before the max_mrs check in do_step(), so subtree updates and source-position advances happen unconditionally. Add an optional 'info' parameter to do_apply() and prepare_apply() so the already-fetched result can be passed through, avoiding a redundant call when below the max MR limit. Co-developed-by: Claude Opus 4.6 Signed-off-by: Simon Glass --- tools/pickman/control.py | 27 +++++++++++---- tools/pickman/ftest.py | 72 ++++++++++++++++++++++++++-------------- 2 files changed, 67 insertions(+), 32 deletions(-) diff --git a/tools/pickman/control.py b/tools/pickman/control.py index f4d4a43c292..a42927f991d 100644 --- a/tools/pickman/control.py +++ b/tools/pickman/control.py @@ -1663,7 +1663,7 @@ def _prepare_get_commits(dbs, source, args): return info, None -def prepare_apply(dbs, source, branch, args=None): +def prepare_apply(dbs, source, branch, args=None, info=None): """Prepare for applying commits from a source branch Get the next commits, set up the branch name and prints info about @@ -1676,15 +1676,18 @@ def prepare_apply(dbs, source, branch, args=None): branch (str): Branch name to use, or None to auto-generate args (Namespace): Parsed arguments with 'push', 'remote', 'target' (needed for subtree updates) + info (NextCommitsInfo): Pre-fetched commit info from + _prepare_get_commits(), or None to fetch it here Returns: tuple: (ApplyInfo, return_code) where ApplyInfo is set if there are commits to apply, or None with return_code indicating the result (0 for no commits, 1 for error) """ - info, ret = _prepare_get_commits(dbs, source, args) - if ret is not None: - return None, ret + if info is None: + info, ret = _prepare_get_commits(dbs, source, args) + if ret is not None: + return None, ret commits = info.commits @@ -1889,18 +1892,20 @@ def execute_apply(dbs, source, commits, branch_name, args, advance_to=None): # return ret, success, conv_log -def do_apply(args, dbs): +def do_apply(args, dbs, info=None): """Apply the next set of commits using Claude agent Args: args (Namespace): Parsed arguments with 'source' and 'branch' attributes dbs (Database): Database instance + info (NextCommitsInfo): Pre-fetched commit info from + _prepare_get_commits(), or None to fetch during prepare Returns: int: 0 on success, 1 on failure """ source = args.source - info, ret = prepare_apply(dbs, source, args.branch, args) + info, ret = prepare_apply(dbs, source, args.branch, args, info=info) if not info: return ret @@ -2851,6 +2856,14 @@ def do_step(args, dbs): process_pipeline_failures(remote, active_mrs, dbs, args.target, args.fix_retries) + # 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) + if ret is not None: + if ret: + return ret + return 0 + # Only block new MR creation if we've reached the max allowed open MRs max_mrs = args.max_mrs if len(active_mrs) >= max_mrs: @@ -2869,7 +2882,7 @@ def do_step(args, dbs): tout.info('No pending pickman MRs, creating new one...') args.push = True args.branch = None # Let do_apply generate branch name - return do_apply(args, dbs) + return do_apply(args, dbs, info=info) def do_poll(args, dbs): diff --git a/tools/pickman/ftest.py b/tools/pickman/ftest.py index af26cbb4229..ced2c79ac87 100644 --- a/tools/pickman/ftest.py +++ b/tools/pickman/ftest.py @@ -2061,16 +2061,21 @@ class TestStep(unittest.TestCase): source_branch='cherry-test', description='Test', ) + mock_info = control.NextCommitsInfo( + commits=['fake'], merge_found=True, advance_to='abc123') with mock.patch.object(control, 'run_git'): with mock.patch.object(gitlab, 'get_merged_pickman_mrs', return_value=[]): with mock.patch.object(gitlab, 'get_open_pickman_mrs', return_value=[mock_mr]): - args = argparse.Namespace(cmd='step', source='us/next', - remote='ci', target='master', - max_mrs=1, fix_retries=3) - with terminal.capture(): - ret = control.do_step(args, None) + with mock.patch.object( + control, '_prepare_get_commits', + return_value=(mock_info, None)): + args = argparse.Namespace( + cmd='step', source='us/next', remote='ci', + target='master', max_mrs=1, fix_retries=3) + with terminal.capture(): + ret = control.do_step(args, None) self.assertEqual(ret, 0) @@ -2109,18 +2114,23 @@ class TestStep(unittest.TestCase): source_branch='cherry-test', description='Test', ) + mock_info = control.NextCommitsInfo( + commits=['fake'], merge_found=True, advance_to='abc123') with mock.patch.object(control, 'run_git'): with mock.patch.object(gitlab, 'get_merged_pickman_mrs', return_value=[]): with mock.patch.object(gitlab, 'get_open_pickman_mrs', return_value=[mock_mr]): - with mock.patch.object(control, 'do_apply', - return_value=0) as mock_apply: - args = argparse.Namespace(cmd='step', source='us/next', - remote='ci', target='master', - max_mrs=5, fix_retries=3) - with terminal.capture(): - ret = control.do_step(args, None) + with mock.patch.object( + control, '_prepare_get_commits', + return_value=(mock_info, None)): + with mock.patch.object(control, 'do_apply', + return_value=0) as mock_apply: + args = argparse.Namespace( + cmd='step', source='us/next', remote='ci', + target='master', max_mrs=5, fix_retries=3) + with terminal.capture(): + ret = control.do_step(args, None) # With 1 open MR and max_mrs=5, it should try to create a new one self.assertEqual(ret, 0) @@ -2138,17 +2148,23 @@ class TestStep(unittest.TestCase): ) for i in range(3) ] + mock_info = control.NextCommitsInfo( + commits=['fake'], merge_found=True, advance_to='abc123') with mock.patch.object(control, 'run_git'): with mock.patch.object(gitlab, 'get_merged_pickman_mrs', return_value=[]): with mock.patch.object(gitlab, 'get_open_pickman_mrs', return_value=mock_mrs): - with mock.patch.object(control, 'do_apply') as mock_apply: - args = argparse.Namespace(cmd='step', source='us/next', - remote='ci', target='master', - max_mrs=3, fix_retries=3) - with terminal.capture(): - ret = control.do_step(args, None) + with mock.patch.object( + control, '_prepare_get_commits', + return_value=(mock_info, None)): + with mock.patch.object(control, 'do_apply') \ + as mock_apply: + args = argparse.Namespace( + cmd='step', source='us/next', remote='ci', + target='master', max_mrs=3, fix_retries=3) + with terminal.capture(): + ret = control.do_step(args, None) # With 3 open MRs and max_mrs=3, should not create new MR self.assertEqual(ret, 0) @@ -6120,19 +6136,25 @@ class TestProcessPipelineFailures(unittest.TestCase): pipeline_status='failed', pipeline_id=100, ) + mock_info = control.NextCommitsInfo( + commits=['fake'], merge_found=True, advance_to='abc123') with mock.patch.object(control, 'run_git'): with mock.patch.object(gitlab, 'get_merged_pickman_mrs', return_value=[]): with mock.patch.object(gitlab, 'get_open_pickman_mrs', return_value=[mock_mr]): with mock.patch.object( - control, 'process_pipeline_failures') as mock_ppf: - args = argparse.Namespace( - cmd='step', source='us/next', - remote='ci', target='master', - max_mrs=1, fix_retries=0) - with terminal.capture(): - control.do_step(args, None) + control, '_prepare_get_commits', + return_value=(mock_info, None)): + with mock.patch.object( + control, + 'process_pipeline_failures') as mock_ppf: + args = argparse.Namespace( + cmd='step', source='us/next', + remote='ci', target='master', + max_mrs=1, fix_retries=0) + with terminal.capture(): + control.do_step(args, None) mock_ppf.assert_not_called() From patchwork Sun Feb 22 15:42:56 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 1937 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=1771775085; bh=FCIWDmOKw8VDJDsHwTbmNkG4A5RQizZWiT1EmroJUMY=; 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=N+2Qw1Ba0wZXnwKcxOotrHf09HQjVUkY/E4O6104FAi5CjiZnDZI2IQDVvMbOe3py QogQEf9NgRxDp+jkBz/PEY+fk9VTNVuVfrp8BR+QmGJ202JZts9hgu6pk1giSRw+FD q5GCiuBJNEmnBSA9Y9o27i+i73Q6anxl9vO7JQkZ1l9oV1FVc+mkMkyKUc7pTeqTkO Fk2Afi0DYbF1az7xdelrPAa4WnzlxQPZZZ+97NbICiS6yFamuiQ7XQa1lca/4jzcmA IHD3ZpDUZSI0T4iUZTuxKduL0I6pLx0jFCJDWch3jz4Ar8E/TJ8UkdBeufvCrYXKkA CnZ2QwF54GfhA== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 9278069D57 for ; Sun, 22 Feb 2026 08:44:45 -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 76In1AmibzRW for ; Sun, 22 Feb 2026 08:44:45 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1771775085; bh=FCIWDmOKw8VDJDsHwTbmNkG4A5RQizZWiT1EmroJUMY=; 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=N+2Qw1Ba0wZXnwKcxOotrHf09HQjVUkY/E4O6104FAi5CjiZnDZI2IQDVvMbOe3py QogQEf9NgRxDp+jkBz/PEY+fk9VTNVuVfrp8BR+QmGJ202JZts9hgu6pk1giSRw+FD q5GCiuBJNEmnBSA9Y9o27i+i73Q6anxl9vO7JQkZ1l9oV1FVc+mkMkyKUc7pTeqTkO Fk2Afi0DYbF1az7xdelrPAa4WnzlxQPZZZ+97NbICiS6yFamuiQ7XQa1lca/4jzcmA IHD3ZpDUZSI0T4iUZTuxKduL0I6pLx0jFCJDWch3jz4Ar8E/TJ8UkdBeufvCrYXKkA CnZ2QwF54GfhA== Received: from mail.u-boot.org (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 7AA7769D48 for ; Sun, 22 Feb 2026 08:44:45 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1771775084; bh=IpEmwWewtt0dIG1uAOJ3xXHew+hnBEx1vHmNJI7xn38=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=iZQ3X9/rFGoJpj/0Aa63Tg4AtCFhkU1wKjxZDiNm5nECIuoAxLnTEun3F8mXVKBo4 4qqEwTc3iB7hylLKlmcst6cjlBj2HKoQoYeonb+hfQpZgLHFU3VhYKEJrLOjnLLtt2 KAsNg09DRXZPm5CoRtHlIreRV9xC9NL4qJtZeu+e3WKoToJ1KznsCf/ORxtnRJOdJ0 hs9Co0zSZYwih+A2Qk76hQLDmYvq118/lLVvUHVslCZVnlnJa/v0O5KyRFks3CPTvp C/k2qJn9v9URfnzTJWahBeitYxziDFB+3o/jIms/2df4/qmxyZgC0UtknI+9JtDqF1 gi8FBkY72Q5Yg== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id C365769C5E; Sun, 22 Feb 2026 08:44: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 10026) with ESMTP id KZLcbnTDONos; Sun, 22 Feb 2026 08:44:44 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1771775080; bh=ToDJZW985yzpZEH2cu9/fIefk0cHBOWXXfxIgwndUBI=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Qfhv8TckTZX+Z5JdnH4G8s9vRni1ddj3qnXHrUIaPdCeEqaT2MwXLJ+699+RBYNq2 03evmSMnDz9B1kTblyOSEGodaRtiXM7gTZl7JYXAR5wPZMUGXS94KY9Q8nBngZr3yL m/vU/kotHo7g1aZg22Pt/A0WxNtw7Oi5SxPGuB74oC5ZcMWIIfifz0gG4xxVC6JSlU /NMENywh08Fr38c4ywjEiogM2JWcVmz19ES1J1L84JQbO6WE8vhkZMgUMwjTFR7dBP WVZy+5t0eDWYT/Z4/2DRwn6Yte2GJbm+zxfhnSc8H7YJ/9aqS1OiLvyWahIKZxUWgW QdlLi4x+ZscDA== Received: from u-boot.org (unknown [73.34.74.121]) by mail.u-boot.org (Postfix) with ESMTPSA id 4BDB169C5C; Sun, 22 Feb 2026 08:44:40 -0700 (MST) From: Simon Glass To: U-Boot Concept Date: Sun, 22 Feb 2026 08:42:56 -0700 Message-ID: <20260222154303.2851319-17-sjg@u-boot.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20260222154303.2851319-1-sjg@u-boot.org> References: <20260222154303.2851319-1-sjg@u-boot.org> MIME-Version: 1.0 Message-ID-Hash: C2DFXLCJ7Q2HMDHNN5YBCVEHLCQSRVUT X-Message-ID-Hash: C2DFXLCJ7Q2HMDHNN5YBCVEHLCQSRVUT 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 16/16] pickman: Strip null bytes from CI job logs 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 CI job traces can contain embedded null bytes, which cause a ValueError ("embedded null byte") when the prompt string is passed to the Claude Agent SDK subprocess. Strip null bytes from the trace after decoding. Co-developed-by: Claude Opus 4.6 Signed-off-by: Simon Glass --- tools/pickman/ftest.py | 35 +++++++++++++++++++++++++++++++++++ tools/pickman/gitlab_api.py | 1 + 2 files changed, 36 insertions(+) diff --git a/tools/pickman/ftest.py b/tools/pickman/ftest.py index ced2c79ac87..5a4d0c433dc 100644 --- a/tools/pickman/ftest.py +++ b/tools/pickman/ftest.py @@ -5787,6 +5787,41 @@ class TestGetFailedJobs(unittest.TestCase): self.assertIn('line 499', result[0].log_tail) + @mock.patch.object(gitlab, 'get_remote_url', + return_value=TEST_SSH_URL) + @mock.patch.object(gitlab, 'get_token', return_value='test-token') + @mock.patch.object(gitlab, 'AVAILABLE', True) + def test_null_bytes_stripped(self, _mock_token, _mock_url): + """Test that null bytes in job logs are stripped""" + trace_bytes = b'before\x00after\nline2\x00end\n' + + mock_job = self._make_mock_job( + 1, 'build:sandbox', 'build', 'https://gitlab.com/job/1', + trace_bytes) + + mock_full_job = mock.MagicMock() + mock_full_job.trace.return_value = trace_bytes + + mock_pipeline = mock.MagicMock() + mock_pipeline.jobs.list.return_value = [mock_job] + + mock_project = mock.MagicMock() + mock_project.pipelines.get.return_value = mock_pipeline + mock_project.jobs.get.return_value = mock_full_job + + mock_glab = mock.MagicMock() + mock_glab.projects.get.return_value = mock_project + + with mock.patch('gitlab.Gitlab', return_value=mock_glab): + with terminal.capture(): + result = gitlab.get_failed_jobs('ci', 100) + + self.assertEqual(len(result), 1) + self.assertNotIn('\0', result[0].log_tail) + self.assertIn('beforeafter', result[0].log_tail) + self.assertIn('line2end', result[0].log_tail) + + class TestBuildPipelineFixPrompt(unittest.TestCase): """Tests for build_pipeline_fix_prompt function.""" diff --git a/tools/pickman/gitlab_api.py b/tools/pickman/gitlab_api.py index d9635d392ee..81918c80c3c 100644 --- a/tools/pickman/gitlab_api.py +++ b/tools/pickman/gitlab_api.py @@ -478,6 +478,7 @@ def get_failed_jobs(remote, pipeline_id, max_log_lines=200): full_job = project.jobs.get(job.id) try: trace = full_job.trace().decode('utf-8', errors='replace') + trace = trace.replace('\0', '') lines = trace.splitlines() log_tail = '\n'.join(lines[-max_log_lines:]) except (AttributeError, gitlab.exceptions.GitlabError):