-
Notifications
You must be signed in to change notification settings - Fork 3.2k
feat: auto-fix license files on PRs and improve CI reliability #1583
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
The licenses script now: - Generates separate license reports per GOOS/GOARCH combination - Groups identical reports together (comma-separated arch names) - Adds a Table of Contents at the top of each platform file - Handles cases where different architectures have different dependencies (e.g., x/sys/unix vs x/sys/windows, mousetrap on Windows only) This addresses the issue discovered in cli/cli where some deps changed which changed the mod graph for different GOARCH and affected the exported licenses because go-licenses tries to find common ancestors.
📜 License files updatedI noticed the third-party license files were out of date and pushed a fix to this PR. What changed: Dependencies were added, removed, or updated, which requires regenerating the license documentation. What I did: Ran Please pull the latest changes before pushing again. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR enhances the license automation workflow by automatically regenerating and committing license files when dependency changes are detected in pull requests, eliminating manual intervention. It also pins the go-licenses tool version for CI reproducibility and excludes third-party dependencies from CodeQL scanning.
Key changes:
- Transforms license-check workflow from fail-only to auto-fix with PR comments
- Pins go-licenses to v2.0.1 (commit hash) in CI environments while keeping @latest for local development
- Excludes third-party directory and license markdown files from CodeQL analysis
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
.github/workflows/license-check.yml |
Complete workflow rewrite: now regenerates license files, commits fixes to PR branch, and posts explanatory comment instead of just failing |
script/licenses |
Adds conditional go-licenses version pinning based on CI environment variable |
script/licenses-check |
Adds conditional go-licenses version pinning and improves error messages with platform-specific context |
.github/workflows/code-scanning.yml |
Adds paths-ignore configuration to exclude third-party directory and license files from CodeQL scanning |
Address review feedback: - Remove bash 4.0+ associative array requirement for macOS compatibility - Add cross-platform hash function (md5sum on Linux, md5 on macOS) - Ensure deterministic iteration order using sorted groups file - Add better error handling for failed go-licenses commands - Fix grammar: 'architecture(s)' -> 'architectures' - Add documentation for third-party/ being a union of all architectures - Use file-based state instead of associative arrays for portability
e382970 to
a27f96f
Compare
📜 License files updatedI noticed the third-party license files were out of date and pushed a fix to this PR. What changed: Dependencies were added, removed, or updated, which requires regenerating the license documentation. What I did: Ran Please pull the latest changes before pushing again. |
29a3e49 to
c5c0934
Compare
bdc44fa to
ee28e6d
Compare
c5c0934 to
be0560b
Compare
ee28e6d to
8855c2c
Compare
- Check now regenerates using ./script/licenses and compares - Add GOROOT/PATH setup in CI to fix go-licenses module info errors - Check both license files AND third-party directory for changes - See: google/go-licenses#244
8855c2c to
eb7d73c
Compare
be0560b to
fca825e
Compare
The sort command uses locale-specific ordering which can differ between systems. Use LC_ALL=C to ensure consistent ordering in CI and locally.
Changes: - Pin go-licenses version in CI for reproducibility (commit 5348b744) - Add GOROOT/PATH setup for 'Package does not have module info' fix - Update license-check.yml to auto-fix and push to PR branches - Add CI=true env var to use pinned go-licenses version - Add dependabot exclusion from auto-fix workflow - Add code-scanning exclusion for third-party files
fca825e to
58f49f5
Compare
After the bot pushes license fixes, check if the PR now only contains license file changes. If so, close it automatically with a comment explaining that the license updates are complete. This prevents stale PRs from accumulating when someone creates a PR just to fix licenses, or when all other changes were already merged to the base branch.
Creates stacked PRs to fix license issues: - Detects when a PR needs license updates - Creates child PR: main <- PR:feature <- PR:license-fix - Tracks PRs with metadata and hash of license changes - Auto-closes if user fixes licenses manually - Auto-closes and recreates if dependencies change - Prevents multiple fix PRs for same base PR Rules: - Only targets PRs against main (not stacked PRs) - Only runs on ready-for-review PRs (not drafts) - Skips bots and forks - Hash-based detection avoids unnecessary work
Dependabot PRs frequently need license updates and can't be merged until fixed. The auto-fix workflow helps by creating a child PR with the license changes, making it easy to merge both together.
- Remove dependabot exclusion (we want to support dependabot PRs) - Comment indentation already fixed - CI env var already set for reproducibility
Moved the 'targets main' check from job if: to workflow on.pull_request.branches. This prevents the workflow from even triggering for PRs targeting other branches, saving CI resources. Draft check is implicit in the types list (opened + ready_for_review). Fork check must stay in if: condition (can't be filtered in on: block).
Combines both workflows into one with two jobs: 1. license-check: Checks licenses, fails if needed, sets outputs 2. auto-create-fix-pr: Creates child PR if needed (only for non-forks) Benefits: - Single workflow file, easier to maintain - Check fails (blocks merge) while still creating helpful fix PR - Fork detection in first job, second job skips for forks - Hash-based tracking prevents duplicate PRs
Much simpler workflow: 1. Always try to auto-commit fix directly to PR branch 2. If push fails (fork without permissions), comment once with instructions 3. Don't create child PRs - just fix in place or give instructions 4. Only comment if not already commented (prevent spam) 5. Always fail check if licenses need updating Benefits: - Much simpler - single job - No child PR management complexity - Clear UX: either fixed or instructed - Works for all PRs (internal/fork/dependabot)
Summary
Improves license file automation to proactively fix license files on PRs, eliminating the common "license check failed, please run script/licenses" workflow friction.
Changes
1. Auto-Commit Workflow
The
license-check.ymlworkflow now:2. Deterministic License Generation
LC_ALL=C sortthroughout script/licenses for locale-independent sorting3. CI Reproducibility
go-licensesversion in CI viaCI=trueenvironment variableBehavior
Internal PRs (or with maintainer permissions)
Fork PRs (without maintainer permissions)
All Cases
Technical Details
Testing
Tested with:
LC_ALL=Censures deterministic sortingResolves
Fixes the manual "run script/licenses" workflow friction while keeping simple, predictable behavior.