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/go: importing "C" with CGO disabled should be an error #12860

Open
pebbe opened this Issue Oct 7, 2015 · 9 comments

Comments

Projects
None yet
5 participants
@pebbe
Contributor

pebbe commented Oct 7, 2015

go version go1.5.1 linux/amd64

Building with CGO_ENABLED=0 silently skips source files that have import "C", leading to incomprehensible error messages from other source files that depend on it.

There should be an error, like:

source.go:10: import "C" failed because CGO is disabled
@minux

This comment has been minimized.

Member

minux commented Oct 7, 2015

@davecheney

This comment has been minimized.

Contributor

davecheney commented Oct 7, 2015

@minux, could we augment go/build to set a field, NeedsCGO, or HasCGO, or
something inside the build.Package structure to detect this ?

On Wed, Oct 7, 2015 at 4:16 AM, Minux Ma notifications@github.com wrote:

cmd/go is a little difficult to diagnose this problem
because when cgo is disabled, go/build will not
report any CgoFiles in struct Package (the files
that need cgo will be in the IgnoredGoFiles slice.)


Reply to this email directly or view it on GitHub
#12860 (comment).

@minux

This comment has been minimized.

Member

minux commented Oct 7, 2015

@rakyll

This comment has been minimized.

Member

rakyll commented Oct 7, 2015

What if the user is providing a non-cgo implementation as well?

// +build !cgo

package main

import "fmt"

func main() {
    fmt.Println("hello")
}

// +build cgo

package main

import "fmt"
import "C"

func main() {
    fmt.Println("hello")
}

We shouldn't fail this package just because it is importing C. In Go, build is constrained to the matching files for the given context. We can improve the current "no buildable Go source files" error message but anything too smart will break the existing use cases.

@rakyll rakyll added this to the Unplanned milestone Oct 7, 2015

@pebbe

This comment has been minimized.

Contributor

pebbe commented Oct 7, 2015

When there are no build directives in any Go source files, this should imply that all sources should be used on all platforms. (Except for files with platform specific names, e.g. source_windows.go)

Any Go files that import C should build always, unless it is set for cgo-only with a build directive. Without such directive, not having cgo enabled should be an error.

@adg

This comment has been minimized.

Contributor

adg commented Oct 8, 2015

@pebbe I can't think of a backward-compatible way of doing that. Can you?

Whether we like it or not, import "C" is an implicit cgo build tag.

I think the solution here is for the "no buildable Go source files" to explain which files were excluded and why. For example:

can't load package: package foo: no buildable Go source files in /Users/adg/src/foo
    foo.go: no match for build tag 'cgo' 
@pebbe

This comment has been minimized.

Contributor

pebbe commented Oct 8, 2015

The problem is when there are some source files that import C, and some that don't, the latter using functions that are defined in the first. Than you get errors like:

github.com/something/something/somefile.go:10: undefined: SomeThing

People report issues because they don't understand this.

I tried adding this to Go sources that import C:

// +build cgo !cgo

That doesn't work.

My current solution is to add an empty C file and at least one Go file that doesn't import C.

Perhaps there could be a build directive that says that it is an error if a file is not used in the build, that overrules the implicit cgo directive, like:

// +build all
@adg

This comment has been minimized.

Contributor

adg commented Oct 9, 2015

What about putting // +build cgo in all the source files in the package? At least then they'll get the "no buildable source files" error, rather than confusing "undefined" errors. (That build directive is also accurate; you don't want that file to build without cgo enabled.)

My current solution is to add an empty C file and at least one Go file that doesn't import C.

How does that solve anything? I thought you already had Go files that don't import C in that package.

@pebbe

This comment has been minimized.

Contributor

pebbe commented Oct 9, 2015

There are two cases. Packages where some Go source files import C, and packages where all Go files import C. Without cgo, in the first case you get the "no buildable source files", and in the second the "undefined" errors. Both are confusing. They don't tell what the real problem is.

In the second case, I only add an empty C file. This results in the error "C source files not allowed when not using cgo or SWIG".

For this to work in the first case, you have to add an empty Go file as well, one that doesn't import C.

@adg adg changed the title from Importing "C" with CGO disabled should be an error to cmd/go: importing "C" with CGO disabled should be an error Oct 12, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment