Bug 241385 - databases/sqlite3: enable OS features
Summary: databases/sqlite3: enable OS features
Status: Open
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Many People
Assignee: freebsd-ports-bugs mailing list
Keywords: buildisok
Depends on:
Reported: 2019-10-20 23:19 UTC by rozhuk.im
Modified: 2019-11-17 12:10 UTC (History)
2 users (show)

See Also:
bugzilla: maintainer-feedback? (pavelivolkov)

patch (956 bytes, patch)
2019-10-20 23:19 UTC, rozhuk.im
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description rozhuk.im 2019-10-20 23:19:35 UTC
Created attachment 208474 [details]

Make script from sqlite3 a bit outdated and does not detect all OS features that can be used by sqlite3.
For example it does not detect pread()/pwrite() and sqlite3 use lseek()+read() instead of pread().
I walk trough all code and collect all HAVE_* switches that available on FreeBSD.

PS: also
+		-DSQLITE_MAX_MMAP_SIZE=0x7fff0000 \
can be set to force use mmap() instead of pread()/pwrite() but is not safe.
Comment 1 Kubilay Kocak freebsd_committer freebsd_triage 2019-10-21 06:18:44 UTC
Doesn't the configure pick these up, or is this a different build than standard autoconf things?
Comment 2 rozhuk.im 2019-10-21 06:40:40 UTC
autoconf does not detect these features, but CPPFLAGS passed to compiler and enable features.
Comment 3 Kubilay Kocak freebsd_committer freebsd_triage 2019-10-21 08:11:43 UTC
Are all flags appropriate for all versions/archs of FreeBSD? If not, they should be scoped accordingly
Comment 4 rozhuk.im 2019-10-21 10:01:48 UTC
IMHO there is no arch depend flags.
Most of flags - enables syscals, not 100% sure but it should be available on 11.2+.
I use it on 12.1 amd64.
Comment 5 Automation User 2019-11-05 00:06:16 UTC
Build info is available at https://gitlab.com/swills/freebsd-ports/pipelines/93636806
Comment 6 Pavel Volkov 2019-11-05 02:51:39 UTC
(In reply to rozhuk.im from comment #0)
You're right. These features (HAVE_PREAD, HAVE_PWRITE) are not detected during configuration.
But your patch requires further study.
For example, the flag SQLITE_MMAP_READWRITE is dangerous for many uses cases. Please read paragraph 4 in the "How Memory-Mapped I/O Works" at the link (https://www.sqlite.org/mmap.html).
Many other options auto detected in the configure script.
Comment 7 rozhuk.im 2019-11-05 03:07:35 UTC
(In reply to Pavel Volkov from comment #6)

SQLITE_MMAP_READWRITE - does not enables mmap() usage, it enables memcpy() to mmaped memory write instead of lseek()+write() / pwrite().

I agree that it can be dangerous, or at least I do not know it there any bugs/features in FreeBSD mmap() implementations.

But mmap() io does not enabled by this patch.
To enable mmaped io some one should add -DSQLITE_DEFAULT_MMAP_SIZE=0x7fff0000 before compile this port, to force all apps use mmap() or execute "PRAGMA mmap_size=0x7fff0000" in app.

I do not insist on SQLITE_MMAP_READWRITE, but if our mmap() implementation works as sqlite3 authors expect, then we get more profit from this.

Yes, ~half of options is detected by autoconf, but I prefer to define it, just to be sure that it set.
Comment 8 Pavel Volkov 2019-11-17 12:10:42 UTC
You can set any compilation options before creating the package.

CPPFLAGS='-DHAVE_PREAD=1 -DHAVE_PWRITE=1' make clean configure

setenv CPPFLAGS '-DHAVE_PREAD=1 -DHAVE_PWRITE=1' && make clean configure