From: Simon Glass <simon.glass@canonical.com> Fix get_next_commits() to use --first-parent when finding the next merge commit. Without this, git log returns commits in topological order which can find merges from different subsystems that were merged later, rather than following the mainline merge order. The fix uses a two-step approach: 1. Use --first-parent to find the next merge on the mainline 2. Get all commits up to that merge (without --first-parent to include the merged branch's commits) Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/pickman/control.py | 40 ++++++++++++++++++++++++++-------------- tools/pickman/ftest.py | 37 ++++++++++++++++++++++++++++++++----- 2 files changed, 58 insertions(+), 19 deletions(-) diff --git a/tools/pickman/control.py b/tools/pickman/control.py index e6b7539ea34..892100f3479 100644 --- a/tools/pickman/control.py +++ b/tools/pickman/control.py @@ -151,7 +151,7 @@ def get_next_commits(dbs, source): """Get the next set of commits to cherry-pick from a source Finds commits between the last cherry-picked commit and the next merge - commit in the source branch. + commit on the first-parent (mainline) chain of the source branch. Args: dbs (Database): Database instance @@ -169,20 +169,38 @@ def get_next_commits(dbs, source): if not last_commit: return None, False, f"Source '{source}' not found in database" - # Get commits between last_commit and source HEAD (oldest first) - # Format: hash|short_hash|author|subject|parents - # Using | as separator since subject may contain colons + # First, find the next merge commit on the first-parent chain + # This ensures we follow the mainline and find merges in order + fp_output = run_git([ + 'log', '--reverse', '--first-parent', '--format=%H|%h|%an|%s|%P', + f'{last_commit}..{source}' + ]) + + if not fp_output: + return [], False, None + + # Find the first merge on the first-parent chain + merge_hash = None + for line in fp_output.split('\n'): + if not line: + continue + parts = line.split('|') + parents = parts[-1].split() + if len(parents) > 1: + merge_hash = parts[0] + break + + # Now get all commits from last_commit to the merge (or end of branch) + # Without --first-parent to include commits from merged branches log_output = run_git([ 'log', '--reverse', '--format=%H|%h|%an|%s|%P', - f'{last_commit}..{source}' + f'{last_commit}..{merge_hash or source}' ]) if not log_output: return [], False, None commits = [] - merge_found = False - for line in log_output.split('\n'): if not line: continue @@ -191,16 +209,10 @@ def get_next_commits(dbs, source): short_hash = parts[1] author = parts[2] subject = '|'.join(parts[3:-1]) # Subject may contain separator - parents = parts[-1].split() commits.append(CommitInfo(commit_hash, short_hash, subject, author)) - # Check if this is a merge commit (has multiple parents) - if len(parents) > 1: - merge_found = True - break - - return commits, merge_found, None + return commits, bool(merge_hash), None def do_next_set(args, dbs): diff --git a/tools/pickman/ftest.py b/tools/pickman/ftest.py index 80aa73f8a5b..784e8dd5d1b 100644 --- a/tools/pickman/ftest.py +++ b/tools/pickman/ftest.py @@ -840,14 +840,27 @@ class TestNextSet(unittest.TestCase): database.Database.instances.clear() - # Mock git log with commits including a merge - log_output = ( + # First-parent log (to find next merge on mainline) + fp_log_output = ( 'aaa111|aaa111a|Author 1|First commit|abc123\n' 'bbb222|bbb222b|Author 2|Second commit|aaa111\n' 'ccc333|ccc333c|Author 3|Merge branch feature|bbb222 ddd444\n' 'eee555|eee555e|Author 4|After merge|ccc333\n' ) - command.TEST_RESULT = command.CommandResult(stdout=log_output) + # Full log (to get all commits up to the merge) + full_log_output = ( + 'aaa111|aaa111a|Author 1|First commit|abc123\n' + 'bbb222|bbb222b|Author 2|Second commit|aaa111\n' + 'ccc333|ccc333c|Author 3|Merge branch feature|bbb222 ddd444\n' + ) + + def mock_git(pipe_list): + cmd = pipe_list[0] if pipe_list else [] + if '--first-parent' in cmd: + return command.CommandResult(stdout=fp_log_output) + return command.CommandResult(stdout=full_log_output) + + command.TEST_RESULT = mock_git args = argparse.Namespace(cmd='next-set', source='us/next') with terminal.capture() as (stdout, _): @@ -931,11 +944,25 @@ class TestGetNextCommits(unittest.TestCase): dbs.source_set('us/next', 'abc123') dbs.commit() - log_output = ( + # First-parent log (to find next merge on mainline) + fp_log_output = ( 'aaa111|aaa111a|Author 1|First commit|abc123\n' 'bbb222|bbb222b|Author 2|Merge branch|aaa111 ccc333\n' + 'ddd444|ddd444d|Author 3|After merge|bbb222\n' ) - command.TEST_RESULT = command.CommandResult(stdout=log_output) + # Full log (to get all commits up to the merge) + full_log_output = ( + 'aaa111|aaa111a|Author 1|First commit|abc123\n' + 'bbb222|bbb222b|Author 2|Merge branch|aaa111 ccc333\n' + ) + + def mock_git(pipe_list): + cmd = pipe_list[0] if pipe_list else [] + if '--first-parent' in cmd: + return command.CommandResult(stdout=fp_log_output) + return command.CommandResult(stdout=full_log_output) + + command.TEST_RESULT = mock_git commits, merge_found, error = control.get_next_commits(dbs, 'us/next') -- 2.43.0