Switch Bookmarks plugin to use Microsoft.Data.Sqlite

[ktos]

In order to prepare a possible ARM64 release (see #2318) the first step would be probably to switch Bookmarks plugin to use a sqlite library available natively for ARM64. System.Data.SQlite which was used is quite old, so this PR switches to current Microsoft.Data.Sqlite, which is based on SQLiteRawPCL offering native sqlite library for many platforms.

The differences are in the connection string, as some parameters were not available for the newer nuget, and in the setting the default RuntimeID to win-x64, which can be overridden at a build step (e.g. to win-arm64 if needed).

This PR also effectively removes support for 32-bit Windows, not sure if it is a problem.

[jjw24]

Hey thanks for the PR.

Could you list what tests you have done with this PR, e.g. loading bookmarks from chrome, edge, etc

[jjw24]

This PR also effectively removes support for 32-bit Windows, not sure if it is a problem.

No issues, we don't support 32-bit Windows.

[jjw24]

Before we can officially publish ARM64 installer, we need to check:

• if any other plugin is not compatible, at the very least address Everything in Explorer plugin being not compatible.
• update the pipeline to publish ARM64 artefact.

Should do all this in another PR. Just noting it down here for now.

[taooceros]

Before we can officially publish ARM64 installer, we need to check:

* if any other plugin is not compatible, at the very least address Everything in Explorer plugin being not compatible.

* update the pipeline to publish ARM64 artefact.

Should do all this in another PR. Just noting it down here for now.

We further need to have a more comprehensive plugin assembly loader. The current one doesn't support deps.json which leads to various issue when publishing crossplatform.

[taooceros]

Remember win-x64 is also not arm. Therefore this may also break the potential arm build.

[ktos]

I decided on it to have "default" win-x64 build, RuntimeIdentifier in .csproj can be overridden by specyfing it in the console build, e.g. dotnet build -r win-arm64. So right now you can build without a problem in VS, and switch to ARM if you really need it.

Alternatively, there is a possibility to put multiple RIDs and compile everytime for all archs.

[taooceros]

Alright that sounds reasonable. Let's just take the default behavior.

[taooceros]

Seems like it is easier than what I think of. I will push the change to this pr.

ref: https://www.reddit.com/r/dotnet/comments/ojl3ah/any_resources_for_resolving_assembly_dependencies/
https://learn.microsoft.com/en-us/dotnet/core/tutorials/creating-app-with-plugin-support

[github-actions]

@check-spelling-bot Report

🔴 Please review

See the 📂 files view or the 📜action log for details.

Unrecognized words (1)

newsqlite

To accept ✔️ these unrecognized words as correct and remove the previously acknowledged and now absent words, run the following commands

... in a clone of the [email protected]:ktos/Flow.Launcher.git repository
on the bookmarks-newsqlite branch (ℹ️ how do I use this?):

curl -s -S -L 'https://raw.githubusercontent.com/check-spelling/check-spelling/main/apply.pl' |
perl - 'https://github.com/Flow-Launcher/Flow.Launcher/actions/runs/6147607468/attempts/1'

Errors (1)

See the 📂 files view or the 📜action log for details.

❌ Errors	Count
❌ check-file-path	1

See ❌ Event descriptions for more information.

If the flagged items are 🤯 false positives

If items relate to a ...

• binary file (or some other file you wouldn't want to check at all).

  Please add a file path to the excludes.txt file matching the containing file.

  File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

  ^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

• well-formed pattern.

  If you can write a pattern that would match it,
  try adding it to the patterns.txt file.

  Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.

  Note that patterns can't match multiline strings.

[taooceros]

@jjw24 I think we can merge this in. There shouldn't be much issue.

[jjw24]

It's an enhancement, we can merge after 1.16.2 is released.

[jjw24]

Also if you have updated the DLL loading mechanism, have you tested that it is working as expected and for some plugins as well?

Otherwise I would seperate this if it's a non-relevant change for the purpose of this PR, so testing is easier.

[ktos]

As for the tests, I made a few more to check if nothing was broken in the process:

I've created a simple console app depending on Flow.Launcher.Plugin.BrowserBookmark and wrote a simple code printing bookmarks loaded by the plugin:

IBookmarkLoader f = new FirefoxBookmarkLoader();

foreach (var item in f.GetBookmarks())
{
    Console.WriteLine(item.Name);
}

I've been building it for win-arm64 and win-x64 and checking manually if the results are actually my bookmarks.

Seems to be working for Edge, Chrome, Firefox and Firefox ESR. I've also checked if native e_sqlite3.dll is the one I wanted after build:

dotnet build -r win-arm64 ConsoleApp1.csproj
file.exe bin\Debug\net7.0-windows\win-arm64\e_sqlite3.dll

bin\Debug\net7.0-windows\win-arm64\e_sqlite3.dll: PE32+ executable (DLL) (console) Aarch64, for MS Windows, 6 sections

So I am quite sure migration to new library didn't broke bookmarks loading.

[taooceros]

Also if you have updated the DLL loading mechanism, have you tested that it is working as expected and for some plugins as well?

Otherwise I would seperate this if it's a non-relevant change for the purpose of this PR, so testing is easier.

In fact, that's the supposed way to do it. I think previously we missed that part to load unmanaged dll, which creates weird issues.

I have vscode workspace installed, which works fine with this branch. That one also need to load some native dll.

https://learn.microsoft.com/en-us/dotnet/core/tutorials/creating-app-with-plugin-support#load-plugins

[jjw24]

I been test driving this change for fair few days now so far no issues I have spotted with this and the code changes look good. Thanks guys.