Repository: beetbox/beets
Pull request: Add loadext plugin
This is a rough draft at the moment of what a `loadext` plugin could look like for loading SQLite extensions. This plugin can be used to fix #3160 by allowing users to load the ICU plugin like so;

```
loadext:
  - libicu
```

Note that this will require the user to compile their own copy of `ext/icu` from SQLite sources.

## Points of Discussion
I've got a few points of discussion I'd like to bring up regarding this PR;

1. The elephant in the room;
 
    ```Python
    # TODO: Find a way of getting a connection without calling a private method.
    conn = lib._connection()
    ```

    This is certainly less than ideal, as we're reaching into beets internals from this plugin. There ~are two~ is one possible solution~s~ I can think of to this;

    * ~Use `SELECT load_extension(...)`~ This doesn't work properly, see [this thread](http://sqlite.1065341.n5.nabble.com/Error-on-loading-ICU-extension-td41219.html) regarding the issues.
    * Add a `load_extension` method to `dbcore.Database`.

2. Where should these extension paths be relative to? In this initial draft the paths are relative to wherever `beets` is running, and as such running beets in different folders breaks this unless you use an absolute path for the extension. I think using the beets config directory for relative paths is probably the best idea, but I'll leave this open to others to decide.

3. How in depth do we want to go into the documentation with this? This plugin seems like a somewhat advanced feature, as it requires the user to compile their own `ext/icu` extension from the SQLite sources.

4. How do we test this? Do we mock out the `load_extension` method and make sure it's called? Or do we compile the ICU extension on Travis and attempt to call one of the methods provided by it?

## Compiling the ICU extension
To compile the ICU extension, you need the following;

 * gcc
 * icu-devtools
 * libicu-dev
 * libicu60 (or any other version I guess?)
 * libsqlite3-dev

```Shell
$ wget https://sqlite.org/2019/sqlite-src-3280000.zip
$ unzip sqlite-src-3280000.zip
$ cd sqlite-src-3280000/ext/icu
$ gcc -shared -fPIC icu.c `icu-config --ldflags` -o libicu.so
$ cp libicu.so [wherever you want]
```

Problem statement:
String queries should be case-insensitive even for non-ASCII characters
### Problem

