Skip to content

Try loading winsqlite3.dll on Windows #290

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

Merged
merged 17 commits into from
Mar 7, 2025

Conversation

benthillerkus
Copy link
Contributor

Windows ships with a sqlite3 dll under the name winsqlite3 since a couple of years. Similar to macOS and Linux this can be used, if no override has been provided

(I accidentally closed #289 by renaming the branch)

Copy link
Owner

@simolus3 simolus3 left a comment

Choose a reason for hiding this comment

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

Thank you for this! I agree that using winsqlite3.dll as a fallback is good, but:

  1. There should be a comment explaining that this is the name of the shared library on windows, perhaps also mentioning this link which seems to be the best source I could find on this.
  2. It would be convenient to test this as part of our CI. Can you see if copying this (while removing the other operating systems as well as the steps to download and install the prebuilt sqlite3 binaries) still passes?

@benthillerkus
Copy link
Contributor Author

benthillerkus commented Mar 7, 2025

  1. There should be a comment explaining that this is the name of the shared library on windows, perhaps also mentioning this link which seems to be the best source I could find on this.

Added a comment. Winsqlite seems to be kinda undocumented by MS, but it should be fine; they cannot just remove it without breaking compatibility.

I kept the code that tries to open sqlite3.dll first, because maybe someone was relying on this (and also I have no clue how sqlite3_flutter_libs works, and maybe that package needs that).

  1. It would be convenient to test this as part of our CI.

I added a new dimension to the test matrix for system provided dlls vs the compiled ones.
There is some weirdness with winsqlite as you can maybe see in the commit messages.

The tests crash when trying to call the function to register a virtual file system. Atleast in CI. I cannot reproduce this on my Windows 11 machine locally.

For some reason either using --debug or enabling randomized test order made the regular test suite pass in CI. This doesn't work for the package with test helpers, so I ended up disabling that for winsqlite testing. I'm kind of out of my depth here.

Copy link
Owner

@simolus3 simolus3 left a comment

Choose a reason for hiding this comment

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

I'm fine with skipping tests on sqlite3_web if they're failing with the system's build on Windows. Thanks for your contribution 👍

@simolus3 simolus3 merged commit 0407796 into simolus3:main Mar 7, 2025
19 of 20 checks passed
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