From: Simon Glass <simon.glass@canonical.com> Add a NextCommitsInfo named tuple with commits and merge_found fields, and return (NextCommitsInfo, error_msg) from get_next_commits() instead of a bare 3-tuple. Update do_next_set() and prepare_apply() to unpack the new return type, along with the corresponding tests. Co-developed-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/pickman/control.py | 49 ++++++++++++++++++++++---------------- tools/pickman/ftest.py | 51 +++++++++++++++++----------------------- 2 files changed, 51 insertions(+), 49 deletions(-) diff --git a/tools/pickman/control.py b/tools/pickman/control.py index 3f034d03d72..cd138612623 100644 --- a/tools/pickman/control.py +++ b/tools/pickman/control.py @@ -87,6 +87,13 @@ CommitInfo = namedtuple('CommitInfo', AgentCommit = namedtuple('AgentCommit', ['hash', 'chash', 'subject', 'applied_as']) +# Named tuple for get_next_commits() result +# +# commits: list of CommitInfo to cherry-pick +# merge_found: True if these commits came from a merge on the source branch +NextCommitsInfo = namedtuple('NextCommitsInfo', + ['commits', 'merge_found']) + # Named tuple for prepare_apply() result # # commits: list of AgentCommit to cherry-pick @@ -751,16 +758,14 @@ def get_next_commits(dbs, source): source (str): Source branch name Returns: - tuple: (commits, merge_found, error_msg) where: - commits: list of CommitInfo tuples - merge_found: bool, True if stopped at a merge commit - error_msg: str or None, error message if failed + tuple: (NextCommitsInfo, error_msg) where error_msg is None + on success """ # Get the last cherry-picked commit from database last_commit = dbs.source_get(source) if not last_commit: - return None, False, f"Source '{source}' not found in database" + return None, f"Source '{source}' not found in database" # Get all first-parent commits to find merges fp_output = run_git([ @@ -769,7 +774,7 @@ def get_next_commits(dbs, source): ]) if not fp_output: - return [], False, None + return NextCommitsInfo([], False), None # Build list of merge hashes on the first-parent chain merge_hashes = [] @@ -799,7 +804,7 @@ def get_next_commits(dbs, source): commits = [c for c in all_commits if not dbs.commit_get(c.hash)] if commits: - return commits, True, None + return NextCommitsInfo(commits, True), None # All commits in this merge are processed, skip to next prev_commit = merge_hash @@ -811,12 +816,12 @@ def get_next_commits(dbs, source): ]) if not log_output: - return [], False, None + return NextCommitsInfo([], False), None all_commits = parse_log_output(log_output, has_parents=True) commits = [c for c in all_commits if not dbs.commit_get(c.hash)] - return commits, False, None + return NextCommitsInfo(commits, False), None def get_commits_for_pick(commit_spec): @@ -890,23 +895,24 @@ def do_next_set(args, dbs): int: 0 on success, 1 if source not found """ source = args.source - commits, merge_found, err = get_next_commits(dbs, source) + info, err = get_next_commits(dbs, source) if err: tout.error(err) return 1 - if not commits: + if not info.commits: tout.info('No new commits to cherry-pick') return 0 - if merge_found: - tout.info(f'Next set from {source} ({len(commits)} commits):') + if info.merge_found: + tout.info(f'Next set from {source} ' + f'({len(info.commits)} commits):') else: - tout.info(f'Remaining commits from {source} ({len(commits)} commits, ' - 'no merge found):') + tout.info(f'Remaining commits from {source} ' + f'({len(info.commits)} commits, no merge found):') - for commit in commits: + for commit in info.commits: tout.info(f' {commit.chash} {commit.subject}') return 0 @@ -1247,16 +1253,18 @@ def prepare_apply(dbs, source, branch): commits to apply, or None with return_code indicating the result (0 for no commits, 1 for error) """ - commits, merge_found, err = get_next_commits(dbs, source) + info, err = get_next_commits(dbs, source) if err: tout.error(err) return None, 1 - if not commits: + if not info.commits: tout.info('No new commits to cherry-pick') return None, 0 + commits = info.commits + # Save current branch to return to later original_branch = run_git(['rev-parse', '--abbrev-ref', 'HEAD']) @@ -1274,7 +1282,7 @@ def prepare_apply(dbs, source, branch): except Exception: # pylint: disable=broad-except pass # Branch doesn't exist, which is fine - if merge_found: + if info.merge_found: tout.info(f'Applying next set from {source} ({len(commits)} commits):') else: tout.info(f'Applying remaining commits from {source} ' @@ -1285,7 +1293,8 @@ def prepare_apply(dbs, source, branch): tout.info(f' {commit.chash} {commit.subject}') tout.info('') - return ApplyInfo(commits, branch_name, original_branch, merge_found), 0 + return ApplyInfo(commits, branch_name, original_branch, + info.merge_found), 0 # pylint: disable=too-many-arguments diff --git a/tools/pickman/ftest.py b/tools/pickman/ftest.py index b943804e6f8..38e5cef5306 100644 --- a/tools/pickman/ftest.py +++ b/tools/pickman/ftest.py @@ -1208,10 +1208,8 @@ class TestGetNextCommits(unittest.TestCase): with terminal.capture(): dbs = database.Database(self.db_path) dbs.start() - commits, merge_found, err = control.get_next_commits(dbs, - 'unknown') - self.assertIsNone(commits) - self.assertFalse(merge_found) + info, err = control.get_next_commits(dbs, 'unknown') + self.assertIsNone(info) self.assertIn('not found', err) dbs.close() @@ -1243,13 +1241,12 @@ class TestGetNextCommits(unittest.TestCase): command.TEST_RESULT = mock_git - commits, merge_found, err = control.get_next_commits(dbs, - 'us/next') + info, err = control.get_next_commits(dbs, 'us/next') self.assertIsNone(err) - self.assertTrue(merge_found) - self.assertEqual(len(commits), 2) - self.assertEqual(commits[0].chash, 'aaa111a') - self.assertEqual(commits[1].chash, 'bbb222b') + self.assertTrue(info.merge_found) + self.assertEqual(len(info.commits), 2) + self.assertEqual(info.commits[0].chash, 'aaa111a') + self.assertEqual(info.commits[1].chash, 'bbb222b') dbs.close() @@ -2870,11 +2867,10 @@ class TestGetNextCommitsEmptyLine(unittest.TestCase): ) command.TEST_RESULT = command.CommandResult(stdout=log_output) - commits, merge_found, err = control.get_next_commits(dbs, - 'us/next') + info, err = control.get_next_commits(dbs, 'us/next') self.assertIsNone(err) - self.assertFalse(merge_found) - self.assertEqual(len(commits), 2) + self.assertFalse(info.merge_found) + self.assertEqual(len(info.commits), 2) dbs.close() def test_get_next_commits_skips_db_commits(self): @@ -2897,13 +2893,12 @@ class TestGetNextCommitsEmptyLine(unittest.TestCase): ) command.TEST_RESULT = command.CommandResult(stdout=log_output) - commits, merge_found, err = control.get_next_commits(dbs, - 'us/next') + info, err = control.get_next_commits(dbs, 'us/next') self.assertIsNone(err) - self.assertFalse(merge_found) + self.assertFalse(info.merge_found) # Only second commit should be returned (first is in DB) - self.assertEqual(len(commits), 1) - self.assertEqual(commits[0].chash, 'bbb222b') + self.assertEqual(len(info.commits), 1) + self.assertEqual(info.commits[0].chash, 'bbb222b') dbs.close() def test_get_next_commits_all_in_db(self): @@ -2928,12 +2923,11 @@ class TestGetNextCommitsEmptyLine(unittest.TestCase): ) command.TEST_RESULT = command.CommandResult(stdout=log_output) - commits, merge_found, err = control.get_next_commits(dbs, - 'us/next') + info, err = control.get_next_commits(dbs, 'us/next') self.assertIsNone(err) - self.assertFalse(merge_found) + self.assertFalse(info.merge_found) # No commits should be returned (all in DB) - self.assertEqual(len(commits), 0) + self.assertEqual(len(info.commits), 0) dbs.close() def test_get_next_commits_skips_processed_merge(self): @@ -2987,14 +2981,13 @@ class TestGetNextCommitsEmptyLine(unittest.TestCase): command.TEST_RESULT = mock_git - commits, merge_found, err = control.get_next_commits(dbs, - 'us/next') + info, err = control.get_next_commits(dbs, 'us/next') self.assertIsNone(err) - self.assertTrue(merge_found) + self.assertTrue(info.merge_found) # Should return commits from second merge (first was skipped) - self.assertEqual(len(commits), 2) - self.assertEqual(commits[0].chash, 'ccc333c') - self.assertEqual(commits[1].chash, 'merge2m') + self.assertEqual(len(info.commits), 2) + self.assertEqual(info.commits[0].chash, 'ccc333c') + self.assertEqual(info.commits[1].chash, 'merge2m') dbs.close() -- 2.43.0