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: 2020-02-22 15:19 UTC (History)
3 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
Comment 9 Matthias Andree freebsd_committer 2020-02-22 15:19:13 UTC
Pavel, the proposal is to add the safe features to the port, for everyone's profit. Is there an automated halfway thorough test suite we can enable so as to trigger pkg-fallout@ mail on the Tier 2/3 architectures?