main: support comma-delimited -tags #4088
Open
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Updated: the upstream fix I made to golang/tools was released in v0.18.0 so this PR has been updated to just use the latest release and pick that up. I updated instances where build tags were still using spaces in the makefile, retained the change for loader/list.go. This also includes a makefile update to allow override of the
go test
command so that utilities like gotestsum can be used:GOTEST="gotestsum --" make test
.Base go tooling originally supported tags as spaced separated list, but more recent versions use comma-separated. The old space delimited form is still supported but deprecated per the latest usage dumps.
In earlier golang discussion around the use of commas, the fixes just added error messages if commas were found, but this was later changed.
golang/go#18800
The current version in
go build
permits the old style (< go1.13) with spaces instead of commas, but does not allow mixing the two forms. So either "tagA tagB tagC" or "tagA,tagB,tagC", but NOT "tagA,tagB tagC". Need to check with tinygo maintainers if they have a preference here.Blame from go lang cmd internals:
https://go.googlesource.com/go/+/80e7832733fd245181af3394077f2df21303a4aa%5E%21/src/cmd/go/internal/work/build.go
Here Russ Cox favors commas and deprecates spaces.
More recent issue related to buildutil.TagsFlag, which only handles the spaced form. Issue remains open so we can't rely on TagsFlag to manage this for us.
golang/go#44787
Golang contributor opened a PR to address it but it stalled in 2021. A golang maintainer requested additional test coverage, which was provided but then no reply after that.
https://go-review.googlesource.com/c/tools/+/284214#message-fc9907b03ae75ae24145f421e623330b4b9b9158
The associated github PR notes that they couldn't locate a CLA for the committer identity golang tools pr 268. I also do not have a CLA for this.
I think it would be best to accept either form, even combinations. But also amenable to stricter validation and just exiting with a message about not mixing forms.
Example with flexible fix in place:
Closes #3966.