ericsink / SQLitePCL.raw

A Portable Class Library (PCL) for low-level (raw) access to SQLite
Apache License 2.0
525 stars 109 forks source link

Consider bringing back dllimport #419

Closed ericsink closed 3 years ago

ericsink commented 3 years ago

During the development of SQLitePCLRaw 2.0, my hope was to get rid of most of the provider implementations and use the "dynamic" provider in all cases.

The basic idea of the dynamic provider is this: Instead of using DllImport attributes, dynamically load the shared library and look up all the names needed.

This was intended to solve two problems.

(1) DllImport requires the name of the shared library to be hard-coded at build time, so the provider has to be recompiled for every name we need. Using a custom SQLite build with the name provided by the app is impossible.

(2) DllImport doesn't allow control over where the shared library is to be found. The hope is that by having finer control over this, we could gain the ability to deal with the nearly constant stream of people having trouble with "e_sqlite3 not found" errors.

Overall, this didn't work out well. I keep encountering cases where it is necessary to just use DllImport anyway. In the end, the dynamic provider probably increased the number of builds instead of its goal of decreasing that number.

And we seem to have just as many people struggling with inability to locate the shared library. Maybe more.

The dynamic provider is implemented using System.Runtime.InteropServices.NativeLibrary. But that API was new in .NET Core 3, so I implemented a subset of it for use on .NET Framework. But there have been problems wherein my implementation doesn't behave the same way. See #389 for example.

Note also that System.Runtime.InteropServices.NativeLibrary includes an API to configure the way DllImport lookups are resolved. I'm not currently using this, but it would present another alternative for the problems above, staying with DllImport plus a configured resolver instead of using NativeLibrary.Load().

It is not clear that the dynamic provider has added anything except complexity.

ericsink commented 3 years ago

FWIW, the dynamic provider is used on .NET Core, and it doesn't seem to be causing any trouble there (that I can remember). There is probably little reason to change this case to use dllimport.

Note also that what I'm talking about here is changing which provider is used for a certain case by a certain bundle. The experimental code change in 1b2e505 changes the bundles to use dllimport providers for net461. But the dynamic provider is still available. Nothing would prevent someone from skipping the bundles and doing things manually and using the dynamic provider on net461 for cases where it is needed.

ericsink commented 3 years ago

pre-release 2.0.5-pre20210521085756 has been pushed up to nuget, including 1b2e505, which changes the bundles to use dllimport providers on .NET Framework.

jeromelaban commented 3 years ago

On this topic, the use of dynamic resolution makes it impossible, as of current .NET for WebAssembly (net5/net6), to invoke sqlite native functions.

Using NuGet tricks to exclude SQLitePCLRaw.provider.dynamic_cdecl and use SQLitePCLRaw.provider.sqlite3 instead can make the interop work properly.

ericsink commented 3 years ago

Note to self:

The previous comment from @jeromelaban appears to contradict the following sentence I wrote a bit further up:

FWIW, the dynamic provider is used on .NET Core, and it doesn't seem to be causing any trouble there (that I can remember). There is probably little reason to change this case to use dllimport.

ericsink commented 3 years ago

Done in 2.0.5