go/build: MatchFile excludes x_js.go when GOOS != js

[neild]

#102835 adds "js" to build.goosList, causing build.Context.MatchFile to exclude files named *_js.go when GOOS is not "js".

We have a number of existing files that match that pattern (mostly generated code containing embedded JavaScript strings), and there are probably more in the world at large. This seems like it might be too common a suffix to change at this time.

[ianlancetaylor]

Approximately a dup of #26329, though there we just documented this change.

If we change this, presumably by removing js from goosList in go/build/syslist.go, there are 13 xxx_js.go files in the standard library that may need explicit build tags.

We could consider changing the GOOS name from js to something less common. But the use of build tags in file names is an ongoing problem that will bite us again in the future. We could simply freeze the current set, and require explicit build tags going forward.

CC @neelance @bradfitz

[rsc]

The specific case where this arises in Google is that we have a go-bindata-like tool that reads x.js and writes x_js.go. Looking at the public go-bindata, though, it just writes one file bindata.go, which doesn't hit this problem. I'm also looking through a very large GOPATH with tons of packages pulled in, and it looks like there are at least a few external projects that already had *_js.go files for use with GopherJS.

It may be that we can leave this as-is and document. I'll report back once I have more data (still downloading).

[cespare]

I think we should explicitly and prominently document that naming files with underscores that aren't instructions to go build is discouraged and likely to lead to broken code in the future. I think this has been apparent to a lot of us for a long time now.

Personally, I really like the feature of naming files with os/arch and I'd dislike it if we moved in the direction of requiring explicit build tags in more situations.

[bradfitz]

FWIW we did document this in the release notes: https://tip.golang.org/doc/go1.11#wasm

[neild]

I think we should explicitly and prominently document that naming files with underscores that aren't instructions to go build is discouraged and likely to lead to broken code in the future.

If we go that route then I would argue that Go 2 should always interpret files with underscores as containing build instructions, regardless of the current set of GOOS values.

[rsc]

I am not convinced we should disallow all use of underscores except for GOOS/GOARCH. I think it would be fine for a project to have storage_aws.go, storage_azure.go, etc. If we do put the go version into the module file (#23969) then the go command could set the GOOS/GOARCH list appropriately when looking at files in that module. That would future-proof things (at least as far as not breaking builds) without imposing additional requirements on users.

Back to the question of how common the _js suffix is.

I have a list of the most commonly imported ~1000 open source repos that I use for testing vgo, plus ~100 of the projects that import the most other things. Scanning those repos and their dependencies, I found:

• 80,931 packages (29,281 after removing vendored code)
• 412,486 files (143,346 after removing vendored code)
• 51 *_js.go files (8 after removing vendored code)

It looks like these are all for GopherJS or the new wasm/js support:

$ find . -name '*_js.go' | grep -v /vendor/
./github.com/jtolds/gls/stack_tags_js.go
./github.com/pborman/uuid/node_js.go
./github.com/google/uuid/node_js.go
./github.com/gogo/protobuf/test/int64support/object_js.go
./github.com/goadesign/goa/uuid/uuid_js.go
./github.com/goadesign/goa/metrics_js.go
./github.com/mattn/go-runewidth/runewidth_js.go
./github.com/gopherjs/gopherjs/compiler/natives/src/internal/poll/fd_poll_js.go
$ 

That suggests it's fine to document and close (already done).