Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/go: override semantics break e.g. GOFLAGS=-ldflags #38522

Open
stapelberg opened this issue Apr 19, 2020 · 7 comments
Open

cmd/go: override semantics break e.g. GOFLAGS=-ldflags #38522

stapelberg opened this issue Apr 19, 2020 · 7 comments

Comments

@stapelberg
Copy link
Contributor

@stapelberg stapelberg commented Apr 19, 2020

I was asked to post this as a separate issue in #26849 (comment), so here goes:

Comment #26849 (comment) already touches upon the override semantics issue (-ldflags=-w -ldflags=-s results in only -s), but since that example would likely be fixed by quoting changes, I’d like to stress another nuance not addressed by quoting: the current override behavior makes setting -ldflags in GOFLAGS impossible when the go tool command lines contain an explicit -ldflags flag.

For my use-case, I want to set the ELF interpreter in my build system, so I figured I’d set the GOFLAGS environment variable like so:

GOFLAGS="-ldflags=-extldflags=-Wl,--dynamic-linker=/ro/glibc-amd64-2.31-4/out/lib/ld-linux-x86-64.so.2"

This works in general, but breaks as soon as the software in question specifies its own ldflags on the command line, like e.g. containerd does in its Makefile.

It’s easy to verify this:

GOFLAGS=-ldflags=-invalid go build # fails
GOFLAGS=-ldflags=-invalid go build -ldflags=-s # does not fail

Notably, from C build systems, I’m used to LDFLAGS generally being extended, never overridden.

For my use-case, I’m currently setting -Wl,--dynamic-linker in CGO_LDFLAGS instead. Not entirely sure how reliable that is longer-term, but it seems to work with the packages I currently care about.

@mvdan
Copy link
Member

@mvdan mvdan commented Apr 19, 2020

I think @myitcv also raised a similar point about -tags. It's impossible to set one tag via GOFLAGS, and some extra tags via command line flags, as the latter will simply overwrite the former.

-ldflags already has semantics that allow merging values in some cases, so I think it's reasonable to extend that to support this use case. The alternative would be to redesign GOFLAGS somehow, or to not support this use case at all, both of which seem like worse options to me.

For example, here's one sample idea to extend the current syntax. Allow pattern+=flags, where += means that the flags should be merged with the resulting list of flags for a pattern, instead of substituting (or being substituted by) other flags. This way, @stapelberg could use GOFLAGS="-ldflags=all+=-extldflags=-Wl,--dynamic-linker=/ro/glibc-amd64-2.31-4/out/lib/ld-linux-x86-64.so.2".

Edit: I realise that all=<flags> and <flags> mean different things today, but I think Michael wants to use all= for his use case anyway, since they are global flags for an entire build.

CC @bcmills @jayconrod

@bcmills
Copy link
Member

@bcmills bcmills commented Apr 20, 2020

@bcmills bcmills added this to the Backlog milestone Apr 20, 2020
@rsc
Copy link
Contributor

@rsc rsc commented Apr 21, 2020

Yes, changing the default behavior is entirely reasonable except when it isn't. For example, what if I set

GOFLAGS=-ldflags=-w

to turn off DWARF by default, but then I want to override that, with go build -ldflags= to get a DWARF binary?

@rsc
Copy link
Contributor

@rsc rsc commented Apr 21, 2020

In general the possible can of complexity we could open here is quite large, and I am wary of anything that sounds "straightforward". For example, supposing we had the += syntax, using it in GOFLAGS would not be enough, because GOFLAGS applies before the command line. The Makefile command line would still need to be changed too. At that point why not make the Makefile accept from a given environment variable the things you want to inject?

@bcmills bcmills modified the milestones: Backlog, Unplanned Apr 21, 2020
@thanm
Copy link
Member

@thanm thanm commented Apr 21, 2020

@rsc in your example if ldflags were indeed "additive", you could enabled a DWARF binary by just adding in -ldflags=-w=0, I think.

@thanm
Copy link
Member

@thanm thanm commented Apr 21, 2020

It is pretty common with other compilers/linkers to have the concept of "rightmost setting wins", e.g. if you say "-w -w=0 -w=1 -w=0" you get -w=0.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Apr 22, 2020

I agree with the rightmost setting wins rule, which is what the linker implements anyhow.

That is how #cgo LDFLAGS: works today with respect to -extldflags. When invoking the external linker we pass the aggregated #cgo LDFLAGS values, then the -extldflags values.

johnboyes added a commit to agilepathway/label-checker that referenced this issue Jun 27, 2020
This commit removes the DWARF debugging information[1]

Decided not to try to also turn off generation of the Go symbol table
using the `-s` flag, as:
1. the GOFLAGS functionality currently only allows 1 ldflag to be set[2]
   (NB this may be fixed in Go 1.15[3])
2. The improvements to the binary size in Go 1.15 [4] mean that the `-s`
   flag will no longer really do anything[5]

Will also try shrinking the binary size further using upx[6] in a
separate pull request.

[1] https://stackoverflow.com/a/22276273
[2] golang/go#38522
[3] golang/go#26849 (comment)
[4] See #64
[5] https://twitter.com/bradfitz/status/1256989624590712834
[6] https://upx.github.io/
johnboyes added a commit to agilepathway/label-checker that referenced this issue Jun 27, 2020
This commit removes the DWARF debugging information[1], resulting in the
Docker image size shrinking from 4.76MB to 2.88MB.

Decided not to try to also turn off generation of the Go symbol table
using the `-s` flag, as:
1. the GOFLAGS functionality currently only allows 1 ldflag to be set[2]
   (NB this may be fixed in Go 1.15[3])
2. The improvements to the binary size in Go 1.15 [4] mean that the `-s`
   flag will no longer really do anything[5]

Will also try shrinking the binary size further using upx[6] in a
separate pull request.

[1] https://stackoverflow.com/a/22276273
[2] golang/go#38522
[3] golang/go#26849 (comment)
[4] See #64
[5] https://twitter.com/bradfitz/status/1256989624590712834
[6] https://upx.github.io/
johnboyes added a commit to agilepathway/label-checker that referenced this issue Jun 27, 2020
This commit removes the DWARF debugging information[1], resulting in the
Docker image size shrinking from 4.76MB to 2.88MB.

Decided not to try to also turn off generation of the Go symbol table
using the `-s` flag, as:
1. the GOFLAGS functionality currently only allows 1 ldflag to be set[2]
   (NB this may be fixed in Go 1.15[3])
2. The improvements to the binary size in Go 1.15 [4] mean that the `-s`
   flag will no longer really do anything[5]

Will also try shrinking the binary size further using upx[6] in a
separate pull request.

[1] https://stackoverflow.com/a/22276273
[2] golang/go#38522
[3] golang/go#26849 (comment)
[4] See #64
[5] https://twitter.com/bradfitz/status/1256989624590712834
[6] https://upx.github.io/
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.