Carry non-Go files into the pure-Go runtime image#882
Conversation
The Go stackbuild's default pure-Go path assembles a distroless-static runtime that copied only /bin/app, silently dropping every other file (README, templates, data dirs). Apps like rfd that read files at runtime relative to /app broke as a result. Copy the built /app tree onto the runtime, excluding Go source and the module/vendor build inputs (**/*.go, go.mod, go.sum, vendor) that the compiled binary never needs. This matches what every other stack already does and removes a footgun from the default Go path.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughChangesThe Go distroless runtime copy step now excludes Go source files, Sequence Diagram(s)Not applicable. Related Issues: None provided. Related PRs: None provided. Suggested labels: documentation, golang, testing Suggested reviewers: None provided. Poem: Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pkg/stackbuild/golang.go (1)
351-355: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winBare
go.mod/go.sum/vendorpatterns only match at the root, not in nested subdirectories.Docker/BuildKit ignore-pattern matching uses
filepath.Matchand patterns are anchored to the copy root unless prefixed with**/.**/*.gocorrectly matches Go source at any depth, butgo.mod,go.sum, andvendorwill only match those names at the top level of/app. For projects using Go workspaces, multi-module layouts, or avendor/directory nested under a subpackage, those files/dirs would still be copied into the runtime image, defeating the intent of the exclusion.♻️ Proposed fix to make exclusions recursive
-var goRuntimeExcludePatterns = []string{"**/*.go", "go.mod", "go.sum", "vendor"} +var goRuntimeExcludePatterns = []string{"**/*.go", "**/go.mod", "**/go.sum", "**/vendor"}Please confirm BuildKit's
ExcludePatternssemantics forgithub.com/moby/buildkitv0.19.0 match this understanding.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/stackbuild/golang.go` around lines 351 - 355, The runtime exclusion list in goRuntimeExcludePatterns only uses root-anchored names for go.mod, go.sum, and vendor, so nested modules or subpackage vendor trees can still be copied. Update the patterns used by the golang.go stack build flow so they apply recursively like the existing **/*.go entry, and verify against BuildKit ExcludePatterns behavior in github.com/moby/buildkit v0.19.0 that the copy root is matched unless prefixed with **/.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@pkg/stackbuild/golang.go`:
- Around line 351-355: The runtime exclusion list in goRuntimeExcludePatterns
only uses root-anchored names for go.mod, go.sum, and vendor, so nested modules
or subpackage vendor trees can still be copied. Update the patterns used by the
golang.go stack build flow so they apply recursively like the existing **/*.go
entry, and verify against BuildKit ExcludePatterns behavior in
github.com/moby/buildkit v0.19.0 that the copy root is matched unless prefixed
with **/.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6a1c0706-fc07-4d62-b92d-a9f2308e364e
📒 Files selected for processing (3)
docs/docs/languages.mdpkg/stackbuild/golang.gopkg/stackbuild/stackbuild_test.go
phinze
left a comment
There was a problem hiding this comment.
This inverts a call I made back in #865, where I made distroless strictly binary-only and left go:embed (or flipping on cgo/JS to route to slim) as the escape hatch. And I think you've got it right to undo it. The tell is that we broke our own rfd tool with its config-in-README trick: if we were leaning on the "files are just there" invariant without thinking about it, that invariant is worth keeping. Better to match every other stack than make Go the one that quietly drops your files.
One thing worth fixing before this goes in: you updated the user-facing docs, but the assembleRuntime header comment (around line 272) still tells the old story, calling this a binary-only image and pointing people at go:embed. That's the misleading bit now, since files come along for free. Whoever edits that function next will read the comment first.
CodeRabbit's point about vendor and go.mod only matching at the repo root is a fair catch, but it's just bloat on nested-module layouts rather than anything broken, so I'll leave that to you.
--p+🤖
The distroless-path bullet still described the image as binary-only and pointed to go:embed, which is misleading now that non-Go files are carried in automatically.
…citly-include-f
Problem
The Go stackbuild's default pure-Go path assembles a distroless-static runtime that copied only
/bin/app, silently dropping every other file in the repo (README, templates, data dirs). This broke apps likerfdthat read files at runtime relative to/app.Only this one build personality was affected — every other stack (Ruby, Python, Node, Bun, Rust) and even Go's own cgo/JS-augmented path already ship the full
/apptree.Fix
In
GoStack.assembleRuntime, the distroless-static branch now copies the built/apptree onto the runtime image, excluding Go source and the module/vendor build inputs the compiled binary never needs:This mirrors the
CopyInfothe debian-slim branch already uses and removes a footgun from the default Go path. Rather than an explicit opt-inincludelist, non-Go files now travel automatically — consistent with every other stack.Scope stays on the pure-Go branch; the debian-slim (cgo/augmented) branch is untouched.
Testing
TestGoRuntimeIncludesNonGoFiles— builds a real image via buildkit and assertsapp/README.mdand nestedapp/data/seed.txtare present whileapp/main.go,app/go.mod,app/go.sumare excluded. ✅TestGo,TestGoCgo,TestGoWithVendorstay green. ✅make lintclean (Go + docs). ✅Docs
Added a "Runtime Files" note to the Go section of
docs/docs/languages.md.