Skip to content

portability: Run the lint suite on macOS and Windows - #14705

Open
atharvaHJoshi wants to merge 1 commit into
google:masterfrom
atharvaHJoshi:portability/lint-ci
Open

atharvaHJoshi wants to merge 1 commit into
google:masterfrom
atharvaHJoshi:portability/lint-ci

Conversation

@atharvaHJoshi

@atharvaHJoshi atharvaHJoshi commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Extends tools/lint.sh and the Lint workflow so the source-level checks run on macOS and Windows, not just Ubuntu.

  • tools/lint.sh: detects host OS/arch (Linux, macOS, Windows via Git Bash/MSYS2/Cygwin; x86_64 + arm64) and fetches the matching actionlint, buildifier, and clang-format assets with pinned SHA256 checksums for every OS+arch combination. sha256_of() handles sha256sum vs shasum -a 256; xargs -0 for the codespell pipeline; extract_archive() and make_executable() helpers.
  • .github/workflows/lint.yml: matrix over ubuntu-latest, macos-latest, windows-latest running bash tools/lint.sh.

Verified: full suite passes on Linux (x86_64) natively and inside a real linux/arm64 QEMU container (all four checks PASS); fresh-cache downloads of every darwin/windows asset checksum-validated; workflow itself passes actionlint.

@google-cla

google-cla Bot commented Sep 11, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@github-actions
github-actions Bot requested a review from trantoji September 11, 2026 19:22
@atharvaHJoshi

Copy link
Copy Markdown
Contributor Author

Re-pushing to re-trigger CLA check

@relkochta

Copy link
Copy Markdown
Collaborator

Is the intent here to allow someone developing gVisor natively on macOS/Windows to run the linter locally? That would be fine, but I think running lints on all three OSes in GitHub Actions is overkill, right? I can't imagine clang-format gives different results on different platforms.

@EtiennePerot

Copy link
Copy Markdown
Collaborator

Linters are too much in flux for this to make sense yet. Please rebase next week where the linters will actually start being enforced.
But also, what @relkochta said: this only makes sense if the linters actually do anything different across platforms. Do they?

Rebase the cross-platform lint run onto the current enforced-linter
baseline and rework the portability layer on top of it.

tools/lint.sh now detects the host OS and architecture instead of
hardcoding Linux:
  - Downloads actionlint, buildifier, and clang-format binaries and
    wheels for Linux, macOS, and Windows (both amd64 and arm64 where
    published), with per-platform checksums.
  - clang-tidy remains Linux-only (its compile_commands.json database
    is produced by Bazel).
  - Fixes platform-specific tooling assumptions: sha256sum -> shasum
    on macOS, nproc -> sysctl / NUMBER_OF_PROCESSORS, GNU-only
    xargs -d '\n', and unzip -> Python zipfile on Windows.
  - Handles .exe suffixes and skips chmod'ing on Windows.
  - Locates Python 3 through a python3/python fallback.

The lint job runs on an ubuntu/macos/windows matrix and calls
'tools/lint.sh' directly with the bash shell, since Windows runners
have neither make nor a POSIX default shell. It also forces
core.autocrlf=false before checkout: GitHub Windows runners default
autocrlf to true, which would check out every tracked file as CRLF
and fail every formatting check.
@atharvaHJoshi

Copy link
Copy Markdown
Contributor Author

Rebased onto current master (the enforced-linter baseline) and redone on top of it. There were a few consequences worth noting:

@relkochta — intent. It's not (primarily) about running the linters locally on macOS/Windows; it's CI coverage. tools/lint.sh runs outside Bazel and pins platform binaries by SHA-256, so the lint gate silently assumed Linux (e.g. sha256sum, nproc, GNU xargs -d '\n', unzip, and Linux-only release assets for actionlint/buildifier/clang-format). The matrix is what enforces that this host-side tooling stays portable once the linters are mandated in CI.

@EtiennePerot — "do the linters do anything different across platforms?" After rebasing I found the answer is now yes, in a sense that makes this worth doing, plus one genuine content-level difference:

  • Rebasing onto the enforced baseline added cpplint to the default run and bumped clang-format to 23.1.1, so I updated the mac/win wheels to the current versions and re-verified every new checksum by downloading each artifact.
  • Line endings are a real per-platform lint difference. The previous Windows job failed ~5,000 whole-file gofmt/clang-format diffs across the tree, not the handful the Linux job flagged — GitHub Windows runners default core.autocrlf=true. The workflow now sets core.autocrlf false before checkout so each OS lints identical bytes; without that step, the Windows run would be checking CRLF rather than the tree and would fail wholesale.
  • clang-tidy is kept Linux-only since its compile_commands.json database is Bazel-produced; it's out of scope for make lint anyway (it's in OPTIONAL_CHECKS).

So: the lint rules are identical everywhere, but the runner plumbing differs per platform (checksums, .exe/.zip, nprocsysctl/NUMBER_OF_PROCESSORS, sha256sumshasum, GNU xargs -d, unzip availability, python3 naming), and Windows line-ending handling changes what the linters see. The matrix is the enforcement that keeps both of those things true from here on.

I verified the full suite passes on Linux locally, and validated the Darwin/Windows install paths (download → checksum → extract → rename) with a mocked uname on this machine; the Linux/macOS/Windows CI runs on this branch should show the enforced checks passing across the matrix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants