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()