-
-
Notifications
You must be signed in to change notification settings - Fork 654
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
sqlite-preprocessed-3350500 in third_party #162
Conversation
* changed headers * removed redundant declarations * ran clang-format * added sqlite3.mk
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.
Looks good. Thanks for your patience on the review.
Yay! I'll close #161 as this got merged. |
I noticed after merging that you ran clang-format on the imported sources. The first commit is intended to be byte-for-byte identical with the upstream source. I'm going to see if I can do a ninja rewrite of your changes. |
Perfect. I did a quick force push of the change with cherry-picked onto the unformatted upstream sources so provenance is clearer. If you pulled master after I merged then you'll need to do a force pull. Sorry about that! Thanks for your contribution! SQLite shell now works like a charm: make -j8 o//third_party/sqlite3
o/third_party/sqlite3/sqlite3.com
SQLite version 3.35.5 2021-04-19 18:32:05
Enter ".help" for usage hints.
Connected to a transient in-memory database.
Use ".open FILENAME" to reopen on a persistent database.
sqlite> select 2 + 2;
4
sqlite> |
I'm also going to do my best to sure POSIX advisory locks work as advertised in a follow-up change. I need it for redbean too. It's probably the most important aspect of the systems integration. |
POSIX advisory locks are now polyfilled across platforms as of 690be54 |
I glossed over "byte-for-byte identical" in the discussion thread and ran |
What are you talking about? We just added 200,000+ lines of code to this codebase. I'm very happy with what you've achieved. It's also obvious reading your diffs that you're starting to really get the feel for how this monorepo works. You've also been helping a long time. The way I see it, any time I spend helping you is likely to pay dividends for the community in the long run. |
Out of curiosity, it looked like you dipped your toes in the water with the sqlite zip virtual table extension and then turned it off. What's your judgement on it? It seems like it's something potentially cool. For example:
|
- Now integrated with `make tags` for Emacs IDE features - Delete some old deprecated broken full-text search engines - Rename .h → .inc files that don't meet our definition of header - Make sure every #include line is normal form so tools understand See #162
I thought #ifndef SQLITE_WAL_H
#define SQLITE_WAL_H
#include "third_party/sqlite3/sqliteInt.inc"
Changing to |
The extension flags in It is possible to add a few more SQLite features/extensions to the build. I looked up the SQLite compile flags and tried adding a few new ones: $(THIRD_PARTY_SQLITE3_A_OBJS): \
OVERRIDE_CFLAGS += \
-DNDEBUG \
-DBUILD_sqlite \
-DHAVE_READLINE=0 \
-DHAVE_EDITLINE=0 \
-DHAVE_MALLOC_USABLE_SIZE \
-DHAVE_FDATASYNC \
-DHAVE_STRCHRNUL \
-DHAVE_LOCALTIME_R \
-DHAVE_GMTIME_R \
-DHAVE_USLEEP \
-DSQLITE_CORE \
-DSQLITE_OS_UNIX \
-DSQLITE_THREADSAFE=0 \
-DSQLITE_HAVE_ZLIB \
-DSQLITE_HAVE_C99_MATH_FUNCS \
-DSQLITE_DEFAULT_MEMSTATUS=0 \
-DSQLITE_ENABLE_BYTECODE_VTAB \
-DSQLITE_ENABLE_DBPAGE_VTAB \
-DSQLITE_ENABLE_DBSTAT_VTAB \
-DSQLITE_ENABLE_DESERIALIZE \
-DSQLITE_ENABLE_EXPLAIN_COMMENTS \
-DSQLITE_ENABLE_FTS3 \
-DSQLITE_ENABLE_FTS3_PARENTHESIS \
-DSQLITE_ENABLE_FTS3_TOKENIZER \
-DSQLITE_ENABLE_FTS4 \
-DSQLITE_ENABLE_FTS5 \
-DSQLITE_ENABLE_GEOPOLY \
-DSQLITE_ENABLE_IOTRACE \
-DSQLITE_ENABLE_JSON1 \
-DSQLITE_ENABLE_MATH_FUNCTIONS \
-DSQLITE_ENABLE_MEMORY_MANAGEMENT \
-DSQLITE_ENABLE_MEMSYS5 \
-DSQLITE_ENABLE_OFFSET_SQL_FUNC \
-DSQLITE_ENABLE_PREUPDATE_HOOK \
-DSQLITE_ENABLE_RTREE \
-DSQLITE_ENABLE_SESSION \
-DSQLITE_ENABLE_SNAPSHOT \
-DSQLITE_ENABLE_STMTVTAB \
-DSQLITE_ENABLE_UNKNOWN_SQL_FUNCTION \
-DSQLITE_ENABLE_UNLOCK_NOTIFY \
-DSQLITE_OMIT_DECLTYPE \
-DSQLITE_OMIT_DEPRECATED \
-DSQLITE_OMIT_LOAD_EXTENSION \
-DSQLITE_OMIT_SHARED_CACHE \
-DSQLITE_TEMP_STORE=1 Adding all these features increases binary size: |
As a data point, we (DB Browser for SQLite project) enable a bunch of extensions in our builds too, as people definitely use them and ask us for them. 😄 |
@ahgamut The internal sqlite headers need to be .inc files because they don't specify their dependencies. For example, btree.h references the type I'm fine adding all the extensions. One thing we can do in the future, if we want tinier SQLite builds, is to create multiple build targets for each extension similar to how it's done in https://github.com/jart/cosmopolitan/blob/master/libc/nt/nt.mk It's a huge pain to configure, but SQLite is written in such a way where extensions can be statically yoinked in that manner. Another thing worth considering, is companies like Apple have published SQLite configs we can reference for inspiration, which are usually reasonably well thought out. Although there's a few things here and there we might disagree with. |
Similar to sqlite amalgamation (#161) but with https://www.sqlite.org/2021/sqlite-preprocessed-3350500.zip instead.
First commit adds all the files (after running
clang-format
), second commit addssqlite3.mk
and does the minimal necessary changes to get SQLite to build.sqlite3.com
is 1036KB withMODE=tiny
.