Conan-center-index: [question] sqlite3 default options

[fulara]

Just tried to take a sqlite3 as my dependency and I see that conan-center-index default options does not match the one in the SQLite documentation:

This option controls whether or not code is included in SQLite to enable it to operate safely in a multithreaded environment. The default is SQLITE_THREADSAFE=1 

Whereas in conan-center-index it is:
https://github.com/conan-io/conan-center-index/blob/master/recipes/sqlite3/all/conanfile.py#L32

What is the aproach here - shouldnt packages try to match default behavior?
If thats the case I can review the flags and make a pr possibly.

[danimtb]

Recipe options do not always resemble 1 to 1 library (build) options. Some of the options might be created on purpose to fit a packaging issue or case not covered by the original library. Default options do not always mean that they have to follow the same defaults of the library. Default options can be changed to use the most common value used by consumers or a value that is relevant for most of its dependents.

The case of sqlite3 was discussed here https://github.com/conan-io/conan-center-index/pull/377#pullrequestreview-331310487 and was needed to continue with sqlitecpp recipe. If you want to consume it with a different value, you can set it in your profile and use --build sqlite3 to get your package built

[fulara]

I've had a read through this.
Reasoning is here:
https://github.com/conan-io/conan-center-index/pull/561

VCPKG does not set threadsafe, which means is 0 by default: https://github.com/microsoft/vcpkg/blob/master/ports/sqlite3/CMakeLists.txt

One package manager convention is more important than the documentation of the original library?

EDIT:
I've just re-read my comment - and vcpkg does not set it - hence uses default.
and as per documentation I've posted the default should be '1'.

you say that:

and was needed to continue with sqlitecpp recipe.

I am not seeing any arguments/discussion raised on that topic

[ericLemanissier]

The fact that sqlitepp requires a specific value does not imply that it has to be the default. It means that it should be built by CCI too.
Keeping defaults consistent with the packaged library should be the rule.
And as @fulara said, vcpkg seems to actually use the actual default value.

EDIT: also, it seems that sqlitecpp supports threadsafe=1 : https://github.com/SRombauts/SQLiteCpp/issues/195#issuecomment-488113454

[uilianries]

Unfortunately we needed to use that option because another package (sqlitecpp), but this is a rare can I say. Also, we don't have a feature for storing more than one different option in CCI, or running with building missing.

[fulara]

couldnt that be solved with def configurein the sqlitecpp?
Either in def configure or default_option depending on the case?

[ericLemanissier]

@uilianries the author of sqlitecpp said that threadsafe=1 would work fine in post https://github.com/SRombauts/SQLiteCpp/issues/195#issuecomment-488113454
Sure, he said you would get yourself into trouble if you start using objects concurrently in different threads, but you would have the same problems with threadsafe=0, and a LOT more in sqlite itself. Also, how do you get to the conclusion that VCPKG does not set threadsafe, which means is 0 by default: https://github.com/microsoft/vcpkg/blob/master/ports/sqlite3/CMakeLists.txtwhen sqlite doc saysIf no SQLITE_THREADSAFE compile-time parameter is present, then serialized mode is used. This can be made explicit with -DSQLITE_THREADSAFE=1`?
sorry for repeating things which were already said, but there is something I don't get here

[madebr]

I've been bitten by this as well in #1570
The poco recipe requires thread safety to be nonzero.