-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Add recent versions of SQLite3 Multiple Ciphers #23227
Add recent versions of SQLite3 Multiple Ciphers #23227
Conversation
This comment has been minimized.
This comment has been minimized.
recipes/sqlite3mc/all/conandata.yml
Outdated
"1.8.3": | ||
url: "https://github.com/utelle/SQLite3MultipleCiphers/archive/refs/tags/v1.8.3.tar.gz" | ||
sha256: "1a9c26082068ee36c33b0a4e061d013db345ad170bab0ab00c8dda2c7c96561a" | ||
"1.8.2": | ||
url: "https://github.com/utelle/SQLite3MultipleCiphers/archive/refs/tags/v1.8.2.tar.gz" | ||
sha256: "c349f45505641a3ee3e5faa4498a18317b65697e9a3cbfbe41a6612e44389c55" | ||
"1.8.1": | ||
url: "https://github.com/utelle/SQLite3MultipleCiphers/archive/refs/tags/v1.8.1.tar.gz" | ||
sha256: "44215d5812ec2cfa1a842d9aae2908be0b14dce4231bc3243394b24b3affb78a" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"1.8.3": | |
url: "https://github.com/utelle/SQLite3MultipleCiphers/archive/refs/tags/v1.8.3.tar.gz" | |
sha256: "1a9c26082068ee36c33b0a4e061d013db345ad170bab0ab00c8dda2c7c96561a" | |
"1.8.2": | |
url: "https://github.com/utelle/SQLite3MultipleCiphers/archive/refs/tags/v1.8.2.tar.gz" | |
sha256: "c349f45505641a3ee3e5faa4498a18317b65697e9a3cbfbe41a6612e44389c55" | |
"1.8.1": | |
url: "https://github.com/utelle/SQLite3MultipleCiphers/archive/refs/tags/v1.8.1.tar.gz" | |
sha256: "44215d5812ec2cfa1a842d9aae2908be0b14dce4231bc3243394b24b3affb78a" |
Intermediate versions are usually only added when there's a specific need for them, especially in the case of patch-version bumps. Please skip them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Intermediate versions are usually only added when there's a specific need for them, especially in the case of patch-version bumps. Please skip them.
Please explain what exactly is a specific need? Who needs what? And who decides what is needed and what not?
And please explain why you think the intermediate versions are just patch-version bumps?
Typically, a new version of SQLite3 Multiple Ciphers is released shortly after a new version of SQLite was released. And that is what most users of SQLite3 Multiple Ciphers expect: encryption support for the latest SQLite version.
SQLite3mc version | based on SQLite3 version |
---|---|
1.8.4 | 3.45.2 |
1.8.3 | 3.45.1 |
1.8.2 | 3.45.0 |
1.8.1 | 3.44.2 |
1.8.0 | 3.44.1 |
The above table lists for each version the related SQLite3 version. Looking at the Conan recipe for SQLite3, I see that all versions mentioned in the table are included - without exception.
So, IMHO it would be somewhat logical to support them all. However, if you insist I can remove the "intermediate" versions.
And what should be done in the future? Should this recipe be updated after a release of a new SQLite version or not? Probably not, because according to your argument those updates would be only patch-version bumps.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We actually just avoid pushing multiple patches due lack of usage, like people will prefer download the latest version (based on download numbers in Conan Center). So publishing multiple versions will only result in more packages in the repository (Sqlite3mc will produce around 100 binaries packages for each version), but most probably people will prefer the latest. Of course, if there is a reason of keep all versions, like someone need that version for a project, we will not refuse keeping it.
As SQlite3 is used not only largely in ConanCenterIndex, but also uses multiple versions, I don't see all problem keeping all versions for this PR, but because is a project strict related to another project tags.
Almost 2 months ago I responded to the review asking some questions. Up to now there was no reaction. |
@utelle Really sorry the huge delay of reviewing your PR! As we have a big number of PRs, some of them can be missed in middle of reviews, unfortunately it happened to your PR here. |
@@ -84,7 +84,7 @@ class sqlite3mc(ConanFile): | |||
"enable_rtree": True, | |||
"enable_uuid": True, | |||
"use_uri": True, | |||
"user_authentication": True, | |||
"user_authentication": False, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see it was change in 1.8.4, is that correct? utelle/SQLite3MultipleCiphers@3bb0339
I would suggest updating the config_options
to follow the default behavior to honor older versions too:
def config_options(self):
if Version(self.version) < "1.8.4":
# INFO: https://github.com/utelle/SQLite3MultipleCiphers/commit/3bb033956816b3301f026abb5e83087799de5bee
self.options.user_authentication = True
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see it was change in 1.8.4, is that correct? utelle/SQLite3MultipleCiphers@3bb0339
Yes, the user authentication extension was deprecated by the SQLite core developers. It may disappear with the next release of SQLite (or anytime later). Therefore I decided to disable it by default.
Whether I will remove the code base of the extension from my project will depend on how the SQLite team will handle this extension in the future.
I would suggest updating the
config_options
to follow the default behavior to honor older versions too:def config_options(self): if Version(self.version) < "1.8.4": # INFO: https://github.com/utelle/SQLite3MultipleCiphers/commit/3bb033956816b3301f026abb5e83087799de5bee self.options.user_authentication = True
I can do that. However, I think I will wait with updating the Conan recipe, because I just saw that the next SQLite version will probably be released within the next 2 weeks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for giving more details. We could deprecate that option as well instead: https://github.com/conan-io/conan-center-index/blob/master/docs/faqs.md#can-i-remove-an-option-from-a-recipe
Than few versions later we can remove from package (if the project does the same).
recipes/sqlite3mc/all/conandata.yml
Outdated
"1.8.3": | ||
url: "https://github.com/utelle/SQLite3MultipleCiphers/archive/refs/tags/v1.8.3.tar.gz" | ||
sha256: "1a9c26082068ee36c33b0a4e061d013db345ad170bab0ab00c8dda2c7c96561a" | ||
"1.8.2": | ||
url: "https://github.com/utelle/SQLite3MultipleCiphers/archive/refs/tags/v1.8.2.tar.gz" | ||
sha256: "c349f45505641a3ee3e5faa4498a18317b65697e9a3cbfbe41a6612e44389c55" | ||
"1.8.1": | ||
url: "https://github.com/utelle/SQLite3MultipleCiphers/archive/refs/tags/v1.8.1.tar.gz" | ||
sha256: "44215d5812ec2cfa1a842d9aae2908be0b14dce4231bc3243394b24b3affb78a" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We actually just avoid pushing multiple patches due lack of usage, like people will prefer download the latest version (based on download numbers in Conan Center). So publishing multiple versions will only result in more packages in the repository (Sqlite3mc will produce around 100 binaries packages for each version), but most probably people will prefer the latest. Of course, if there is a reason of keep all versions, like someone need that version for a project, we will not refuse keeping it.
As SQlite3 is used not only largely in ConanCenterIndex, but also uses multiple versions, I don't see all problem keeping all versions for this PR, but because is a project strict related to another project tags.
Are the download numbers publicly visible somewhere? I fully understand that keeping multiple versions doesn't make much sense if no one downloads them. So, I really have no problem with removing all interim versions except for the latest.
Of course, the latest version is usually preferred. However, not all projects update package dependencies as soon as new versions are available.
What I would like to know is how updates should be handled in the future. For example, should the entry for version 1.8.4 be removed, when version 1.8.5 will be released? What are the criteria to keep or drop a prior version?
I simply want to understand the Conan policy when to add, keep, or drop a version from a recipe. |
No, but there is a discussion about it where you can ask more information about. We had in the past, but we dropped because the counter was incorrect:
True, even more in C and C++ ecosystem. That's why I mentioned we include old versions when people open issues asking about. We have it documented (very generic) here: https://github.com/conan-io/conan-center-index/blob/master/docs/adding_packages/sources_and_patches.md#adding-old-versions
There is no precise rule, but a generic one: https://github.com/conan-io/conan-center-index/blob/master/docs/adding_packages/sources_and_patches.md#removing-old-versions We check if more packages are using that specific version (e.g sqlite3/3.41.2 is not new, but still used in few recipes), plus, if people commented in issues and pull requests about that version. Then we can consider good to go if some newer minor or patch version is available and the older is not mentioned, consumed by other recipes and aged by months. Keep in mind removing a version from a recipe (config.yml and conandata.yml) will not remove that version from Conan Center, it will be always available there, Conan Center does not remove packages (except in case of security reasons). Once removed, the dropped will not receive new updates. It should not be a problem, because users are recommended to use lockfiles when using CCI in production, to avoid any breakage. |
- Comment out versions 1.8.1 to 1.8.3 (uncomment if there is a demand for those versions in the future) - Change default of option 'user_authentication' zo False (extension 'User Authentication' is deprecated) - Use default True for option 'user_authentication' for versions below 1.8.4
For now, I commented out the versions 1.8.1 to 1.8.3. Should there be a demand for those versions in the future, these entries can be uncommented.
Well, it can be discussed in the future if old versions should be removed or not. As proposed by you I added a check to change the default of option |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thank you for your version update and sorry again our delay in review this PR.
Conan v1 pipeline ✔️All green in build 6 (
Conan v2 pipeline ✔️
All green in build 6 ( |
Specify library name and version: sqlite3mc/1.8.1, sqlite3mc/1.8.2, sqlite3mc/1.8.3, sqlite3mc/1.8.4
A Conan recipe was added for SQLite3 Multiple Ciphers version 1.8.0 in November 2023, but it was approved only recently. In the meantime further versions of SQLite3 Multiple Ciphers were released. This PR adds those new versions to the Conan recipe.
One compile time option was adjusted, namely
ENABLE_USER_AUTHENTICATION
. The user authentication extension was deprecated by the SQLite core developer team. Therefore this option is now disabled by default. Most likely, the source code of this extension will be removed in a future version.