Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cmd/test2json: CGO_ENABLED is set, when running tests from prebuilt binary #54545

Open
cardil opened this issue Aug 19, 2022 · 5 comments · May be fixed by #54554
Open

cmd/test2json: CGO_ENABLED is set, when running tests from prebuilt binary #54545

cardil opened this issue Aug 19, 2022 · 5 comments · May be fixed by #54554
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unplanned

Comments

@cardil
Copy link

cardil commented Aug 19, 2022

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

$ go version
go version go1.18.1 linux/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
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/ksuszyns/.cache/go-build"
GOENV="/home/ksuszyns/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/ksuszyns/.gvm/pkgsets/go1.18.1/global/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/ksuszyns/.gvm/pkgsets/go1.18.1/global"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/ksuszyns/git/golang-go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/ksuszyns/git/golang-go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.18.1"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build2603697588=/tmp/go-build -gno-record-gcc-switches"

What did you do?

  1. Checkout the reproduction repo: https://github.com/cardil/repro-cgo-drift
  2. Run make repro

What did you expect to see?

I expect to see the test pass regardless if executed via:

go test -count=1 ./

or

go test -c -o ./repro.test ./
go tool test2json -t ./repro.test -test.count=1

What did you see instead?

The process fails with the following output:

go test -c -o ./repro.test ./
go tool test2json -t ./repro.test -test.count=1
{"Time":"2022-08-19T12:42:13.742590288+02:00","Action":"output","Test":"TestCgo","Output":"--- FAIL: TestCgo (0.00s)\n"}
{"Time":"2022-08-19T12:42:13.743167948+02:00","Action":"output","Test":"TestCgo","Output":"    cgo_test.go:12: Expecting CGO_ENABLED equal \"\", got \"1\"\n"}
{"Time":"2022-08-19T12:42:13.743197548+02:00","Action":"fail","Test":"TestCgo","Elapsed":0}
{"Time":"2022-08-19T12:42:13.743212794+02:00","Action":"output","Output":"FAIL\n"}
{"Time":"2022-08-19T12:42:13.743499898+02:00","Action":"fail","Elapsed":0.006}

Additional context

I stumbled on this problem while running tests via the Goland IDE. The Goland IDE uses the pair of commands to execute tests:

go test -c -o /tmp/Goland/test.bin ./package/path
go tool test2json -t /tmp/Goland/test.bin -test.v -test.paniconexit0

We are executing google/ko in our tests to built test OCI images. KO sets the CGO_ENABLED=0 by default because the default base image is distroless, which doesn't contain any system libraries.

When I execute tests from Goland IDE, I have effectively CGO_ENABLED=1 when the test starts. This overrides the KO default value, and the produced image is invalid. The faulty image contains dynamic binary, which can't run on distroless.

When the tests are executed with go test directly, all is well.

I think running tests via go tool test2json -t shouldn't change the execution environment for the test.

See also: knative-extensions/control-protocol#204
@cardil
Copy link
Author

cardil commented Aug 19, 2022

I think the following fragment is responsible for this discrepancy:

go/src/cmd/go/main.go

Lines 202 to 213 in 6001c04

// Set environment (GOOS, GOARCH, etc) explicitly.
// In theory all the commands we invoke should have
// the same default computation of these as we do,
// but in practice there might be skew
// This makes sure we all agree.
cfg.OrigEnv = os.Environ()
cfg.CmdEnv = envcmd.MkEnv()
for _, env := range cfg.CmdEnv {
if os.Getenv(env.Name) != env.Value {
os.Setenv(env.Name, env.Value)
}
}

@joedian joedian added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 19, 2022
@seankhliao
Copy link
Member

Arguably the test is both fragile and wrong, it tests for an unset value (ie use the default) when it actually needs the specific value of off.

@cardil cardil linked a pull request Aug 19, 2022 that will close this issue
Open
@gopherbot
Copy link

Change https://go.dev/cl/424999 mentions this issue: cmd/test2json: use original environment to mirror go test

@cardil
Copy link
Author

cardil commented Aug 19, 2022 •
edited

The point is that the test2json uses effective env - same as go env, but go test actually uses the original env:

go/src/cmd/go/internal/test/test.go

Line 1358 in bf4e35b

env := cfg.OrigEnv[:len(cfg.OrigEnv):len(cfg.OrigEnv)]

The PR #54554 align this to be the same.

@bcmills
Copy link
Contributor

bcmills commented Aug 19, 2022

go tool test2json is not a supported way to run tests. If you want precise control over the process's environment, use go tool -n test2json to obtain the path to the tool, and then run it with whatever environment you need for it.

@seankhliao seankhliao added this to the Unplanned milestone Aug 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unplanned
Development

Successfully merging a pull request may close this issue.

cmd/test2json: use original environment to mirror go test cardil/golang-go
5 participants
@cardil @bcmills @gopherbot @seankhliao @joedian