From da4a6bf45b8b16b7483bce9fe37e0e1e7eaa7d47 Mon Sep 17 00:00:00 2001 From: Thales Maciel Date: Thu, 16 Apr 2026 16:49:17 -0300 Subject: [PATCH] Add lint targets, fix gofmt drift, broaden Makefile build inputs Three small operational improvements. 1. Makefile build dependencies now cover everything under cmd/ and internal/, not just *.go. The previous GO_SOURCES find pattern missed embedded assets (catalog.json today, anything else added later), so editing a JSON manifest didn't trigger a rebuild and left the binary stale. New BUILD_INPUTS covers all files; go's own build cache absorbs any redundant invocations. GO_SOURCES is kept for fmt/lint targets which still want only Go files. 2. New `make lint` (default + lint-go + lint-shell): - lint-go: gofmt -l (fail if any output) and go vet ./... - lint-shell: shellcheck --severity=error on scripts/*.sh The shell floor is set at error-level for now; the legacy make-rootfs-*.sh / make-*-kernel.sh / customize.sh scripts have warning-level findings (sudo-cat redirects, heredoc quoting) that would block landing this if we tightened immediately. Documented as tech debt in docs/kernel-catalog.md alongside a note about eventually replacing the per-distro bash with a uniform Go tool. 3. gofmt drift fixed in internal/daemon/imagemgr/build.go, session/session.go, and vm_create_ops.go (trailing newline + gofmt's preferred function-definition wrapping). Now `make lint` passes cleanly; future drift will fail CI/local lint instead of accumulating. AGENTS.md gains a one-line note on make lint. Co-Authored-By: Claude Sonnet 4.6 --- AGENTS.md | 1 + Makefile | 29 +++++++++++++++++++++++---- docs/kernel-catalog.md | 24 ++++++++++++++++++++++ internal/daemon/imagemgr/build.go | 1 - internal/daemon/session/session.go | 32 ++++++++++++++++++++---------- internal/daemon/vm_create_ops.go | 8 ++++---- 6 files changed, 76 insertions(+), 19 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 1ab1f7f..c935bf2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -15,6 +15,7 @@ Always run `make build` before commit. - `make build` builds `./build/bin/banger`, `./build/bin/bangerd`, and `./build/bin/banger-vsock-agent`. - `make test` runs `go test ./...`. +- `make lint` runs `gofmt -l`, `go vet ./...`, and `shellcheck --severity=error` on `scripts/*.sh`. Run before commits. - `./build/bin/banger doctor` checks host readiness. - `./build/bin/banger image build --from-image ` builds a managed image from an existing registered image. - `./build/bin/banger image register ...` registers an unmanaged host-side image stack. diff --git a/Makefile b/Makefile index f6dffa0..991e0ac 100644 --- a/Makefile +++ b/Makefile @@ -15,6 +15,12 @@ BANGERD_BIN ?= $(BUILD_BIN_DIR)/bangerd VSOCK_AGENT_BIN ?= $(BUILD_BIN_DIR)/banger-vsock-agent BINARIES := $(BANGER_BIN) $(BANGERD_BIN) $(VSOCK_AGENT_BIN) GO_SOURCES := $(shell find cmd internal -type f -name '*.go' | sort) +# BUILD_INPUTS is everything that can change a binary's bytes: Go sources +# plus embedded assets (catalog.json, future static files). Listing +# everything is cheaper than missing a rebuild — go's own cache absorbs +# any redundant invocations. +BUILD_INPUTS := $(shell find cmd internal -type f | sort) +SHELL_SOURCES := $(shell find scripts -type f -name '*.sh' | sort) VOID_IMAGE_NAME ?= void VOID_VM_NAME ?= void-dev ALPINE_RELEASE ?= 3.23.3 @@ -27,7 +33,7 @@ GO_LDFLAGS := -X banger/internal/buildinfo.Version=$(VERSION) -X banger/internal .DEFAULT_GOAL := help -.PHONY: help build banger bangerd test fmt tidy clean rootfs rootfs-void void-kernel void-register void-vm verify-void alpine-kernel rootfs-alpine alpine-register alpine-vm verify-alpine install bench-create +.PHONY: help build banger bangerd test fmt tidy clean rootfs rootfs-void void-kernel void-register void-vm verify-void alpine-kernel rootfs-alpine alpine-register alpine-vm verify-alpine install bench-create lint lint-go lint-shell help: @printf '%s\n' \ @@ -36,6 +42,7 @@ help: ' make bench-create Benchmark vm create and SSH readiness with scripts/bench-create.sh' \ ' make install Build and install banger, bangerd, and the companion vsock helper' \ ' make test Run go test ./...' \ + ' make lint Run gofmt + go vet + shellcheck (errors)' \ ' make fmt Format Go sources under cmd/ and internal/' \ ' make tidy Run go mod tidy' \ ' make clean Remove built Go binaries' \ @@ -53,21 +60,35 @@ help: build: $(BINARIES) -$(BANGER_BIN): $(GO_SOURCES) go.mod go.sum +$(BANGER_BIN): $(BUILD_INPUTS) go.mod go.sum mkdir -p "$(BUILD_BIN_DIR)" $(GO) build -ldflags '$(GO_LDFLAGS)' -o "$(BANGER_BIN)" ./cmd/banger -$(BANGERD_BIN): $(GO_SOURCES) go.mod go.sum +$(BANGERD_BIN): $(BUILD_INPUTS) go.mod go.sum mkdir -p "$(BUILD_BIN_DIR)" $(GO) build -ldflags '$(GO_LDFLAGS)' -o "$(BANGERD_BIN)" ./cmd/bangerd -$(VSOCK_AGENT_BIN): $(GO_SOURCES) go.mod go.sum +$(VSOCK_AGENT_BIN): $(BUILD_INPUTS) go.mod go.sum mkdir -p "$(BUILD_BIN_DIR)" CGO_ENABLED=0 GOOS=linux GOARCH=amd64 $(GO) build -ldflags '$(GO_LDFLAGS)' -o "$(VSOCK_AGENT_BIN)" ./cmd/banger-vsock-agent test: $(GO) test ./... +lint: lint-go lint-shell + +lint-go: + @unformatted="$$($(GOFMT) -l $(GO_SOURCES))"; \ + if [ -n "$$unformatted" ]; then \ + printf 'gofmt: the following files are not formatted:\n%s\n' "$$unformatted" >&2; \ + exit 1; \ + fi + $(GO) vet ./... + +lint-shell: + @command -v shellcheck >/dev/null 2>&1 || { echo 'shellcheck is required for make lint-shell' >&2; exit 1; } + shellcheck --severity=error $(SHELL_SOURCES) + fmt: $(GOFMT) -w $(GO_SOURCES) diff --git a/docs/kernel-catalog.md b/docs/kernel-catalog.md index c616e8d..315a748 100644 --- a/docs/kernel-catalog.md +++ b/docs/kernel-catalog.md @@ -115,3 +115,27 @@ never committed). R2's free tier covers the expected traffic comfortably If hosting ever moves, catalog entries can be migrated by reuploading the tarballs and editing the URLs in `catalog.json` — no other code changes required. + +## Tech debt: kernel-build scripts + +`scripts/make-void-kernel.sh` and `scripts/make-alpine-kernel.sh` are +procedural bash that fetches and patches per-distro kernel sources. +Each new distro means a new bespoke script. They're "good enough" +because catalog refreshes are infrequent and only the maintainer runs +them, but they are the bottleneck if the catalog ever wants to grow +beyond two distros. + +A future iteration should: + +- Move kernel acquisition into a Go (or at least uniform) tool with a + per-distro plugin/config rather than per-distro scripts. +- Encode kernel config and required modules declaratively so a Debian + or Fedora target is a config addition, not a new script. +- Run unattended in CI once banger goes public — the manual + `scripts/publish-kernel.sh` flow scales until then. + +Until that happens, `make lint-shell` only runs at `--severity=error`. +Tightening to `--severity=warning` would surface real issues in the +legacy build scripts (mostly `sudo cat > file` redirects and +heredoc-quoting concerns); fixing those is a prerequisite to bumping +the lint floor. diff --git a/internal/daemon/imagemgr/build.go b/internal/daemon/imagemgr/build.go index 3bffcf9..51a338d 100644 --- a/internal/daemon/imagemgr/build.go +++ b/internal/daemon/imagemgr/build.go @@ -245,4 +245,3 @@ func shellArray(values []string) string { func shellQuote(value string) string { return "'" + strings.ReplaceAll(value, "'", `'"'"'`) + "'" } - diff --git a/internal/daemon/session/session.go b/internal/daemon/session/session.go index 4407520..bb42743 100644 --- a/internal/daemon/session/session.go +++ b/internal/daemon/session/session.go @@ -53,16 +53,28 @@ func RelativeStateDir(id string) string { return strings.TrimPrefix(StateDir(id), "/root/") } -func ScriptPath(id string) string { return filepath.ToSlash(filepath.Join(StateDir(id), "run.sh")) } -func PIDPath(id string) string { return filepath.ToSlash(filepath.Join(StateDir(id), "pid")) } -func MonitorPIDPath(id string) string { return filepath.ToSlash(filepath.Join(StateDir(id), "monitor_pid")) } -func ExitCodePath(id string) string { return filepath.ToSlash(filepath.Join(StateDir(id), "exit_code")) } -func StdinPipePath(id string) string { return filepath.ToSlash(filepath.Join(StateDir(id), "stdin.pipe")) } -func StdinKeepalivePIDPath(id string) string { return filepath.ToSlash(filepath.Join(StateDir(id), "stdin_keepalive.pid")) } -func StatusPath(id string) string { return filepath.ToSlash(filepath.Join(StateDir(id), "status")) } -func ErrorPath(id string) string { return filepath.ToSlash(filepath.Join(StateDir(id), "error")) } -func StdoutLogPath(id string) string { return filepath.ToSlash(filepath.Join(StateDir(id), "stdout.log")) } -func StderrLogPath(id string) string { return filepath.ToSlash(filepath.Join(StateDir(id), "stderr.log")) } +func ScriptPath(id string) string { return filepath.ToSlash(filepath.Join(StateDir(id), "run.sh")) } +func PIDPath(id string) string { return filepath.ToSlash(filepath.Join(StateDir(id), "pid")) } +func MonitorPIDPath(id string) string { + return filepath.ToSlash(filepath.Join(StateDir(id), "monitor_pid")) +} +func ExitCodePath(id string) string { + return filepath.ToSlash(filepath.Join(StateDir(id), "exit_code")) +} +func StdinPipePath(id string) string { + return filepath.ToSlash(filepath.Join(StateDir(id), "stdin.pipe")) +} +func StdinKeepalivePIDPath(id string) string { + return filepath.ToSlash(filepath.Join(StateDir(id), "stdin_keepalive.pid")) +} +func StatusPath(id string) string { return filepath.ToSlash(filepath.Join(StateDir(id), "status")) } +func ErrorPath(id string) string { return filepath.ToSlash(filepath.Join(StateDir(id), "error")) } +func StdoutLogPath(id string) string { + return filepath.ToSlash(filepath.Join(StateDir(id), "stdout.log")) +} +func StderrLogPath(id string) string { + return filepath.ToSlash(filepath.Join(StateDir(id), "stderr.log")) +} // -- Script generators ------------------------------------------------------ diff --git a/internal/daemon/vm_create_ops.go b/internal/daemon/vm_create_ops.go index b27dc90..fa43aa8 100644 --- a/internal/daemon/vm_create_ops.go +++ b/internal/daemon/vm_create_ops.go @@ -11,10 +11,10 @@ import ( "banger/internal/model" ) -func (op *vmCreateOperationState) ID() string { return op.snapshot().ID } -func (op *vmCreateOperationState) IsDone() bool { return op.snapshot().Done } -func (op *vmCreateOperationState) UpdatedAt() time.Time { return op.snapshot().UpdatedAt } -func (op *vmCreateOperationState) Cancel() { op.cancelOperation() } +func (op *vmCreateOperationState) ID() string { return op.snapshot().ID } +func (op *vmCreateOperationState) IsDone() bool { return op.snapshot().Done } +func (op *vmCreateOperationState) UpdatedAt() time.Time { return op.snapshot().UpdatedAt } +func (op *vmCreateOperationState) Cancel() { op.cancelOperation() } type vmCreateProgressKey struct{}