main: support comma-delimited -tags

[omnide]

trafficstars

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. https://github.com/golang/go/issues/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. https://github.com/golang/go/issues/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:

❯ for t in "comma,delim" "space delim" "comma,and space delim"; do tinygo info -tags ${t} | grep "build tags"; done
build tags:        linux amd64 tinygo math_big_pure_go gc.precise scheduler.tasks serial.none comma delim
build tags:        linux amd64 tinygo math_big_pure_go gc.precise scheduler.tasks serial.none space delim
build tags:        linux amd64 tinygo math_big_pure_go gc.precise scheduler.tasks serial.none comma and space delim

Closes #3966.

[omnide]

Converted to draft, I'll also update loader/list.go noted in the issue ticket.

[aykevl]

I've commented on https://github.com/golang/go/issues/44787, let's see whether we get a response in a reasonable timeframe.

[aykevl]

@omnide can you try to get this fixed upstream? See https://github.com/golang/go/issues/44787#issuecomment-1901114568 for details.

While we can work around this in TinyGo, the proper place to fix this would be upstream. So if at all possible, I'd prefer that over rolling our own solution.

[omnide]

@omnide can you try to get this fixed upstream? See golang/go#44787 (comment) for details.

While we can work around this in TinyGo, the proper place to fix this would be upstream. So if at all possible, I'd prefer that over rolling our own solution.

Hey @aykevl, I totally agree about upstream fix being preferred here. I'll follow up on golang/go#44787 and the linked gerrit.

[omnide]

I opened https://go-review.googlesource.com/c/tools/+/558115 and will keep it moving until we have a resolution. I now have a signed CLA for golang and can contribute there.

[deadprogram]

The upstream change appears to have been merged, so what needs to happen to this PR now?