Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion packages/envd/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@
# which truncates ENVD_COMMIT_SHA to 7 chars (`git rev-parse --short` can
# emit more than 7 when 7 is ambiguous).
BUILD := $(shell git rev-parse HEAD | cut -c1-7)
LDFLAGS=-ldflags "-X=main.commitSHA=$(BUILD)"

Check warning on line 8 in packages/envd/Makefile

View check run for this annotation

Claude / Claude Code Review

Unused LDFLAGS variable left dead

The `LDFLAGS` variable defined at packages/envd/Makefile:8 is now unused — this PR switches `build-debug` (its only consumer) over to the new `DEBUG_LDFLAGS`. It's dead code left over from the refactor and can be safely deleted.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 The LDFLAGS variable defined at packages/envd/Makefile:8 is now unused — this PR switches build-debug (its only consumer) over to the new DEBUG_LDFLAGS. It's dead code left over from the refactor and can be safely deleted.

Extended reasoning...

What happened: This PR adds a new DEBUG_LDFLAGS variable (line 10) with the extra -linkmode external -extldflags=-static flags needed to statically link the race-instrumented debug binary so it can run on musl guests. It correctly updates the build-debug target (line 80) to use ${DEBUG_LDFLAGS} instead of ${LDFLAGS}.

However, the original LDFLAGS=-ldflags \"-X=main.commitSHA=$(BUILD)\" definition on line 8 was left in place. Before this PR, build-debug was the sole consumer of LDFLAGS (the build target has always used the separate PROD_LDFLAGS). After this PR, nothing references LDFLAGS anywhere in the file.

Proof:

  1. Before the PR: grep LDFLAGS packages/envd/Makefile shows LDFLAGS defined on line 8 and consumed via ${LDFLAGS} on line 80 (build-debug).
  2. After the PR: line 80 now reads CGO_ENABLED=1 go build -race -tags osusergo,netgo -gcflags=all=\"-N -l\" -o bin/debug/envd ${DEBUG_LDFLAGS} — the only other ldflags usage is PROD_LDFLAGS in the unrelated build target (line 73).
  3. A boundary-aware grep for the bare LDFLAGS token (excluding PROD_LDFLAGS/DEBUG_LDFLAGS) matches only the definition on line 8, confirming it has zero remaining references.

Impact: None functionally — Make does not warn on unused variables, and there's no implicit rule here that would pick it up accidentally, so the build behaves identically whether or not the line is removed. It's purely a leftover from the refactor that muddies the file (a future reader might wonder if it's still used somewhere, or reintroduce a stale reference).

Fix: Delete line 8 (LDFLAGS=-ldflags \"-X=main.commitSHA=$(BUILD)\") since PROD_LDFLAGS and DEBUG_LDFLAGS now cover both build targets.

PROD_LDFLAGS=-ldflags "-X=main.commitSHA=$(BUILD) -s -w -buildid="
DEBUG_LDFLAGS=-ldflags "-X=main.commitSHA=$(BUILD) -linkmode external -extldflags=-static"

AWS_BUCKET_PREFIX ?= $(PREFIX)$(AWS_ACCOUNT_ID)-
GCP_BUCKET_PREFIX ?= $(GCP_PROJECT_ID)-
Expand Down Expand Up @@ -71,8 +72,12 @@
build:
CGO_ENABLED=0 GOOS=linux GOARCH=$(BUILD_ARCH) go build -trimpath -buildvcs=false -a -o bin/envd ${PROD_LDFLAGS}

# This binary is injected into every sandbox guest, including musl ones, so it
# must not need a dynamic loader; -race requires cgo, hence the static external
# link. osusergo/netgo keep user and DNS lookups pure Go, as in the CGO_ENABLED=0
# build above, so only the race instrumentation differs from what ships.
build-debug:
CGO_ENABLED=1 go build -race -gcflags=all="-N -l" -o bin/debug/envd ${LDFLAGS}
CGO_ENABLED=1 go build -race -tags osusergo,netgo -gcflags=all="-N -l" -o bin/debug/envd ${DEBUG_LDFLAGS}

start-docker:
make build
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ func TestTemplateBuildDistroFamilies(t *testing.T) {
name string
templateName string
fromImage string
skipReason string
}{
{
name: "Debian family",
Expand All @@ -43,23 +42,13 @@ func TestTemplateBuildDistroFamilies(t *testing.T) {
name: "Alpine on OpenRC",
templateName: "test-distro-alpine",
fromImage: "alpine:3.24",
// The integration host installs the envd from `make build-debug`, and
// -race requires cgo, so that binary is glibc-linked and cannot exec
// on musl: envd never answers and the base layer times out. Released
// envd is built with CGO_ENABLED=0 and runs on Alpine. Drop this once
// the host installs a static envd.
skipReason: "host envd is glibc-linked; a musl guest cannot exec it",
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()

if tc.skipReason != "" {
t.Skip(tc.skipReason)
}

var logMessages []string
logHandler := func(alias string, entry api.BuildLogEntry) {
logMessages = append(logMessages, entry.Message)
Expand Down
Loading