Go: cmd/go: escape quoted strings in GOFLAGS

Created on 7 Aug 2018  ·  18Comments  ·  Source: golang/go

In https://golang.org/cl/126656, @rsc added the GOFLAGS environment variable.

The variable is space-separated, but it is sometimes useful to include spaces within flags passed to the go command (for example, as the final argument to list -f). It would be nice to recognize at least a subset of the usual quoting conventions when parsing GOFLAGS.

~/go/src/cmd/go$ go list -e -deps -f="{{if .Error}}{{.Error}}{{end}}"

~/go/src/cmd/go$ export GOFLAGS='-e -deps -f="{{if .Error}}{{.Error}}{{end}}"'

~/go/src/cmd/go$ go list
go: parsing $GOFLAGS: non-flag ".Error}}{{.Error}}{{end}}\""

FeatureRequest NeedsDecision

Most helpful comment

That's OK, GOFLAGS is for things like -ldflags=-w.

My usage of -ldflags almost always also includes spaces -- I frequently use -d -s -w in order to get smaller static binaries (where size is the primary concern); combined with -tags netgo and CGO_ENABLED=0, this works pretty well.

I think -tags is another great problematic example -- multiple instances of -tags replace each other, and the argument to -tags is space separated, so -tags can't easily be used via GOFLAGS today except for the absolute simplest cases.

Respectfully, I would request that this be reopened and reconsidered, especially since the primary use case where this feature got me excited (building the same binary for a load of different architectures in a row) aren't actually helped by this feature today. :heart: :sweat_smile:

(I commented on the CL and was redirected here. :heart:)

All 18 comments

@bcmills if you want to support cmd.exe users on Windows, you should consider how quoting works in cmd.exe. For example, this http://daviddeley.com/autohotkey/parameters/parameters.htm#WINCMDRULES shows cmd.exe rules.

Alex

Sorry, but no. I looked at this when I added GOFLAGS, and we don't do any special quoting for the other environment variables (go help environment | grep FLAGS)

This means you cant use GOFLAGS for your example.
That's OK, GOFLAGS is for things like -ldflags=-w.

That's OK, GOFLAGS is for things like -ldflags=-w.

My usage of -ldflags almost always also includes spaces -- I frequently use -d -s -w in order to get smaller static binaries (where size is the primary concern); combined with -tags netgo and CGO_ENABLED=0, this works pretty well.

I think -tags is another great problematic example -- multiple instances of -tags replace each other, and the argument to -tags is space separated, so -tags can't easily be used via GOFLAGS today except for the absolute simplest cases.

Respectfully, I would request that this be reopened and reconsidered, especially since the primary use case where this feature got me excited (building the same binary for a load of different architectures in a row) aren't actually helped by this feature today. :heart: :sweat_smile:

(I commented on the CL and was redirected here. :heart:)

@rsc I'm not sure I understand the argument for not permitting quoting in the environment variable given the special support we have for quoting in -gcflags and friends. It seems to me that there are clear uses for passing multiple options to all builds, such as the -l -N that delve uses.

Ping @rsc @bcmills.

Leaving for Go 1.13. GOFLAGS is relatively new and the workaround is to use the command line, like you had to do before Go 1.11.

In case we do loosen this constraint on GOFLAGS, I think we will also need to add support for accepting multiple -tags settings.

At the moment the last -tags setting "wins":

$ GOFLAGS="-tags=banana" go list -tags apple -f "{{with context}}{{.BuildTags}}{{end}}" runtime
[apple]

Given my use-case, this is relatively easy to work around by parsing the space-separated GOFLAGS for -tags=-prefixed strings and adding the results to the last command line flag.

But if we allow escape quoted strings in GOFLAGS, this parsing becomes more difficult.

I suspect allowing multiple instances of -tags would be useful in any case? I can always spin off another proposal/CL if needs be.

I suspect allowing multiple instances of -tags would be useful in any case?

Also note that the per-package flags like -ldflags also overwrite each other, but not always. For example, in -ldflags=-w -ldflags=-s the second wins, but in -ldflags=foo=-w -ldflags=bar=-s both apply (to different packages). In that flag, it makes sense as it allows to override a previous per-package flag instead of only appending to it.

Not saying that I disagree with allowing multiple -tags flags; I actually think what you're trying to do is reasonable. My point is that I think we should fix the GOFLAGS interaction with all these list-like flags in a consistent way.

Re-reading what I wrote in https://github.com/golang/go/issues/26849#issuecomment-454446697 I realise it wasn't that clear.

At the moment, because GOFLAGS is:

A space-separated list of -flag=value settings

it can't contain a multi-value -tags flag value because -tags is:

a space-separated list of build tags to consider satisfied during the build

Add to that, cmd/go does not understand multiple -tags values; the last one wins.

Hence at the moment, it is not possible to specify a multi-value -tags via GOFLAGS. This becomes problematic in situations like https://github.com/golang/go/issues/27898#issuecomment-433723027 where the GOFLAGS variable could otherwise be used as a means of passing build tags in the environment go generate sets for each directive it runs.

