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(-)
@@ -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):
@@ -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')