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/go: do not modify environment for 'go run' #11709

Closed
calmh opened this issue Jul 14, 2015 · 8 comments
Closed

cmd/go: do not modify environment for 'go run' #11709

calmh opened this issue Jul 14, 2015 · 8 comments
Labels
FrozenDueToAge
Milestone
Go1.5

Comments

@calmh
Copy link
Contributor

calmh commented Jul 14, 2015

For example compiling for dragonfly/amd64 on darwin/amd64:

(Via a build script, but what happens is just a setting of GOPATH for vendoring and some ldflags for version tagging being added)

jb@syno:~/src/github.com/syncthing/syncthing (go15) $ go run build.go -goos dragonfly
*** Unknown Go version "go version devel +b6ead9f Tue Jul 7 21:53:11 2015 +0000 darwin/amd64".
*** This isn't known to work, proceed on your own risk.
GOPATH=/Users/jb/src/github.com/syncthing/syncthing/Godeps/_workspace:/Users/jb
go install -v -ldflags -w -X main.Version=v0.11.15+6-gff03813 -X main.BuildStamp=1436877241 -X main.BuildUser=jb -X main.BuildHost=syno -X main.BuildEnv=default ./cmd/...
runtime/cgo
# runtime/cgo
/usr/local/go-exp/src/runtime/cgo/gcc_dragonfly_amd64.c:37:2: error: implicit declaration of function 'SIGFILLSET' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
/usr/local/go-exp/src/runtime/cgo/gcc_dragonfly_amd64.c:37:13: error: variable 'ign' is uninitialized when used here [-Werror,-Wuninitialized]
/usr/local/go-exp/src/runtime/cgo/gcc_dragonfly_amd64.c:32:14: note: initialize the variable 'ign' to silence this warning
exit status 2
exit status 1

I'm not sure what it's trying to do, but the error sounds like it's trying to compile stuff for dragonfly using the system compiler, which is obviously quite different from what would exist on actual dragonfly. Disabling cgo makes it work:

jb@syno:~/src/github.com/syncthing/syncthing (go15) $ CGO_ENABLED=0 go run build.go -goos dragonfly
*** Unknown Go version "go version devel +b6ead9f Tue Jul 7 21:53:11 2015 +0000 darwin/amd64".
*** This isn't known to work, proceed on your own risk.
GOPATH=/Users/jb/src/github.com/syncthing/syncthing/Godeps/_workspace:/Users/jb
go install -v -ldflags -w -X main.Version=v0.11.15+6-gff03813 -X main.BuildStamp=1436877241 -X main.BuildUser=unknown-user -X main.BuildHost=syno -X main.BuildEnv=default ./cmd/...

Is this expected? (cgo not working is obviously expected, what surprises me is that it seems to try to enable it by default.)

(I see similar but different errors on linux/amd64 -> dragonfly/amd64 so this one at least is not darwin specific)
@ianlancetaylor
Copy link
Contributor

We need to know what your build.go program does.

Normally CGO_ENABLED is set to false for a cross-compiler. That code is in defaultContext in the go/build package, and it has not changed since 1.4. Apparently something has changed somewhere, but without knowing what build.go does I don't know what it is.

@ianlancetaylor ianlancetaylor changed the title go1.5beta1 cross compilations fail due to cgo, where they previously succeeded build: go1.5beta1 cross compilations fail due to cgo, where they previously succeeded Jul 14, 2015
@ianlancetaylor ianlancetaylor added this to the Go1.5 milestone Jul 14, 2015
@calmh
Copy link
Contributor Author

calmh commented Jul 14, 2015

Yeah, I figured it out. The build script itself does nothing fancy apart from building the parameter lists above and then running "go" with them. However, when I go run build.go, the first instance of go adds CGO_ENABLED, GOGCCFLAGS and a few other variables to the environment. These are then inherited when the build script execs "go install ...". I can filter them out, by inspecting the environment set by go run and removing the stuff I don't want.

Something here seems to have changed, but it may as well just be the fact that packages are built on the fly for cross compilation rather than as it was before.

@ianlancetaylor
Copy link
Contributor

I'll close this, but feel free to reopen if you find out what changed and think it should be changed back.

@calmh
Copy link
Contributor Author

calmh commented Jul 15, 2015

What changed is that "go run" now sets a number of environment variables it didn't before:

jb@syno:~ $ cat test.go 
package main

import (
    "os"
    "os/exec"
)

func main() {
    c := exec.Command("/bin/bash", "-c", "export")
    c.Stdout = os.Stdout
    c.Run()
}

# Go 1.5:
jb@syno:~ $ go run test.go | grep GO
declare -x CGO_ENABLED="1"
declare -x GOARCH="amd64"
declare -x GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fno-common"
declare -x GOHOSTARCH="amd64"
declare -x GOHOSTOS="darwin"
declare -x GOOS="darwin"
declare -x GOPATH="/Users/jb"
declare -x GOROOT="/usr/local/go-exp"
declare -x GOTOOLDIR="/usr/local/go-exp/pkg/tool/darwin_amd64"

# Go 1.4:
jb@syno:~ $ GOROOT=/usr/local/go1.4.2 /usr/local/go1.4.2/bin/go run test.go | grep GO
declare -x GOPATH="/Users/jb"
declare -x GOROOT="/usr/local/go1.4.2"
jb@syno:~ $

If this is intentional, I guess it's fine. Possibly it should be documented somewhere.

@ianlancetaylor
Copy link
Contributor

Thanks for looking into it. The change was intentional: https://go-review.googlesource.com/6409 .

Reopening to document this.

@ianlancetaylor ianlancetaylor changed the title build: go1.5beta1 cross compilations fail due to cgo, where they previously succeeded cmd/go: document that go commands, notably go run, now set environment variables Jul 15, 2015
@rsc
Copy link
Contributor

rsc commented Jul 21, 2015

The change was intentional for things like go tool compile. It was not meant to affect go run. That's a bug and should be fixed.

@rsc rsc changed the title cmd/go: document that go commands, notably go run, now set environment variables cmd/go: do not modify environment for 'go run' Jul 21, 2015
@rsc rsc removed the Documentation label Jul 21, 2015
@gopherbot
Copy link

CL https://golang.org/cl/12483 mentions this issue.

@gopherbot
Copy link

CL https://golang.org/cl/13449 mentions this issue.

rsc added a commit that referenced this issue Aug 11, 2015
@rsc
cmd/go: run test binaries in original environment
58035ec 
Fixes #12096.
Followup to CL 12483, which fixed #11709 and #11449.

Change-Id: I9031ea36cc60685f4d6f65c39f770c89b3e3395a
Reviewed-on: https://go-review.googlesource.com/13449
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@golang golang locked and limited conversation to collaborators Aug 10, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.5
Development

No branches or pull requests
4 participants
@rsc @calmh @ianlancetaylor @gopherbot