[Deepin-Kernel-SIG] [linux 6.6-y] [CI] CI: add check depend all workflow#1482
[Deepin-Kernel-SIG] [linux 6.6-y] [CI] CI: add check depend all workflow#1482opsiff wants to merge 2 commits intodeepin-community:linux-6.6.yfrom
Conversation
Reviewer's GuideAdds a GitHub Actions workflow and supporting script to check whether commits in the local kernel tree have corresponding upstream fixes with satisfied dependencies, by scanning commit ranges against torvalds/linux and reporting candidates based on Fixes: tags and subject matching. Sequence diagram for check depend all workflow invoking kernel commit checksequenceDiagram
actor Developer
participant GitHub
participant Actions as GitHub_Actions
participant Job as Job_check_depend_all
participant Runner as Self_hosted_runner
participant Script as check_kernel_commits_sh
participant LocalGit as Local_git_repo
participant RemoteTorvalds as Remote_torvalds_linux
Developer->>GitHub: Push commits or open PR
GitHub-->>Actions: Trigger workflow check depend all
Actions->>Runner: Start job check-depend-all
Runner->>Job: Run job steps
Job->>LocalGit: actions/checkout@v4
Job->>Runner: Configure git user.email and user.name
Job->>Script: Run .github/tools/check_kernel_commits.sh v6.6..torvalds/master head_sha
Script->>LocalGit: git remote add torvalds REMOTE_URL
Script->>LocalGit: git fetch torvalds
Script->>LocalGit: Verify TARGET_BRANCH exists
Script->>LocalGit: git log TARGET_BRANCH subjects to tmp_target
Script->>LocalGit: git log range for commits
Script->>Script: AWK scan Fixes tags and match against target subjects
Script-->>Job: Print matching commits needing dependency checks
Job-->>Actions: Step completes with results
Actions-->>GitHub: Attach logs to workflow run for review
Flow diagram for check_kernel_commits.sh commit dependency analysisflowchart TD
Start["Start script with range and optional branch"] --> ValidateArgs{"Args valid?"}
ValidateArgs -- No --> Usage["Print usage and exit"]
ValidateArgs -- Yes --> SetVars["Set range and TARGET_BRANCH"]
SetVars --> AddRemote["git remote add torvalds REMOTE_URL"]
AddRemote --> FetchRemote["git fetch torvalds"]
FetchRemote --> VerifyBranch{"TARGET_BRANCH exists?"}
VerifyBranch -- No --> ErrorBranch["Print error and exit"]
VerifyBranch -- Yes --> ExportTargets["git log TARGET_BRANCH subjects to tmp_target"]
ExportTargets --> CountTargets["Count target subjects"]
CountTargets --> CountRange["Compute total commits in range"]
CountRange --> PrepareScan["git log range piped into awk"]
subgraph AWK_Processing["AWK commit scanning loop"]
LoadTargets["Load target subjects into array target"] --> NextCommit["Read next commit hash, subject, body"]
NextCommit --> CommitEnd{"Commit end?"}
CommitEnd -- No --> NextCommit
CommitEnd -- Yes --> ProcessCommit
ProcessCommit --> CheckDup{"subject in target?"}
CheckDup -- Yes --> IncDup["Increment dup"] --> Progress["Update progress"] --> NextCommit
CheckDup -- No --> FindFixes["Scan body lines for Fixes: tag"]
FindFixes --> HasFixes{"Found Fixes: ?"}
HasFixes -- No --> IncNoFixes["Increment no_fixes"] --> Progress --> NextCommit
HasFixes -- Yes --> ExtractTitle["Extract ref_title from ('...')"]
ExtractTitle --> CheckRef{"ref_title in target?"}
CheckRef -- No --> IncNoRef["Increment no_ref"] --> Progress --> NextCommit
CheckRef -- Yes --> Match["Increment matched and print hash subject"] --> Progress --> NextCommit
Progress --> LoopEnd{"More commits?"}
LoopEnd -- Yes --> NextCommit
LoopEnd -- No --> Summary["Print summary stats to stderr"]
end
PrepareScan --> AWK_Processing
Summary --> Cleanup["Remove temp files"]
Cleanup --> End["End script"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider parameterizing the upstream repo/branch and the local commit range in both the script and workflow (via inputs/env) so the check can be reused across branches and forks without code changes.
- If the script currently matches against upstream by commit title, it may be more robust to prefer
Fixes:commit hashes or other stable identifiers to reduce false positives from similar or rewritten titles. - It would be helpful if the script’s output clearly distinguishes between hard failures and informational warnings (e.g., via a machine-readable summary or exit codes aligned with the workflow steps), so maintainers can quickly interpret CI results.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider parameterizing the upstream repo/branch and the local commit range in both the script and workflow (via inputs/env) so the check can be reused across branches and forks without code changes.
- If the script currently matches against upstream by commit title, it may be more robust to prefer `Fixes:` commit hashes or other stable identifiers to reduce false positives from similar or rewritten titles.
- It would be helpful if the script’s output clearly distinguishes between hard failures and informational warnings (e.g., via a machine-readable summary or exit codes aligned with the workflow steps), so maintainers can quickly interpret CI results.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
9b100b9 to
c0fbbdb
Compare
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Avenger-285714 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Pull request overview
This PR introduces a new CI workflow to detect kernel commits in the Deepin kernel repository that reference upstream fixes via "Fixes:" tags, and verify whether those referenced commits exist in the upstream Linux kernel. The workflow runs on pull requests, manual triggers, and daily via cron schedule.
Changes:
- Added a GitHub Actions workflow that runs daily and on pull requests to check commit dependencies
- Introduced a bash script that compares local commit titles and "Fixes:" references against upstream torvalds/linux commits
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 10 comments.
| File | Description |
|---|---|
| .github/workflows/check-depend-all.yml | New workflow definition for automated dependency checking with multiple trigger types |
| .github/tools/check_kernel_commits.sh | Bash script with AWK logic to parse git logs and verify Fixes: tag references against upstream |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| check-depend-all: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v6 |
There was a problem hiding this comment.
The workflow uses actions/checkout@v6, but other workflows in the codebase consistently use actions/checkout@v3. This inconsistency should be addressed to maintain uniformity across the repository. For reference, see build-kernel.yml:23, build-kernel-arm64.yml:23, and other workflow files which all use @V3.
There was a problem hiding this comment.
The workflow uses actions/checkout@v6, but other workflows in the codebase consistently use actions/checkout@v3. This inconsistency should be addressed to maintain uniformity across the repository. For reference, see build-kernel.yml:23, build-kernel-arm64.yml:23, and other workflow files which all use @V3.
No. Just all use v6
| - name: "Check Depend for all commit" | ||
| run: | | ||
| # .config | ||
| bash .github/tools/check_kernel_commits.sh v6.6..torvalds/master ${{ github.event.pull_request.head.sha }} |
There was a problem hiding this comment.
The workflow passes a PR head SHA as the second argument to the script, which expects a branch name for TARGET_BRANCH. Looking at the script logic on line 7, this parameter is used to specify which branch to extract commit titles from (used in line 30: 'git log --format="%s" "$TARGET_BRANCH"'). Passing a commit SHA here doesn't align with the script's intended usage. The workflow should either: (1) pass 'torvalds/master' as the second argument if that's the intended target branch, (2) remove the second argument to use the default 'master', or (3) clarify the intended behavior in both the workflow and script.
| bash .github/tools/check_kernel_commits.sh v6.6..torvalds/master ${{ github.event.pull_request.head.sha }} | |
| bash .github/tools/check_kernel_commits.sh v6.6..torvalds/master torvalds/master |
There was a problem hiding this comment.
No , the scripts logic is fomat all log from ${{ github.event.pull_request.head.sha }} to the prev parent and parent commit, you can run 'git log --format="%s" linux-6.6.y', and check fix commit from (v6.6,torvalds/master] which fix the commit in the tree but not contain the fix.
|
|
||
| - name: "Check Depend for all commit" | ||
| run: | | ||
| # .config |
There was a problem hiding this comment.
The comment '# .config' appears to be copied from other workflows where it's relevant (preceding make commands with config files). However, in this context where we're running a check script, this comment is misleading and should either be removed or replaced with a more appropriate comment describing what the check does.
| # .config | |
| # Run dependency check script for all commits in the pull request |
| - name: "Check Depend for all commit" | ||
| run: | | ||
| # .config | ||
| bash .github/tools/check_kernel_commits.sh v6.6..torvalds/master ${{ github.event.pull_request.head.sha }} |
There was a problem hiding this comment.
When the workflow is triggered by a schedule event (daily at 2 AM), the expression '${{ github.event.pull_request.head.sha }}' will be empty/null since there is no pull request associated with scheduled runs. This will result in passing an empty second argument to the script. The script logic should handle this case gracefully, or the workflow should use a conditional expression to pass different values based on the trigger type (e.g., using github.sha for scheduled runs).
| bash .github/tools/check_kernel_commits.sh v6.6..torvalds/master ${{ github.event.pull_request.head.sha }} | |
| bash .github/tools/check_kernel_commits.sh v6.6..torvalds/master ${{ github.event.pull_request.head.sha || github.sha }} |
| - name: "Check Depend for all commit" | ||
| run: | | ||
| # .config | ||
| bash .github/tools/check_kernel_commits.sh v6.6..torvalds/master ${{ github.event.pull_request.head.sha }} |
There was a problem hiding this comment.
The range 'v6.6..torvalds/master' will select commits that are in torvalds/master but not in v6.6, which are upstream commits. However, based on the PR description ("check if upstream have someone fix which commits in our tree"), the intent appears to be checking commits in the local repository against upstream. The range should likely be something like 'v6.6..HEAD' or 'v6.6..origin/main' to examine local commits, not 'v6.6..torvalds/master'. This appears to be a fundamental logic error that would cause the check to analyze the wrong set of commits.
| bash .github/tools/check_kernel_commits.sh v6.6..torvalds/master ${{ github.event.pull_request.head.sha }} | |
| bash .github/tools/check_kernel_commits.sh v6.6..HEAD ${{ github.event.pull_request.head.sha }} |
|
|
||
| local range="$1" | ||
|
|
||
| git remote add torvalds "${REMOTE_URL}" |
There was a problem hiding this comment.
The script attempts to add a remote named 'torvalds' without checking if it already exists. If this workflow runs multiple times or if the remote already exists, the 'git remote add' command will fail with a fatal error. Since the script uses 'set -euo pipefail', this will cause the entire script to exit. Add a check to see if the remote exists before attempting to add it, or use 'git remote add' with error suppression followed by 'git remote set-url' to update it.
| git remote add torvalds "${REMOTE_URL}" | |
| if ! git remote get-url torvalds >/dev/null 2>&1; then | |
| git remote add torvalds "${REMOTE_URL}" | |
| else | |
| git remote set-url torvalds "${REMOTE_URL}" | |
| fi |
There was a problem hiding this comment.
the case is not happened in the case.
There was a problem hiding this comment.
ok, if anyone run it many times, it is useful
| fi | ||
|
|
||
| local tmp_target=$(mktemp) | ||
| local tmp_commits=$(mktemp) |
There was a problem hiding this comment.
The variable tmp_commits is created but never used in the script. This temporary file should either be utilized for its intended purpose or removed to avoid unnecessary resource allocation.
| state = 0 | ||
| } | ||
|
|
||
| /---COMMIT_END---/ { |
There was a problem hiding this comment.
There's an extra space at the beginning of line 60 before the pattern match, creating inconsistent indentation. This should be removed to maintain proper AWK script formatting.
| /---COMMIT_END---/ { | |
| /---COMMIT_END---/ { |
|
|
||
| set -euo pipefail | ||
|
|
||
| REMOTE_NAME="${REMOTE_NAME:-torvalds}" |
There was a problem hiding this comment.
The REMOTE_NAME environment variable is defined but never used in the script. The script hardcodes 'torvalds' as the remote name on line 17. Either use the REMOTE_NAME variable consistently (replace 'torvalds' with '$REMOTE_NAME' on lines 17, 19, etc.), or remove the unused REMOTE_NAME variable definition.
| log_error() { echo -e "\033[31m[ERROR]\033[0m $*" >&2; } | ||
|
|
||
| main() { | ||
| [ $# -lt 1 ] && { echo "Usage: $0 <range> [branch]" >&2; exit 1; } |
There was a problem hiding this comment.
The usage message states 'Usage: $0 [branch]' but the script's parameter naming is potentially confusing. The second parameter is called TARGET_BRANCH but is actually used to specify which branch's commit titles to load for comparison (line 30). Based on the workflow invocation and PR description, it would be clearer to rename this parameter or improve the usage message to explicitly state what each parameter does, e.g., 'Usage: $0 [upstream-branch]' to clarify that the branch parameter is for the upstream comparison target.
| [ $# -lt 1 ] && { echo "Usage: $0 <range> [branch]" >&2; exit 1; } | |
| [ $# -lt 1 ] && { echo "Usage: $0 <commit-range> [upstream-branch]" >&2; exit 1; } |
deepin inclusion category: other check if upstream have someone fix which commits in our tree. Signed-off-by: Wentao Guan <guanwentao@uniontech.com>
deepin inclusion category: other Also remove schedule everyday, if we need we can bring it back. Signed-off-by: Wentao Guan <guanwentao@uniontech.com>
4aa0bf5 to
dedc45f
Compare
deepin inclusion
category: other
check if upstream have someone fix which commits in our tree.
Summary by Sourcery
Add an automated CI workflow to detect kernel commits in this tree that correspond to upstream fixes and ensure their dependencies are satisfied against the upstream Linux master branch.
Build:
CI: