There is in fact a 'msg' member for Commit, set up by the
Patchstream._close_commit() function. Declare it in the docs and set it
to empty when the Commit is created.
Signed-off-by: Simon Glass <simon.glass@canonical.com>
When a patch is co-developed with an AI assistant, the Co-developed-by
tag typically uses a noreply email address. Since Signed-off-by requires
a real person with a valid email address, skip the name/email mismatch
warning when the Co-developed-by email contains "noreply@".
Cover-letter:
checkpatch: Sync with Linux v6.18
This series syncs checkpatch.pl with Linux v6.18 and adds several
U-Boot-specific improvements:
- Fix patman's checkpatch message parsing for consecutive messages
- Allow test macros (UNIT_TEST, etc.) immediately after function close
- Allow AI Co-developed-by tags without requiring matching Signed-off-by
END
Co-developed-by: Claude <noreply@anthropic.com>
Signed-off-by: Simon Glass <simon.glass@canonical.com>
U-Boot test declarations (e.g., FS_TEST, UNIT_TEST) should immediately
follow the closing brace of the test function with no blank line.
Add an exception to the blank-line-after-declaration check for *TEST
macros, and add a new check that warns if there IS a blank line before
a test declaration macro.
Update the patman test to handle duplicate warnings now that message
parsing correctly separates consecutive checkpatch messages.
Co-developed-by: Claude <noreply@anthropic.com>
Signed-off-by: Simon Glass <simon.glass@canonical.com>
Checkpatch.pl doesn't always output blank lines between messages. The
current parser splits on '\n\n' which fails to separate consecutive
messages, causing "Internal error: some problems lost" warnings.
Fix by splitting on message-starting patterns (ERROR:, WARNING:, CHECK:,
etc.) instead of blank lines.
Add a test that verifies consecutive messages are parsed correctly.
Co-developed-by: Claude <noreply@anthropic.com>
Signed-off-by: Simon Glass <simon.glass@canonical.com>
When a cherry pick has already been completed this can confuse pickman
and make it hard for a reviewer to figure out what is going on.
Attempt to detect already-applied commits (by commit subject) as a way
to reduce duplicate cherry-picks and improve the reliability of the
automation.
The agent now compares actual patch content using git show and diff,
only skipping commits that are similar with minor differences like
line numbers or conflict resolutions. This should reduces false
positives while maintaining robust duplicate detection.
Series-to: concept
Cover-letter:
pickman: Provide better ways to check cherry-picks
After a few weeks of using this tool a couple of things have come to
light, mostly in a recent attempt to cherry-pick ~260 commits.
Firstly, it is hard to manually check a cherry-pick against the
original. The GitLab GUI allows you to click on both, but it does not
give a sense of the detail.
Secondly, commits have sometimes already been applied to the tree, but
in this case effort is still paid to cherry pick it, sometimes with
unfortunate results.
Thirdly, building once at the end does not catch every problems. In the
case in question, about five commits were mangled, three of which would
have been caught by doing a build.
This series introduces a new 'check' command which provides a simple
check of the diff delta between an original commit and its cherry pick.,
thus providing a list of suspect cherry-picks, e.g. (omitting some long
lines):
Cherry-pick Delta% Original Subject
----------- ------ ---------- -------
aaea489b2a 100 9bab7d2a7c net: wget: let wget_with_dns work with
e557daec17 100 f0315babfb hash: Plumb crc8 into the hash functions
08a86f1769 40 6acada5daa configs: j7*: Enable TI_COMMON_CMD_OPTIONS
de37f6abb6 100 fc37a73e66 fdt: Swap the signature for
d1437b065a 100 e2cc9b4fc1 tools: binman: add 'fit, encrypt' property
09b800b0df 100 12d7be498a Docker/CI: Only test Xtensa on amd64 hosts
0256d8140c 100 ece1631f5e test/cmd/wget: replace bogus response with
e005799d8f 33 15e0c5e390 lmb: Remove lmb_alloc_addr_flags()
eaba5aae8f 79 99afa58e6d sandbox: Correct guard around readq/writeq
c347fb4b1a 41 99145eec2d x86: select CONFIG_64BIT for X86_64
14192a60e0 89 60a684e0a9 trace: add support for 'trace wipe'
8f58cfe76d 66 905204ddcf test: test_trace.py: test 'trace wipe'
12 problem commit(s) found
The -d option shows a diff of the patch diffs, which can aid a fast
review.
This series also includes some updates to the agent prompt to build
after each commit, some refactoring and tidy-ups to remove pylint
warnings and a way to tell the agent about possible already-applied
commits.
Further refinements will likely be needed as time goes on.
END
Co-developed-by: Claude <noreply@anthropic.com>
Signed-off-by: Simon Glass <simon.glass@canonical.com>
Create an AgentCommit namedtuple for passing data to the agent to save
confusing about ordering.
Document CommitInfo while we are here.
Co-developed-by: Claude <noreply@anthropic.com>
Signed-off-by: Simon Glass <simon.glass@canonical.com>
Add a --diff/-d option to the check command that displays the difference
between the original commit and the cherry-picked commit for commits
which show a large discrepancy. This helps identify what changed during
the cherry-pick process.
Also add a --no-colour option to disable coloured output when needed,
e.g. when outputting to a log file.
Features:
- Shows unified diff between original and cherry-picked patch content
- Uses coloured output by default, can be disabled with --no-colour
- Works in both verbose and summary modes
- Includes comprehensive tests for both colour modes
Co-developed-by: Claude <noreply@anthropic.com>
Signed-off-by: Simon Glass <simon.glass@canonical.com>
Some cherry-picks end up with large deltas compared to their original
commits, indicating potential problems. Add a check command to identify
these commits for review.
The check command:
- Analyses all commits on current branch vs ci/master
- Compares original vs cherry-picked commit statistics
- Shows commits with deltas above threshold (default 20%)
- Supports verbose mode with detailed analysis
- Color codes results: red ≥50%, yellow ≥threshold
- Skips merge commits and small commits (default <10 lines)
Also enhance the cherry-pick agent to:
- Check delta after each commit and build validation
- Attempt recovery for large deltas before continuing
- Build each commit individually for early problem detection
Co-developed-by: Claude <noreply@anthropic.com>
Signed-off-by: Simon Glass <simon.glass@canonical.com>
Add per-commit delta-checking and build-validation to the cherry-pick
and review agents:
- Check commit deltas after each cherry-pick to catch problems early
- Run buildman after each commit instead of only at the end
- Add recovery strategies for commits with large deltas
- Include delta checking in rebase operations
- Improve error handling and reporting
Co-developed-by: Claude <noreply@anthropic.com>
Signed-off-by: Simon Glass <simon.glass@canonical.com>
Replace long, hardcoded URLs in tests with named constants defined at
the top of the file. This improves code maintainability by providing a
single point of change for test URLs and helps with line-length
violations.
Co-developed-by: Claude <noreply@anthropic.com>
Signed-off-by: Simon Glass <simon.glass@canonical.com>
Break long lines to comply with 80-character limit and remove trailing
whitespace across agent.py, control.py, database.py, gitlab_api.py, and
__main__.py
Co-developed-by: Claude <noreply@anthropic.com>
Signed-off-by: Simon Glass <simon.glass@canonical.com>
First, thanks to Simon Glass and also Linaro, we now have access to a
few fast arm64 host machines in our Gitlab instance, to use as CI
runners. This series finishes the work that I pushed earlier and Simon
had started that enables arm64 hosts to be used for most things now.
The first notable change, especially if you use this on your own Gitlab
instance is that "DEFAULT_TAG" is now unused and we instead have:
- DEFAULT_ALL_TAG:
- DEFAULT_ARM64_TAG:
- DEFAULT_AMD64_TAG:
- DEFAULT_FAST_AMD64_TAG:
This lets us say that some jobs can be run on all runners, because they
are small enough that anything we'd connect to CI is fast enough and it
also does not depend on the underlying host architecture. Next we have
tags for any arm64 host, or any amd64 host. Finally, we have a tag for
fast amd64 hosts. What these last three are for is that we have a few
jobs that need to run on amd64 hosts and so we have to restrict them
there, but we also have now reworked the world build jobs to build
(almost) everything in a single job and on the fast amd64 machines this
is still as quick as the old way was, in practice.
To reach this point, we say that the Xtensa jobs can only run on amd64
hosts. Our targets only work with the binary-only toolchain and so this
is a reasonable limit and we exclude them from the world build jobs. We
also need to deal with ensuring the right toolchain is used regardless
what the host architecture is and that we don't use the host toolchain
by accident. Finally, because some of these changes needed to be worked
out in the linter, fix some of the general warnings that notes as well.
(cherry picked from commit 5947cd76ac)
Dropped tag changes:
Signed-off-by: Simon Glass <simon.glass@canonical.com>
When CI has fewer CPUs than boards, one thread may process multiple
boards. The .config from the first board persists in the thread's work
directory, causing kconfig_changed_since() to return True for the next
board's first commit, triggering an extra reconfig.
Fix by forcing -T4 to ensure each board gets its own thread with an
isolated work directory.
Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com>
Signed-off-by: Simon Glass <simon.glass@canonical.com>
Exclude tools/qconfig.py from coverage since it's a separate tool with
its own tests. Add allow_failures for core buildman modules that don't
have 100% test coverage yet (builder.py, builderthread.py, cfgutil.py,
control.py, toolchain.py).
This fixes the --coverage test returning error code 1.
Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com>
Signed-off-by: Simon Glass <simon.glass@canonical.com>
Add automatic detection of Kconfig and defconfig file changes to trigger
reconfiguration. When any Kconfig* file or the board's defconfig is newer
than the board's .config file, buildman forces a reconfigure even if the
build was previously successful.
This is enabled by default and can be disabled with -Z/--no-kconfig-check.
Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com>
Signed-off-by: Simon Glass <simon.glass@canonical.com>
Add a new test_bsettings.py with 12 tests to achieve 100% coverage for
bsettings.py
Series-to: concept
Co-developed-by: Claude <noreply@anthropic.com>
Signed-off-by: Simon Glass <simon.glass@canonical.com>
Series-links: 1:86
When rebasing MRs, two CI pipelines are triggered: one for the push
and another when GitLab detects the MR source branch update. Also,
the MR title can inadvertently be changed during rebases.
Fix this by:
- Removing --run-ci flag from the push-branch command in the review
agent prompt (GitLab triggers an MR pipeline automatically)
- Adding an instruction not to update the MR title during rebases
Update the CI Pipelines documentation to reflect that all pushes
(both initial and updates) skip the push pipeline.
Co-developed-by: Claude <noreply@anthropic.com>
Signed-off-by: Simon Glass <simon.glass@canonical.com>
When commits have already been applied to the target branch via a
different path (with different hashes), cherry-picking fails with
conflicts. Pickman has no way to detect this situation, resulting
in confusing MRs and requiring manual intervention.
Add a signal mechanism so the agent can communicate this status back
to pickman. When the first cherry-pick fails with conflicts, the agent
checks if commits are already present in the target branch by matching
subjects. If all commits are found:
- The agent aborts the cherry-pick and writes a signal file
(.pickman-signal) with 'already_applied' status
- Pickman reads this signal and handles it appropriately:
- Marks commits as 'skipped' in the database
- Updates source position to advance past these commits
- Creates an MR with [skipped] prefix to record the attempt
This allows automated workflows to continue without manual intervention.
Co-developed-by: Claude <noreply@anthropic.com>
Signed-off-by: Simon Glass <simon.glass@canonical.com>
Fix two issues with push-branch:
1. Handle missing remote branch: When pushing a new branch for the
first time, the fetch to update tracking refs fails because the
branch doesn't exist on the remote yet. Handle this by catching
the fetch failure and using regular --force instead of
--force-with-lease for new branches.
2. Detect aborted cherry-picks: When the agent aborts a cherry-pick
(e.g., because commits are already applied), it may delete the
branch. Verify the branch exists after the agent runs to avoid
attempting to push a non-existent branch.
Cover-letter:
pickman and CI improvements
This series contains pickman enhancements and a CI fix:
- Process MRs oldest first to handle rebases chronologically
- Add --run-ci option for push-branch to trigger pipelines after rebase
- Fix force-push tracking ref issues by fetching before push
- Handle edge cases: new branches and agent-aborted cherry-picks
- Push master to GitHub for ReadTheDocs documentation rebuilds
END
Co-developed-by: Claude <noreply@anthropic.com>
Signed-off-by: Simon Glass <simon.glass@canonical.com>
When using --force-with-lease with an HTTPS URL (instead of a remote
name), git cannot find the tracking refs automatically. This causes
"stale info" errors when the local tracking ref is out of date.
Fix by fetching the branch first to update the tracking ref before
pushing.
Co-developed-by: Claude <noreply@anthropic.com>
Signed-off-by: Simon Glass <simon.glass@canonical.com>
Add a --run-ci flag to push-branch that triggers the CI pipeline instead
of skipping it. This is needed when pushing rebased branches where the
MR pipeline should run to verify the changes.
Update the review agent prompt to use --run-ci when pushing after
rebase operations.
Co-developed-by: Claude <noreply@anthropic.com>
Signed-off-by: Simon Glass <simon.glass@canonical.com>
Sort merge requests by created_at ascending so older MRs are processed
before newer ones. This ensures that MRs needing rebase are handled in
chronological order.
Co-developed-by: Claude <noreply@anthropic.com>
Signed-off-by: Simon Glass <simon.glass@canonical.com>
Add support for pushing branches using the GitLab API token, so commits
appear as coming from the token owner (e.g., a bot account) rather than
the user's configured git credentials.
Add get_push_url() function that creates a token-authenticated URL in
the format https://oauth2:TOKEN@host/project.git. Update push_branch()
to use this URL when available.
Add a new 'push-branch' command that the agent can use for pushing:
./tools/pickman/pickman push-branch <branch> -r <remote> [-f]
Update the review agent prompt to use this command instead of direct
git push, ensuring all pickman commits come from the same account.
Series-to: concept
Cover-letter:
pickman: Improvements for robustness and automation
These changes improve pickman's robustness when interrupted, add the
ability to skip problematic MRs during review, support multiple
concurrent MRs, and ensure all commits come from the pickman bot
account.
This series adds several improvements to pickman:
- Fix rebase-detection for open MRs
- Add skip/unskip support via review comments (pickman: skip/unskip)
- Skip already-processed commits to handle interruptions gracefully
- Handle 409 errors when MR already exists
- Document the commit-selection algorithm and skip feature
- Add --max-mrs option for parallel MR creation
- Run a CI pipeline after review-comment changes
- Use GitLab API token for push authentication (bot-account support)
END
Co-developed-by: Claude <noreply@anthropic.com>
Signed-off-by: Simon Glass <simon.glass@canonical.com>
Remove ci.skip from the push command in the review agent prompt. When
pushing changes after addressing review comments, a new pipeline should
run to verify the changes.
The ci.skip option is still used in gitlab_api.push_branch() for
initial MR creation, since the MR creation itself triggers a pipeline.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-developed-by: Claude <noreply@anthropic.com>
Add a --max-mrs option to the step and poll commands to allow multiple
MRs to be open simultaneously. Previously, step would block creating
new MRs if any were pending. Now it checks if the count of active
(non-skipped) MRs is below the limit before creating a new one.
The default is 5 MRs, allowing parallel review of multiple cherry-pick
sets while still preventing unbounded growth.
Each MR targets master independently, so there's no complex retargeting
needed when earlier MRs merge.
Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com>
Signed-off-by: Simon Glass <simon.glass@canonical.com>
Add two new sections to README.rst:
- Commit Selection: Explain how pickman groups commits into MRs
based on merge commits, with a concrete example
- Skipping MRs: Document the skip/unskip comment commands
Co-developed-by: Claude <noreply@anthropic.com>
Signed-off-by: Simon Glass <simon.glass@canonical.com>
If pickman crashes or is interrupted after pushing a branch but before
recording the MR, the next run would try to create an MR for the same
branch and receive a 409 Conflict error. Handle this gracefully by
finding the existing MR and returning its URL.
Add MrCreateError exception class to allow testing without importing
the gitlab module.
Co-developed-by: Claude <noreply@anthropic.com>
When determining the next commits to cherry-pick in get_next_commits(),
check the database and skip commits that are already tracked. This
prevents re-processing if pickman is interrupted after adding commits to
the database but before updating the source position.
The key changes:
- Build list of all merge commits on first-parent chain
- For each merge, get its commits and filter out those in database
- If any unprocessed commits remain, return them
- Otherwise skip to the next merge
- Finally check for remaining commits after all merges
Co-developed-by: Claude <noreply@anthropic.com>
Split the for loop body into a separate function to reduce complexity
and improve readability. The new process_single_mr() function handles
all processing for a single MR including skip/unskip handling, rebase,
and comment addressing.
Co-developed-by: Claude <noreply@anthropic.com>
Allow reviewers to skip an MR by commenting "pickman skip" (or
"pickman: skip", "@pickman skip"). When skipped, the MR title is
updated to include "[skipped]" and pickman continues creating new
MRs for subsequent cherry-picks.
To resume processing a skipped MR, comment "pickman unskip" which
removes the [skipped] tag from the title.
Skipped MRs are ignored for rebase processing and don't block
creation of new MRs.
Fix some double-quotes while here.
Co-developed-by: Claude <noreply@anthropic.com>
Signed-off-by: Simon Glass <simon.glass@canonical.com>
The GitLab list() API returns inaccurate detailed_merge_status values.
Fetch the full MR details for open MRs to get accurate rebase status.
Co-developed-by: Claude <noreply@anthropic.com>
Signed-off-by: Simon Glass <simon.glass@canonical.com>
Add pylint disable comments for too-many-arguments, too-many-locals,
and too-many-branches warnings on functions that necessarily have
complex logic due to the nature of building agent prompts and
processing MR reviews.
Co-developed-by: Claude <noreply@anthropic.com>
Extract build_review_context() and build_review_prompt() from
run_review_agent() to reduce function complexity and improve
readability.
Fix the use of 'crosfw' while we are here.
Co-developed-by: Claude <noreply@anthropic.com>
Check GitLab merge request status to detect when a rebase is needed,
even if there are no review comments. This handles the case where
GitLab shows "Merge request must be rebased" or "Merge conflicts must
be resolved".
Add has_conflicts and needs_rebase fields to PickmanMr namedtuple,
populated from GitLab's detailed_merge_status and diverged_commits_count
fields.
Update process_mr_reviews() to trigger the agent for rebase when needed,
even without review comments. The agent prompt is adjusted based on
whether it needs to handle comments, rebase, or both.
Also:
- Pass MR description to agent for context from previous work
- After processing reviews, restore the original branch
- Use -o ci.skip when pushing to avoid duplicate pipelines
Series-to: concept
Series-links: 2:81 1:80
Cover-letter:
pickman: Improve handling of rebasing
This little series includes a few patches to ensure that rebases are
made against -master and detection for needing a rebase without the user
having explicitly request it.
It also passes the previous context to the agent when handling reviews,
which should improve its response.
END
Co-developed-by: Claude <noreply@anthropic.com>
Signed-off-by: Simon Glass <simon.glass@canonical.com>
Series-version: 2
Add git fetch to ensure pickman uses the latest remote state:
- Before creating a new MR, fetch the remote first
- Before processing review comments, fetch the remote
This ensures that when creating MRs, the base branch (ci/master) is
up to date, and when the agent handles review comments that request
a rebase, it has the latest commits available.
Also update the agent prompt to use the configured remote/target for
rebase operations instead of a hardcoded value.
Co-developed-by: Claude <noreply@anthropic.com>
Signed-off-by: Simon Glass <simon.glass@canonical.com>
Add pickman test suite to the existing tool test job alongside binman,
buildman, dtoc and patman tests.
Series-to: concept
Cover-letter:
pickman: Refine the feature set
This series adds a few more features and tweaks to make pickman work
better in practice.
The main issue is that the branch logic is not correct, so it selects
the wrong branch once one has already been applied.
Other improvements in this series:
- stop polling if an error occurs
- allow using a config file for the gitlab credentials (so we can set up
a 'pickman' user), along with a command to check gitlab access
- add the Claude log to the merge request, when it responds to comments
- maintain comments in a database so we know what was addressed
- add a command to figure out how many merges are pending and to show
the next 10
- measure test coverage, improve it (not 100%) and add to CI
END
Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com>
Signed-off-by: Simon Glass <simon.glass@canonical.com>
Remove SJG_LAB CI variable from push since lab tests are now
triggered automatically for MR pipelines via .gitlab-ci.yml rules.
Keep ci.skip to avoid running a pipeline on push; the MR pipeline
will run when the merge request is created.
Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com>
Signed-off-by: Simon Glass <simon.glass@canonical.com>
Add check-gitlab command to verify GitLab permissions for the
configured token. This helps diagnose issues like 403 Forbidden
errors when trying to create merge requests.
Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com>
Signed-off-by: Simon Glass <simon.glass@canonical.com>
Add support for storing a GitLab API token in ~/.config/pickman.conf
instead of an environment variable. This allows using a dedicated bot
account for pickman without affecting the user's personal GitLab
credentials.
Config file format:
[gitlab]
token = glpat-xxxxxxxxxxxxxxxxxxxx
This falls back to GITLAB_TOKEN environment variable if config file is
not present.
Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com>
Signed-off-by: Simon Glass <simon.glass@canonical.com>
Change poll to stop on errors rather than continuing, since errors
like permission denied are not recoverable by retrying.
Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com>
Signed-off-by: Simon Glass <simon.glass@canonical.com>
Add command to show the total number of merge commits remaining to be
processed from a source branch. This helps track progress through a
large backlog of merges.
Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com>
Signed-off-by: Simon Glass <simon.glass@canonical.com>
Update the review agent to:
- Create local branch with version suffix (-v2, -v3, etc.)
- Force push to original remote branch to update existing MR
- Use --keep-empty when rebasing to preserve empty merge commits
After processing comments:
- Mark them as processed in database to avoid reprocessing
- Update MR description with agent conversation log
- Append review handling to .pickman-history
Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com>
Signed-off-by: Simon Glass <simon.glass@canonical.com>
Add function to update a merge request's description via the GitLab API.
This is used to update the MR with the agent's conversation log after
processing review comments.
Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com>
Signed-off-by: Simon Glass <simon.glass@canonical.com>
Add schema v3 with a processed_comment table to track which MR comments
have been addressed by the review agent. This prevents re-processing
the same comments when running review or poll commands.
Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com>
Signed-off-by: Simon Glass <simon.glass@canonical.com>
The regex for extracting commit hashes from MR descriptions was matching
short numbers like "1" from the conversation log (e.g., "- 1 board built").
Fix by requiring commit hashes to be at least 7 characters, which is the
minimum length for git short hashes.
Shorten the argument name to 'desc' while we are here.
Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com>
Signed-off-by: Simon Glass <simon.glass@canonical.com>
Add a new next-merges command that shows the next N merges to be applied
from a source branch, useful for seeing what's coming up.
Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com>
Signed-off-by: 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>
