Skip to content
New issue

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

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

Already on GitHub? # to your account

actually link to <sqlite3.h> when -tags libsqlite3 #339

Merged
merged 2 commits into from
Oct 28, 2016

Conversation

brauner
Copy link
Contributor

@brauner brauner commented Oct 17, 2016

Building with -tags libsqlite3 used the sqlite3.h from the system but the go
compiler will compile all *.{c,h} files in the same direcory:

"When the Go tool sees that one or more Go files use the special import
"C", it will look for other non-Go files in the directory and compile
them as part of the Go package. Any .c, .s, or .S files will be compiled
with the C compiler." (https://golang.org/cmd/cgo/)

So if users actually want to link against the system sqlite3 we should make
sqlite3-binding.* a noop.

Signed-off-by: Christian Brauner christian.brauner@canonical.com

Closes #330.

After the patch:

  1. linking against system sqlite3:
chb@conventiont|~/source/go/src/github.com/mattn/go-sqlite3|2016-10-17/fix_libsqlite3_build>
> go build -tags libsqlite3 _example/simple/simple.go
chb@conventiont|~/source/go/src/github.com/mattn/go-sqlite3|2016-10-17/fix_libsqlite3_build %>
> ldd simple | grep sqlite
        libsqlite3.so.0 => /usr/lib/x86_64-linux-gnu/libsqlite3.so.0 (0x00007f860d0a4000)
  1. using vendored sqlite3:
chb@conventiont|~/source/go/src/github.com/mattn/go-sqlite3|2016-10-17/fix_libsqlite3_build %>
> go build _example/simple/simple.go
chb@conventiont|~/source/go/src/github.com/mattn/go-sqlite3|2016-10-17/fix_libsqlite3_build %>
> ldd simple | grep sqlite

@coveralls
Copy link

coveralls commented Oct 17, 2016

Coverage Status

Coverage remained the same at 62.274% when pulling 78124cb on brauner:2016-10-17/fix_libsqlite3_build into e5a3c16 on mattn:master.

@brauner
Copy link
Contributor Author

brauner commented Oct 27, 2016

@mattn ping. We'd really like to see this issue resolved. Would be great if you could take a look whether this pr is enough. :)

@mattn
Copy link
Owner

mattn commented Oct 28, 2016

please update tool/upgrade.go too

@brauner
Copy link
Contributor Author

brauner commented Oct 28, 2016

@mattn, done.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.6%) to 61.668% when pulling 0554f97 on brauner:2016-10-17/fix_libsqlite3_build into e5a3c16 on mattn:master.

1 similar comment
@coveralls
Copy link

coveralls commented Oct 28, 2016

Coverage Status

Coverage decreased (-0.6%) to 61.668% when pulling 0554f97 on brauner:2016-10-17/fix_libsqlite3_build into e5a3c16 on mattn:master.

Christian Brauner added 2 commits October 28, 2016 10:28
Building with -tags libsqlite3 used the sqlite3.h from the system but the go
compiler will compile all *.{c,h} files in the same direcory:

	"When the Go tool sees that one or more Go files use the special import
	"C", it will look for other non-Go files in the directory and compile
	them as part of the Go package. Any .c, .s, or .S files will be compiled
	with the C compiler." (https://golang.org/cmd/cgo/)

So if users actually want to link against the system sqlite3 we should make
sqlite3-binding.* a noop.

Signed-off-by: Christian Brauner <christian.brauner@canonical.com>
This makes it possible to correctly link against the system sqlite. Also, don't
use defers with log.Fatal() since they won't be run on exit.

Signed-off-by: Christian Brauner <christian.brauner@canonical.com>
@brauner brauner force-pushed the 2016-10-17/fix_libsqlite3_build branch from 0554f97 to 420dc66 Compare October 28, 2016 08:28
@coveralls
Copy link

coveralls commented Oct 28, 2016

Coverage Status

Coverage decreased (-0.9%) to 61.668% when pulling 420dc66 on brauner:2016-10-17/fix_libsqlite3_build into 955dae4 on mattn:master.

@mattn
Copy link
Owner

mattn commented Oct 28, 2016

LGTM!

@mattn mattn merged commit dbe15b0 into mattn:master Oct 28, 2016
@mattn
Copy link
Owner

mattn commented Oct 28, 2016

Thanks!

@marienz
Copy link

marienz commented Nov 5, 2016

See also #318. This was previously handled using a +build !libsqlite3 build tag. The go tool recognizes such tags even in non-Go source, and does not invoke the C compiler at all. I'd argue that's a tidier solution than this one, which runs the C compiler on an effectively empty file.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

go-sqlite3 still using built-in copy despite libsqlite3 tag
4 participants