Parameter / flag parsing from YAML does not remove quotes around quoted arguments

[juliusv]

The parameter parsing code at

promu/util/sh/sh.go

Lines 46 to 50 in 0455049

	// SplitParameters splits shell command parameters, taking quoting in account.
	func SplitParameters(s string) []string {
	r := regexp.MustCompile(`'[^']*'|[^ ]+`)
	return r.FindAllString(s, -1)
	}

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.

[simonpasquier]

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.

[juliusv]

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).

[simonpasquier]

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.)

[juliusv]

@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 :)

[simonpasquier]

@juliusv I didn't know either until today :-) I suspect that they moved away from the space-separated list because quoting is hard.

[jan--f]

I suspect this can be closed?

[juliusv]

@jan--f From what I understand, the original bug still exists, I just ended up working around it for my use case back then.

[emilykmarx]

FWIW, I have another use case for this (similar to #235), that I haven't yet found a workaround for - I'd like to build prometheus with debug flags for delve, specifically -gcflags="all=-N -l" .
Initially I thought of passing -gcflags multiple times as a workaround, but I believe that if -gcflags is passed multiple times to go build, the last match will win, based on my reading of https://github.com/golang/go/blob/ac803b5949f6dbc5bfa559afe506d35f9e1b3195/src/cmd/go/testdata/script/gcflags_patterns.txt#L13 and https://github.com/golang/go/blob/ac803b5949f6dbc5bfa559afe506d35f9e1b3195/src/cmd/go/internal/load/flag_test.go#L26-L84