Add reproducibility checks to CI workflows#1222
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR updates two GitHub Actions workflows (analysis.yml and build.yml) to pin Flutter versions via a shared FLUTTER_VERSION environment variable and introduces reproducibilityCheck jobs. These jobs enforce lockfile consistency ( Changes
Sequence Diagram(s)sequenceDiagram
participant Trigger as Workflow Trigger
participant RepCheck as reproducibilityCheck
participant Git as Git Repo
participant Flutter as Flutter Tooling
participant Build as buildAndroid/Linux/Windows
Trigger->>RepCheck: start (tag or push)
RepCheck->>Git: validate tag vs pubspec/build_data (build.yml only)
RepCheck->>Flutter: pub get --enforce-lockfile
RepCheck->>Git: diff check pubspec.yaml/pubspec.lock
RepCheck->>RepCheck: run patch-jni-build-id.sh
RepCheck-->>Build: pass/fail (needs gate)
Build->>Flutter: setup with FLUTTER_VERSION
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
.github/workflows/analysis.yml (1)
33-48: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated with
build.yml; consider extracting to a shared composite action.The "Check locked dependencies" and "Check JNI build-id patch" steps are byte-for-byte identical to the ones added in
build.yml. Any future change (e.g., patch path or lockfile check tweak) needs to be kept in sync manually in two places.Consider extracting these into a local composite action (e.g.,
.github/actions/reproducibility-check) invoked from both workflows.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/analysis.yml around lines 33 - 48, The “Check locked dependencies” and “Check JNI build-id patch” steps are duplicated in this workflow and in build.yml, so factor them into a shared local composite action and invoke it from both places. Create a reusable action such as reproducibility-check that contains the flutter pub get --enforce-lockfile, git diff --exit-code, scripts/release/patch-jni-build-id.sh, and the jni CMakeLists.txt grep check, then replace the repeated step blocks with a single call to that action..github/workflows/build.yml (2)
66-81: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReproducibility checks duplicate logic likely present in analysis.yml.
Per the PR objectives/stack, analysis.yml gets an equivalent
reproducibilityCheckjob validating the lockfile and JNI build-id patch. Maintaining the same bash logic in two workflow files risks drift (e.g., one gets updated, the other doesn't). Consider extracting the "Check locked dependencies" and "Check JNI build-id patch" steps into a reusable composite action or a shared workflow invoked viaworkflow_call.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/build.yml around lines 66 - 81, The reproducibility validation in the workflow duplicates the same lockfile and JNI build-id checks used elsewhere, so the two workflow definitions can drift. Move the shared logic from the “Check locked dependencies” and “Check JNI build-id patch” steps into a reusable composite action or a workflow called via workflow_call, then have both this workflow and reproducibilityCheck invoke that shared entrypoint instead of keeping separate bash implementations.
73-81: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant post-patch verification.
scripts/release/patch-jni-build-id.shalready fails (exit 1) if noCMakeLists.txtmatchingjni-*is found under the pub cache, per its own internal checks. The extragrep -R+ exit here largely duplicates that guarantee. Not harmful, just extra surface to maintain.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/build.yml around lines 73 - 81, The “Check JNI build-id patch” workflow step is duplicating validation that scripts/release/patch-jni-build-id.sh already performs internally. Update the build.yml step to rely on the script’s own failure behavior and remove the extra pub cache grep-based post-check, keeping only the direct script invocation in the Check JNI build-id patch job.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/analysis.yml:
- Around line 22-27: The checkout step in the reproducibilityCheck job is
missing the same credential hardening used elsewhere, so update the
actions/checkout invocation to disable credential persistence. Add
persist-credentials: false alongside the existing fetch-depth and submodules
settings in the checkout step so the job does not leave the GITHUB_TOKEN on
disk.
- Around line 14-48: Add an explicit least-privilege permissions block for the
analysis workflow/reproducibilityCheck job so the default GitHub token scopes
are not used. Update the workflow definition near the existing
`jobs.reproducibilityCheck` setup to grant only the minimal access needed for
`actions/checkout` and the local validation steps, using a restrictive
`permissions` entry rather than relying on defaults.
---
Nitpick comments:
In @.github/workflows/analysis.yml:
- Around line 33-48: The “Check locked dependencies” and “Check JNI build-id
patch” steps are duplicated in this workflow and in build.yml, so factor them
into a shared local composite action and invoke it from both places. Create a
reusable action such as reproducibility-check that contains the flutter pub get
--enforce-lockfile, git diff --exit-code, scripts/release/patch-jni-build-id.sh,
and the jni CMakeLists.txt grep check, then replace the repeated step blocks
with a single call to that action.
In @.github/workflows/build.yml:
- Around line 66-81: The reproducibility validation in the workflow duplicates
the same lockfile and JNI build-id checks used elsewhere, so the two workflow
definitions can drift. Move the shared logic from the “Check locked
dependencies” and “Check JNI build-id patch” steps into a reusable composite
action or a workflow called via workflow_call, then have both this workflow and
reproducibilityCheck invoke that shared entrypoint instead of keeping separate
bash implementations.
- Around line 73-81: The “Check JNI build-id patch” workflow step is duplicating
validation that scripts/release/patch-jni-build-id.sh already performs
internally. Update the build.yml step to rely on the script’s own failure
behavior and remove the extra pub cache grep-based post-check, keeping only the
direct script invocation in the Check JNI build-id patch job.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d62951c9-3fa3-4e6c-b5fa-4fe22c853e9d
📒 Files selected for processing (2)
.github/workflows/analysis.yml.github/workflows/build.yml
1 participant
Summary
FLUTTER_VERSIONvalue.Testing
Summary by CodeRabbit
New Features
Bug Fixes