go/build: name_$(GOOS)_$(GOARCH)_test.syso is accepted, but included in non-test binaries

[TBBle]

What version of Go are you using (go version)?

$ go version
go version go1.18.1 windows/amd64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

go env Output

$ go env
set GO111MODULE=
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\paulh\AppData\Local\go-build
set GOENV=C:\Users\paulh\AppData\Roaming\go\env
set GOEXE=.exe
set GOEXPERIMENT=
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMODCACHE=C:\Users\paulh\go\pkg\mod
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=C:\Users\paulh\go
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=C:\Program Files\Go
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLDIR=C:\Program Files\Go\pkg\tool\windows_amd64
set GOVCS=
set GOVERSION=go1.18.1
set GCCGO=gccgo
set GOAMD64=v1
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=NUL
set GOWORK=
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=C:\Users\paulh\AppData\Local\Temp\go-build3103084805=/tmp/go-build -gno-record-gcc-switches

What did you do?

• Given containerd, where cmd/containerd/... depends on pkg/os/....
• Generated a file pkg/os/rsrc_windows_amd64_test.syso in order to add a manifest to the Windows AMD64 build of the test binary generated for that package, which did not include the "run as administrator" flag.
• Generated a file cmd/containerd/rsrc_windows_amd64.syso in order to add a manifest for the containerd.exe generated for that package, which included the "run as administrator" flag.

containerd/containerd#6945 is where this work took place.

If this isn't clear, I could put together a minimal reproduction fairly easily.

What did you expect to see?

• go test pkg/os/... would act as if it had a manifest, correctly identifying the Windows version on which it was running.
• go install cmd/containerd/... would produce conatinerd.exe that correctly identifies the Windows version on which it is running, and is marked to require Administrator permissions.

What did you see instead?

• go test pkg/os/... functioned as expected.
• go install cmd/containerd/... did not have the 'run as administrator" flag.
• go install cmd/containerd/... logged the following output:

C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/11.3.0/../../../../x86_64-w64-mingw32/bin/ld.exe: .rsrc merge failure: multiple non-default manifests
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/11.3.0/../../../../x86_64-w64-mingw32/bin/ld.exe: .rsrc merge failure: multiple non-default manifests

Both of these issues went away if I deleted pkg/os/rsrc_windows_amd64_test.syso, demonstrating that it was including that file in the build of cmd/containerd/....

I had a quick look at src/go/build/build.go, and the problem seems to be that goodOSArchFile accepts name_$(GOOS)_$(GOARCH)_test.* and other _test.* variants, but the code that splits out the _test variants only processes _test.go.

I think perhaps goodOSArchFile should reject name_$(GOOS)_$(GOARCH)_test.* and accept explicitly name_$(GOOS)_$(GOARCH)_test.go so that I would have gotten much earlier feedback that what I had wasn't going to do the right thing. I'd prefer that the _test.* suffix work for all extensions, but that looks like a much larger change, as the Package struct doesn't have a way to represent non-Go Test files.

This problem is not syso-specific, so I imagine if someone was trying to use name_$(GOOS)_$(GOARCH)_test.s they'd have the same issue, but I guess this specific use-case is fairly specific to Windows binary resource tables integrated using syso files.

I plan to work around this using some variant of #42477 (comment), so this bug is mostly that the build didn't fail for the non-sensical input it was given.