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.

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

cmd/cgo: if CGO_ENABLED=0, cgo files are silently ignored #16981

Closed
FiloSottile opened this issue Sep 3, 2016 · 6 comments

Comments

@FiloSottile
Copy link
Member

commented Sep 3, 2016

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

go version go1.7 darwin/amd64

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

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/filippo/go"
GORACE=""
GOROOT="/usr/local/Cellar/go/1.7/libexec"
GOTOOLDIR="/usr/local/Cellar/go/1.7/libexec/pkg/tool/darwin_amd64"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/v8/xdj2snz51sg2m2bnpmwl_91c0000gn/T/go-build466071352=/tmp/go-build -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="1"

What did you do?

Built a package with both cgo and non-cgo files, with CGO_ENABLED=0:

CGO_ENABLED=0 go get github.com/FiloSottile/justcgo/plusgo

What did you expect to see?

An error about not being able to build cgo files.

What did you see instead?

cgo files were silently dropped, and the binary built anyway.


This is the simplified case, which might seem like the correct behavior, but how I found this was by cross-compiling github.com/mattn/go-sqlite3/_example/simple, which imports _ github.com/mattn/go-sqlite3, which register a database/sql driver.

I expected the command to fail, instead it built a binary. I was very puzzled. Eventually, I tried that binary and it complained about an unknown driver. Turns out github.com/mattn/go-sqlite3 has one file with no cgo directives: doc.go. Which makes failures to compile the package completely silent.

I don't think that should be intended behavior. A cgo-specific error is much more useful than an attempt at building anyway, which can lead to the confusing:

  • broken binary (missing side effects)
  • "no buildable files found"
  • variable/method/function not found
@FiloSottile

This comment has been minimized.

Copy link
Member Author

commented Sep 3, 2016

I initially thought this was a regression, because I never saw it happen before 1.7. Turns out it's more subtle than that: older versions of Go would still ignore.go files with cgo directives, but balk at .c files.

$ CGO_ENABLED=0 go14 build github.com/mattn/go-sqlite3/_example/simple
../go/src/github.com/mattn/go-sqlite3/_example/simple/simple.go:6:2: C source files not allowed when not using cgo: sqlite3-binding.c
$ CGO_ENABLED=0 go14 build github.com/FiloSottile/justcgo/plusgo
$ CGO_ENABLED=0 go build github.com/mattn/go-sqlite3/_example/simple
@rakyll

This comment has been minimized.

Copy link
Member

commented Sep 3, 2016

This behaviour has landed in Go 1.4. If cgo is not used, go build is ignoring the C files now.

When CGO_ENABLED=1 without import "C", C files are not allowed and build should fail.

@FiloSottile

This comment has been minimized.

Copy link
Member Author

commented Sep 3, 2016

Did you mean 1.4? My test above shows different behaviors is 1.4 and 1.7.

I understand this might have made the behavior more consistent, but it made real-world cases like the sqlite one above much more confusing.

@minux

This comment has been minimized.

Copy link
Member

commented Sep 4, 2016

@rakyll

This comment has been minimized.

Copy link
Member

commented Sep 4, 2016

Did you mean 1.4?

Rejecting the C files introduced in 1.4. Sorry, it sounds like skipping them has introduced.
go build -compiler=gc prohibiting builds with C files (when CGO_ENABLED=0) happened at https://codereview.appspot.com/149720043 (go1.4).

It is more of a requirement now to ship a package with cgo and !cgo implementations because C compilers are not primarily required for Go. Implicitly scoping C files without a build tag to cgo builds is not bad but handy.

sqlite package should maybe add cgo build tag to restrict itself to cgo builds.

@quentinmit quentinmit added this to the Go1.8Maybe milestone Sep 6, 2016

@rsc

This comment has been minimized.

Copy link
Contributor

commented Oct 20, 2016

This is working as intended: it lets you write a cgo implementation and a non-cgo implementation in the same directory. It sounds like the package you are building has a non-cgo implementation that does nothing. You can write files that are compiled only when not using cgo by adding "// +build !cgo" tags to them.

@rsc rsc changed the title cgo: if CGO_ENABLED=0, cgo files are silently ignored cmd/cgo: if CGO_ENABLED=0, cgo files are silently ignored Oct 20, 2016

@rsc rsc closed this Oct 20, 2016

@golang golang locked and limited conversation to collaborators Oct 20, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
6 participants
You can’t perform that action at this time.