Skip to content

Conversation

camdenfullmer
Copy link

The following PR adds support for setting SQLITE_ENABLE_FTS5=1 as an environment variable when compiling SwiftToolchainCSQLite to enable FTS5.

This is the best way I thought to enable this feature when swift-toolchain-sqlite is not directly used as a dependency (e.g. when using https://github.com/stephencelis/SQLite.swift).

Also, when working on this I think the defines that are used currently are in the wrong location. They are defined as part of the sqlite executable, but they should be in the cSettings of SwiftToolchainCSQLite. For example SQLITE_OMIT_LOAD_EXTENSION is not taken into account when compiling sqlite3.c.

@jakepetroules
Copy link
Collaborator

From this package's description:

Copy of SQLite for use by clients within the Swift toolchain. This is not a general-purpose wrapper for SQLite.

It's really meant to support packages internal to the Swift toolchain, rather than something third parties should build general SQLite abstractions on top of.

That said, it's hard to truly prevent that, and I suppose I'd be open to making these sort of additions as long as it doesn't conflict with this package's goals of serving the toolchain build. Have you considered adding this flag via package traits?

@camdenfullmer
Copy link
Author

It's really meant to support packages internal to the Swift toolchain, rather than something third parties should build general SQLite abstractions on top of.

I totally understand where you're coming from and the intended use of this package. I guess I came here first because the SQLite package was using this package as a dependency and thought I could add functionality if someone also wanted to have FTS capabilities upstream.

The other solution I was thinking of was to enable a way to bypass the amalgamation file and use SQLite that is installed on the system. This is what I was doing before with the SQLite package, but unfortunately updating to use this package broke that.

Have you considered adding this flag via package traits?

I didn't know about this feature and seems like a much better solution than the environment variables!

I'm totally ok with going to the SQLite package and adding back in the functionality to use the system installed SQLite and not adding FTS support to this package.

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