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

Proposal: sub-packages to enable optional features #1134

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

emersion
Copy link

@emersion emersion commented Feb 9, 2023

A common pain point when using go-sqlite3 is that enabling optional features requires adding a build tag to all go command invocations. For instance, when a program depends on FTS5, it's no longer possible to run go build, go test, etc as usual. End users need to explicitly add -tags sqlite_fts5 every time.

Here is a proposal to improve this. A new fts5 sub-package is introduce. Any program which depends on FTS5 can import it to turn on FTS5:

import _ "github.com/mattn/go-sqlite3/fts5"

What do you think?

@rittneje
Copy link
Collaborator

rittneje commented Feb 9, 2023

Have you tested this? AFAIK the #cgo directives are package-scoped. Consequently, specifying them in a sub-package like this won't have any effect.

@emersion
Copy link
Author

emersion commented Feb 9, 2023

Yes, I have tested this and it works correctly.

AFAIU the cgo flags are not package-scoped, they are module-scoped. See: golang/go#40041

@rittneje
Copy link
Collaborator

rittneje commented Feb 9, 2023

I just ran a simplified test case. From what I can tell, the #cgo directives are indeed package-scoped, not module-scoped.

cgotest/go.mod

module cgotest

go 1.18

cgotest/main.go

package main

import _ "cgotest/subpackage"

/*
#include <sqlite3.h>
*/
import "C"

func main() {
	C.sqlite3_open_v2(nil, nil, 0, nil)
}

cgotest/subpackage/c.go

package subpackage

/*
#cgo LDFLAGS: -lsqlite3
*/
import "C"
$ go run main.go 
# command-line-arguments
Undefined symbols for architecture x86_64:
  "_sqlite3_open_v2", referenced from:
      __cgo_6884829902c1_Cfunc_sqlite3_open_v2 in _x002.o
     (maybe you meant: __cgo_6884829902c1_Cfunc_sqlite3_open_v2)
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

@emersion
Copy link
Author

emersion commented Feb 9, 2023

On my system this builds just fine (and fails at runtime because of the nil parameters):

> go version
go version go1.20 linux/amd64
> go run main.go
fatal error: unexpected signal during runtime execution
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x7f21b6f2da2d]

@rittneje
Copy link
Collaborator

rittneje commented Feb 9, 2023

Okay, based on further testing, it seems this behavior changed in Go 1.19. However, it was not mentioned in the release notes. And I'm not sure if it was intentional, since the cgo docs make no mention of modules or sub-packages. I'm going to file an issue with the Go authors for this.

In the meantime, I don't think we can accept this change, since this module intends to support Go 1.18.

@emersion
Copy link
Author

Note, the result of this discussion is at golang/go#58438 (comment).

@emersion
Copy link
Author

In the end I opted to bundle FTS5 in a separate Go package: https://git.sr.ht/~emersion/go-sqlite3-fts5

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.

None yet

2 participants