Conan-center-index: [package] sqlite3/all: Discussion of default options

Created on 2 Dec 2019  路  7Comments  路  Source: conan-io/conan-center-index

Having a look at the options of the sqlite3 recipe you see most of them defaulted to false. However, there are some recipes like qt from bincrafters that already require this package with specific option enabled:

https://github.com/bincrafters/conan-qt/blob/594e40f036adcbf22d23dc2da228f389d45a76dc/conanfile.py#L256

Moreover, the sqlite3 is provided with a custom CMakeLists.txt with the CMake options, so we could say there are no defaults for these options recommended by the library authors/maintainers.

Additionally, these options are enabled by default in vcpkg repo:

-DSQLITE_ENABLE_RTREE
-DSQLITE_ENABLE_UNLOCK_NOTIFY
-DSQLITE_ENABLE_COLUMN_METADATA

This issue is open for discussion regarding the options of this package and its default values.

cc/ @ericLemanissier @SSE4 @uilianries

help wanted look into question

Most helpful comment

my 2 cents, looking at sqlite3 options:

  • column_metadata - enables additional APIs, making library more feature complete. almost harmless (the only side-effect is a bit larger library, but cost is very low IMO)
  • threadsafe - very contradictory. enabling it makes library more safe and stable, but has a significant performance penalty. this is something that depends on consumer's usage (does he call sqlite3 APIs from different threads?). so it appears option should be enabled only if it's really needed (default = false)
  • rtree - another API extension, should be safe to enable by default
  • unlock_notify - one more API extension
    therefore, we may safely enable all API extensions by default (column metadata, rtree, unlock notify).
    for threadsafe - I vote for false by default. if possible, build both threadsafe=false and threadsafe=true.

All 7 comments

I just know that qt needs SQLITE_ENABLE_COLUMN_METADATA. https://www.sqlite.org/compile.html clearly indicates that this option is Normally Turned Off. However, I think the best default is what most of the consumer are using. the only other bincrafters package setting sqlite options is conan-sqlitecpp, activating enable_column_metadata and threadsafe=2. I'll try to do a global search on github to see most common options used

Thanks for the additional info in the sqlite site.

I completely agree with you regarding

the best default is what most of the consumers are using

Let's try to see if we can have a common consensus about the default options to use in the recipes. Thanks a lot!

So, searching for sqlite3 in github files named conanfile.py and conanfile.txt showed that these files never change sqlite3 options, except conan-sqlitecpp and conan-qt (enable_column_metadata) and a handful packages tweaking the threadsafe option. It's not the best outcome for qt, but it looks like the current defaults are not so bad.

Thanks for the research @ericLemanissier! This is crucial to implement support to build packages with non-default options but we have to handle with care in order to avoid massive binary matrix.

my 2 cents, looking at sqlite3 options:

  • column_metadata - enables additional APIs, making library more feature complete. almost harmless (the only side-effect is a bit larger library, but cost is very low IMO)
  • threadsafe - very contradictory. enabling it makes library more safe and stable, but has a significant performance penalty. this is something that depends on consumer's usage (does he call sqlite3 APIs from different threads?). so it appears option should be enabled only if it's really needed (default = false)
  • rtree - another API extension, should be safe to enable by default
  • unlock_notify - one more API extension
    therefore, we may safely enable all API extensions by default (column metadata, rtree, unlock notify).
    for threadsafe - I vote for false by default. if possible, build both threadsafe=false and threadsafe=true.

Thanks for the research @SSE4! I think it makes a lot of sense to have the API extension options enabled by default in this case. That will ease the adoption when adding new dependent packages in the future.

So let's add the same api extensions options as in vcpkg and keep the threadsafe one as is

Was this page helpful?
0 / 5 - 0 ratings