feat: Add Wasm platform by conditionally building without swift-nio - #110
Open
scottmarchant wants to merge 9 commits into
Open
feat: Add Wasm platform by conditionally building without swift-nio#110scottmarchant wants to merge 9 commits into
scottmarchant wants to merge 9 commits into
Conversation
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: Construct the `tm` with the zeroing initializer, which compiles against every libc regardless of the exact field set. The memberwise call also set `tm_wday` and `tm_yday` to -1, but as review pointed out, `timegm()` ignores both fields entirely, so the all-zero struct is enough on its own. 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. It can build without any of that, 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: The previous commit gated the observation hook API out of the SwiftNIO-free configuration wholesale, on the grounds that it is built on `NIOThreadPool` and `NIOLockedValueBox` throughout. Review pointed out that is the wrong reading: the hook API's public surface has no NIO dependency at all, not even a `ByteBuffer`, and it is already `async`. The thread pool is only involved because event loops are unsuitable for CPU-bound work, and where SwiftNIO is absent there is no event loop to protect; the code can simply run where it is called. Modifications: Narrow the whole-file `canImport(NIOCore)` gate in `SQLiteConnection+Hooks.swift` to the two NIO imports, and route the six hook registration methods through `withBlockingIO(_:)`, the same helper the rest of the shared `async` surface uses, in place of direct `NIOThreadPool.runIfActive` calls. Move the `observerBuckets` property out of the SwiftNIO-only section of `SQLiteConnection`. Where SwiftNIO is absent it is backed by the single-threaded `NIOLockedValueBox` stand-in `Exports.swift` already provides, so the hook implementation compiles unchanged, and `close()` no longer needs a gate around its `clearAllHooks()` call. The C dispatcher machinery needs no changes: SQLite invokes those callbacks synchronously on whatever thread runs the statement, so none of it ever depended on NIO. Result: Where SwiftNIO is present, nothing changes. Where it is absent, the full hook API (update, commit, rollback, and authorizer observers and validators) is now available with upstream semantics; on the single-threaded targets that configuration supports, callbacks fire inline during statement execution.
Motivation: SwiftNIO cannot be built for `wasm32-unknown-wasip1`. Dependency resolution succeeds and NIOCore compiles, but NIOPosix does not: it is built on POSIX sockets and threads, neither of which WASI preview 1 provides. A WASI build of sqlite-nio therefore fails inside a dependency, before 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. Gate the test suite's SwiftNIO imports on the same condition. Those tests drive connections through an `EventLoopGroup` and a `NIOThreadPool`, so they are only meaningful where SwiftNIO is present, and `swift build` evaluates `--explicit-target-dependency-import-check` for every target in the graph, including the test target it does not build. 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, since 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.
Author
|
Just a heads up, I confirmed that CI checks all pass when ran from the PassiveLogic fork: |
Author
|
I've tested pulling this changes into a private downstream repo of mine and have been able to successfully run sqlite queries running the browser using a Wasm built with these changes. |
This was referenced Jul 30, 2026
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #110 +/- ##
==========================================
+ Coverage 67.72% 69.72% +2.00%
==========================================
Files 10 10
Lines 979 991 +12
==========================================
+ Hits 663 691 +28
+ Misses 316 300 -16
🚀 New features to boost your workflow:
|
Motivation: The stand-in supplied where NIOCore cannot be imported takes no lock at all. That is only correct because the single target selecting that configuration today is single-threaded, and nothing in the source records the requirement, so a future target could pick up the configuration and silently get unsynchronized storage. Review asked for a guard that makes widening the configuration impossible without someone revisiting this type. Modifications: Add a `#if _runtime(_multithreaded)` tripwire above the type that fails the build with an explanatory diagnostic. The condition is threading rather than platform, because that is the actual requirement: `wasm32-unknown-wasip1` is single-threaded but `wasm32-unknown-wasip1-threads` is not, so a platform check would admit a target the type cannot serve. `NIOConcurrencyHelpers` keys its own lock implementations off the same condition. Add a debug-only reentrancy assertion to `withLockedValue(_:)`. Taking no lock also means the box cannot nest, and the hook dispatcher invokes callbacks synchronously from inside libsqlite3, so nesting is reachable rather than theoretical. Result: Where SwiftNIO is present, nothing changes: the tripwire sits inside the branch that is only compiled when NIOCore is absent. Where it is absent and the runtime is single-threaded, nothing changes either. A multithreaded target selecting this configuration now fails to compile with a message naming the fix.
Motivation: Consolidating the blocking-call helper rerouted several public entry points through `withBlockingIO(_:)`, and three of them had no test exercising them at all: `lastAutoincrementID()`, `uninstall(customFunction:)`, and the `EventLoopFuture` query primitive. The reshape was therefore unverified for those paths, and coverage reporting on the pull request flagged exactly those lines. Modifications: Add a test asserting `lastAutoincrementID()` reports the rowid of the most recent insert, and that it advances across two inserts. Add a test installing a custom function, calling it, uninstalling it, and asserting the call then fails. The assertion is against any error rather than a specific `SQLiteError` reason, because the failure originates in libsqlite3's unknown-function handling and pinning the reason would make the test brittle. Add a test driving the `EventLoopFuture` query surface end to end. It goes through the `[SQLiteRow]`-returning overload, which delegates to the `logger:`-taking primitive, so the legacy path is exercised without constructing a logger. Result: The suite goes from 59 tests to 62, all passing. The three previously unexercised lines now execute, and the reshaped `async` surface has direct coverage rather than relying on the paths that happened to be tested already.
Motivation: This manifest spelled `nonWASIPlatforms` as a literal list that omits `.wasi`, while the other packages adopting the same approach derive it by filtering `.wasi` out of the full platform list. Review asked for the two to be consistent. The filter form is also the clearer of the two. It names the platform being excluded rather than leaving a reader to notice an absence, and it keeps a future edit from quietly reintroducing `.wasi` by appending it to the list. Modifications: Derive `nonWASIPlatforms` from `allPlatforms` by filtering. Drop the line anchor from the link to SPM's platform list and restore the sentence capitalization, so the whole comment block matches as well. Result: `swift package dump-package` is byte-identical before and after, so the resolved package description, and with it every target dependency condition, is unchanged.
Author
|
Note on the code coverage report, I added coverage for everything I could, but one touchpoint (the |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Builds and tests sqlite-nio on
wasm32-unknown-wasip1without SwiftNIO. Dependency resolution succeeds there and NIOCore compiles, but NIOPosix does not (it needs the POSIX sockets and threads WASI preview 1 lacks), so the package cannot be built for that platform today. Compiling under Embedded Swift needs a further set of gates; that work is kept out of this PR and is not being proposed yet.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 theNIOLockedValueBoxthat the open path acquired in 1.13.0 and the hook observers store their state in.async. Registration runs through the same inline helper as the rest of the sharedasyncsurface, the observer storage rides the single-threadedNIOLockedValueBoxstand-in, and the C dispatcher machinery is untouched, since SQLite invokes those callbacks synchronously on whatever thread runs the statement.tmin the FoundationEssentials date fallback is constructed with the zeroing initializer: the memberwise initializer spells out every libc-specific field and does not compile against wasi-libc, and per review thetm_wday/tm_ydayvalues it was assigning were never needed anyway, sincetimegm()ignores both. Nothing had caught the wasi-libc break 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). The test suite's SwiftNIO imports gate on the same condition, becauseswift buildevaluates--explicit-target-dependency-import-checkfor every target in the graph, including the test target it does not build.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 and the thread pool. TheasyncAPI, already the recommended interface, is what remains, hooks included. 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. A wasmtime smoke run exercises the hook API end to end on the NIO-free flavor: update events in order, observer cancellation tearing down the dispatcher, and a commit-validator veto rolling back. Theunit-tests / wasmlane runs the same build in CI.Notes for review: the
canImport(NIOCore)gates are build-wide, live inSources/only (never in a manifest, per the discussion on #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.