experimental: switch to SQLitePCL.raw #292
Conversation
…LitePCL.raw. this was mostly done by putting 'using NativeMethods = SQLitePCL.raw;' (and similar for Sqlite3Handle and Sqlite3StmtHandle) at the top of source files. a few other minor code changes were necessary as well. plus a couple of new things in SQLitePCL.raw itself, now planned for the 1.1.x release of same. ability to use winsqlite3.dll removed, as this capability is built-into and handled-by SQLitePCL.raw. this makes it possible for Microsoft.Data.Sqlite to no longer care about which particular instance of the SQLite library is being used. most tests pass. 3 test failures. 2 of them look irrelevant, 1 needs further investigation.
|
Hi @ericsink, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution! TTYL, DNFBOT; |
|
Out of curiosity, how do you support binding to winsqlite3.dll? Does it work with .NET Native? See #249 for the issues we've ran into. |
|
I haven't tested with .NET Native yet, but I am interested in doing so. The work I've done to be compatible with other AOT environments makes me optimistic about those aspects, but there could be other problems lurking. SQLitePCL.raw has a system of "providers", which are implementations of ISQLite3Provider. Each implementation corresponds to a particular instance of the SQLite native library. The main assembly needs one of these providers to be injected as part of initialization. winsqlite3.dll is handled by a different provider. Same goes for sqlcipher, and for bundled SQLite builds to satisfy Android N, and so on. |
|
Wait. Does support for UWP10 mean that I actually have tested with .NET Native? |
|
|
||
| namespace Microsoft.Data.Sqlite | ||
| { | ||
| public class WinSqlite3TestFramework : XunitTestFramework |
There was a problem hiding this comment.
This is an important part of our testing. We use this to verify that our implementation works with the "winsqlite3" library that is bundled in Windows 10 (desktop). Does this not work with SQLitePCL?
There was a problem hiding this comment.
winsqlite3.dll is supported, but it's done differently. I didn't set that up as part of the proof-of-concept stuff I was doing, but it's not difficult.
| @@ -21,7 +21,7 @@ | |||
| ] | |||
| }, | |||
| "dependencies": { | |||
| "SQLite": "3.12.2", | |||
| "SQLitePCLRaw.bundle_e_sqlite3": "1.1.0-pre20160928150051", | |||
There was a problem hiding this comment.
How would the consumer of Microsoft.Data.Sqlite swap this out for another one?
There was a problem hiding this comment.
Or would we need to do something like this?
There was a problem hiding this comment.
There are a couple of options here.
The technique shown in the PR was to take the dependency on a bundle. This allows me to call SQLitePCL.Batteries_V2.Init() from Microsoft.Data.Sqlite itself (in a static constructor) in portable code. The bait-and-switch technique replaces this with a real implementation. This is accomplished by adding the bundle to the tests assembly.
The main reason I used the technique above was that it made the xUnit tests work without sprinkling Init() calls into them.
Another option would be to make Microsoft.Data.Sqlite depend on SQLitePCLRaw.core (the main assembly) and to do the Init() call somewhere else, more explicitly.
There was a problem hiding this comment.
Ah, yes, the discussion over in item #148 mentions some of the same techniques that I've needed to use.
|
FWIW I think this is an idea we've floated before. #21. I think this could be a good move. We haven't yet made our library work on AndroidN/iOS. It looks like SQLitePCL.raw has already solved some platforms problems we haven't even discovered. But as mentioned above, we should check carefully this wouldn't regress our support UWP and .NET Native.
@ericsink not necessarily. .NET Native tool chain is disabled in "Debug" mode. Did you compile and run in "Release" mode? Did you dry deploying to x86/x64/ARM devices? |
|
Embarrassingly enough, I've been thinking of .NET Native as yet another .NET thing that I need to test and figure out. I've heard inklings of UWP builds being AOT, but I never realized the connection. I've done some fixes specifically for UWP users who have reported problems, but there have definitely been some holes in my testing along these lines. I'll report back after I do some additional testing with .NET Native and UWP and release builds and such. |
...well, actually...there is the CoreRT project (https://github.com/dotnet/corert) which yet another AOT thing...but we don't support it yet. For now, by .NET Native we mean "AOT for Universal Windows Platform". |
|
Thanks for the tip on the .NET Native/UWP/release-mode stuff. It turns out I did have a bug there. It showed up only for the winsqlite3 provider. The e_sqlite3 provider was fine. Anyway, the compiler error message recommended ExactSpelling=true on the DllImport attributes, and that was the fix. So my test suite is now passing for UWP release mode builds. |
|
👍 I'm all for making this move too. It offloads most of the platform-specific problems we have and will encounter. We just need to sort out the logistics... I'm marked #21 for re-triage to discuss. |
|
Sounds good. Interesting to read issue #21 and see how much has changed in the TWO YEARS since. :-) The 1.1.0 release of SQLitePCL.raw should happen next week, and will contain fixes that Microsoft.Data.Sqlite would need. I will let you guys drive this issue as you see fit. Just let me know if/how/when I can help. |
|
It looks like the Azure SDK team has taken a dependency on SQLitePCL.raw (in Microsoft.Azure.Mobile.Client.SQLiteStore), so we shouldn't run into any problems. |
Yes, I believe the "Android N SQLite Apocalypse" was what pushed them toward the change.
Or rather, if you do have any problems, you should be in excellent company. :-) |
|
Closing, but we'll use this as info when we look at moving. |
I consider this more of an FYI than a literal pull request.
As a proof of concept, I deleted most of the stuff under Interop, added a reference to SQLitePCL.raw, remapped NativeMethods/Sqlite3Handle/Sqlite3StmtHandle, fixed a few things, and it basically works.
I realize that taking a dep on a community open source package like SQLitePCL.raw would be a risk and a big decision. I'm not pushing for that. I'm just pleased that it seems to work.
In part, I am looking toward possibilities of an ADO.NET style SQLite API (or maybe even EF Core) on iOS and Android. Building on SQLitePCL.raw makes that straightforward. It also provides a path toward things like custom SQLite builds, or swapping the SQLite library for something like SQLCipher.
In a nutshell, this approach would allow Microsoft.Data.Sqlite to not care about the specific instance of the native SQLite library. You could just target, say, netstandard1.1 and let SQLitePCL.raw deal with all the drama of Xamarin pinvokes.
My original commit message:
experimental idea. change Microsoft.Data.Sqlite to build on top of SQLitePCL.raw. this was mostly done by putting 'using NativeMethods = SQLitePCL.raw;' (and similar for Sqlite3Handle and Sqlite3StmtHandle) at the top of source files. a few other minor code changes were necessary as well. plus a couple of new things in SQLitePCL.raw itself, now planned for the 1.1.x release of same. ability to use winsqlite3.dll removed, as this capability is built-into and handled-by SQLitePCL.raw. this makes it possible for Microsoft.Data.Sqlite to no longer care about which particular instance of the SQLite library is being used. most tests pass. 3 test failures. 2 of them look irrelevant, 1 needs further investigation.