Page MenuHomeFreeBSD

Mk/Uses/go.mk: Omit symbol table and debug information by default
ClosedPublic

Authored by dmgk on Sep 20 2019, 2:58 PM.
Tags
None
Referenced Files
Unknown Object (File)
Feb 20 2024, 7:53 AM
Unknown Object (File)
Dec 22 2023, 11:58 PM
Unknown Object (File)
Dec 21 2023, 11:06 PM
Unknown Object (File)
Dec 14 2023, 3:44 PM
Unknown Object (File)
Dec 11 2023, 11:46 AM
Unknown Object (File)
Nov 10 2023, 2:57 PM
Unknown Object (File)
Nov 8 2023, 3:02 PM
Unknown Object (File)
Nov 7 2023, 8:03 AM

Details

Summary

Mk/Uses/go.mk: Add linker flags to omit symbol table and debug information
unless WITH_DEBUG is defined or port sets its own -ldflags.

PR: 233335

Test Plan

Tested with:

poudriere testport devel/awless (doesn't set custom -ldflags)
poudriere testport ports-mgmt/modules2tuple (sets custom -ldflags, but without "-s -w")

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

I did some more testing and it turns out that this patch actually breaks a few ports (www/gohugo, devel/gitaly, mail/aerc) due to GO_BUILDFLAGS being expanded too early.
It should be possible to pass linker flags in GOFLAGS though, I'll look into that and update the patch.

tobik requested changes to this revision.Sep 20 2019, 4:45 PM
tobik added inline comments.
Mk/Uses/go.mk
79 ↗(On Diff #62349)

Maybe use

GO_BUILDFLAGS+=	-ldflags=-s -ldflags=-w

instead to not break mail/aerc and others.

This revision now requires changes to proceed.Sep 20 2019, 4:45 PM
Mk/Uses/go.mk
79 ↗(On Diff #62349)

No, the issue is that empty(GO_BUILDFLAGS:M-ldflags*) expands GO_BUILDFLAGS and in case of e.g. www/gohugo it breaks the port because DISTINFO_FILE used in the shell command is not yet defined. I didn't look closely at other ports breakages but I guess it's the same issue.

And multiple -ldflags syntax doesn't work anyway unfortunately because second -ldflags overrides the first.

dmgk added inline comments.
Mk/Uses/go.mk
79 ↗(On Diff #62349)

Actually you were right about -ldflags - surprisingly, GOFLAGS -flag=value syntax doesn't allow spaces in value [1]. Just -s (Omit the symbol table and debug information) seems like a good compromise for now, it produces smaller executables and is enough to silence the strip warning.

[1] https://github.com/golang/go/issues/29096

Pass just -s because GOFLAGS doesn't support spaces in values.

This change depends on https://reviews.freebsd.org/D21735.

Tested with poudriere bulk over all Go ports.

IIUC this potentially changes all Go binaries, so all USES=go ports that do not set ldflags themselves probably need a PORTREVISION bump afterwards to relink them?

Mk/Uses/go.mk
79 ↗(On Diff #62349)

Yeah, -s seems to be enough.

I misunderstood how -ldflags works. go help build is pretty clear that "the latest match on the command line wins." The semantics of -ldflags are weird to me. If it would append to previous values (like with rustc -Clink-arg) integrating this into ports would be much easier.

IIUC this potentially changes all Go binaries, so all USES=go ports that do not set ldflags themselves probably need a PORTREVISION bump afterwards to relink them?

A quick search

for f in $(find /usr/ports -name Makefile | xargs grep -l 'USES=.*\bgo\b'); do grep -q '\bldflags\b' $f || echo $f; done | wc -l

shows that it would require bumping PORTREVISION of 198 ports. Recompiling will make binaries about 25% smaller on average but there are no other functional changes. I can prepare a separate review if we're OK with this.

In D21730#475087, @dmgk wrote:

IIUC this potentially changes all Go binaries, so all USES=go ports that do not set ldflags themselves probably need a PORTREVISION bump afterwards to relink them?

A quick search

for f in $(find /usr/ports -name Makefile | xargs grep -l 'USES=.*\bgo\b'); do grep -q '\bldflags\b' $f || echo $f; done | wc -l

shows that it would require bumping PORTREVISION of 198 ports. Recompiling will make binaries about 25% smaller on average but there are no other functional changes. I can prepare a separate review if we're OK with this.

Please do so, seems very beneficial!
Thanks for all this work.

Is it worth do an exp-run on this one too?

dmgk added inline comments.
Mk/Uses/go.mk
78 ↗(On Diff #62375)

@mat Is there a way to avoid early evaluation by empty here?

dmgk marked an inline comment as not done.Sep 30 2019, 10:25 PM

Please do so, seems very beneficial!

Review for PORTREVISION bump of ports without explicit -ldflags: https://reviews.freebsd.org/D21854

The exp-run (https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=240986) doesn't seem to be having any progress, I'm going to do a local test build of this change along with https://reviews.freebsd.org/D21854 and commit.

This revision was not accepted when it landed; it landed in state Needs Review.Nov 16 2019, 12:39 PM
This revision was automatically updated to reflect the committed changes.