Skip to content

Commit 95af54d

Browse files
authored
Unbreak lint: drop vars context that the pinned actionlint rejects (#8490)
## Summary ACTIONLINT has been failing on `main` since #8481, which introduced `vars.PYTORCH_GREENLIGHT_MAX_DIFF_LINES` / `_BYTES` in `greenlight-pr-review.yml`: ``` Error (ACTIONLINT) [expression] undefined variable "vars". available variables are "env", "github", "inputs", "job", "matrix", "needs", "runner", "secrets", "steps", "strategy" 178 | MAX_DIFF_LINES: ${{ vars.PYTORCH_GREENLIGHT_MAX_DIFF_LINES || '2000' }} 179 | MAX_DIFF_BYTES: ${{ vars.PYTORCH_GREENLIGHT_MAX_DIFF_BYTES || '500000' }} ``` `vars` is valid GitHub Actions syntax. The linter is simply stale: `.lintrunner.toml` pins actionlint **1.6.21** via `s3_init_config.json`, and the `vars` context was added in **1.6.24**. ## Why this blocks everything The ACTIONLINT linter's `include_patterns` are `.github/workflows/*.yml` / `*.yaml`, so it lints **every** workflow regardless of what a PR touched. Any PR therefore inherits this failure. Confirmed on `main`: | run | commit | result | |---|---|---| | 31446317087 | `a6fa78f0b9` | failure | | 31444723388 | `0e82c9f495` | failure | | 31443941296 | `982575bb72` ← #8481 | failure | | 31432664642 | `030feb596f` | success | ## The fix Replaces the two lookups with literals carrying the same defaults, plus a comment recording why and how to revert. **Bumping actionlint is the better fix** — `vars` is legitimate and the linter should understand it. I did not do that here because it needs new binaries published to the `oss-clang-format` S3 bucket that `s3_init_config.json` points at, which I can't write to. Worth doing as a follow-up; this unblocks the repo in the meantime. ## Trade-off The runtime override via repo variables is lost until the binary is bumped — tuning the caps becomes a one-line edit rather than a repo setting. Given the alternative is `main` staying red, that seemed the right call, but say the word if you'd rather I wait for an actionlint bump instead. Behaviour is otherwise unchanged: the values were already the defaults, and the step validates them as non-negative integers before use, so nothing downstream is affected. ## Test plan ``` $ grep -rn 'vars\.' .github/workflows/ | grep -v 'env/vars.sh' (only the explanatory comment remains) $ node -e "yaml.load(...)" YAML OK | jobs: [ announce_start, review, record ] sizecheck env: {"MAX_DIFF_LINES":"2000","MAX_DIFF_BYTES":"500000"} ``` actionlint itself was not run locally — the binary is fetched by `lintrunner init` from S3 — so CI is the real check. If `lintrunner` goes green here, that confirms these two lines were the only remaining ACTIONLINT failures. cc @atalman --- 🤖 Authored with assistance from Claude Code.
1 parent 7732fe4 commit 95af54d

1 file changed

Lines changed: 9 additions & 2 deletions

File tree

.github/workflows/greenlight-pr-review.yml

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -175,8 +175,15 @@ jobs:
175175
- name: Decline oversized diffs
176176
id: sizecheck
177177
env:
178-
MAX_DIFF_LINES: ${{ vars.PYTORCH_GREENLIGHT_MAX_DIFF_LINES || '2000' }}
179-
MAX_DIFF_BYTES: ${{ vars.PYTORCH_GREENLIGHT_MAX_DIFF_BYTES || '500000' }}
178+
# Literals rather than `vars.*`: the actionlint pinned in .lintrunner.toml
179+
# is 1.6.21, which predates the `vars` context (added in 1.6.24) and so
180+
# reports it as an undefined variable. That failure is repo-wide, because
181+
# the ACTIONLINT linter checks every workflow rather than only changed
182+
# ones. Restore the `vars.PYTORCH_GREENLIGHT_MAX_DIFF_*` lookups once the
183+
# pinned binary is bumped -- the step already rejects non-integer values,
184+
# so it is safe against a malformed override.
185+
MAX_DIFF_LINES: "2000"
186+
MAX_DIFF_BYTES: "500000"
180187
run: |
181188
set -euo pipefail
182189
# Line count is the primary gate: the model reads only ~2000 lines of the diff, so a

0 commit comments

Comments
 (0)