Lint cache follow-ups: same-checkout runs abort instead of queueing, and make clean leaves .lint-cache/ #130

Open
opened 2026-08-09 16:49:13 +02:00 by clawbot · 0 comments
Collaborator

Non-blocking follow-ups from the review of PR #128. Neither affects correctness of that change; both are small and belong in one commit.

1. Two concurrent make lint in the same checkout still abort

PR #128 scopes TMPDIR per checkout, which eliminates lock contention between checkouts — the dominant case. Within one checkout the lock is still shared, and golangci-lint aborts after its 5s timeout rather than waiting. Measured: two concurrent make lint in one tree with a cold cache, one exits rc=2 with parallel golangci-lint is running.

The realistic trigger is script/precommit overlapping a make check, which is not exotic.

script/lint's comment currently claims the lock "serialis[es] runs that share one cache". It does not — it aborts. --allow-serial-runners (flagsets.go:59, consumed at run.go:498 to skip the timeout) is the flag that makes that sentence true: it keeps the mutual-exclusion guard and makes the second run queue.

Note this is not --allow-parallel-runners, which removes the guard entirely. PR #128's body and an earlier upstream comment of mine both wrongly described --allow-parallel-runners as the only alternative; that has been corrected on prompts #30.

2. make clean does not remove .lint-cache/

Makefile:43 is rm -rf bin/. The new cache is ~80MB per checkout with no trimming, and this repo's agents work in throwaway worktrees, so it accumulates unboundedly.

Definition of done

  1. script/lint passes --allow-serial-runners so overlapping runs in one checkout queue rather than abort. Verify by launching two concurrent make lint in a single checkout with a cold cache and confirming both exit 0 — the current behaviour is one rc=2, so this is a test that can fail.
  2. The script/lint comment describes what the lock now actually does.
  3. make clean removes .lint-cache/ as well as bin/. Confirm a make lint followed by make clean leaves no .lint-cache/.
  4. script/lint stays POSIX sh (#!/bin/sh, set -eu, no bashisms); sh -n clean.
  5. Cross-checkout isolation from PR #128 must not regress — re-run a concurrent multi-checkout probe and confirm no run reports a path beginning with ../ or outside its own tree.
  6. make check green; TODO.md updated in the same commit.

Commit title ends with (closes #N).

Constraints

  • Do not modify .golangci.yml (sha256 021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb) or the golangci-lint pin.
  • Do not suppress, narrow, or disable any lint check.
  • Do not rename .lint-cache/ to anything without a leading dot. The dot is load-bearing: TMPDIR lives inside the linted tree, and this is safe only because the Go tool skips dot-prefixed directories when expanding ./.... A rename would silently feed the linter its own temp files. Add a comment saying so.
  • DNS is never mocked in this repository; nothing here touches test behaviour.

Sequencing

Blocked until PR #128 merges — every line referenced here only exists on that branch.

Non-blocking follow-ups from the review of [PR #128](https://git.eeqj.de/sneak/dnswatcher/pulls/128). Neither affects correctness of that change; both are small and belong in one commit. ## 1. Two concurrent `make lint` in the *same* checkout still abort [PR #128](https://git.eeqj.de/sneak/dnswatcher/pulls/128) scopes `TMPDIR` per checkout, which eliminates lock contention *between* checkouts — the dominant case. Within one checkout the lock is still shared, and golangci-lint aborts after its 5s timeout rather than waiting. Measured: two concurrent `make lint` in one tree with a cold cache, one exits `rc=2` with `parallel golangci-lint is running`. The realistic trigger is `script/precommit` overlapping a `make check`, which is not exotic. `script/lint`'s comment currently claims the lock "serialis[es] runs that share one cache". It does not — it aborts. **`--allow-serial-runners`** (`flagsets.go:59`, consumed at `run.go:498` to skip the timeout) is the flag that makes that sentence true: it keeps the mutual-exclusion guard and makes the second run *queue*. Note this is **not** `--allow-parallel-runners`, which removes the guard entirely. [PR #128](https://git.eeqj.de/sneak/dnswatcher/pulls/128)'s body and an earlier upstream comment of mine both wrongly described `--allow-parallel-runners` as the only alternative; that has been corrected on [prompts #30](https://git.eeqj.de/sneak/prompts/issues/30). ## 2. `make clean` does not remove `.lint-cache/` `Makefile:43` is `rm -rf bin/`. The new cache is ~80MB per checkout with no trimming, and this repo's agents work in throwaway worktrees, so it accumulates unboundedly. ## Definition of done 1. `script/lint` passes `--allow-serial-runners` so overlapping runs in one checkout queue rather than abort. Verify by launching two concurrent `make lint` in a single checkout with a cold cache and confirming **both exit 0** — the current behaviour is one `rc=2`, so this is a test that can fail. 2. The `script/lint` comment describes what the lock now actually does. 3. `make clean` removes `.lint-cache/` as well as `bin/`. Confirm a `make lint` followed by `make clean` leaves no `.lint-cache/`. 4. `script/lint` stays POSIX `sh` (`#!/bin/sh`, `set -eu`, no bashisms); `sh -n` clean. 5. Cross-checkout isolation from [PR #128](https://git.eeqj.de/sneak/dnswatcher/pulls/128) must not regress — re-run a concurrent multi-checkout probe and confirm no run reports a path beginning with `../` or outside its own tree. 6. `make check` green; `TODO.md` updated in the same commit. Commit title ends with ` (closes #N)`. ## Constraints - Do not modify `.golangci.yml` (sha256 `021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb`) or the golangci-lint pin. - Do not suppress, narrow, or disable any lint check. - **Do not rename `.lint-cache/` to anything without a leading dot.** The dot is load-bearing: `TMPDIR` lives inside the linted tree, and this is safe only because the Go tool skips dot-prefixed directories when expanding `./...`. A rename would silently feed the linter its own temp files. Add a comment saying so. - DNS is never mocked in this repository; nothing here touches test behaviour. ## Sequencing **Blocked until [PR #128](https://git.eeqj.de/sneak/dnswatcher/pulls/128) merges** — every line referenced here only exists on that branch.
clawbot added this to the 1.0 milestone 2026-08-09 16:49:13 +02:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/dnswatcher#130