go/build: name_$(GOOS)_$(GOARCH)_test.syso is accepted, but included in non-test binaries
#52,921 opened on May 16, 2022
Repository metrics
- Stars
- (133,883 stars)
- PR merge metrics
- (No merged PRs in 30d)
Description
What version of Go are you using (go version)?
Does this issue reproduce with the latest release?
Yes.
What operating system and processor architecture are you using (go env)?
What did you do?
- Given containerd, where
cmd/containerd/...depends onpkg/os/.... - Generated a file
pkg/os/rsrc_windows_amd64_test.sysoin 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.sysoin order to add a manifest for the containerd.exe generated for that package, which included the "run as administrator" flag.
https://github.com/containerd/containerd/pull/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 https://github.com/golang/go/issues/42477#issuecomment-725591875, so this bug is mostly that the build didn't fail for the non-sensical input it was given.