sqlite3.dart icon indicating copy to clipboard operation
sqlite3.dart copied to clipboard

Enable extension loading

Open FaFre opened this issue 2 years ago • 5 comments

This PR is a draft to enable manual extensions loading via manual C-API call (https://www.sqlite.org/c3ref/enable_load_extension.html) for the built binaries.

Also I removed SQLITE_OMIT_GET_TABLE which is removing functions that are required by some extensions e.g. spatialite.

FaFre avatar Sep 01 '23 05:09 FaFre

Just FYI, the native assets feature and toolchain integration is far enough along to compile SQLite with it on all Dart supported native platforms. I've started playing around with a package that provides an SQLite API and makes use of native assets. Here is the build.dart file that defines how to build the native code, for reference. It's awesome how easy this makes shipping native code.

Support for native assets is available for standalone Dart when passing --enable-experiment=native-assets. On the Flutter side, support has landed for iOS, macOS, Linux and Windows on the master channel, with the PR for Android already open.

blaugold avatar Sep 28 '23 18:09 blaugold

That's awesome @blaugold! It's also pretty neat that we're able to use NativeCallables now.

The only feature missing for this library is being able to access native fields with this functionality (https://github.com/dart-lang/sdk/issues/50551). We use that to bind sqlite3_temp_directory. It's currently blocked on a refactoring of how @Native functions are implemented, I hope I'll be able to implement it quickly once that blocker is gone.

I'll see if I can already integrate native assets for the other things though.

simolus3 avatar Sep 28 '23 18:09 simolus3

As a workaround for accessing sqlite3_temp_directory you could do something like this: https://github.com/blaugold/sqlite_dart_native/commit/a4fd7c434fa7267d4da76a56e1e2ed187a83722d

blaugold avatar Sep 28 '23 19:09 blaugold

I've played around with it a bit in #185 - I also had to apply some workarounds for variadic @Native functions, but it works.

simolus3 avatar Sep 28 '23 21:09 simolus3

I've played around with it a bit in #185 - I also had to apply some workarounds for variadic @Native functions, but it works.

Nice! 🤓

blaugold avatar Sep 29 '23 04:09 blaugold

Hi @simolus3 This is the exact functionality I am also looking for. I need to enable extension loading specifically for linux and windows. Can I ask why you would not want to enable extension loading?

I can create a separate PR to remove only the SQLITE_OMIT_LOAD_EXTENSION flag but would like to clarify first.

mugikhan avatar May 30 '24 13:05 mugikhan

Can I ask why you would not want to enable extension loading?

My main concern is that sqlite_flutter_libs should behave similarly on all platforms it supports. Enabling extension loading only on Windows and Linux introduces platform-specific behavior that I'm trying to avoid. I also don't want to enable extensions (which may bring security concerns) for all users unconditionally.

At the moment we're using native buildscripts to compile sqlite3 (CMake on Windows+Linux, NDK on Android and CocoaPods on iOS and macOS). This makes it impossible for users to customize the sqlite3 that they're getting in the end. Once native assets are stable, we can migrate from sqlite3_flutter_libs towards a solution that works for all Dart projects and doesn't require any platform-specific builds. That would also be easier to customize, so we can offer options to enable extension loading for those who need it.

If you only need Windows and Linux either way, you can pretty much copy the CMakeLists.txt from this repository and adapt them to your needs to enable extensions. You pretty much just have to copy these parts, replacing $PLUGIN_NAME with sqlite3. Then you put target_link_libraries(${BINARY_NAME} PRIVATE sqlite3) somewhere and you have sqlite3 in your Flutter app without sqlite3_flutter_libs.

simolus3 avatar May 30 '24 16:05 simolus3

Can I ask why you would not want to enable extension loading?

My main concern is that sqlite_flutter_libs should behave similarly on all platforms it supports. Enabling extension loading only on Windows and Linux introduces platform-specific behavior that I'm trying to avoid. I also don't want to enable extensions (which may bring security concerns) for all users unconditionally.

At the moment we're using native buildscripts to compile sqlite3 (CMake on Windows+Linux, NDK on Android and CocoaPods on iOS and macOS). This makes it impossible for users to customize the sqlite3 that they're getting in the end. Once native assets are stable, we can migrate from sqlite3_flutter_libs towards a solution that works for all Dart projects and doesn't require any platform-specific builds. That would also be easier to customize, so we can offer options to enable extension loading for those who need it.

If you only need Windows and Linux either way, you can pretty much copy the CMakeLists.txt from this repository and adapt them to your needs to enable extensions. You pretty much just have to copy these parts, replacing $PLUGIN_NAME with sqlite3. Then you put target_link_libraries(${BINARY_NAME} PRIVATE sqlite3) somewhere and you have sqlite3 in your Flutter app without sqlite3_flutter_libs.

I forgot to mention that it is already enabled on Android, iOS and macOS. There is an extension that I am loading and it works correctly on all those platforms (gradle on Android and pod on iOS and macOS). I was wondering why it isn't enabled for the remaining two platforms. My testing only indicated Windows and Linux as an issue unless I am missing something.

mugikhan avatar May 30 '24 23:05 mugikhan

Oops - I have to apologize for pointlessly leaving this hanging so long then! I think in that case it's justified to get extension support working on Windows and Linux as well. I've just checked, we also don't have OMIT_GET_TABLE on Android/iOS/macOS.

So I'm fine with this except that OMIT_LOAD_EXTENSION should still be there for wasm builds, since there's no way to support it there at the moment.

I'm not sure if @FaFre is still looking for this functionality or if he will be able to merge this change. @simolus3 I also see this PR removes the OMIT_LOAD_EXTENSION flag for wasm.

I can create a new PR if @FaFre cannot make those changes and merge this PR. Does sqlite_flutter_libs and sqlcipher_flutter_libs require a minor or patch version bump?

mugikhan avatar Jun 01 '24 18:06 mugikhan

A new PR would be helpful, thanks!

Does sqlite_flutter_libs and sqlcipher_flutter_libs require a minor or patch version bump?

I think a patch bump is fine, this is more of a consistency fix than a new feature.

simolus3 avatar Jun 01 '24 21:06 simolus3

Cool that this found its way into the package now! Guess I can close this now :)

FaFre avatar Jun 12 '24 04:06 FaFre