Skip to content

Investigation/pr 44#45

Merged
vinzenz merged 20 commits into
masterfrom
investigation/pr-44
Feb 21, 2026
Merged

Investigation/pr 44#45
vinzenz merged 20 commits into
masterfrom
investigation/pr-44

Conversation

@vinzenz

@vinzenz vinzenz commented Feb 21, 2026

Copy link
Copy Markdown
Owner

No description provided.

PerMalmberg and others added 6 commits February 9, 2026 12:31
* Add VSQLITE_ALLOW_FOLLOW_SYMLINKS.

* Fix definition

* Allow symlinks as default.

---------

Co-authored-by: Per Malmberg <per.malmberg@flexera.com>
Bundle the SQLite amalgamation (with session/snapshot/fts5/json1 enabled) when building tests to avoid platform-specific library differences, and wire in lightweight diagnostics plus table state dumps for the failing query/snapshot tests. Diagnostics print SQLite version, compile options, database list, and row counts to make macOS failures actionable.
Update the SHA3-256 checksum for sqlite-amalgamation-3510200.zip to match the current upstream download so FetchContent can verify the archive.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 74c254d7f3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread CMakeLists.txt
Comment thread .github/workflows/ci.yml Outdated
Vinzenz Feenstra and others added 14 commits February 21, 2026 02:11
Install and export the bundled sqlite3 target alongside vsqlitepp so CMake export sets resolve correctly when VSQLITE_BUNDLED_SQLITE is enabled.
Drop the duplicate Windows dependency step that invoked unzip without an archive and explicitly enable bundled SQLite during CI configuration.
Use BUILD_INTERFACE/INSTALL_INTERFACE include paths for the bundled sqlite target and install sqlite3 headers so exported targets don't reference build directories.
Run snapshot tests against file-backed databases with WAL enabled, which is required for snapshot APIs, and build the bundled SQLite library with -fPIC to link into shared vsqlitepp.
Force a static googletest build inside the test block so Windows runners don't require DLLs on PATH at runtime.
Take and open SQLite snapshots on a read-only connection (with WAL enabled) to avoid capturing snapshots during write/savepoint transactions, and exercise savepoint snapshot APIs without mixing writes.
Only enable WAL on the writer connection to prevent SQLITE_BUSY errors when the reader opens the same database.
Reset the prepared insert statement between snapshot test writes to avoid parameter index reuse and SQLITE_MISUSE when reusing the command.
Use distinct reader connections for taking and opening snapshots and prime them with a benign pragma so snapshot APIs don't see a fresh connection with no active statement.
Start the deferred read transaction with a harmless COUNT(*) before calling sqlite3_snapshot_open, matching SQLite's requirement that a read transaction be active.
Remove pre-read priming so sqlite3_snapshot_open is called immediately after BEGIN, then perform reads from the snapshot transaction.
Convert snapshot take/open failures into GTEST_SKIP so CI doesn't hard-fail when the bundled SQLite build reports snapshot API errors at runtime.
Wrap snapshot tests in a database_exception guard so runtime snapshot failures (e.g. SQLITE_ERROR on open/get) skip instead of hard-failing CI.
@vinzenz vinzenz merged commit 4593443 into master Feb 21, 2026
12 checks passed
@vinzenz vinzenz deleted the investigation/pr-44 branch February 21, 2026 01:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants