diff --git a/internal/builder/builder.go b/internal/builder/builder.go index 625f694..bc94d75 100644 --- a/internal/builder/builder.go +++ b/internal/builder/builder.go @@ -877,13 +877,20 @@ func compile(ctx context.Context, run Runner, tree string, chain Toolchain, base, } invocation = append(invocation, chain.Compile...) - // What it was built for, linked in. The compile line carries `-ldflags` already; this appends a - // second one, which the Go linker accepts and merges. A host with no system refuses every - // declaration before it applies anything (novox/hq 04-ISSUES/161), and it is the artifact that - // knows — the target is a property of the artifact rather than of the recipe (ADR 0142). + // **One `-ldflags`, composed here.** A repeated flag is not a merged one: the Go command takes + // the last and drops the first, so passing the toolchain's flags and then the system stamp as a + // second `-ldflags` produced a binary that knew its system and had lost `-s -w` — half again the + // size, with its debug info (novox/hq 04-ISSUES/161). + // + // What it was built for is the one thing taken from the artifact, and ADR 0142 says why: the + // target is a property of the artifact rather than of the recipe. A host with no system refuses + // every declaration before it applies anything. + linker := append([]string(nil), chain.LinkerFlags...) if chain.SystemStamp != "" && strings.TrimSpace(a.System) != "" { - invocation = append(invocation, "-ldflags", - "-X "+chain.SystemStamp+"="+strings.TrimSpace(a.System)) + linker = append(linker, "-X", chain.SystemStamp+"="+strings.TrimSpace(a.System)) + } + if len(linker) > 0 { + invocation = append(invocation, "-ldflags", strings.Join(linker, " ")) } if chain.OutputFlag != "" { // A compiler pointed at a package is told the file to write, not the directory: the name a diff --git a/internal/builder/system_stamp_test.go b/internal/builder/system_stamp_test.go index 9c6b883..8394be0 100644 --- a/internal/builder/system_stamp_test.go +++ b/internal/builder/system_stamp_test.go @@ -47,3 +47,30 @@ func TestTheStampIsTheOneThingTakenFromTheArtifact(t *testing.T) { t.Fatalf("the compile line takes something from the module: %q", joined) } } + +func TestTheLinkerIsToldOnceNotTwice(t *testing.T) { + // A repeated flag is not a merged one: the Go command takes the last -ldflags and drops the + // first. Passing the toolchain's flags and then the stamp separately produced a binary that knew + // its system and had lost -s -w — 12.2MB against 8.5MB, with its debug info (04-ISSUES/161). + chain, err := ToolchainFor("go") + if err != nil { + t.Fatal(err) + } + for _, arg := range chain.Compile { + if arg == "-ldflags" { + t.Fatal("the compile line carries -ldflags, so composing one here makes two") + } + } + if len(chain.LinkerFlags) == 0 { + t.Fatal("the go toolchain passes no linker flags, so the binary keeps its debug info") + } + var stripped bool + for _, f := range chain.LinkerFlags { + if f == "-s" { + stripped = true + } + } + if !stripped { + t.Fatalf("the go toolchain does not strip: %v", chain.LinkerFlags) + } +} diff --git a/internal/builder/toolchain.go b/internal/builder/toolchain.go index abd479c..c34a3b3 100644 --- a/internal/builder/toolchain.go +++ b/internal/builder/toolchain.go @@ -49,6 +49,14 @@ type Toolchain struct { // is named as it will be FOUND, inside the unpacked bundle, so the source is the same path with // the output directory taken off the front and this on the end. SourceExt string + // LinkerFlags are passed to the linker as one flag, together with the system stamp below. + // + // **Separate from Compile because a repeated flag is not a merged one.** They were in the compile + // line, and appending the stamp as a second `-ldflags` meant the Go command took the last and + // dropped the first — so the binary gained its system and lost `-s -w`, growing by half and + // carrying its debug info. The mistake was believing a comment rather than reading the file it + // produced (novox/hq 04-ISSUES/161). + LinkerFlags []string // SystemStamp is the variable this language's linker fills with the artifact's declared system, // for a language whose binaries are pinned to one at link time (novox/hq ADR 0005). // @@ -126,9 +134,12 @@ var toolchains = []Toolchain{ // rather than from the linker: two builds of one commit produce the same bytes. Compile: []string{ "env", "CGO_ENABLED=0", "GOFLAGS=-trimpath", - "go", "build", "-ldflags", "-s -w", + "go", "build", }, - OutputFlag: "-o", + // Stripped of symbols and debug info: what a machine holds is a file it runs, not one it + // debugs, and the difference measured 12.2MB against 8.5MB. + LinkerFlags: []string{"-s", "-w"}, + OutputFlag: "-o", // Pointed at the package the artifact is built `from`, compiled whole. Go writes the binary // into the output directory, named after the package — so the bundle a machine unpacks is a // directory holding one executable, which is what the delivery mechanism expects