-rw-r--r-- 1 mgorny mgorny 9,6K 10-24 10:29 sqlite-3.34.1-build_1.1.patch -rw-r--r-- 1 mgorny mgorny 14K 10-24 10:29 sqlite-3.34.1-build_1.2.patch -rw-r--r-- 1 mgorny mgorny 11K 10-24 10:29 sqlite-3.34.1-build_2.1.patch -rw-r--r-- 1 mgorny mgorny 12K 10-24 10:29 sqlite-3.34.1-build_2.2.patch -rw-r--r-- 1 mgorny mgorny 9,6K 10-24 10:29 sqlite-3.35.0-build_1.1.patch -rw-r--r-- 1 mgorny mgorny 14K 10-24 10:29 sqlite-3.35.0-build_1.2.patch -rw-r--r-- 1 mgorny mgorny 11K 10-24 10:29 sqlite-3.35.0-build_2.1.patch -rw-r--r-- 1 mgorny mgorny 12K 10-24 10:29 sqlite-3.35.0-build_2.2.patch It is pretty obvious that these patches have not been split in some logical way but are a single patch arbitrarily split in order to workaround a QA check. In either case, these patch don't belong in FILESDIR and I think we should consider deliberately circumventing checks for that a major offense. Please remove these patches from ::gentoo.
Furthermore, given how deeply these patches modify SQLite3, I'd like to request that a proper explanation and rationale is added to them, so that the future maintainers of the package don't have to figure out WTF the previous maintainer thought.
These patches are not a single patch arbitrarily split, but about 24 patches partially merged for easier maintenance (less work during rebasing of patches for version bumps). About 20 of these patches modify adjacent lines in src/main.c and tool/mksqlite3c.tcl: - E.g. patch for enabling amatch extension (https://sqlite.org/src/file?name=ext/misc/amatch.c&ci=trunk) adds call to sqlite3AmatchInit() in src/main.c and adds amatch.c to tool/mksqlite3c.tcl. - E.g. patch for enabling carray extension (https://sqlite.org/src/file?name=ext/misc/carray.c&ci=trunk) adds call to sqlite3CarrayInit() in src/main.c and adds carray.c to tool/mksqlite3c.tcl. - E.g. patch for enabling completion extension (https://sqlite.org/src/file?name=ext/misc/completion.c&ci=trunk) adds call to sqlite3CompletionVtabInit() in src/main.c and adds completion.c to tool/mksqlite3c.tcl. - E.g. patch for enabling csv extension (https://sqlite.org/src/file?name=ext/misc/csv.c&ci=trunk) adds call to sqlite3CsvInit() in src/main.c and adds csv.c to tool/mksqlite3c.tcl. - E.g. patch for enabling dbdata extension (https://sqlite.org/src/file?name=ext/misc/dbdata.c&ci=trunk) adds call to sqlite3DbdataRegister() in src/main.c and adds dbdata.c to tool/mksqlite3c.tcl. - E.g. patch for enabling decimal extension (https://sqlite.org/src/file?name=ext/misc/decimal.c&ci=trunk) adds call to sqlite3DecimalInit() in src/main.c and adds decimal.c to tool/mksqlite3c.tcl. ... Anyway I am working on splitting patches again into separate patches, if it is so desired. Patches are documented, but I will re-phrase some documentation and add references to files in upstream repository.
Please also make sure that for each patch it's documented: - why do we need to apply it? - how does that affect compatibility between Gentoo and other distributions?
(In reply to Michał Górny from comment #3) > Please also make sure that for each patch it's documented: > > - why do we need to apply it? > - how does that affect compatibility between Gentoo and other distributions? - upstream status
SQLite contains over hundred optional features and configuration-time options. For some of them, build system provides options (https://sqlite.org/compile.html or some arguments for `configure`), but for some them, no options are written yet. In case of patches enabling extensions, these patches enable only small minority of available extensions, which seem more useful and have no effect when functionality provided by them is not used. Upstream rejects even trivial, technically correct patches. https://sqlite.org/copyright.html "In order to keep SQLite completely free and unencumbered by copyright, the project does not accept patches. If you would like to suggest a change and you include a patch as a proof-of-concept, that would be great. However, please do not be offended if we rewrite your patch from scratch."
(In reply to Arfrever Frehtes Taifersar Arahesis from comment #5) > SQLite contains over hundred optional features and configuration-time > options. > […] > In case of patches enabling extensions, these patches enable only small > minority of available extensions, which seem more useful and have no effect > when functionality provided by them is not used. How do you determine "more useful"? Wouldn't it be better to have local USE flags instead? > > > Upstream rejects even trivial, technically correct patches. > > https://sqlite.org/copyright.html > > "In order to keep SQLite completely free and unencumbered by copyright, the > project does not accept patches. If you would like to suggest a change and > you include a patch as a proof-of-concept, that would be great. However, > please do not be offended if we rewrite your patch from scratch." I don't read it like a rejection but that (your) suggestions will not be copied to the code (and maybe not make it into the code base at all).
(In reply to Michał Górny from comment #1) > Furthermore, given how deeply these patches modify SQLite3, I'd like to > request that a proper explanation and rationale is added to them, so that > the future maintainers of the package don't have to figure out WTF the > previous maintainer thought. I see them as highly problematic. Most of the extras enabled with these patches look quite experimental. They reside in SQLite's sources under ext/misc. One of them is an eval-function that evaluates statements recursively. I don't want that in my hardened Gentoo. I'm also puzzled about carray-function, being able to treat C-pointers as virtual tables. I have absolutely no idea why SQLite must be able to read data from arbitrary pointers by default. (Both from 1.1) By enabling regexp Gentoo's SQLite variant is incompatible with the docs: https://anongit.gentoo.org/git/repo/gentoo.git. I assume (!) that an already defined function cannot be re-defined. Anyone who provided a user function either already dealt with the error or gets the SQLite-implementation and not his own (didn't check the precedence). (Stems from 1.2) Finally all the I/O-things like reading files, CSV or ZIP makes the library unnecessarily complex. Docs state: not enabled by default. In my opinion Gentoo shouldn't enable this anyway. If a project requires it it can easily include the sources/ have the bindings and keeps the system's lib clean. Wouldn't it be sufficient to drop the patch-line from the ebuild first/ create a new version without it?
(In reply to onkobu from comment #7) > By enabling regexp Gentoo's SQLite variant is incompatible with the docs > I assume (!) that an already defined function cannot be re-defined. icu extension (ext/icu/icu.c) (available with USE="icu") also defines regexp() function. Initialization of regexp extension (ext/misc/regexp.c) happens after initialization of icu extension (ext/icu/icu.c), and implementation of regexp() function from regexp extension (ext/misc/regexp.c) wins. This shows that re-definition is possible. (regexp() function from icu extension can be made available under additional alternative name, e.g. regexp_icu().) (Since version 3.36.0, regexp extension (ext/misc/regexp.c) also provides case-insensitive regexpi() function.)
(In reply to onkobu from comment #6 and comment #7) Some programs may actually need some extensions. E.g. dev-qt/qt-creator currently bundles SQLite with carray extension: https://code.qt.io/cgit/qt-creator/qt-creator.git/commit/?id=36fd58fbe97c2159eaec9bfebfb992531eb1e0ac https://code.qt.io/cgit/qt-creator/qt-creator.git/tree/src/libs/3rdparty/sqlite dev-qt/qt-creator could be fixed to link against system version of SQLite with carray extension... If necessary, I can add USE flags to SQLite, but I would be against removing these features unconditionally. I have split patches ready for SQLite 3.36.0, and I am investigating some error in test suite...
In restructured patchset: Inclusion of extensions in libsqlite3.so library made conditional on SQLITE_ENABLE_* macros (e.g. SQLITE_ENABLE_AMATCH, SQLITE_ENABLE_CARRAY, ...). (This is similar to options described in https://sqlite.org/compile.html) I am asking for opinion on what is preferred in ebuild: 1. Enabling these extensions unconditionally: append-cppflags -DSQLITE_ENABLE_AMATCH append-cppflags -DSQLITE_ENABLE_CARRAY ... 2. Adding USE flags enabled by default (since they have no additional dependencies): IUSE="+amatch +carray ..." use amatch && append-cppflags -DSQLITE_ENABLE_AMATCH use carray && append-cppflags -DSQLITE_ENABLE_CARRAY ... 3. Adding USE flags disabled by default: IUSE="amatch carray ..." use amatch && append-cppflags -DSQLITE_ENABLE_AMATCH use carray && append-cppflags -DSQLITE_ENABLE_CARRAY ... (Actual code in ebuild would look differently and have list of these extensions in separate variable.) (Number of new USE flags would be currently 28, with small growth possible over years.) (With SQLITE_ENABLE_* macros present in patchset, potential changing of ebuild from one solution to another solution, if desired, would be trivial.)
Also I want to remind that not everything (although majority) in patchset is about enabling extensions. Some parts of patchset fix unrelated problems in build system like using static linking.
(In reply to Arfrever Frehtes Taifersar Arahesis from comment #9) > Some programs may actually need some extensions. Have there been any related bug reports since 3.37.0 was added without the patches to enable these extensions? I think most of us would strongly prefer to keep the patchset as small as possible. If nothing in Gentoo actually requires the functionality, let's drop it.
(In reply to Arfrever Frehtes Taifersar Arahesis from comment #11) > Some parts of patchset fix unrelated problems in build system like using > static linking. Please submit a PR to re-introduce any patches that you think are actually necessary.
(In reply to Arfrever Frehtes Taifersar Arahesis from comment #10) > 2. Adding USE flags enabled by default (since they have no additional > dependencies): > IUSE="+amatch +carray ..." > use amatch && append-cppflags -DSQLITE_ENABLE_AMATCH > use carray && append-cppflags -DSQLITE_ENABLE_CARRAY > ... > > 3. Adding USE flags disabled by default: > IUSE="amatch carray ..." > use amatch && append-cppflags -DSQLITE_ENABLE_AMATCH > use carray && append-cppflags -DSQLITE_ENABLE_CARRAY > ... As variants of these solutions, there could be one USE flag called e.g. "miscellaneous-extensions": 2A. IUSE="+miscellaneous-extensions" 3A. IUSE="miscellaneous-extensions" (This name because source code of these extensions is in ext/misc directory in SQLite repository.)
The bug has been closed via the following commit(s): https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=6911314208cb54f79a643db77f6eba7874fd0d8a commit 6911314208cb54f79a643db77f6eba7874fd0d8a Author: Jakov Smolić <jsmolic@gentoo.org> AuthorDate: 2022-02-02 11:10:26 +0000 Commit: Jakov Smolić <jsmolic@gentoo.org> CommitDate: 2022-02-02 12:14:07 +0000 dev-db/sqlite: drop 3.35.5, 3.37.0 Starting with version 3.37.2 we're no longer carrying custom undocumented patchset in stable versions of sqlite. Given that there was no clear rationale behind adding these patches and no consumers in Gentoo that actually use the functionality enabled we are removing them. Closes: https://bugs.gentoo.org/825278 Signed-off-by: Jakov Smolić <jsmolic@gentoo.org> dev-db/sqlite/Manifest | 4 - dev-db/sqlite/files/sqlite-3.35.0-build_1.1.patch | 375 ---------------- dev-db/sqlite/files/sqlite-3.35.0-build_1.2.patch | 500 ---------------------- dev-db/sqlite/files/sqlite-3.35.0-build_2.1.patch | 292 ------------- dev-db/sqlite/files/sqlite-3.35.0-build_2.2.patch | 441 ------------------- dev-db/sqlite/sqlite-3.35.5.ebuild | 428 ------------------ dev-db/sqlite/sqlite-3.37.0.ebuild | 428 ------------------ 7 files changed, 2468 deletions(-)
