promu
promu copied to clipboard
Parameter / flag parsing from YAML does not remove quotes around quoted arguments
The parameter parsing code at https://github.com/prometheus/promu/blob/04550494378ed57e36ba704721cbefd26f4253ee/util/sh/sh.go#L46-L50 allows extracting quoted arguments (to allow such things as -tags 'foo bar'
), but it does not remove the quotes around quoted arguments before passing this to the execution layer. Thus, quoted arguments aren't currently working correctly.
I wonder overall whether we should try to parse this as one line or go the "normal" way of just having a YAML list of command-line arguments with no postprocessing needed after YAML parsing.
Hmm do you have an example showing what isn't working? In any case, I agree that it would be better to have a list.
Yeah, I tried changing
https://github.com/prometheus/prometheus/blob/53ea6d63901b96a13bb79ff598c96af4a979adce/.promu.yml#L15
...to:
flags: -mod=vendor -a -tags 'netgo release'
...and it didn't set the release
tag (I guess it also didn't set the netgo
tag then, but nowadays it appears that it results in a static binary even without that tag).
Hmm, I swear that I had tested something similar and it did take into account the build tags but now I see what you describe.
As a quick workaround, we can change -tags foo bar
to -tags foo,bar
.
From https://golang.org/cmd/go/#hdr-Compile_packages_and_dependencies
-tags tag,list a comma-separated list of build tags to consider satisfied during the build. For more information about build tags, see the description of build constraints in the documentation for the go/build package. (Earlier versions of Go used a space-separated list, and that form is deprecated but still recognized.)
@simonpasquier Thanks! I didn't know that Go 1.13 supports comma-based separation now, that failed for me yesterday with an older Go version, but now it works :)
@juliusv I didn't know either until today :-) I suspect that they moved away from the space-separated list because quoting is hard.
I suspect this can be closed?
@jan--f From what I understand, the original bug still exists, I just ended up working around it for my use case back then.