When searching for metadata written in English or, in other words, in latin script, `beet ls` performs case-insensitive search, which is very nice. With non-latin scripts (I've checked with cyrillic, since it is the only one in which I have tracks and which has the case concept),  however, case-insensitive search does not work.

Running this command:

```sh
$ beet ls сплин
```

outputs nothing, while this one:

```sh
$ beet ls Сплин
```

shows all the tagged files correctly.

I would expect this to work for any kind of scripts, probably based on unicode case transformations.

### Setup

* OS: Archlinux
* Python version: 3.7.2
* beets version: 1.4.7
* Turning off plugins made problem go away (yes/no): no

My configuration (output of `beet config`) is:

```yaml
lyrics:
    bing_lang_from: []
    auto: yes
    bing_client_secret: REDACTED
    bing_lang_to:
    google_API_key: REDACTED
    google_engine_ID: REDACTED
    genius_api_key: REDACTED
    fallback:
    force: no
    local: no
    sources:
    - google
    - lyricwiki
    - musixmatch
    - genius
directory: ~/Music
library: ~/.local/share/beets/music.db
sort_item: artist+ year+ album+ disc+ track+
original_date: yes

ui:
    color: yes

import:
    move: no
    log: ~/.local/share/beets/import.log
    incremental: yes

paths:
    default: $albumartist/$year - $album%aunique{albumartist year album}/%if{$multidisc,Disc $disc/}$track - $title
    comp: Compilations/$album%aunique{}/$track - $title
    singleton: Singletons/$artist - $title
item_fields:
    multidisc: 'True if disctotal > 1 else False

        '

plugins: chroma fromfilename edit embedart fetchart lyrics zero bpd convert info export alternatives types inline web
chroma:
    auto: no
embedart:
    compare_threshold: 50
    maxwidth: 0
    auto: yes
    ifempty: no
    remove_art_file: no
fetchart:
    store_source: yes
    auto: yes
    minwidth: 0
    maxwidth: 0
    enforce_ratio: no
    cautious: no
    cover_names:
    - cover
    - front
    - art
    - album
    - folder
    sources:
    - filesystem
    - coverart
    - itunes
    - amazon
    - albumart
    google_key: REDACTED
    google_engine: 001442825323518660753:hrh5ch1gjzm
    fanarttv_key: REDACTED
zero:
    fields: month day
    auto: yes
    keep_fields: []
    update_database: no
bpd:
    host: 127.0.0.1
    port: 6600
    password: REDACTED
    volume: 100
convert:
    never_convert_lossy_files: yes
    format: opus
    dest:
    pretend: no
    threads: 12
    formats:
        aac:
            command: ffmpeg -i $source -y -vn -acodec aac -aq 1 $dest
            extension: m4a
        alac:
            command: ffmpeg -i $source -y -vn -acodec alac $dest
            extension: m4a
        flac: ffmpeg -i $source -y -vn -acodec flac $dest
        mp3: ffmpeg -i $source -y -vn -aq 2 $dest
        opus: ffmpeg -i $source -y -vn -acodec libopus -ab 96k $dest
        ogg: ffmpeg -i $source -y -vn -acodec libvorbis -aq 3 $dest
        wma: ffmpeg -i $source -y -vn -acodec wmav2 -vn $dest
    max_bitrate: 500
    auto: no
    tmpdir:
    quiet: no
    embed: yes

    paths: {}
    no_convert: ''
    copy_album_art: no
    album_art_maxwidth: 0
types:
    phone_export: bool
alternatives:
    phone:
        directory: ~/tmp/phone-music
        formats: opus mp3 ogg
        query: phone_export:true
export:
    default_format: json
    json:
        formatting:
            ensure_ascii: no
            indent: 4
            separators: [',', ': ']
            sort_keys: yes
edit:
    albumfields: album albumartist
    itemfields: track title artist album
    ignore_fields: id path
pathfields: {}
album_fields: {}
web:
    host: 127.0.0.1
    port: 8337
    cors: ''
    cors_supports_credentials: no
    reverse_proxy: no
    include_paths: no
```

Hints from issue discussion:
Sounds good! See also #2626 for some prior discussion.
I think the the solutions has two parts: first, the user of beets should have the ICU extension for SQLite on their machine (at least for Linux systems, where system SQLite libraries are usually used). This seems to be quite obscure, unfortunately - it seems that there is no ICU extension anywhere in Archlinux packages, including AUR, which suggests than it is even worse in other distros (maybe sans Gentoo and similar distributions). All instructions on the net suggest building the extension manually.

Second, beets should load this extension when it queries the database. From what I understand, this should be an explicit action on the client side of the database. After it is done, the `LIKE` queries should handle character case automatically.

I wonder, could this (loading an SQLite extension) be done through a plugin? If yes, then I think adding a plugin with an optional explicit path to the extension shared object (possibly using some system directories, if it is not specified), which loads this extension, could be a nice solution.
Building sqlite with ICU module enabled does indeed help, and makes it work automatically without any changes necessary from the beets side. This kind of solves this issue specifically for me, since I can build sqlite with necessary options. But I wonder - across all platforms, how often maintainers build sqlite with ICU extension built-in?
That's great to hear @netvl, thanks for doing some digging! It's awesome to hear that beets doesn't need any changes to support this, and I've been playing around with using a dynamic library for it too.

It does seem that we can load the ICU extension into beets' SQLite database at startup using a plugin, although we do need to reach into the beets internals a bit;

```Python
from __future__ import division, absolute_import, print_function

from beets.plugins import BeetsPlugin

class LoadIcuPlugin(BeetsPlugin):
    def __init__(self):
        super(LoadIcuPlugin, self).__init__()
        self.register_listener('library_opened', self.library_opened)

    def library_opened(self, lib):
        lib._connection().load_extension('libicu.so')
```

[It seems that we could also call `db.enable_load_extension(True)` inside `dbcore` and then just use the `Transaction#query` method to run `SELECT load_extension("libicu.so")`](https://docs.python.org/2/library/sqlite3.html#sqlite3.Connection.enable_load_extension), which doesn't require calling any private methods (like `_connection()`).

I think this could be made into a simple `LoadExtPlugin` which takes a list of plugins to load from your beets configuration.
That's cool! We could even fold this into beets core if we could make it optional. Is it easy to load the plugin if it's available, and silently do nothing if it's not?
The main issue is that you need a path to the plugin, I guess we could make it a core configuration option to provide the path? I'm not sure if there's a "standardised" path for SQLite plugins.
Ah, of course, that makes sense! In that case, a beets plugin for loading these could be the right path.

Commit message: Add load_extension method for loading SQLite extensions

Patch to review:
diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py
index 43c0445727..97a4a7ce31 100755
--- a/beets/dbcore/db.py
+++ b/beets/dbcore/db.py
@@ -850,16 +850,21 @@ class Database(object):
     """A container for Model objects that wraps an SQLite database as
     the backend.
     """
+
     _models = ()
     """The Model subclasses representing tables in this database.
     """
 
+    supports_extensions = hasattr(sqlite3.Connection, 'enable_load_extension')
+    """Whether or not the current version of SQLite supports extensions"""
+
     def __init__(self, path, timeout=5.0):
         self.path = path
         self.timeout = timeout
 
         self._connections = {}
         self._tx_stacks = defaultdict(list)
+        self._extensions = []
 
         # A lock to protect the _connections and _tx_stacks maps, which
         # both map thread IDs to private resources.
@@ -909,6 +914,13 @@ def _create_connection(self):
             py3_path(self.path), timeout=self.timeout
         )
 
+        if self.supports_extensions:
+            conn.enable_load_extension(True)
+
+            # Load any extension that are already loaded for other connections.
+            for path in self._extensions:
+                conn.load_extension(path)
+
         # Access SELECT results like dictionaries.
         conn.row_factory = sqlite3.Row
         return conn
@@ -937,6 +949,18 @@ def transaction(self):
         """
         return Transaction(self)
 
+    def load_extension(self, path):
+        """Load an SQLite extension into all open connections."""
+        if not self.supports_extensions:
+            raise ValueError(
+                    'this sqlite3 installation does not support extensions')
+
+        self._extensions.append(path)
+
+        # Load the extension into every open connection.
+        for conn in self._connections.values():
+            conn.load_extension(path)
+
     # Schema setup and migration.
 
     def _make_table(self, table, fields):


Code review comments:
diff_hunk: @@ -0,0 +1,53 @@
+Load Extension Plugin
+=====================
+
+Beets uses an SQLite database to store and query library information, which
+has support for extensions to extend its functionality.
+
+One of the primary uses of this within beets is with the `"ICU" extension`_,
+which adds support for case insensitive querying of non-ASCII characters.
+
+.. _"ICU" extension: https://www.sqlite.org/src/dir?ci=7461d2e120f21493&name=ext/icu
+
+Configuration
+-------------
+
+To configure the plugin, make a ``loadext`` section in your configuration
+file. The section must consist of a list of paths to extensions to load.
+
+If a relative path is specified, it is resolved relative to the beets
+configuration directory.
+
+No file extension is required, as SQLite will automatically use the correct
+extension for the current platform.
+
+Building the ICU extension
+--------------------------
+This section is for **advanced** users only, and is not an in-depth guide on
+building the extension.
+
+To compile the ICU extension, you will need a few dependencies:
+
+ - gcc
+ - icu-devtools
+ - libicu
+ - libicu-dev
+ - libsqlite3-dev
+
+The following commands can be then be used to compile the ICU extension:
+
+.. code-block:: shell
+
+    $ wget https://sqlite.org/2019/sqlite-src-3280000.zip
+    $ unzip sqlite-src-3280000.zip
+    $ cd sqlite-src-3280000/ext/icu
+    $ gcc -shared -fPIC icu.c `icu-config --ldflags` -o libicu.so
+    $ cp libicu.so [your beets config directory]
path: docs/plugins/loadext.rst
text: @user1:
To make this a little more digestible, I'd probably just say:

```
$ cp libicu.so ~/.config/beets
```

Readers (advanced ones, anyway) will be able to sort out that they need to think about where to copy the file.