From a91f1446d060e5223f0bcef616ffed703a35a8b1 Mon Sep 17 00:00:00 2001 From: Joshua Yanchar Date: Sun, 14 Jun 2026 22:10:08 -0700 Subject: [PATCH 1/2] ruff: format + lint as a first-class gate Lands the ruff half of the Python toolchain (plan \xc2\xa7B2). Config: [tool.ruff] in //:pyproject.toml \xe2\x80\x94 line-length 100, target py314, src = [meta, tools], select E/F/I/B/UP/SIM/RUF/S. Per-file ignores carve out S603/S607 for meta/scripts (subprocess calls to first-party tools by name) and S101/S105/S106/S108/S311 for tests. Where ruff runs: - Pre-commit: ruff-check --fix then ruff-format (fix order matters; check re-sorts imports, format finishes). - CI: new `ruff` job mirrors golangci-lint\xe2\x80\x99s shape, uses astral-sh/ruff-action with version pinned alongside the Dockerfile ARG. Added to build-and-test-per-target.needs to gate merge. - Devcontainer: installed via uv tool install during image build. UV_TOOL_DIR is forced to /usr/local/share/uv-tools so the root-owned install is reachable by the vscode user. - VS Code: charliermarsh.ruff in extensions.json + devcontainer.json; settings.json sets ruff as the default Python formatter with source.fixAll.ruff + source.organizeImports.ruff as save-time actions. Renovate: new regex matcher for .github/workflows/*.yml picks up the `version:` input under `# renovate: ...` (mirrors the Dockerfile pattern), and a ruff packageRule groups both pins into one PR. Existing meta/scripts/*.py reformatted by `ruff format` and the four real findings fixed (SIM102 nested-if, two E501 long error messages, SIM115 NamedTemporaryFile-without-with). Test files\xe2\x80\x99 RUF005 fixed inline. README: new ruff row in the CI checks table, ruff-check / ruff-format / uv-lock-fresh rows in the pre-commit table, charliermarsh.ruff in the recommended extensions, and a `ruff (diagnostics + format)` row in the on-save table. --- .devcontainer/Dockerfile | 14 +++- .devcontainer/devcontainer.json | 1 + .github/workflows/ci.yml | 16 ++++ .pre-commit-config.yaml | 17 ++++ .vscode/extensions.json | 1 + .vscode/settings.json | 8 ++ README.md | 26 ++++-- meta/scripts/_workspace.py | 9 +- meta/scripts/check_go_modules.py | 19 +++-- meta/scripts/check_go_work.py | 2 +- meta/scripts/check_no_cgo.py | 16 ++-- meta/scripts/smoke_py/smoke_test.py | 2 +- meta/scripts/test__workspace.py | 8 +- meta/scripts/test_check_go_modules.py | 43 +++++----- meta/scripts/test_check_go_work.py | 16 ++-- meta/scripts/test_check_no_cgo.py | 109 +++++++++++++------------ meta/scripts/test_check_secrets_dir.py | 3 +- pyproject.toml | 34 ++++++++ renovate.json | 12 +++ 19 files changed, 238 insertions(+), 118 deletions(-) diff --git a/.devcontainer/Dockerfile b/.devcontainer/Dockerfile index 530efef..f70ca7a 100644 --- a/.devcontainer/Dockerfile +++ b/.devcontainer/Dockerfile @@ -9,11 +9,23 @@ ARG BUILDIFIER_VERSION=v8.5.1 # uv: the workspace's Python package manager. Single source of truth for the # uv.lock + requirements_lock.txt chain consumed by rules_python's pip.parse # and enforced by the `uv-lock-fresh` pre-commit hook. Installed first so -# later PRs can layer `uv tool install ruff ty pre-commit` on top of it. +# later layers can layer `uv tool install ruff ty pre-commit` on top of it. # Pinned tag bumped by Renovate's docker manager (matches the pattern used in # ~/.dotfiles/.devcontainer/Dockerfile). COPY --from=ghcr.io/astral-sh/uv:0.11.21 /uv /uvx /usr/local/bin/ +# ruff: format + lint. Installed via `uv tool install` so the binary lives in +# /usr/local/bin and `ruff` is on PATH for the pre-commit hooks and the VS Code +# extension. UV_TOOL_DIR is forced to a world-readable system location because +# the install runs as root; the default ~/.local/share/uv would land under +# /root (mode 700) and the vscode user could not follow the bin/ symlink. +# Renovate's regex manager tracks RUFF_VERSION via the comment above the ARG. +# renovate: datasource=pypi depName=ruff +ARG RUFF_VERSION=0.15.17 +ENV UV_TOOL_BIN_DIR=/usr/local/bin \ + UV_TOOL_DIR=/usr/local/share/uv-tools +RUN uv tool install --no-cache "ruff==${RUFF_VERSION}" + RUN apt-get update && DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends \ build-essential \ ca-certificates \ diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json index c64d93c..777264f 100644 --- a/.devcontainer/devcontainer.json +++ b/.devcontainer/devcontainer.json @@ -55,6 +55,7 @@ "vscode": { "extensions": [ "bazelbuild.vscode-bazel", + "charliermarsh.ruff", "cnshenj.vscode-task-manager", "esbenp.prettier-vscode", "github.vscode-github-actions", diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1f2525d..d4c637b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -79,6 +79,21 @@ jobs: with: working-directory: ${{ matrix.module }} + # Python's sibling of golangci-lint. Format + lint in one job; config lives in + # `[tool.ruff]` in //:pyproject.toml. Renovate's regex manager tracks the + # version pin via the comment above (matches the Dockerfile pattern). + ruff: + name: ruff + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - uses: astral-sh/ruff-action@0ce1b0bf8b818ef400413f810f8a11cdbda0034b # v4.0.0 + with: + # renovate: datasource=pypi depName=ruff + version: "0.15.17" + args: format --check + - run: ruff check + # Build and test runs once per supported target platform, on a runner whose host matches # the target. (Running tests natively per platform is the only way (without an emulation # layer we do not have) to actually exercise platform-specific code paths and catch regressions @@ -97,6 +112,7 @@ jobs: secrets-check, no-cgo-check, golangci-lint, + ruff, ] strategy: fail-fast: false diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 041c6bf..4d9f148 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -18,6 +18,23 @@ repos: pass_filenames: false files: ^(pyproject\.toml|uv\.lock|requirements_lock\.txt)$ + # ruff check first (with --fix), then ruff format — check's import + # sorting can produce output the formatter wants to retouch. Both fix in + # place; pre-commit re-flags modified files so the user re-stages. + - id: ruff-check + name: ruff check --fix + language: system + entry: ruff check --fix + pass_filenames: false + files: \.py$ + + - id: ruff-format + name: ruff format + language: system + entry: ruff format + pass_filenames: false + files: \.py$ + - id: gazelle name: gazelle language: system diff --git a/.vscode/extensions.json b/.vscode/extensions.json index f26ca4f..e6640d7 100644 --- a/.vscode/extensions.json +++ b/.vscode/extensions.json @@ -1,6 +1,7 @@ { "recommendations": [ "bazelbuild.vscode-bazel", + "charliermarsh.ruff", "esbenp.prettier-vscode", "github.vscode-github-actions", "golang.go", diff --git a/.vscode/settings.json b/.vscode/settings.json index 80dc749..79402f6 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -18,6 +18,14 @@ "prettier.proseWrap": "always", "prettier.trailingComma": "all", "python.languageServer": "Default", + "// Ruff format-on-save": "charliermarsh.ruff surfaces diagnostics inline; this block makes it the default Python formatter so `editor.formatOnSave` calls ruff format, and runs ruff check --fix + import-organize as save-time code actions. Config is read from `[tool.ruff]` in //:pyproject.toml — single source of truth shared with CI + pre-commit.", + "[python]": { + "editor.defaultFormatter": "charliermarsh.ruff", + "editor.codeActionsOnSave": { + "source.fixAll.ruff": "explicit", + "source.organizeImports.ruff": "explicit" + } + }, "// Task Manager favorites": "Pins the Bazel build/test tasks to the cnshenj.vscode-task-manager sidebar (recommended in the devcontainer; see devcontainer.json). Names must match the `label` fields in tasks.json. The `UnnaturalDesigns/Workspace/` prefix is Task Manager's own scoping convention.", "taskManager.favorites": [ "UnnaturalDesigns/Workspace/Bazel Build all", diff --git a/README.md b/README.md index a97297b..03be6a1 100644 --- a/README.md +++ b/README.md @@ -159,6 +159,7 @@ Two GitHub Actions workflows run on every push and pull request to `main`. | Secrets check | Always - verifies the `secrets/` directory contains no committed files | | No-cgo policy check | Always - rejects `import "C"` and transitive deps that compile C/C++/cgo/SWIG | | golangci-lint | After module check passes - runs per Go module | +| ruff | Always - `ruff format --check` and `ruff check` over all Python | | Build and test | After all checks above pass | | Coverage | After build and test - `bazel coverage //...`, uploads merged lcov to Codecov | @@ -181,16 +182,19 @@ each commit. To install: pre-commit install ``` -Only hooks that either fix the problem they detect (`bazel-mod-tidy`, `gazelle`) or prevent unsafe -content from entering the repo (`check-secrets-dir`) run here. Verification-only checks live in the -editor instead (see **Editor integration** below) so they can surface findings without blocking a -commit when you want to switch contexts. +Only hooks that either fix the problem they detect (`bazel-mod-tidy`, `gazelle`, `uv-lock-fresh`, +`ruff-check`, `ruff-format`) or prevent unsafe content from entering the repo (`check-secrets-dir`) +run here. Verification-only checks live in the editor instead (see **Editor integration** below) so +they can surface findings without blocking a commit when you want to switch contexts. -| Hook | Triggers on | -| ------------------- | ----------------------------- | -| `bazel-mod-tidy` | `go.mod`, `go.work`, `go.sum` | -| `gazelle` | `*.go` files | -| `check-secrets-dir` | files under `secrets/` | +| Hook | Triggers on | +| ------------------- | -------------------------------------------- | +| `bazel-mod-tidy` | `go.mod`, `go.work`, `go.sum` | +| `uv-lock-fresh` | `pyproject.toml`, `uv.lock`, `requirements_lock.txt` | +| `ruff-check` | `*.py` files | +| `ruff-format` | `*.py` files | +| `gazelle` | `*.go` files | +| `check-secrets-dir` | files under `secrets/` | **Editor integration** (via `.vscode/`) - runs the non-fixing checks on save. Works in VS Code and VS Code-derived editors (e.g. Google Antigravity). Recommended extensions @@ -198,6 +202,9 @@ VS Code-derived editors (e.g. Google Antigravity). Recommended extensions - [`golang.go`](https://marketplace.visualstudio.com/items?itemName=golang.go) - runs `golangci-lint` on save at package scope, surfacing inline findings that match what CI enforces. +- [`charliermarsh.ruff`](https://marketplace.visualstudio.com/items?itemName=charliermarsh.ruff) - + surfaces `ruff check` diagnostics inline and applies `ruff format` on save, matching what the CI + `ruff` job and the pre-commit hooks enforce. - [`emeraldwalk.runonsave`](https://marketplace.visualstudio.com/items?itemName=emeraldwalk.RunOnSave) - triggers the repo-health scripts on save. - [`ryanluker.vscode-coverage-gutters`](https://marketplace.visualstudio.com/items?itemName=ryanluker.vscode-coverage-gutters) - @@ -206,6 +213,7 @@ VS Code-derived editors (e.g. Google Antigravity). Recommended extensions | On-save check | Triggers on | | -------------------- | ------------------------------------------ | | `golangci-lint` | `*.go` files | +| `ruff` (diagnostics + format) | `*.py` files | | `check-go-modules` | `go.mod`, workflow `.yml`, `.golangci.yml` | | `check-go-work` | `go.mod`, `go.work` | diff --git a/meta/scripts/_workspace.py b/meta/scripts/_workspace.py index bccd2a8..017a7e6 100644 --- a/meta/scripts/_workspace.py +++ b/meta/scripts/_workspace.py @@ -14,7 +14,9 @@ def workspace_root() -> Path: result = subprocess.run( ["git", "rev-parse", "--show-toplevel"], - capture_output=True, text=True, check=True, + capture_output=True, + text=True, + check=True, ) return Path(result.stdout.strip()) @@ -27,8 +29,7 @@ def is_skipped(path: Path) -> bool: that should never contain repo-managed sources. """ return any( - part in _SKIP_DIR_NAMES or part.startswith(_SKIP_DIR_PREFIXES) - for part in path.parts + part in _SKIP_DIR_NAMES or part.startswith(_SKIP_DIR_PREFIXES) for part in path.parts ) @@ -54,7 +55,7 @@ def col_range(file: Path, lineno: int, needle: str) -> tuple[int, int]: line = file.read_text().splitlines()[lineno - 1] start = line.index(needle) + 1 return start, start + len(needle) - except (OSError, IndexError, ValueError): + except OSError, IndexError, ValueError: # bad path / past-EOF lineno / needle-not-on-line all collapse to the same fallback. return 1, 2 diff --git a/meta/scripts/check_go_modules.py b/meta/scripts/check_go_modules.py index dba4dd0..d439ebb 100644 --- a/meta/scripts/check_go_modules.py +++ b/meta/scripts/check_go_modules.py @@ -19,7 +19,7 @@ # where rules_python already makes the import resolvable. sys.path.insert(0, str(Path(__file__).resolve().parents[2])) -from meta.scripts._workspace import col_range, found_modules, workspace_root # noqa: E402 +from meta.scripts._workspace import col_range, found_modules, workspace_root def workflow_module_lists( @@ -52,7 +52,7 @@ def workflow_module_lists( text = workflow_file.read_text() result: list[tuple[str, int, dict[Path, int]]] = [] - state = "scanning" # scanning | in_matrix | in_module + state = "scanning" # scanning | in_matrix | in_module matrix_indent = -1 module_indent = -1 module_key_line = -1 @@ -109,9 +109,8 @@ def workflow_module_lists( module_key_line = lineno current = {} - elif state == "in_module": - if stripped.startswith("- "): - current[Path(stripped[2:].strip())] = lineno + elif state == "in_module" and stripped.startswith("- "): + current[Path(stripped[2:].strip())] = lineno # End of file while still inside a module list. if state == "in_module" and current is not None: @@ -146,7 +145,10 @@ def check_workflow_matrices(root: Path, modules: set[Path]) -> int: for mod in sorted(matrix_set - modules): line = matrix_entries[mod] start, end = col_range(wf_file, line, str(mod)) - print(f"{rel}:{line}:{start}-{end}: [{job_name}] stale matrix entry ./{mod} (no go.mod)") + print( + f"{rel}:{line}:{start}-{end}: " + f"[{job_name}] stale matrix entry ./{mod} (no go.mod)" + ) errors += 1 return errors @@ -174,7 +176,10 @@ def check_golangci_configs(root: Path, modules: set[Path]) -> int: candidate = candidate.parent if not found: # Anchor on the module's go.mod — no specific token at fault. - print(f"{mod}/go.mod:1:1-2: no .golangci.yml reachable from ./{mod} (module dir or any parent up to repo root)") + print( + f"{mod}/go.mod:1:1-2: no .golangci.yml reachable from ./{mod} " + f"(module dir or any parent up to repo root)" + ) errors += 1 return errors diff --git a/meta/scripts/check_go_work.py b/meta/scripts/check_go_work.py index 4ef00ad..db2d0d9 100755 --- a/meta/scripts/check_go_work.py +++ b/meta/scripts/check_go_work.py @@ -16,7 +16,7 @@ # where rules_python already makes the import resolvable. sys.path.insert(0, str(Path(__file__).resolve().parents[2])) -from meta.scripts._workspace import ( # noqa: E402 +from meta.scripts._workspace import ( col_range, found_modules, registered_modules, diff --git a/meta/scripts/check_no_cgo.py b/meta/scripts/check_no_cgo.py index c749833..96fe509 100644 --- a/meta/scripts/check_no_cgo.py +++ b/meta/scripts/check_no_cgo.py @@ -34,7 +34,7 @@ # where rules_python already makes the import resolvable. sys.path.insert(0, str(Path(__file__).resolve().parents[2])) -from meta.scripts._workspace import ( # noqa: E402 +from meta.scripts._workspace import ( find_files, registered_modules, workspace_root, @@ -55,7 +55,7 @@ def find_cgo_in_sources(root: Path) -> list[Path]: for go_file in find_files(root, "*.go"): try: content = go_file.read_text() - except (UnicodeDecodeError, OSError): + except UnicodeDecodeError, OSError: continue if _CGO_IMPORT_RE.search(content): offenders.append(go_file.relative_to(root)) @@ -92,9 +92,7 @@ def find_cgo_in_deps(module_dir: Path) -> list[str]: check=False, ) if result.returncode != 0: - raise RuntimeError( - f"`go list` failed in {module_dir}:\n{result.stderr.strip()}" - ) + raise RuntimeError(f"`go list` failed in {module_dir}:\n{result.stderr.strip()}") return [line.strip() for line in result.stdout.splitlines() if line.strip()] @@ -104,7 +102,7 @@ def check(root: Path) -> int: sources = find_cgo_in_sources(root) if sources: exit_code = 1 - print("Found `import \"C\"` in repo source files:") + print('Found `import "C"` in repo source files:') for path in sources: print(f" {path}") print() @@ -130,9 +128,7 @@ def check(root: Path) -> int: continue if offenders: exit_code = 1 - print( - f"Module //{module_rel} has dependencies that compile C/C++/cgo/SWIG:" - ) + print(f"Module //{module_rel} has dependencies that compile C/C++/cgo/SWIG:") for path in offenders: print(f" {path}") print() @@ -143,7 +139,7 @@ def check(root: Path) -> int: else: print( "Pure-Go policy violation. See docs/future-considerations.md " - "(\"Introducing cgo or Python C-Extensions\") for the implications " + '("Introducing cgo or Python C-Extensions") for the implications ' "and required infrastructure changes." ) diff --git a/meta/scripts/smoke_py/smoke_test.py b/meta/scripts/smoke_py/smoke_test.py index c8c8c27..7a2bba7 100644 --- a/meta/scripts/smoke_py/smoke_test.py +++ b/meta/scripts/smoke_py/smoke_test.py @@ -8,7 +8,7 @@ class PyPiHubReachableTest(unittest.TestCase): def test_third_party_import_resolves(self) -> None: - import requests # noqa: PLC0415 - import inside test is the point + import requests self.assertTrue(hasattr(requests, "get")) diff --git a/meta/scripts/test__workspace.py b/meta/scripts/test__workspace.py index da445b6..d9d9520 100644 --- a/meta/scripts/test__workspace.py +++ b/meta/scripts/test__workspace.py @@ -13,7 +13,6 @@ class TestIsSkipped(unittest.TestCase): - def test_plain_path_not_skipped(self): self.assertFalse(is_skipped(Path("foo/bar.py"))) @@ -38,7 +37,6 @@ def test_substring_match_does_not_skip(self): class TestFindFiles(unittest.TestCase): - def test_no_matches(self): with tempfile.TemporaryDirectory() as tmp: self.assertEqual(find_files(Path(tmp), "*.go"), []) @@ -81,11 +79,9 @@ def test_pattern_matches_exact_filename(self): class TestColRange(unittest.TestCase): - def _write(self, content: str) -> Path: - tmp = tempfile.NamedTemporaryFile(mode="w", delete=False, suffix=".txt") - tmp.write(content) - tmp.close() + with tempfile.NamedTemporaryFile(mode="w", delete=False, suffix=".txt") as tmp: + tmp.write(content) return Path(tmp.name) def test_finds_needle_at_start(self): diff --git a/meta/scripts/test_check_go_modules.py b/meta/scripts/test_check_go_modules.py index 15e7d25..d409097 100644 --- a/meta/scripts/test_check_go_modules.py +++ b/meta/scripts/test_check_go_modules.py @@ -11,7 +11,6 @@ workflow_module_lists, ) - # ── Helpers ──────────────────────────────────────────────────────────────────── @@ -61,9 +60,7 @@ def make_workflow(root: Path, name: str, content: str) -> Path: """ -def make_module_workflow( - root: Path, name: str, jobs: dict[str, list[str]] -) -> Path: +def make_module_workflow(root: Path, name: str, jobs: dict[str, list[str]]) -> Path: """Create a workflow file with one job per entry in jobs, each with a module matrix.""" job_blocks = "".join( _JOB_TEMPLATE.format( @@ -79,7 +76,6 @@ def make_module_workflow( class TestFoundModules(unittest.TestCase): - def test_no_modules(self): with tempfile.TemporaryDirectory() as tmp: self.assertEqual(found_modules(Path(tmp)), set()) @@ -114,7 +110,6 @@ def test_excludes_git_directory(self): class TestWorkflowModuleLists(unittest.TestCase): - def _parse(self, content: str) -> list[tuple[str, int, dict[Path, int]]]: with tempfile.TemporaryDirectory() as tmp: path = Path(tmp) / "workflow.yml" @@ -380,7 +375,6 @@ def test_real_security_yml_shape(self): class TestCheckWorkflowMatrices(unittest.TestCase): - def test_consistent_single_job(self): with tempfile.TemporaryDirectory() as tmp: root = Path(tmp) @@ -393,10 +387,14 @@ def test_consistent_multiple_jobs_same_modules(self): root = Path(tmp) make_module(root, "tools/foo") make_module(root, "libs/bar") - make_module_workflow(root, "security.yml", { - "govulncheck": ["tools/foo", "libs/bar"], - "golangci-lint": ["tools/foo", "libs/bar"], - }) + make_module_workflow( + root, + "security.yml", + { + "govulncheck": ["tools/foo", "libs/bar"], + "golangci-lint": ["tools/foo", "libs/bar"], + }, + ) self.assertEqual(check_workflow_matrices(root, found_modules(root)), 0) def test_module_missing_from_matrix(self): @@ -411,9 +409,13 @@ def test_stale_entry_in_matrix(self): with tempfile.TemporaryDirectory() as tmp: root = Path(tmp) make_module(root, "tools/foo") - make_module_workflow(root, "security.yml", { - "scan": ["tools/foo", "tools/nonexistent"], - }) + make_module_workflow( + root, + "security.yml", + { + "scan": ["tools/foo", "tools/nonexistent"], + }, + ) self.assertEqual(check_workflow_matrices(root, found_modules(root)), 1) def test_missing_and_stale_are_both_counted(self): @@ -429,10 +431,14 @@ def test_two_jobs_both_missing_same_module(self): root = Path(tmp) make_module(root, "tools/foo") make_module(root, "tools/bar") - make_module_workflow(root, "security.yml", { - "govulncheck": ["tools/foo"], - "golangci-lint": ["tools/foo"], - }) + make_module_workflow( + root, + "security.yml", + { + "govulncheck": ["tools/foo"], + "golangci-lint": ["tools/foo"], + }, + ) # tools/bar missing from both matrices = 2 errors self.assertEqual(check_workflow_matrices(root, found_modules(root)), 2) @@ -469,7 +475,6 @@ def test_no_modules_no_matrices(self): class TestCheckGolangciConfigs(unittest.TestCase): - def test_all_modules_have_config(self): with tempfile.TemporaryDirectory() as tmp: root = Path(tmp) diff --git a/meta/scripts/test_check_go_work.py b/meta/scripts/test_check_go_work.py index 38b6d68..b225e7c 100644 --- a/meta/scripts/test_check_go_work.py +++ b/meta/scripts/test_check_go_work.py @@ -23,7 +23,6 @@ def write_go_mod(root: Path, module_path: str) -> None: class TestRegisteredModules(unittest.TestCase): - def test_empty_go_work(self): with tempfile.TemporaryDirectory() as tmp: root = Path(tmp) @@ -67,7 +66,6 @@ def test_toolchain_directive_ignored(self): class TestFoundModules(unittest.TestCase): - def test_no_modules(self): with tempfile.TemporaryDirectory() as tmp: self.assertEqual(found_modules(Path(tmp)), set()) @@ -93,7 +91,6 @@ def test_excludes_bazel_symlinks(self): class TestConsistency(unittest.TestCase): - def test_consistent(self): with tempfile.TemporaryDirectory() as tmp: root = Path(tmp) @@ -125,17 +122,20 @@ def test_stale_use_entry(self): class TestMain(unittest.TestCase): - def _run(self, *, found, registered): """registered may be a list of paths (each defaulting to line 1) or {path: line} dict.""" if isinstance(registered, dict): reg_locs = {Path(p): n for p, n in registered.items()} else: reg_locs = {Path(p): 1 for p in registered} - with mock.patch.object(check_go_work, "workspace_root", return_value=Path("/fake")), \ - mock.patch.object(check_go_work, "found_modules", return_value={Path(p) for p in found}), \ - mock.patch.object(check_go_work, "registered_modules", return_value=reg_locs), \ - mock.patch("sys.stdout", new_callable=io.StringIO) as stdout: + with ( + mock.patch.object(check_go_work, "workspace_root", return_value=Path("/fake")), + mock.patch.object( + check_go_work, "found_modules", return_value={Path(p) for p in found} + ), + mock.patch.object(check_go_work, "registered_modules", return_value=reg_locs), + mock.patch("sys.stdout", new_callable=io.StringIO) as stdout, + ): rc = check_go_work.main() return rc, stdout.getvalue() diff --git a/meta/scripts/test_check_no_cgo.py b/meta/scripts/test_check_no_cgo.py index 3fd42c2..076b67a 100644 --- a/meta/scripts/test_check_no_cgo.py +++ b/meta/scripts/test_check_no_cgo.py @@ -26,7 +26,6 @@ def write(root: Path, rel: str, content: str) -> Path: class TestFindCgoInSources(unittest.TestCase): - def test_no_go_files(self): with tempfile.TemporaryDirectory() as tmp: self.assertEqual(find_cgo_in_sources(Path(tmp)), []) @@ -40,49 +39,37 @@ def test_go_file_without_cgo(self): def test_detects_import_c(self): with tempfile.TemporaryDirectory() as tmp: root = Path(tmp) - write(root, "pkg/foo.go", ( - 'package foo\n' - '\n' - '/*\n' - '#include \n' - '*/\n' - 'import "C"\n' - )) + write(root, "pkg/foo.go", ('package foo\n\n/*\n#include \n*/\nimport "C"\n')) self.assertEqual(find_cgo_in_sources(root), [Path("pkg/foo.go")]) def test_detects_import_c_with_other_imports(self): with tempfile.TemporaryDirectory() as tmp: root = Path(tmp) - write(root, "pkg/foo.go", ( - 'package foo\n' - '\n' - 'import (\n' - ' "fmt"\n' - ')\n' - '\n' - 'import "C"\n' - )) + write(root, "pkg/foo.go", ('package foo\n\nimport (\n "fmt"\n)\n\nimport "C"\n')) self.assertEqual(find_cgo_in_sources(root), [Path("pkg/foo.go")]) def test_does_not_match_string_literal(self): with tempfile.TemporaryDirectory() as tmp: root = Path(tmp) - write(root, "pkg/foo.go", ( - 'package foo\n' - '\n' - 'var s = `import "C"`\n' # backtick string, not a top-level import - )) + write( + root, + "pkg/foo.go", + ( + "package foo\n" + "\n" + 'var s = `import "C"`\n' # backtick string, not a top-level import + ), + ) self.assertEqual(find_cgo_in_sources(root), []) def test_does_not_match_quoted_in_doc_comment(self): with tempfile.TemporaryDirectory() as tmp: root = Path(tmp) - write(root, "pkg/foo.go", ( - 'package foo\n' - '\n' - '// We do not use `import "C"` here.\n' - 'import "fmt"\n' - )) + write( + root, + "pkg/foo.go", + ('package foo\n\n// We do not use `import "C"` here.\nimport "fmt"\n'), + ) self.assertEqual(find_cgo_in_sources(root), []) def test_finds_across_multiple_files(self): @@ -106,12 +93,13 @@ def test_excludes_git_dir(self): self.assertEqual(find_cgo_in_sources(root), []) -def _completed(stdout: str = "", stderr: str = "", returncode: int = 0) -> subprocess.CompletedProcess: +def _completed( + stdout: str = "", stderr: str = "", returncode: int = 0 +) -> subprocess.CompletedProcess: return subprocess.CompletedProcess(args=[], returncode=returncode, stdout=stdout, stderr=stderr) class TestFindCgoInDeps(unittest.TestCase): - def test_empty_output_returns_empty_list(self): with mock.patch.object(subprocess, "run", return_value=_completed(stdout="")) as m: self.assertEqual(check_no_cgo.find_cgo_in_deps(Path("/tmp/m")), []) @@ -143,7 +131,9 @@ def test_strips_whitespace_and_blank_lines(self): ) def test_nonzero_exit_raises(self): - with mock.patch.object(subprocess, "run", return_value=_completed(stderr="boom", returncode=1)): + with mock.patch.object( + subprocess, "run", return_value=_completed(stderr="boom", returncode=1) + ): with self.assertRaises(RuntimeError) as cm: check_no_cgo.find_cgo_in_deps(Path("/tmp/m")) self.assertIn("boom", str(cm.exception)) @@ -151,8 +141,9 @@ def test_nonzero_exit_raises(self): class TestCheck(unittest.TestCase): - - def _patches(self, *, sources=None, deps_by_module=None, modules=("tools/foo",), go_present=True): + def _patches( + self, *, sources=None, deps_by_module=None, modules=("tools/foo",), go_present=True + ): sources = sources or [] deps_by_module = deps_by_module or {} @@ -160,15 +151,23 @@ def fake_find_cgo_in_deps(module_dir: Path): return deps_by_module.get(module_dir.name, []) return [ - mock.patch.object(check_no_cgo, "find_cgo_in_sources", return_value=[Path(p) for p in sources]), + mock.patch.object( + check_no_cgo, "find_cgo_in_sources", return_value=[Path(p) for p in sources] + ), mock.patch.object(check_no_cgo, "find_cgo_in_deps", side_effect=fake_find_cgo_in_deps), - mock.patch.object(check_no_cgo, "registered_modules", return_value={Path(m): 1 for m in modules}), - mock.patch.object(check_no_cgo.shutil, "which", return_value="/usr/bin/go" if go_present else None), + mock.patch.object( + check_no_cgo, "registered_modules", return_value={Path(m): 1 for m in modules} + ), + mock.patch.object( + check_no_cgo.shutil, "which", return_value="/usr/bin/go" if go_present else None + ), ] def _run(self, *, root=Path("/tmp/repo"), **kwargs): - with mock.patch("sys.stdout", new_callable=io.StringIO) as stdout, \ - mock.patch("sys.stderr", new_callable=io.StringIO) as stderr: + with ( + mock.patch("sys.stdout", new_callable=io.StringIO) as stdout, + mock.patch("sys.stderr", new_callable=io.StringIO) as stderr, + ): patches = self._patches(**kwargs) for p in patches: p.start() @@ -215,12 +214,17 @@ def test_source_and_dep_offenders_both_reported(self): def test_runtime_error_in_deps_check_fails(self): def explode(module_dir: Path): raise RuntimeError("simulated `go list` failure") - with mock.patch.object(check_no_cgo, "find_cgo_in_sources", return_value=[]), \ - mock.patch.object(check_no_cgo, "find_cgo_in_deps", side_effect=explode), \ - mock.patch.object(check_no_cgo, "registered_modules", return_value={Path("tools/foo"): 1}), \ - mock.patch.object(check_no_cgo.shutil, "which", return_value="/usr/bin/go"), \ - mock.patch("sys.stdout", new_callable=io.StringIO), \ - mock.patch("sys.stderr", new_callable=io.StringIO) as stderr: + + with ( + mock.patch.object(check_no_cgo, "find_cgo_in_sources", return_value=[]), + mock.patch.object(check_no_cgo, "find_cgo_in_deps", side_effect=explode), + mock.patch.object( + check_no_cgo, "registered_modules", return_value={Path("tools/foo"): 1} + ), + mock.patch.object(check_no_cgo.shutil, "which", return_value="/usr/bin/go"), + mock.patch("sys.stdout", new_callable=io.StringIO), + mock.patch("sys.stderr", new_callable=io.StringIO) as stderr, + ): rc = check_no_cgo.check(Path("/tmp/repo")) self.assertEqual(rc, 1) self.assertIn("simulated `go list` failure", stderr.getvalue()) @@ -235,17 +239,22 @@ def test_go_missing_fails_loudly(self): class TestMain(unittest.TestCase): - def test_main_delegates_to_check_at_workspace_root(self): - with mock.patch.object(check_no_cgo, "workspace_root", return_value=Path("/fake/root")) as wr, \ - mock.patch.object(check_no_cgo, "check", return_value=0) as ck: + with ( + mock.patch.object( + check_no_cgo, "workspace_root", return_value=Path("/fake/root") + ) as wr, + mock.patch.object(check_no_cgo, "check", return_value=0) as ck, + ): self.assertEqual(check_no_cgo.main(), 0) wr.assert_called_once_with() ck.assert_called_once_with(Path("/fake/root")) def test_main_propagates_exit_code(self): - with mock.patch.object(check_no_cgo, "workspace_root", return_value=Path("/fake/root")), \ - mock.patch.object(check_no_cgo, "check", return_value=1): + with ( + mock.patch.object(check_no_cgo, "workspace_root", return_value=Path("/fake/root")), + mock.patch.object(check_no_cgo, "check", return_value=1), + ): self.assertEqual(check_no_cgo.main(), 1) diff --git a/meta/scripts/test_check_secrets_dir.py b/meta/scripts/test_check_secrets_dir.py index 07b57f8..d47a6ea 100644 --- a/meta/scripts/test_check_secrets_dir.py +++ b/meta/scripts/test_check_secrets_dir.py @@ -7,12 +7,11 @@ def run(files: list[str]) -> int: - with patch("sys.argv", ["check_secrets_dir.py"] + files): + with patch("sys.argv", ["check_secrets_dir.py", *files]): return main() class TestCheckSecretsDir(unittest.TestCase): - def test_allowed_file_passes(self): self.assertEqual(run(["secrets/secrets.md"]), 0) diff --git a/pyproject.toml b/pyproject.toml index adef785..fa8d9d5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -31,3 +31,37 @@ required-version = ">=0.9" [tool.uv.workspace] members = [] + +# ruff: format + lint. Single repo-wide config. Per-project pyproject.toml +# files inherit unless they redeclare `[tool.ruff]`. CI runs `ruff format +# --check .` and `ruff check .`; pre-commit runs the fixing variants. +[tool.ruff] +line-length = 100 +target-version = "py314" +# Where first-party code lives — affects import grouping (`I` rules) so internal +# packages sort together. Extend as new top-level Python roots land. +src = ["meta", "tools"] +extend-exclude = [ + "bazel-*", + ".venv", + "venv", +] + +[tool.ruff.lint] +# Strict-but-reasonable. E/F = pycodestyle + pyflakes baseline; I = import sort; +# B = bugbear (real bugs, not style); UP = pyupgrade (modern syntax); SIM = +# code-simplification; RUF = ruff's own catches; S = bandit security rules. +select = ["E", "F", "I", "B", "UP", "SIM", "RUF", "S"] +[tool.ruff.lint.per-file-ignores] +# meta/scripts shell out to first-party tools (`git`, `go`) by name. S607 +# (partial executable path) and S603 (untrusted subprocess input) flag those +# calls, but the executables come from a controlled devcontainer/CI PATH and +# the arguments are not externally controlled — false positives in this +# context. +"meta/scripts/**" = ["S603", "S607"] +# Test conveniences that aren't actual problems in test code: +# S101 = `assert` (the whole point of a test); S105/S106/S311 = throwaway +# "secrets" and non-crypto randomness in fixtures; S108 = stable fake paths +# like "/tmp/repo" used as mock arguments (no actual temp file is created). +"**/test_*.py" = ["S101", "S105", "S106", "S108", "S311"] +"**/*_test.py" = ["S101", "S105", "S106", "S108", "S311"] diff --git a/renovate.json b/renovate.json index 78acd30..de32d47 100644 --- a/renovate.json +++ b/renovate.json @@ -42,6 +42,13 @@ "matchStrings": [ "#\\s*renovate:\\s*datasource=(?[a-z-]+?)\\s+depName=(?\\S+?)\\s*\\n\\w+=(?\\S+)" ] + }, + { + "customType": "regex", + "managerFilePatterns": [ "/^\\.github/workflows/.*\\.ya?ml$/" ], + "matchStrings": [ + "#\\s*renovate:\\s*datasource=(?[a-z-]+?)\\s+depName=(?\\S+?)\\s*\\n\\s*version:\\s*\"(?[^\"]+)\"" + ] } ], "packageRules": [ @@ -61,6 +68,11 @@ "matchManagers": [ "custom.regex", "devcontainer" ], "matchDepNames": [ "go", "python" ], "groupName": "Language toolchain SDKs" + }, + { + "matchDatasources": [ "pypi" ], + "matchDepNames": [ "ruff" ], + "groupName": "ruff" } ] } From 660c95dee1cb433db38b1572a54da8d12e43f95a Mon Sep 17 00:00:00 2001 From: Joshua Yanchar Date: Sun, 14 Jun 2026 22:41:18 -0700 Subject: [PATCH 2/2] ci: pin python to 3.14 in jobs that run meta/scripts directly MODULE.bazel pins rules_python to 3.14 and pyproject.toml targets py314. Ubuntu-24.04 runners still ship 3.12 as `python3`, so the CI sites that invoke a script outside Bazel run on the wrong interpreter. Ruff 0.15.17, correctly honouring target-version=py314, applies the PEP 758 transform `except (A, B, C):` -> `except A, B, C:` to _workspace.py; that\xe2\x80\x99s valid 3.14 syntax but a SyntaxError on 3.12, which is what tanked the four go-* / no-cgo checks on PR 123\xe2\x80\x99s last run. Adds `actions/setup-python@v5` with `python-version: "3.14"` to the five sites that call `python3 meta/scripts/...`. The renovate workflow-yaml matcher is widened to accept `python-version:` alongside `version:` so the new pins are tracked and grouped with the MODULE.bazel python_version under "Language toolchain SDKs". --- .github/workflows/ci.yml | 24 ++++++++++++++++++++++++ .github/workflows/security.yml | 4 ++++ renovate.json | 2 +- 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d4c637b..3868937 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -20,11 +20,23 @@ jobs: buildbuddy-api-key: ${{ secrets.BUILDBUDDY_API_KEY }} - run: bazel run --config=ci //:gazelle -- -mode=diff + # The repo's `meta/scripts/*.py` target Python 3.14 (matches the rules_python + # toolchain pin in MODULE.bazel and the `target-version` in pyproject.toml's + # [tool.ruff]). Ubuntu-24.04 still ships 3.12 as `python3`, so the language- + # toolchain pin needs to be applied explicitly on every job that runs a script + # outside Bazel. The `# renovate:` comment above each `python-version` is + # picked up by the regex matcher in renovate.json and grouped with the + # MODULE.bazel python_version under "Language toolchain SDKs". + go-modules-check: name: Go module completeness check runs-on: ubuntu-latest steps: - uses: actions/checkout@v6 + - uses: actions/setup-python@v5 + with: + # renovate: datasource=python-version depName=python + python-version: "3.14" - run: python3 meta/scripts/check_go_modules.py go-work-check: @@ -32,6 +44,10 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v6 + - uses: actions/setup-python@v5 + with: + # renovate: datasource=python-version depName=python + python-version: "3.14" - run: python3 meta/scripts/check_go_work.py secrets-check: @@ -39,6 +55,10 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v6 + - uses: actions/setup-python@v5 + with: + # renovate: datasource=python-version depName=python + python-version: "3.14" - run: python3 meta/scripts/check_secrets_dir.py no-cgo-check: @@ -46,6 +66,10 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v6 + - uses: actions/setup-python@v5 + with: + # renovate: datasource=python-version depName=python + python-version: "3.14" # `go list -deps` is what surfaces transitive cgo, so the runner needs Go in PATH. # Use go.work as the source-of-truth — it's the workspace-level Go version directive # and matches the version Bazel installs in MODULE.bazel (per-module go.mod files diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index 3febec4..3784aed 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -23,6 +23,10 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v6 + - uses: actions/setup-python@v5 + with: + # renovate: datasource=python-version depName=python + python-version: "3.14" - run: python3 meta/scripts/check_go_modules.py # ── Static Application Security Testing (SAST) ─────────────────────────────── diff --git a/renovate.json b/renovate.json index de32d47..888f10e 100644 --- a/renovate.json +++ b/renovate.json @@ -47,7 +47,7 @@ "customType": "regex", "managerFilePatterns": [ "/^\\.github/workflows/.*\\.ya?ml$/" ], "matchStrings": [ - "#\\s*renovate:\\s*datasource=(?[a-z-]+?)\\s+depName=(?\\S+?)\\s*\\n\\s*version:\\s*\"(?[^\"]+)\"" + "#\\s*renovate:\\s*datasource=(?[a-z-]+?)\\s+depName=(?\\S+?)\\s*\\n\\s*[a-z-]*version:\\s*\"(?[^\"]+)\"" ] } ],