feat: Add option to auto-load built-in extension

[lucydodo]

This patch changes macOS* users can set to automatically load the sqlean extension when loading DB files.
This PR is related to issue #3357.

Detailed Changes and Code-Flow

Changes

Add a checkbox to the 'Extension' tab of the 'PreferencesDialog' for "Automatically load the 'sqlean' extension"

Code-Flow

sqlitebrowser/src/sqlitedb.cpp

Lines 2174 to 2181 in 83649a3

	if (Settings::getValue("extensions", "autoload_sqlean_extension").toBool())
	{
	const QString sqleanExtensionPath = qApp->applicationDirPath() + "/../Extensions/sqlean.dylib";
	if (loadExtension(sqleanExtensionPath) == false) {
	QMessageBox::warning(nullptr, QApplication::applicationName(), tr("Error loading 'sqlean' extension. Option is disabled."));
	Settings::setValue("extensions", "autoload_sqlean_extension", false);
	}
	}

When loading the DB, get the path to the current app execution if that option is enabled, then locate and load the extension.
(This extension path is determined during packaging, see the following collapse section for more information)

Part of the CI workflow for adding the `sqlean` extension during packaging

sqlitebrowser/.github/actions/notarize-macos/action.yml

Lines 37 to 58 in 0038f0f

	- name: Add the 'nalgeon/sqlean' extension to the app bundle
	shell: bash
	run: |
	gh auth login --with-token <<< "${{ inputs.GH_TOKEN }}"
	gh release download --pattern "sqlean-macos-arm64.zip" --repo "nalgeon/sqlean"
	unzip sqlean-macos-arm64.zip
	for TARGET in $(find build -name "DB Browser for SQL*.app" | sed -e 's/ /_/g'); do
	TARGET=$(echo $TARGET | sed -e 's/_/ /g')
	mkdir "$TARGET/Contents/Extensions"
	clang -I /opt/homebrew/opt/db4subsqlitefts@5/include -L /opt/homebrew/opt/db4subsqlitefts@5/lib -fno-common -dynamiclib src/extensions/extension-formats.c -o "$TARGET/Contents/Extensions/formats.dylib"
	if [ -f "$TARGET/Contents/Extensions/formats.dylib" ]; then
	install_name_tool -id "@executable_path/../Extensions/formats.dylib" "$TARGET/Contents/Extensions/formats.dylib"
	ln -s formats.dylib "$TARGET/Contents/Extensions/formats.dylib.dylib"
	fi
	cp sqlean.dylib "$TARGET/Contents/Extensions/"
	if [ -f "$TARGET/Contents/Extensions/sqlean.dylib" ]; then
	install_name_tool -id "@executable_path/../Extensions/sqlean.dylib" "$TARGET/Contents/Extensions/sqlean.dylib"
	ln -s sqlean.dylib "$TARGET/Contents/Extensions/sqlean.dylib.dylib"
	fi
	done

and if it fails to load the sqlean extension, it will warn the user and deselect the option.

Screenshot

Test Environment

• macOS Sonoma 14.2.1 (23C71)
• Windows 11 Home, 23H2 (22631.2792)

Outdated

[!WARNING]
This change applies to MacOS builds only. Other operating systems are not affected.
Also, when I say testing in a Windows environment, I mean that I have confirmed that this feature is disabled.

Thank you.

*: This PR was changed to support cross-platform not only macOS during review process.
For further information, see the review comments.

[lucydodo]

You can download the test builds from the following link: https://github.com/sqlitebrowser/sqlitebrowser/releases/tag/83649a3-3357

[justinclift]

The concept sounds good. 😄

With the new Automatically load the 'sqlean' extension option, how does it interact with the option above it Allow loading extensions from SQL code?

Does it only work if the Allow loading extensions from SQL code option is enabled? 😄

[lucydodo]

No, the two options work independently of each other.

[lucydodo]

As a note, the CI for Windows keeps failing, and that's because I'm running tests on some CIs right now.
You can ignore it, I'll check it before the final merge. Now fixed.

[justinclift]

Cool. I don't remember C++ enough to really review this.

Hopefully @MKleusberg or @mgrojo have a few minutes to look at it? The code itself seems pretty short. 😄

[lucydodo]

Thank you for the review. In fact, I'm wondering if there's a better way to get the path fo the extension.

Additionally, issue #3503 enlightened me: we have another extension-formats extension besides sqlean.
Why don't we see it to load that as well? For example, we could change the option to something like
Load built-in extensions on startup, but in this case, the project name sqlean would not be visible. 🤔

[justinclift]

Well, "so far" our extension support has been a bit of a mess.

Though stuff improves as people feel motivated to put time into it. 😁

The old PR #1930 ("Create SQLite extensions pack installer for Windows") is still open, and that was created back in 2019.

So it'd be good to get a fairly complete (but simple) approach in place, like this sqlean package.

We can hopefully get it working across all our supported OS's at some point. 😄

For now though, having the sqlean one be (optionally) automatically loaded sounds like a win.

And we should probably have that option turned on by default so things "work out of the box" for the majority of our users.