For my specific case there are at least two possible non-mutually exclusive solutions:

  • allowing quoted strings in GOFLAGS (this proposal)
  • having cmd/go accept -tags multiple times, i.e. space-joining the multiple -tags values

It is for this second option that I can create a separate issue if required.

Another alternative that would only work for -tags would be to accept comma separated tags. This feels more natural anyway, and would match the syntax used in the +build lines anyway.
This would be very helpful for people needing to set multiple tags and have them obeyed by tools invoked by their editors.

Change https://golang.org/cl/173438 mentions this issue: cmd/go: change -tags to a comma-separated list

Funnily enough, I thought that was how -tags worked years ago. That is why I filed https://github.com/golang/go/issues/18800 and mailed a change for cmd/go to give a more obvious error. So I agree with @ianthehat; I'm not sure why that idea hadn't popped up again since 2017.

Don't mean to bump the thread, but - is this planned for 1.14, or is it likely to be moved to 1.15 if noone works on it?

If so, I really want a fix for this problem, so I can work on a fix for 1.14. We'd just need to agree on either of these solutions:

1) Support basic forms of quoting in GOFLAGS, as per @bcmills
2) Support commas in -ldflags and -gcflags, similar to the recent change in -tags, as per @ianthehat

I personally think option 2 is the most consistent, and probably the simplest. Like with -tags, we can continue to allow spaces as a separator for now, since it can't be part of a valid flag argument anyway.

That's OK, GOFLAGS is for things like -ldflags=-w.

But IIUC it doesn't work once you specify more than one flag to -ldflags? I expected the environment variable called "GOFLAGS" to be an exact alternative to specifying Go flags. Something like

go build $MYFLAGS .
===
GOFLAGS="$MYFLAGS" go build . 

Personally, I don't find the statement "we don't do any special quoting for the other environment variables" to be a compelling reason not to strive for this invariant as it disregards a sensible request based on a present technical limitation.

A better response might be "treating specific environment variables differently will increase the maintenance burden beyond what we're happy with given our CLI architecture." Perhaps due to cross-platform considerations. I still don't find it compelling, but I'd understand.

In support of this as a use case, take this example:

go build -ldflags="-X 'main.Version=v1.0.0' -X 'app/build.User=$(id -u -n)' -X 'app/build.Time=$(date)'"

~ https://www.digitalocean.com/community/tutorials/using-ldflags-to-set-version-information-for-go-applications

I specifically want to use GOFLAGS to achieve the same thing, because I want to use ko publish to build and push my binary to a docker registry:

ko publish -ldflags="-X 'main.Version=v1.0.0' -X 'app/build.User=$(id -u -n)' -X 'app/build.Time=$(date)'"

This doesn't work, as ko assumes (much like I did) that the user can use GOFLAGS to decorate its go build invocation.

29096 was de-duped into this bug, so I’m posting here, but please let me know if I should open a separate issue for this:

Comment https://github.com/golang/go/issues/26849#issuecomment-454481195 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.

@stapelberg I think you raise a valid point, and I've thought about it as well, but I think that should be filed as a separate issue. That is, when cmd/go merges GOFLAGS with command-line flags, the behavior is simply that the flags are concatenated and then interpreted. So, any flags that override previous ones (like -tags, as per @myitcv above, or your example with -ldflags) aren't very usable with GOFLAGS.

@myitcv already mentioned raising this issue separately in https://github.com/golang/go/issues/26849#issuecomment-454446697 for -tags. I think one such issue should be filed for all these flags at once, explaining the unfortunate interaction with GOFLAGS.

I didn't get much of a response on https://github.com/golang/go/issues/26849#issuecomment-533866215 aside form a couple of reactions, so I didn't feel like there was enough consensus to try something out for 1.14.

This 1.15 cycle is being weird, but I'd like to give this a try in the two remaining weeks.

@bcmills do you still prefer adding simple quoting support to GOFLAGS?

@ianthehat do you still prefer adding support for flags like -ldflags to take commas, just like we did for -tags?

I used to prefer the second option, but I'm starting to think that simple quoting support is a better long-term fix. Commas are really only a short-term fix, don't support nesting of list parameters which also use commas, and IMO make complex list flags like -ldflags harder to read.

So, I now propose that we add support for two quoting syntaxes on all platforms equally:

  • single quotes, which end at the next single quote. no support for any form of escaping. very similar to single quotes in POSIX shell, and to raw string literals in Go.
  • double quotes, with Go syntax semantics. That is, they can contain nested double quotes like GOFLAGS=-ldflags="-foo=\"bar baz\"".

These should be fairly easy to implement. For example, the second would reuse https://golang.org/pkg/strconv/#Unquote for unquoting. If this generally seems like a good idea, I can send a CL this coming week and we can give it a try during the freeze.

I think one such issue should be filed for all these flags at once, explaining the unfortunate interaction with GOFLAGS.

Filed https://github.com/golang/go/issues/38522. Please amend as you see fit.

Was this page helpful?
0 / 5 - 0 ratings