mirror of
https://github.com/koala73/worldmonitor.git
synced 2026-04-25 17:14:57 +02:00
fix(build): pin sebuf plugin via PATH in make generate (#3371)
* fix(build): pin sebuf plugin via PATH in `make generate`
Without this, developers who have an older sebuf protoc-gen-openapiv3
binary installed via a package manager (Homebrew ships v0.7.0 at
/opt/homebrew/bin) hit this failure on a fresh `make generate`:
Failure: file ".../AviationService.openapi.yaml" was generated
multiple times: once by plugin "protoc-gen-openapiv3" and again
by plugin "protoc-gen-openapiv3"
Root cause: `buf.gen.yaml` declares three `protoc-gen-openapiv3`
invocations — the default yaml, a `format=json` variant, and a
`bundle_only=true, strategy: all` unified bundle. Sebuf v0.11.x
honors both `format=json` (emits .json extension) and `bundle_only=true`
(skips per-service emission), so the three invocations write distinct
files. Sebuf v0.7.x does NOT honor either option — it silently emits
the same per-service .yaml filenames from all three plugins and buf
rejects the collision.
`Makefile: install-plugins` installs v0.11.1 (SEBUF_VERSION) to
$HOME/go/bin. But the `generate` target doesn't prepend that to PATH,
so `which protoc-gen-openapiv3` resolves to the stale Homebrew binary
for anyone with both installed.
Verified by `go version -m`:
/opt/homebrew/bin/protoc-gen-openapiv3 — mod sebuf v0.7.0
/Users/eliehabib/go/bin/protoc-gen-openapiv3 — mod sebuf v0.11.1
Fix: prepend $$HOME/go/bin to PATH in the `generate` recipe. Matches
what .husky/pre-push:151-153 already does before invoking this target,
so CI and local behavior converge. No sebuf upstream bug.
* fix(build): follow GOBIN-then-GOPATH/bin for plugin PATH prefix
Reviewer (Codex) on PR #3371: the previous patch hardcoded
\$HOME/go/bin, which is only the default fallback when GOBIN is unset
AND GOPATH defaults to \$HOME/go. On machines with a custom GOBIN or
a non-default GOPATH, `go install` targets a different directory — so
hardcoding \$HOME/go/bin can force a stale binary from there to win
over the freshly-installed SEBUF_VERSION sitting at the actual install
location.
Fix: resolve the install dir the same way `go install` does:
GOBIN first, then GOPATH/bin.
Shell expression: `go env GOBIN` returns an empty string (exit 0) when
unset, so `||` alone doesn't cascade. Using explicit `[ -n "$gobin" ]`
instead.
Also dropped the misleading comment that claimed the pre-push hook
used the same rule — it still hardcodes \$HOME/go/bin. Called that out
in a note, but left the hook alone because its PATH prepend is
belt-and-suspenders (only matters for locating `buf` itself; the
Makefile's own recipe-level prepend decides plugin resolution).
Verified on a machine with empty GOBIN:
resolved → /Users/eliehabib/go/bin
And \`make generate\` succeeds without manual PATH overrides.
* fix(build): use first GOPATH entry for plugin PATH prefix
Reviewer (Codex) on commit 6db0b53c2: the GOBIN-empty fallback used
`$(go env GOPATH)/bin`, which silently breaks on setups where GOPATH
is a colon-separated list. Example:
GOPATH=/p1:/p2
previous code → "/p1:/p2/bin"
^ two PATH entries; neither is the actual install target /p1/bin
`go install` writes binaries only into the first GOPATH entry's bin,
so the stale-plugin case this PR is trying to fix can still bite.
Fix: extract the first entry via `cut -d: -f1`. Matches Go's own
behavior in cmd/go/internal/modload/init.go:gobin(), which uses
filepath.SplitList + [0].
Verified:
- Single-entry GOPATH (this machine) → /Users/eliehabib/go/bin ✓
- Simulated GOPATH=/fake/path1:/fake/path2 → /fake/path1/bin ✓
- make generate succeeds in both cases.
* fix(build): resolve buf via caller PATH; prepend plugin dir only (review P1/P3)
Codex P1: the previous recipe prepended the Go install dir to PATH
before invoking `buf generate`, which also changed which `buf` binary
ran. On a machine with a stale buf in GOBIN/$HOME/go/bin, the recipe
would silently downgrade buf itself and reintroduce version-skew
failures — the exact class of bug this PR was trying to fix.
Fix: two-stage resolution.
1. `BUF_BIN=$(command -v buf)` resolves buf using the CALLER's PATH
(Homebrew, Go install, distro package — whichever the developer
actually runs day-to-day).
2. Invoke the resolved buf via absolute path ("$BUF_BIN"), with a
PATH whose first entry is the Go install dir. That affects ONLY
plugin lookup inside `buf generate` (protoc-gen-ts-*,
protoc-gen-openapiv3) — not buf itself, which was already resolved.
Adds a loud failure when `buf` is not on PATH:
buf not found on PATH — run: make install-buf
Previously a missing buf would cascade into a confusing error deeper
in the pipeline.
Codex P3: added tests/makefile-generate-plugin-path.test.mjs — scrapes
the generate recipe text and asserts:
- `command -v buf` captures buf before the PATH override
- Missing-buf case fails loudly
- buf is invoked via "$BUF_BIN" (absolute path)
- GOBIN + GOPATH/bin resolution is present
- Install-dir prepend precedes $$PATH (order matters)
- The subshell expression resolves on the current machine
Codex P2 (Windows GOPATH semicolon delimiter) is acknowledged but
not fixed here — this repo does not support Windows dev per CLAUDE.md,
the pre-push hook and CI are Unix-only, and a cross-platform
implementation would require a separate Make detection or a
platform-selected helper script. Documented inline as a known
Unix assumption.
Verified:
- `make generate` clean
- `command -v buf` → /opt/homebrew/bin/buf
- protoc-gen-openapiv3 via plugin-PATH → ~/go/bin/protoc-gen-openapiv3
- New test suite 6/6 pass
- npm run typecheck clean
* fix(build): silence recipe comments with @# (review P2)
Codex P2: recipe lines starting with `#` are passed to the shell
(which ignores them) but Make still echoes them to stdout before
execution. Running `make generate` printed all 34 comment lines
verbatim. Noise for developers, and dilutes the signal when the
actual error output matters.
Fix: prefix every in-recipe `#` comment with `@` so Make suppresses
the echo. No semantic change — all comments still read identically
in the source.
Verified: `make generate` now prints only "Clean complete!", the
buf invocation line (silenced with @... would hide the invocation
which helps debugging, so leaving that audible), and "Code
generation complete!".
* fix(build): fail closed when go missing; verify plugin is present (review High)
Codex High: previous recipe computed PLUGIN_DIR from `go env GOBIN` /
`go env GOPATH` without checking that `go` itself was on PATH. When
go is missing:
- `go env GOBIN` fails silently, gobin=""
- `go env GOPATH` fails silently, cut returns ""
- printf '%s/bin' "" yields "/bin"
- PATH becomes "/bin:$PATH" — doesn't override anything
- `buf generate` falls back to whatever stale sebuf plugin is on
PATH, reintroducing the exact duplicate-output failure this PR
was supposed to fix.
Fix (chained in a single shell line so any guard failure aborts):
1. `command -v go` — fail with clear "install Go" message.
2. `command -v buf` — fail with clear "run: make install-buf".
3. Resolve PLUGIN_DIR via GOBIN / GOPATH[0]/bin.
4. `[ -n "$$PLUGIN_DIR" ]` — fail if resolution returned empty
(shouldn't happen after the go-guard, but belt-and-suspenders
against future shell weirdness).
5. `[ -x "$$PLUGIN_DIR/protoc-gen-ts-client" ]` — fail if the
plugin isn't installed, telling the user to run
`make install-plugins`. Catches the case where `go` exists but
the user has never installed sebuf locally.
6. `PATH="$$PLUGIN_DIR:$$PATH" "$$BUF_BIN" generate`.
Verified failure modes:
- go missing → "go not found on PATH — run: ... install-plugins"
- buf missing → "buf not found on PATH — run: make install-buf"
- happy path → clean `Code generation complete!`
Extended tests/makefile-generate-plugin-path.test.mjs with:
- `fails loudly when go is not on PATH`
- `verifies the sebuf plugin binary is actually present before invoking buf`
- Rewrote `PATH override order` to target the new PLUGIN_DIR form.
All 8 tests pass. Typecheck clean.
* fix(makefile): guard all sebuf plugin binaries, not just ts-client
proto/buf.gen.yaml invokes THREE sebuf binaries:
- protoc-gen-ts-client
- protoc-gen-ts-server
- protoc-gen-openapiv3 (× 3 plugin entries)
The previous guard only verified protoc-gen-ts-client was present in
the pinned Go install dir. If the other two were missing from that
dir (or only partially installed by a prior failed `make install-plugins`),
`PATH="$PLUGIN_DIR:$PATH" buf generate` would fall through to whatever
stale copy happened to be earlier on the caller's normal PATH —
exactly the mixed-sebuf-version failure this PR is meant to eliminate.
Fix: iterate every plugin name in a shell `for` loop. Any missing
binary aborts with the same `Run: make install-plugins` remediation
the previous guard showed.
Tests:
- Update the existing plugin-presence assertion to require all three
binaries by name AND the `[ -x "$PLUGIN_DIR/$p" ]` loop pattern.
- Add a cross-reference test that parses proto/buf.gen.yaml, extracts
every `local:` plugin, and fails if any declared plugin is missing
from the Makefile guard list. This catches future drift without
relying on a human to remember the dependency.
Closes the PR #3371 High finding that a `ts-server` or `openapiv3`
missing from $PLUGIN_DIR would silently re-enable the stale-plugin bug.
* fix(pre-push): don't shadow caller's buf with stale ~/go/bin/buf
The proto-freshness hook's unconditional
`export PATH="$HOME/go/bin:$PATH"` defeated the Makefile-side
caller-PATH-first invariant: on machines with both a preferred buf
(Homebrew, /usr/local, etc.) AND an older `go install buf@<old>`
leftover at `~/go/bin/buf`, the prepend placed the stale copy first.
`make generate` then resolved buf via `command -v buf` and picked up
the shadowed stale binary — recreating the mixed-version failure
this PR is meant to eliminate.
Fix:
1. Only prepend `$HOME/go/bin` when buf is NOT already on the caller's
PATH. Now buf's Homebrew/system copy always wins; `~/go/bin/buf` is
a pure fallback.
2. Widen the plugin-presence check to accept either a PATH-resolvable
`protoc-gen-ts-client` OR the default go-install location
`$HOME/go/bin/protoc-gen-ts-client`. `make generate` now resolves
plugins via its own PLUGIN_DIR (GOBIN, then first-entry GOPATH/bin),
so requiring them on PATH is too strict.
3. Drop the redundant plugin-only PATH prepend — the Makefile's own
plugin-path resolution handles it authoritatively.
Tests: add a regression guard that reads the hook, verifies the
prepend is gated on `! command -v buf`, and explicitly asserts the
OLD buggy pattern is not present.
Closes the PR #3371 High finding about the hook's unconditional
prepend defeating the Makefile-side caller-PATH-first invariant.
This commit is contained in:
Elie Habib
@@ -151,10 +151,26 @@ fi
|
||||
|
||||
echo "Running proto freshness check..."
|
||||
if git diff --name-only origin/main -- proto/ src/generated/ docs/api/ Makefile | grep -q .; then
|
||||
if command -v buf >/dev/null 2>&1 || [ -x "$HOME/go/bin/buf" ]; then
|
||||
# Only prepend $HOME/go/bin when buf is NOT already resolvable on the
|
||||
# caller's PATH. An unconditional prepend would shadow a developer's
|
||||
# preferred `buf` (e.g. Homebrew) with a potentially stale
|
||||
# ~/go/bin/buf left over from an older `go install buf@<old>`, which
|
||||
# is the exact mixed-version failure this PR is trying to eliminate.
|
||||
# Plugin resolution is handled inside `make generate` via a pinned
|
||||
# PLUGIN_DIR, so the hook no longer needs to prepend for plugin
|
||||
# discovery — only for the `buf` binary itself, and only when there's
|
||||
# no other candidate.
|
||||
if ! command -v buf >/dev/null 2>&1 && [ -x "$HOME/go/bin/buf" ]; then
|
||||
export PATH="$HOME/go/bin:$PATH"
|
||||
fi
|
||||
if command -v buf &>/dev/null && command -v protoc-gen-ts-client &>/dev/null; then
|
||||
# Plugin-presence check: consider a plugin "available" if it's on
|
||||
# PATH OR at the default `go install` location ($HOME/go/bin).
|
||||
# `make generate` resolves plugins via its own PLUGIN_DIR (GOBIN,
|
||||
# then first-entry GOPATH/bin), which matches the $HOME/go/bin
|
||||
# default on almost every dev setup. Anything exotic (custom GOBIN
|
||||
# not on PATH) still runs `make generate` cleanly because the
|
||||
# Makefile's own plugin-executable guard fires there.
|
||||
if command -v buf &>/dev/null && { command -v protoc-gen-ts-client &>/dev/null || [ -x "$HOME/go/bin/protoc-gen-ts-client" ]; }; then
|
||||
make generate
|
||||
if ! git diff --exit-code src/generated/ docs/api/; then
|
||||
echo ""
|
||||
|
||||
59
Makefile
59
Makefile
@@ -54,7 +54,64 @@ lint: ## Lint protobuf files
|
||||
|
||||
generate: clean ## Generate code from proto definitions
|
||||
@mkdir -p $(GEN_CLIENT_DIR) $(GEN_SERVER_DIR) $(DOCS_API_DIR)
|
||||
cd $(PROTO_DIR) && buf generate
|
||||
@# Ensure the Makefile-declared sebuf protoc plugins ($(SEBUF_VERSION))
|
||||
@# installed by `install-plugins` win over any stale sebuf binary that
|
||||
@# a package manager (Homebrew, etc.) may have placed earlier on PATH.
|
||||
@# Without this, `buf generate` spawns an older sebuf v0.7.x plugin
|
||||
@# that ignores `bundle_only=true` / `format=json` and fails with
|
||||
@# duplicate-output errors.
|
||||
@#
|
||||
@# Two-stage resolution:
|
||||
@#
|
||||
@# 1. Resolve `buf` using the CALLER's PATH so we pick up whatever
|
||||
@# `buf` version was installed via Homebrew / go install / etc. —
|
||||
@# whichever the developer actually runs day-to-day. We do NOT
|
||||
@# want the Go install dir to override the buf binary itself; an
|
||||
@# earlier version of this Makefile did, which could silently
|
||||
@# downgrade `buf` on machines with a stale GOBIN copy.
|
||||
@#
|
||||
@# 2. Invoke the resolved `buf` via absolute path, but give it a
|
||||
@# PATH whose FIRST entry is the Go install dir. This affects
|
||||
@# only plugin lookup inside `buf generate` (protoc-gen-ts-*,
|
||||
@# protoc-gen-openapiv3) — not `buf` itself, which is already
|
||||
@# resolved. Plugins find the Makefile-pinned version first.
|
||||
@#
|
||||
@# Go install dir resolution mirrors `go install`'s own logic:
|
||||
@# GOBIN first, then the FIRST entry of GOPATH + "/bin". `go install`
|
||||
@# writes binaries only into the first GOPATH entry's bin dir — GOPATH
|
||||
@# can be a path-list (colon-separated on Unix, semicolon on Windows),
|
||||
@# so naïvely appending "/bin" to the whole value produces a bogus
|
||||
@# path. The `cut -d:` fallback works on Linux/macOS shells; Windows
|
||||
@# (MSYS/cmd) is not a supported dev platform for this repo, so the
|
||||
@# Unix assumption is acceptable here.
|
||||
@#
|
||||
@# .husky/pre-push still prepends $$HOME/go/bin for the outer shell
|
||||
@# discovering `buf` — that's a broader prepend (it affects the shell's
|
||||
@# command resolution before `make` runs) and is harmless here because
|
||||
@# this recipe resolves `buf` via its own PATH before building the
|
||||
@# plugin PATH.
|
||||
@command -v go >/dev/null 2>&1 || { echo 'go not found on PATH — make generate needs `go` to resolve the sebuf plugin install dir. Install Go, then run: make install-plugins' >&2; exit 1; }
|
||||
@command -v buf >/dev/null 2>&1 || { echo 'buf not found on PATH — run: make install-buf' >&2; exit 1; }
|
||||
@# All plugin-resolution, plugin-presence, and invocation in a single
|
||||
@# shell line chained with `&&` so any failed guard aborts the recipe
|
||||
@# BEFORE `buf generate` runs. `$$(go env GOBIN)` without -n guard
|
||||
@# would silently become "/bin" when unset, failing to override PATH
|
||||
@# and letting a stale sebuf on the normal PATH win — the exact
|
||||
@# failure mode this PR is trying to prevent (Codex high-severity on
|
||||
@# 9c0058a). The plugin-executable check covers ALL three sebuf
|
||||
@# binaries invoked by proto/buf.gen.yaml (protoc-gen-ts-client,
|
||||
@# protoc-gen-ts-server, protoc-gen-openapiv3) — guarding only one
|
||||
@# would let `buf generate` fall through to a stale copy of the
|
||||
@# others on PATH, recreating the mixed-version failure mode. Keep
|
||||
@# this list in sync with proto/buf.gen.yaml.
|
||||
cd $(PROTO_DIR) && \
|
||||
PLUGIN_DIR=$$(gobin=$$(go env GOBIN); if [ -n "$$gobin" ]; then printf '%s' "$$gobin"; else printf '%s/bin' "$$(go env GOPATH | cut -d: -f1)"; fi) && \
|
||||
[ -n "$$PLUGIN_DIR" ] || { echo 'Could not resolve Go install dir from GOBIN/GOPATH — refusing to run buf generate without a pinned plugin location.' >&2; exit 1; } && \
|
||||
for p in protoc-gen-ts-client protoc-gen-ts-server protoc-gen-openapiv3; do \
|
||||
[ -x "$$PLUGIN_DIR/$$p" ] || { echo "$$p not found at $$PLUGIN_DIR/. Run: make install-plugins" >&2; exit 1; }; \
|
||||
done && \
|
||||
BUF_BIN=$$(command -v buf) && \
|
||||
PATH="$$PLUGIN_DIR:$$PATH" "$$BUF_BIN" generate
|
||||
@echo "Code generation complete!"
|
||||
|
||||
breaking: ## Check for breaking changes against main
|
||||
|
||||
222
tests/makefile-generate-plugin-path.test.mjs
Normal file
222
tests/makefile-generate-plugin-path.test.mjs
Normal file
@@ -0,0 +1,222 @@
|
||||
// Regression guard for the `generate` target's plugin-path resolution.
|
||||
//
|
||||
// The Makefile's `generate` recipe must satisfy two invariants:
|
||||
//
|
||||
// 1. `buf` is resolved via the CALLER's PATH. Overriding buf's own
|
||||
// location can silently downgrade the build tool on machines with a
|
||||
// stale binary in GOBIN.
|
||||
// 2. Proto plugins (protoc-gen-ts-*, protoc-gen-openapiv3) resolve
|
||||
// from the Go install dir FIRST — GOBIN when set, otherwise the
|
||||
// first entry of GOPATH + "/bin". This mirrors `go install`'s own
|
||||
// resolution order.
|
||||
//
|
||||
// This suite scrapes the recipe text from the Makefile and asserts the
|
||||
// shell expression matches both invariants. It does not shell out to
|
||||
// `make generate` — that's covered by the pre-push proto-freshness hook.
|
||||
// We're guarding against future Makefile edits that break the pattern
|
||||
// without having to run the whole proto pipeline to notice.
|
||||
//
|
||||
// Closes the PR #3371 P3 finding about missing automated coverage for
|
||||
// the path-resolution behavior.
|
||||
|
||||
import { strict as assert } from 'node:assert';
|
||||
import { test, describe } from 'node:test';
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { resolve, dirname } from 'node:path';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
import { execSync } from 'node:child_process';
|
||||
|
||||
const __dirname = dirname(fileURLToPath(import.meta.url));
|
||||
const MAKEFILE = readFileSync(resolve(__dirname, '../Makefile'), 'utf-8');
|
||||
|
||||
function extractGenerateRecipe() {
|
||||
// Match `generate:` through the next blank line or non-indented line.
|
||||
const m = MAKEFILE.match(/^generate:.*?\n((?:\t[^\n]*\n|#[^\n]*\n|\s*\n)+)/m);
|
||||
if (!m) throw new Error('generate target not found in Makefile');
|
||||
return m[0];
|
||||
}
|
||||
|
||||
describe('Makefile generate target — plugin path resolution', () => {
|
||||
const recipe = extractGenerateRecipe();
|
||||
|
||||
test('resolves buf via command -v before invoking it', () => {
|
||||
// `command -v buf` must appear before the PATH override so the
|
||||
// caller's buf is captured first. Any version pinned by PATH
|
||||
// manipulation below only affects plugin resolution.
|
||||
assert.match(
|
||||
recipe,
|
||||
/BUF_BIN=\$\$\(command -v buf\)/,
|
||||
'generate recipe must resolve buf via `command -v buf` before invoking it',
|
||||
);
|
||||
});
|
||||
|
||||
test('fails loudly when buf is not on PATH', () => {
|
||||
// Must not silently fall through when buf is absent.
|
||||
assert.match(
|
||||
recipe,
|
||||
/buf not found on PATH/i,
|
||||
'generate recipe must emit a clear error when buf is missing',
|
||||
);
|
||||
});
|
||||
|
||||
test('fails loudly when go is not on PATH', () => {
|
||||
// `go env GOBIN` failing silently would let PLUGIN_DIR resolve to
|
||||
// "/bin" (empty + "/bin" suffix), which doesn't override PATH — a
|
||||
// stale sebuf on the normal PATH would win and the duplicate-output
|
||||
// failure this PR is trying to prevent would come back. Codex
|
||||
// high-severity on commit 9c0058a.
|
||||
assert.match(
|
||||
recipe,
|
||||
/command -v go/,
|
||||
'generate recipe must check that `go` is on PATH before attempting plugin resolution',
|
||||
);
|
||||
assert.match(
|
||||
recipe,
|
||||
/go not found on PATH/i,
|
||||
'generate recipe must emit a clear error when go is missing',
|
||||
);
|
||||
});
|
||||
|
||||
test('verifies EVERY sebuf plugin binary referenced by buf.gen.yaml is present', () => {
|
||||
// `go` can be installed without the user having ever run
|
||||
// `make install-plugins`. Guarding only one plugin would let buf
|
||||
// fall through to a stale copy of the OTHERS on the normal PATH
|
||||
// and recreate the mixed-version bug this PR is meant to prevent.
|
||||
// The list here must stay in sync with proto/buf.gen.yaml.
|
||||
const required = ['protoc-gen-ts-client', 'protoc-gen-ts-server', 'protoc-gen-openapiv3'];
|
||||
for (const bin of required) {
|
||||
// The guard iterates a shell `for` loop, so the literal plugin
|
||||
// name must appear in the recipe's plugin list AND the
|
||||
// remediation string must reference install-plugins.
|
||||
assert.ok(
|
||||
recipe.includes(bin),
|
||||
`generate recipe must include ${bin} in the plugin-executable guard list`,
|
||||
);
|
||||
}
|
||||
// The loop must check each entry with `[ -x "$PLUGIN_DIR/$p" ]`
|
||||
// and bail with a message that points users to `make install-plugins`.
|
||||
assert.match(
|
||||
recipe,
|
||||
/\[ -x "\$\$PLUGIN_DIR\/\$\$p" \]/,
|
||||
'generate recipe must verify each plugin is executable via [ -x "$PLUGIN_DIR/$p" ]',
|
||||
);
|
||||
assert.match(
|
||||
recipe,
|
||||
/Run: make install-plugins/,
|
||||
'generate recipe must tell the user the remediation when a plugin is missing',
|
||||
);
|
||||
});
|
||||
|
||||
test('plugin guard list covers every plugin declared in buf.gen.yaml', () => {
|
||||
// Cross-reference proto/buf.gen.yaml plugin entries against the
|
||||
// Makefile's guard list. If buf.gen.yaml ever adds a new `local:`
|
||||
// plugin (e.g. a future protoc-gen-*), the guard must grow too —
|
||||
// otherwise the new binary can silently fall through to a stale
|
||||
// PATH copy.
|
||||
const BUF_GEN = readFileSync(resolve(__dirname, '../proto/buf.gen.yaml'), 'utf-8');
|
||||
const declared = new Set();
|
||||
for (const m of BUF_GEN.matchAll(/^\s*-\s*local:\s*(\S+)\s*$/gm)) {
|
||||
declared.add(m[1]);
|
||||
}
|
||||
assert.ok(declared.size > 0, 'buf.gen.yaml must declare at least one local plugin');
|
||||
for (const bin of declared) {
|
||||
assert.ok(
|
||||
recipe.includes(bin),
|
||||
`buf.gen.yaml declares '${bin}' but the Makefile guard does not check it — stale ${bin} on PATH could still win`,
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
test('invokes buf via absolute path (via "$BUF_BIN"), not via PATH lookup', () => {
|
||||
// Using "$$BUF_BIN" generate ensures the plugin-PATH override
|
||||
// (added only for this command) does not also redirect which `buf`
|
||||
// binary runs. The whole point of the two-stage resolution.
|
||||
assert.match(
|
||||
recipe,
|
||||
/"\$\$BUF_BIN" generate/,
|
||||
'generate recipe must invoke buf via absolute path "$BUF_BIN"',
|
||||
);
|
||||
});
|
||||
|
||||
test('prepends GOBIN-or-GOPATH/bin to PATH for plugin lookup', () => {
|
||||
// Plugin resolution follows `go install`'s own rule:
|
||||
// GOBIN when set, otherwise GOPATH/bin using the FIRST entry of
|
||||
// GOPATH (GOPATH can be a path-list).
|
||||
assert.ok(recipe.includes('go env GOBIN'),
|
||||
'generate recipe must consult `go env GOBIN`');
|
||||
assert.ok(recipe.includes('go env GOPATH | cut -d:'),
|
||||
'generate recipe must extract first GOPATH entry via `go env GOPATH | cut -d:`');
|
||||
assert.ok(recipe.includes(':$$PATH"'),
|
||||
'generate recipe must prepend to $$PATH (install-dir:$$PATH, not the other way around)');
|
||||
});
|
||||
|
||||
test('PATH override order: install-dir comes first, then original PATH', () => {
|
||||
// PLUGIN_DIR must appear BEFORE $$PATH in the PATH assignment.
|
||||
// Reversing them would let any earlier PATH entry (e.g. Homebrew
|
||||
// plugins) shadow the Makefile-pinned version.
|
||||
const pathAssignMatch = recipe.match(/PATH="\$\$PLUGIN_DIR:\$\$PATH"/);
|
||||
assert.ok(
|
||||
pathAssignMatch,
|
||||
'recipe must contain PATH="$$PLUGIN_DIR:$$PATH" — resolved plugin dir first, original PATH second',
|
||||
);
|
||||
// Cross-check: PLUGIN_DIR must have been computed before the PATH
|
||||
// assignment uses it.
|
||||
const pluginDirAssignIdx = recipe.indexOf('PLUGIN_DIR=');
|
||||
const pathAssignIdx = recipe.indexOf('PATH="$$PLUGIN_DIR');
|
||||
assert.ok(pluginDirAssignIdx >= 0, 'recipe must set PLUGIN_DIR');
|
||||
assert.ok(pathAssignIdx > pluginDirAssignIdx,
|
||||
'PATH assignment must come AFTER the PLUGIN_DIR computation');
|
||||
// The GOBIN lookup happens in the PLUGIN_DIR assignment, which
|
||||
// precedes the PATH assignment — verified above.
|
||||
});
|
||||
|
||||
test('pre-push hook does not unconditionally prepend $HOME/go/bin', () => {
|
||||
// The Makefile's caller-PATH-first invariant is only meaningful
|
||||
// if the hook invoking it doesn't first shadow the caller's `buf`.
|
||||
// An unconditional `export PATH="$HOME/go/bin:$PATH"` would let a
|
||||
// stale `~/go/bin/buf` (from an old `go install`) win over a newer
|
||||
// Homebrew-installed `buf`, defeating the whole point of this PR.
|
||||
//
|
||||
// The hook MUST guard the prepend on "buf is not already on PATH"
|
||||
// so the prepend only fires as a fallback when buf has no other
|
||||
// candidate.
|
||||
const HOOK = readFileSync(resolve(__dirname, '../.husky/pre-push'), 'utf-8');
|
||||
// Locate the proto-freshness block by its echo line.
|
||||
const start = HOOK.indexOf('Running proto freshness check');
|
||||
assert.ok(start >= 0, 'pre-push hook must contain the proto-freshness block');
|
||||
const block = HOOK.slice(start, start + 2000);
|
||||
// The prepend MUST be gated on `! command -v buf` so it only fires
|
||||
// when buf has no other candidate. Any `export PATH="$HOME/go/bin:$PATH"`
|
||||
// inside this block must appear inside an `if ! command -v buf ...`
|
||||
// arm — never directly under the block or under a bare `if command -v buf`.
|
||||
assert.match(
|
||||
block,
|
||||
/if\s+!\s+command\s+-v\s+buf[^\n]*\n[^\n]*export PATH="\$HOME\/go\/bin:\$PATH"/,
|
||||
'pre-push hook must gate the $HOME/go/bin prepend on `! command -v buf` so it only fires as a fallback — ' +
|
||||
'otherwise a stale ~/go/bin/buf would shadow the caller\'s preferred buf binary',
|
||||
);
|
||||
// Explicit regression guard for the PRIOR buggy pattern that this
|
||||
// PR is replacing. The old hook did
|
||||
// `if command -v buf ... || [ -x "$HOME/go/bin/buf" ]; then export PATH=...`
|
||||
// which ALWAYS prepended whenever buf was reachable anywhere —
|
||||
// exactly the stale-buf-wins failure mode.
|
||||
assert.ok(
|
||||
!/if\s+command\s+-v\s+buf[^\n]*\|\|\s*\[\s*-x\s+"\$HOME\/go\/bin\/buf"\s*\][^\n]*;\s*then\s*\n\s*export PATH="\$HOME\/go\/bin:\$PATH"/.test(block),
|
||||
'pre-push hook must not use the old `buf-on-PATH-OR-at-~/go/bin -> prepend` pattern — it shadowed Homebrew buf with stale go-install buf',
|
||||
);
|
||||
});
|
||||
|
||||
test('path expansion succeeds on current machine', () => {
|
||||
// The shell expression is syntactically correct and resolves to
|
||||
// an existing directory on this runner. Catches obvious typos
|
||||
// (e.g. mismatched parens, wrong subshell syntax) at test time
|
||||
// instead of at first `make generate` attempt.
|
||||
const out = execSync(
|
||||
`bash -c 'gobin=$(go env GOBIN); if [ -n "$gobin" ]; then printf "%s" "$gobin"; else printf "%s/bin" "$(go env GOPATH | cut -d: -f1)"; fi'`,
|
||||
{ encoding: 'utf-8' },
|
||||
).trim();
|
||||
assert.ok(out.length > 0, 'install-dir expression must produce a non-empty path');
|
||||
assert.ok(out.endsWith('/bin') || out.includes('go'),
|
||||
`install-dir "${out}" should end with /bin or contain "go"`);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user