chore: skip CI Rust build on non-Rust-only changes - #267
Merged
Conversation
`.github/workflows/ci.yml` had no path filter on either trigger, so every pull request ran the full Rust toolchain even when the diff contained no Rust. Both jobs are pure Rust (`cargo fmt`, `cargo clippy`, `cargo test`, and `cargo check --workspace` on the declared MSRV), so on a documentation-only or release-YAML-only PR they compiled the entire tree to prove nothing. PR #264 is the motivating case: it changed only `release.yml`, two composite actions, and `ARCHITECTURE.md`, and the `ci` job was still pending roughly ten minutes later, at which point the PR was merged without it. A check that cannot say anything about the diff still gates attention, and in practice gets ignored, which erodes the signal for the PRs where it does matter. Add a `paths-ignore` blocklist to both the `push` and `pull_request` triggers. A blocklist rather than a `paths` allowlist: an allowlist fails unsafe, because a new crate or workspace member nobody remembered to add would silently stop being built, while a blocklist fails safe, because anything unrecognized still runs the full build and the worst case is a wasted run rather than an unvalidated merge. Both triggers get the same list so the merge commit on `main` does not run the very build its PR had just skipped. GitHub Actions has no YAML anchors, so the list is duplicated with a comment saying to keep the copies in sync. Every entry was verified against the current tree rather than assumed: no file in the workspace uses `include_str!`, `include_bytes!`, or `#[doc = include_str!(...)]`, so no Markdown or `docs/` asset reaches the compiler, and no Rust source reads `docs/`, `LICENSE`, or `NOTICE` at test time. `debian_build.yml` and `launchpad_ppa.yml` trigger only on `workflow_run` and `workflow_dispatch`, so they never had PR-time coverage from this workflow to lose. `debian/**`, `.githooks/**`, and `tools/**` were deliberately left off the list, keeping the blocklist to paths that both change often and are provably inert. `ci.yml` itself is deliberately absent from the list, so a change to the CI definition still validates itself; the sibling workflows are enumerated one by one for the same reason, so a workflow added later is not ignored by default. The header comment records the branch-protection constraint: `main` has no protection today (`gh api repos/lablup/bssh/branches/main/protection` returns 404), so neither job is a required status check, but GitHub does not treat "not run" as "passed", so enabling required checks later means migrating to an always-reporting gate job or to per-step `if:` conditions first. Validated by parsing the workflow with `yaml.safe_load` and by replaying the ignore globs over all 428 tracked files: no `.rs`, `.toml`, `.lock`, or `ci.yml` path matches the filter, every pattern matches something that actually exists, and scenario checks confirm doc-only and release-YAML-only diffs skip while Rust sources, manifests, `benches/`, `tests/`, `crates/`, unlisted new workflows, and mixed diffs all still run both jobs. Refs #265
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
.github/workflows/ci.ymlhad no path filter on either trigger, so every pull request ran the full Rust toolchain even when the diff contained no Rust at all. Both jobs are pure Rust, so on a documentation-only or release-YAML-only PR they compiled the whole tree to prove nothing. This adds apaths-ignoreblocklist to both triggers.What changed
.github/workflows/ci.yml: added an identicalpaths-ignorelist to thepushandpull_requesttriggers, plus a header comment recording the blocklist rationale, the two editing rules, and the branch-protection constraint.Final ignore list as committed:
No other file is touched.
.github/workflows/release.ymland.github/workflows/update_homebrew_formula.ymlappear in the list by path only; their contents are untouched.Approach chosen: option 1, workflow-level
paths-ignoreOption 1 from the issue, chosen because
mainhas no branch protection today (gh api repos/lablup/bssh/branches/main/protectionreturns 404 "Branch not protected", re-verified during implementation), so neithercinormsrvis a required status check and a skipped run blocks nothing. It is the smallest change that fixes the problem, and it costs zero runner seconds rather than the few seconds per PR that a path-detection job would.What must change if required status checks are introduced later. GitHub does not treat "not run" as "passed" for a required check, so the moment either job name is registered as required under branch protection, a doc-only PR would sit un-mergeable forever. This filter must be migrated before such a protection rule goes live, to one of:
paths-ignorefrom the workflow, add a lightweight job whose name is the one registered as the required check, and have itneeds:the conditional Rust jobs. It reports success when the heavy jobs are skipped, so the required check is always satisfied.dorny/paths-filter), and gate the expensive steps on its output withif:. Both jobs always report, at the cost of a few seconds of runner time per PR.The same note is recorded as a comment block at the top of
ci.yml, which is where someone enabling branch protection is most likely to look.Design notes
Blocklist, not allowlist. An allowlist fails unsafe: a new crate, workspace member, or
build.rsthat nobody remembered to add would silently stop being built, and nobody notices until something breaks onmain. A blocklist fails safe: anything unrecognized still runs the full build, and the worst case is a wasted run rather than an unvalidated merge.ci.ymlis deliberately absent from the list, so a change to the CI definition still validates itself. The sibling workflows are enumerated individually rather than globbed as.github/workflows/*.ymlfor the same fail-safe reason: a workflow added later is not ignored by default, and it also keepsci.ymlexcluded by construction rather than by a negation pattern.Both triggers, not just
pull_request. Filtering only pull requests would leave the merge commit onmainrunning the very build its PR had just skipped, which reintroduces the wasted run one step later.'**.md'rather than'**/*.md'.'**.md'is GitHub's own documented pattern for "all Markdown files in the repository" and unambiguously covers root-level files;'**/*.md'depends on**/matching zero path segments. Both cover the same 24 files in this tree, so the documented form was chosen.The Markdown entry intentionally takes precedence over the
crates/andtests/carve-outs.crates/bssh-russh-sftp/README.mdandtests/pdsh_compat/README.mdare ignored. That is safe only because nothing in the workspace usesinclude_str!,include_bytes!, or#[doc = include_str!(...)], verified by grep acrosssrc/,tests/,benches/, andcrates/, and because there is nobuild.rs. If Markdown is ever embedded into the binary, this entry must be narrowed. That condition is written into the workflow comment.Each entry verified against the current tree, not copied from the issue. All nine patterns match at least one tracked file. No Rust source reads
docs/,LICENSE, orNOTICEat test time.debian_build.ymlandlaunchpad_ppa.ymltrigger only onworkflow_runandworkflow_dispatch, so they never had PR-time coverage from this workflow to lose.debian/**,.githooks/**, andtools/**were considered and deliberately left off: they are inert forcargo, but they change rarely, and keeping the blocklist to paths that both change often and are provably inert limits the number of chances to get an entry wrong.Verification
Static verification, all passing:
.github/workflows/ci.ymlparses underyaml.safe_load; both jobs (ci,msrv) and their display names are unchanged.pushandpull_requestignore lists are byte-identical..rs,.toml,.lock, orci.ymlpath matches. 42 files match, and every one is Markdown, adocs/asset,LICENSE,NOTICE, a composite action, or one of the four listed sibling workflows.docs/**-only, and release-YAML-plus-composite-action diffs skip. Rust sources,Cargo.toml/Cargo.lock,benches/,tests/*.rs,crates/**/src, an unlisted new workflow, a new crate at an unknown path,debian/**, and any mixed doc-plus-Rust diff all still run both jobs.No Rust build was run, because this change cannot affect one.
Live verification is only half complete
This PR touches
.github/workflows/ci.yml, which the issue requires not be ignored. So this PR proves the positive half of acceptance criterion 5 only: a PR touchingci.ymlstill starts both jobs. The job results observed on this PR are recorded in a comment below.The negative half, that a doc-only PR skips both jobs, is not verified and is not claimed. Proving it requires opening a separate throwaway PR, which is outside this change's scope. The test that would confirm it: open a PR against
mainwhose diff touches onlyREADME.md(or any file underdocs/), and confirm the Checks tab shows noCIand noMSRVrun at all, not a skipped or passing one. The static glob replay above predicts this, but prediction is not observation.Test plan
python3 -c "import yaml; yaml.safe_load(open('.github/workflows/ci.yml'))"parses cleanlypushandpull_requestignore lists verified identicalinclude_str!,include_bytes!,#[doc = include_str!], orbuild.rsanywhere in the workspacemainhas no branch protection (404)ciandmsrvobserved running on this PR (positive half of AC5)Closes #265