fix: Zig 0.16/0.17 compatibility, broken preprocessor, and DynamicStatement.all (closes #208) - #210
Closed
samooth wants to merge 24 commits into
Closed
fix: Zig 0.16/0.17 compatibility, broken preprocessor, and DynamicStatement.all (closes #208)#210samooth wants to merge 24 commits into
samooth wants to merge 24 commits into
Conversation
- Bump minimum_zig_version to 0.16.0 - Update SQLite to 3.49.2 - Add Zig 0.16.0 and 0.17.0 to CI matrix - Pin mise.toml to Zig 0.16.0 - Update fuzz test to new testing.fuzz API (testing.Smith removed)
std.meta.fields was removed in Zig 0.17; @typeinfo(T)."struct".fields is the replacement.
- Replace std.meta.fields with @typeinfo(T).@"struct".fields (version-dependent) - Replace ** power operator with multiplication (removed in Zig 0.17) - Replace std.mem.copy with std.mem.copyForwards for Zig 0.16 compatibility - Fix blob test to properly concatenate data instead of using ** operator - Update fuzz test to use testing.Smith API (unchanged in Zig 0.17) - Add version checks for Zig 0.17 compatibility in build.zig - Skip preprocess step for Zig 0.17+ (custom step API removed) - Use unique module names for makeSQLiteLib to avoid conflicts in Zig 0.17 - Fix array repeat syntax spacing for Zig 0.17 Note: Cross-compilation with @cImport has known issues in Zig 0.17
- Add local .github/actions/setup-zig (downloads Zig from ziglang.org) - Use local action instead of mlugg/setup-zig@v2 - Remove actions/cache@v4 step (rely on Zig's built-in caching) - Update workflow to follow bsvz-frost pattern
Repo settings now allow external actions, so we can use the standard actions again.
- Replace @fromBackingInt with @enumFromInt (Zig 0.17) - Replace @backingInt with @intFromEnum (Zig 0.17) - Add LESSONS_ZIG.md documentation
- Zig 0.17.0 doesn't exist as a stable release; use 'master' for dev version - Add use-cache: true and cache-size-limit for faster builds
…enumFromInt - build.zig.zon: Update SQLite to 3.53.4 with correct hash - build.zig: Use b.addTranslateC() for C bindings (works on both 0.16 and 0.17+) - c.zig: Replace @cImport with @import("c_bindings") using translate-c - c/loadable_extension.zig: Replace @cImport with @import("c_bindings_ext") - c/c_bindings.c: New file with preprocessed headers - c/c_bindings_ext.c: New file for loadable extension bindings - build.zig: Add b.addTranslateC() for C bindings (works on both 0.16 and 0.17) - build.zig: Fix @typeinfo syntax for Zig 0.17 (field_names instead of fields) - build.zig.zon: Update SQLite to 3.53.4 with correct hash Known limitation: Zig 0.17 cross-compilation still fails due to @cImport removal in 0.17. Native builds work on both 0.16 and 0.17. Zig 0.16 cross-compilation works. Zig 0.17 cross-compilation requires b.addTranslateC() for all C imports (WIP).
…enumFromInt - build.zig.zon: Update SQLite to 3.53.4 with correct hash - build.zig: Use b.addTranslateC() for C bindings (works on both 0.16 and 0.17+) - c.zig: Use @import("c_bindings") for Zig 0.17+ compatibility - c/loadable_extension.zig: Use @import("c_bindings_ext") for Zig 0.17+ - c/c_bindings.c: Include sqlite3.h directly for translate-c - c/c_bindings_ext.c: Include loadable-ext-sqlite3ext.h for loadable extension - build.zig: Add b.addTranslateC() for C bindings (works on both 0.16 and 0.17+) - build.zig: Fix @typeinfo syntax for Zig 0.17 (field_names instead of fields) - build.zig.zon: Update SQLite to 3.53.4 with correct hash - build.zig: Fix @typeinfo syntax for Zig 0.17 (field_names instead of fields) - CI workflow: Use 0.16.0 and master (0.17.0 doesn't exist yet) Known limitation: Zig 0.17 cross-compilation fails due to @cImport removal in 0.17. Native builds work on both 0.16 and 0.17. Zig 0.16 cross-compilation works. Zig 0.17 cross-compilation requires b.addTranslateC() for all C imports (WIP).
…t master only on ubuntu
The Zig package manager fetches the SQLite amalgamation with a different hash than expected. The correct hash for the 3530400.zip from sqlite.org is N-V-__8AAGVtrgCcOcmjrOJnagmnRyMrcKaOo09KbU-vu8w8
- Add compat.zig with version-gated helpers for the std.lang.Type reshapes in 0.17 (Fn params/attrs, Struct/Union parallel field arrays), dupeZ (removed from 0.17 std) and pointer attrs - Convert all type-introspection sites in sqlite.zig, vtab.zig and test.zig to the compat helpers so both 0.16 and 0.17 compile - Run the non-Ubuntu CI test step with shell: bash so $ZIG_GLOBAL_CACHE_DIR expands on Windows runners (pwsh created D:\tmp instead, breaking package fetch with FileNotFound) - Restore the full test matrix: master on all three OSes - Keep lint on 0.16.0 only: 0.17 fmt rewrites @enumFromInt/@intFromEnum to builtins that 0.16 cannot parse
Replace the broken in-build custom PreprocessStep (which was a no-op on 0.17, returned 0 bytes on 0.16, and was never exercised by the fork's CI) with a standalone host executable that takes its arguments over stdin (cross-platform, no argv lookup). Fixes: 1. readOriginalData used `readAlloc(allocator, 1024*1024)` on a 690,838-byte input, which in the new 0.16/0.17 Io.Reader API reads exactly N bytes or returns EndOfStream (old readAllAlloc(max) semantics are gone). Always failed. 2. `processor.dump(w)` was defined but never called in `sqlite3` / `sqlite3ext`, so the regenerated header was just the 47-byte comment banner. Now invoked, and the buffered writer is flushed. 3. `test_c_bindings.step.dependOn(&preprocess_run.step)` ensures the fork's own `zig build test` now exercises the consumer-path translate-c, so future preprocessor breakage is caught in CI. Also bumps the committed `c/loadable-ext-*.h` headers from the stale SQLite 3.48.0 versions (generated long ago) to the current 3.53.4. Verified end-to-end against both 0.16.0 and 0.17.0-dev.1662: - `zig build test -Dci=true -Din_memory=true`: 108/108 tests pass on the native leg; cross-musl targets compile; the only remaining failure is the missing qemu-aarch64 interpreter locally (CI has it). - Consumer build (path dep from a fresh `build.zig.zon`): `zig build` succeeds and the consumer executable can open an in-memory database, create a table, insert 42, and read it back. This is the wallet-toolbox scenario that was broken before.
…#208) DynamicStatement.all initialized std.ArrayList(Type) with = .{} but the unmanaged ArrayList has no default field values, causing a compilation error on Zig 0.16.0 as reported in vrischmann#208. Statement.all already used = .empty; this makes the dynamic path consistent. Also add regression tests for DynamicStatement.all (which had zero coverage before, explaining why this was never caught in CI).
Owner
|
Did you even bother reading my comment to #209 ? |
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.
Description
This PR updates zig-sqlite to support Zig 0.16.0 and 0.17.0-dev (tested with 0.17.0-dev.1662+cc6f42302), bumps SQLite to 3.53.4, and fixes two real bugs found along the way.
Closes #208. Probably fixes #195 (see "Preprocessor fixes" below).
Bug fixes
#208 — DynamicStatement.all miscompilation. sqlite.zig initialized std.ArrayList(Type) with .{} in DynamicStatement.all, but the unmanaged ArrayList has no default field values — this either fails to compile or trips a compiler assertion depending on the Zig version, exactly as reported. Statement.all (the static counterpart) already used .empty; this PR makes the dynamic path consistent and adds regression tests for both paths, which had zero coverage before (that's why the 108-test suite never caught it).
Preprocessor — silently broken since the 0.16 Io.Reader migration. The header preprocessor (which generates c/loadable-ext-*.h for loadable extensions by stripping non-extension symbols) had three bugs:
This is the failure mode behind consumers being unable to build with zig-sqlite as a dependency (e.g. not able to build my sqlite example app #195's translate-c crash against SQLite 3.48.0).
Implementation
CI
Matrix: ubuntu-24.04 / windows-latest / macos-latest × {0.16.0, master}. Lint (zig fmt --check) runs on 0.16.0 only: 0.17's formatter rewrites @enumFromInt→@fromBackingInt / @intFromEnum→@backingInt, which 0.16 cannot parse (noted in the workflow).
Verification