WASI support: build sqlite-nio without SwiftNIO - #4
Conversation
|
FWIW, https://github.com/vapor/sqlite-nio/releases/tag/1.13.0 now includes the |
88f4f62 to
ad4624e
Compare
@gwynne Thanks so much for the heads up, very timely and helpful! I ran the idea I'm working on here past 0xTim a few weeks ago on Discord, but I should have looped you in as well. I'm working to make sqlite-nio, sql-kit, and sqlite-kit conditionally have nio-free options (eg. Swift Concurrency backing instead of swift-nio). This will support WASI now, and other platforms could in theory take the approach, as well. I was originally, going to use a Swift trait to control the behavior so that non-WASI platforms can select the option as well. But I eventually settled on the current approach which uses Would love any opinions or feedback if you have them Thank you! 🙌 |
Motivation: `SQLiteInt64` exists so that SQLite's 64-bit `INTEGER` affinity survives on 32-bit platforms, where carrying it in `Int` would trap above `Int32.max`, and both construction sites convert with `.init(...)` so the type comes from the typealias. Nothing exercises that range: the suite never stores a value wider than 32 bits, so a regression back to a hardcoded `Int(...)` conversion would compile everywhere and only be caught by trapping on a 32-bit target at runtime. Modifications: Add tests covering the full 64-bit range through both paths that construct `SQLiteData.integer` from libsqlite3: the column-read path, and the `sqlite3_value` path that custom functions see. Result: The wide-integer behavior is pinned by the suite everywhere, including the 32-bit targets (`wasm32`, `armv7k`) where `SQLiteInt64` resolves to `Int64`.
Motivation: The pre-macOS-12 date parsing fallback constructs a `tm` with the memberwise initializer, which spells out every field of the struct. That field list is not portable: wasi-libc's `struct tm` carries an extra `__tm_nsec` member, so the call does not compile there. The file's import list already anticipates WASI (`#elseif os(WASI)` selects WASILibc), but with no wasm lane in this package's CI the initializer call was never actually compiled against it. Modifications: Use the zeroing initializer and assign the two fields with non-zero values. `tm_isdst`, `tm_gmtoff`, and `tm_zone` were being set to their zero values already. Result: Identical behavior on every platform that compiled before, and the file now also compiles against wasi-libc. The `#available` check guarding the fallback is always true off Darwin, so the code remains unreachable everywhere except old Darwin hosts.
Motivation: SwiftNIO cannot be built for `wasm32-unknown-wasip1`: NIOPosix is built on POSIX sockets and threads, neither of which WASI preview 1 provides. SQLiteNIO itself needs very little of SwiftNIO — an `EventLoopFuture` API, `ByteBuffer` for blobs, and an `NIOThreadPool` to keep blocking libsqlite3 calls off the event loops — so it can build without it, given somewhere to put the differences. The goal is one implementation with a few conditional declarations, not a second copy of the package that has to be kept in step by hand. Modifications: Gate on `#if canImport(NIOCore)` rather than on a platform check, so the sources follow whatever the manifest actually resolves for the target being built. When SwiftNIO is present every gate is true and nothing changes. Where it is absent: - `Exports.swift` defines `ByteBuffer` as `[UInt8]` and supplies, as internal members, the five `ByteBuffer` APIs this package uses, plus a single-threaded stand-in for `NIOLockedValueBox` so the `sqlite3_initialize()` guard in the open sequence reads identically in both configurations. - `SQLiteConnection` keeps one class. The `EventLoopFuture` members are gated; the `async` members are shared, with `withBlockingIO(_:)` standing in for `NIOThreadPool.runIfActive` where there is no pool to offload to, and `openHandle(storage:logger:)` holding the open sequence that every `open(…)` overload needs. - `SQLiteConnection.execute(_:_:_:)` now holds the prepare/bind/step loop, and the futures-based and `async` query entry points both call it, so the loop exists once instead of twice. - `SQLiteDatabase` keeps one protocol declaration, and its `async` requirements are exactly the upstream ones, shared by both configurations; only the `EventLoop`-shaped requirements and the futures-routed default implementations are conditional. Read blobs with `ByteBuffer(bytes:)` instead of allocating and then writing, and build `Data` from `readableBytesView` instead of `Data(buffer:byteTransferStrategy:)`, so those expressions compile against both blob representations. Both are equivalent to what they replace. The observable hook API is the one thing with no counterpart here: it is built on `NIOThreadPool` and `NIOLockedValueBox` throughout, so the file is gated whole. Beyond the no-op locked-value stand-in, no synchronization is introduced: `SQLiteConnectionHandle` is already `@unchecked Sendable` for the reasons documented on it, and row collection uses the same `nonisolated(unsafe)` accumulator the futures-based overload uses. Result: On every platform that links SwiftNIO the public API, the resolved symbol graph, and the behavior are unchanged; `diagnose-api-breaking-changes` reports nothing, and no symbol is added. Where SwiftNIO is absent, SQLiteNIO offers the same `async` API over the same libsqlite3 handle, with no SwiftNIO module in the build graph at all.
Motivation: SwiftNIO cannot be built for `wasm32-unknown-wasip1`, so sqlite-nio currently fails to configure for that platform at all — the failure is in dependency resolution, before any of the conditional sources in the previous commit get a chance to matter. Modifications: Gate the SwiftNIO products on `.when(platforms: nonWASIPlatforms)`. Target dependency conditions are evaluated per platform, so on WASI the products are simply not linked and the `canImport(NIOCore)` gates select the `async` API. `NIOFoundationCompat` keeps its existing Darwin-only condition, which already excludes WASI. `.when(platforms:)` can only include, never exclude, so excluding one platform means enumerating the others; the list is the set SPM 6.1 knows about, noted as such so it is not extended without also raising the manifest's tools version. Result: On every other platform the resolved dependency set is byte-identical to before. On WASI the build graph contains no SwiftNIO module — not NIOPosix, not NIOCore, not NIOConcurrencyHelpers. `SQLITE_THREADSAFE` is left at `1` everywhere: wasi-libc supplies the mutex primitives SQLite needs, and keeping serialized mode means a threaded WASI target is correct rather than silently unprotected.
Motivation: `vapor/ci`'s reusable unit-test workflow already knows how to build a package for `wasm32-unknown-wasip1`; sqlite-nio simply had not opted in, so nothing stops the SwiftNIO-free configuration from regressing unnoticed. Modifications: Pass `with_wasm: true` to the reusable workflow, as sql-kit already does. Result: The WASI build is checked on every pull request.
ad4624e to
d80081e
Compare
| // `__tm_nsec` member, for example). The zeroing initializer plus two assignments | ||
| // compiles against all of them; the remaining fields were being set to zero anyway. | ||
| var stm = tm() | ||
| (stm.tm_wday, stm.tm_yday) = (-1, -1) |
There was a problem hiding this comment.
As it turns out, I didn't actually need to set these to -1; setting them to zero is functionally the same. Both fields' values are ignored entirely by timegm() et al. The zeroing initializer is thus sufficient (and I really should have used it myself anyway, since I was trying so hard to make the code not take up much visual space, as I imagine is painfully obvious. 🤣🤣). Really, this fallback is a shameful abomination. It just happens to be good enough for the purpose it serves, especially considering that I doubt pre-Monterey macOS is a critical deployment platform for very many users. 😆
| (stm.tm_wday, stm.tm_yday) = (-1, -1) |
| // The hook API is built on `NIOThreadPool` and `NIOLockedValueBox`; it has no NIO-free | ||
| // counterpart, so it is elided along with the rest of the SwiftNIO surface. |
There was a problem hiding this comment.
| // The hook API is built on `NIOThreadPool` and `NIOLockedValueBox`; it has no NIO-free | |
| // counterpart, so it is elided along with the rest of the SwiftNIO surface. |
Sure, but so is the rest of SQLiteNIO. The thread pool is just there because event loops are not suitable for CPU-bound work. In a fully Concurrency-based world without NIO, you can just let the code run without the thread pool using your withBlockingIO() helper. The public API of the hooks support has no NIO dependency whatsoever, literally not even so much as a ByteBuffer, and it's already async; the counterpart that's missing isn't the NIO-free one, it's the NIO-using one!
2 participants
Builds and tests sqlite-nio on
wasm32-unknown-wasip1without SwiftNIO. SwiftNIO does not build for WASI (NIOPosix needs POSIX sockets and threads), so today the package fails at dependency resolution before a single source file is considered. Embedded Swift is out of scope here and stacked separately as #5.Changes:
#if canImport(NIOCore)selects an async-only configuration. TheasyncAPI is 1.13.0's own: the protocol'sasyncrequirements are shared unchanged, blocking libsqlite3 calls run inline (correct on a single-threaded target), andByteBufferbecomes a[UInt8]typealias carrying the small slice of buffer API the package uses, plus a no-op stand-in for theNIOLockedValueBoxthe open path acquired in 1.13.0.tmin the FoundationEssentials date fallback is initialized field by field: the memberwise initializer spells out every libc-specific field and does not compile against wasi-libc. Nothing had caught that because sqlite-nio has no wasm CI lane yet. This commit stands alone and could be taken by itself..when(platforms: nonWASIPlatforms); NIOFoundationCompat keeps its existing Darwin-only condition, which already excludes WASI. SwiftPM has no "every platform except" form, so the list is spelled out; Add support for WASILibc apple/swift-nio#2671 and swift-crypto use the same idiom, and a comment pins the list to the manifest's tools version (6.1).SQLiteInt64range land as their own first commit: 1.13.0 introduced the typealias, but the suite never stores a value wider than 32 bits, and this branch makes 32-bit targets real.vapor/cialready provides (with_wasm: true), as sql-kit already does.Where SwiftNIO is present, nothing changes: every gate is unconditionally true,
swift package diagnose-api-breaking-changesagainst the base reports no breaking changes, and no public symbol is added.What WASI loses: the
EventLoopFutureAPI, the future-based hook API, and the thread pool. TheasyncAPI, already the recommended interface, is what remains. Since 1.13.0 reworked that API to run directly on the thread pool instead of hopping through event loops, the two configurations now share theasyncmethod bodies outright; the previous revision of this branch had to keep a parallel async form of thequeryrequirement, and that fork is gone.Verification:
swift buildandswift test: 59 tests green (the 54 active upstream tests plus 5 new covering the full 64-bit integer range through both read paths, blob andDataround trips), including with CI's--explicit-target-dependency-import-check error -Xswiftc -require-explicit-sendable.--swift-sdk swift-6.3.1-RELEASE_wasm): green, with zero SwiftNIO object files and no NIO, EventLoop, or ByteBuffer symbols in the built objects. Theunit-tests / wasmcheck skips drafts, so the lane is proven by the local build until this is marked ready.Notes for review: the
canImport(NIOCore)gates are build-wide, live inSources/only (never in a manifest, per the discussion on vapor#98), and all three packages in this stack gate the same modules on the same condition; a package that did build NIOCore for WASI would fail loudly at compile. No SwiftPM traits, no versioned manifest, no tools-version bump, no new dependencies.Base
base/vapor-mainis an exact snapshot of vapor/sqlite-nio@main at 553a6be (1.13.0, "Modernize for Swift 6.1"), excluding this fork's two extra commits (the NIOAsyncRuntime approach from #3, superseded here). TheSQLiteInt64fix this branch used to carry is native to 1.13.0, so that commit is dropped and only its wide-range tests remain. Rebasing onto the realvapor/mainis a no-op.