Allowing other extensions to be (manually) added to that dialog for loading also gives options to the people that need something special.

[lucydodo]

Yes, that makes sense.
Alternatively, we could add the extension-formats extension to the 'sqlean' and build and deploy it. This would eliminate the need for additional options. (However, we could need to confirm this with the original author).

and I'll also have 'sqlean' built-in for Windows builds too in the near future. :)

[justinclift]

Alternatively, we could add the extension-formats extension to the 'sqlean' ...

That makes sense too. It might have already been considered, but it doesn't hurt to ask. 😄

[lucydodo]

Yes, I will complete the Windows side first and then ask to the original author. :)

[mgrojo]

I don't oppose to the PR, but ideally, we should be agnostic from the application about what extensions we deploy along with it and automatically load. It should also be OS independent. As I suggested in #1930, we could generate an initial configuration for the list of loaded extensions from what we are installing. One way to do it is to call the application itself in this way after determining the list of extensions:

sqlitebrowser --save-option extensions/list=\path\to\extension1.dll,\path\to\extension2.dll --quit

The truth is that --quit did not save the option for me. But that should be considered a bug and fixed. Maybe there are other ways to do that initial configuration, though.

This will allow users to later remove some of the loaded extensions and not others, if they want to use different versions or implementations.

The directory of the built-in extensions could include a copy of LICENSE-EXTENSIONS for user awareness of included extensions and respective licenses.

[lucydodo]

Although I understand your sentiment, there is a specific reason for this PR.
In order to have notarized when deploying it on macOS, it is necessary to turn on the 'strict code signing' option.
Unfortunately, this also requires us to sign the libraries included in the app with our certificate, which is why I included and signed sqlean it self. (In addition, 'sqlean' includes extensions for most of the known SQLite extensions) :)

Of course, if the user has superuser privileges,
they can force us to break the code signing for app and any libraries they want and load them.

[justinclift]

we should be agnostic from the application about what extensions we ... automatically load.

This doesn't really sound right to me, as there aren't competing sides who we'd be picking favourites from.

We're making this GUI for our users, and it'll make things that little bit easier for them if we have the sqlean extension enabled by default.

It's one less thing for people to have to worry about, and if their situation is some kind of edge case then they can disable it anyway.

Does that make sense?

[mgrojo]

we should be agnostic from the application about what extensions we ... automatically load.

This doesn't really sound right to me, as there aren't competing sides who we'd be picking favourites from.

Probably, I didn't explain myself correctly. It's not that we should be neutral, what I mean is that 'sqlean' (or any other extension that we distributed and install) should be loaded as default, but through our usual configuration mechanism. That is, I would prefer that users see, when starting the application after installation, just a list of preloaded extensions that they can just edit as usual, adding new extensions or removing them.

We're making this GUI for our users, and it'll make things that little bit easier for them if we have the sqlean extension enabled by default.

It's one less thing for people to have to worry about, and if their situation is some kind of edge case then they can disable it anyway.

Agreed. My remark was only about how to do that, not about not doing it at all.

But if we don't find another way, then I don't mind if this enters the codebase. It's only that I would prefer a more generic approach.

[mgrojo]

What if we simply do something like this:

diff --git a/src/Settings.cpp b/src/Settings.cpp
index bb1e31c4..b36c4187 100644
--- a/src/Settings.cpp
+++ b/src/Settings.cpp
@@ -403,8 +403,13 @@ QVariant Settings::getDefaultValue(const std::string& group, const std::string&
         return true;
 
     // extensions/list?
-    if(group == "extensions" && name == "list")
+    if(group == "extensions" && name == "list") {
+#ifdef Q_OS_MACX
+        return QStringList(qApp->applicationDirPath() + "/../Extensions/sqlean.dylib");
+#else
         return QStringList();
+#endif
+    }
 
     // extensions/disableregex?
     if(group == "extension" && name == "disableregex")

[lucydodo]

Got it. So, if I understand correctly, instead of offering only one option like this PR,
we should provide a list of a default extensions and allow the user to choose from them?

I draw a simple prototype to make sure I understood it well :)

[mgrojo]

I was thinking of just treating them as any other extension that the user has loaded, but done automatically by default (like in the patch I suggested). But your approach is also good, and has the advantage of not losing the reference to the file if the user unselects some built-in extension.

[lucydodo]

In app bundles on macOS, built-in extensions are stored and distributed in the
DB Browser for SQLite.app/Contents/Extensions,
so you mean it would be better to automatically list-up the files in this path to the PreferencesDialog?

[lucydodo]

For now, this PR requires additional code changes, so I'll move it to draft.

[lucydodo]

I've modified the code based on the review, and I'm ready for a new review now.

Now, get a list of extensions that are included in the app bundle, and use checkboxes to let the user decide whether or not to laod them by checking or unchecking them. (Implemented as shown in the screenshot above) Thank you. :)

[justinclift]

Awesome. I can't do a proper technical review of this personally. But I did notice some small potential items that seemed a bit strange, so made comments on those.

The potential change of the currentIndex value though is probably the only one that's actually important. 😄