Skip to content

build/ci.go: don't set CC for go run ci.go, only inside#15784

Merged
fjl merged 1 commit into
ethereum:masterfrom
karalabe:ci-cc-flag-noenv
Jan 2, 2018
Merged

build/ci.go: don't set CC for go run ci.go, only inside#15784
fjl merged 1 commit into
ethereum:masterfrom
karalabe:ci-cc-flag-noenv

Conversation

@karalabe
Copy link
Copy Markdown
Member

@karalabe karalabe commented Dec 30, 2017

Our build/ci.go script + travic config combo was incorrect from a Go tooling standpoint, but worked correctly up to Go 1.9 due to limitations of Go itself. Go 1.10 fixes the limitations, causing our CI script to break.

The issue lies in the way we pass the C compiler for cross compilation to ci.go. Currently we just set CC before calling go run build/ci.go, which will get passed to the inside cross compilation invocation. The issue is that this in reality sets CC for the outer go run too, not just cross compilation. So in theory if ci.go depended on CGO, things would blow up because we try to use some cross compiler to build for the host.

Unfortunately, ci.go does indeed depend on CGO through some arbitrary dependency path. The reason we never hit this issue is because Go 1.9 and below just looked at runtime/cgo.a, saw that it was recent and didn't try to rebuild. Go 1.10 gets a bit smarter, and notices that the C compiler changed, so it will try to rebuild runtime/cgo, which will result in all kinds of failures when the C compiler is actually a cross compilat for a different platform/architecture (e.g. arm).

This PR fixes the issue by not specifying the C compiler through the outer CC env var, rather sets it as a flag to ci.go install, which will internally convert it into a CC env var before invoking the cross compiling Go step.

More details and discussion on the Go issue tracker: golang/go#23288

@fjl fjl merged commit 3e0113f into ethereum:master Jan 2, 2018
mariameda pushed a commit to NiluPlatform/go-nilu that referenced this pull request Aug 23, 2018
This avoids setting CC for the go run invocation, which fails on go1.10.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants