[RFC] Add basic extension loader.

[toofar]

I wanted to see if I could move all my config.py snippets into extensions. I could, because I am not doing any configurating in config.py, just registering commands. This works (for me) but it could use some more attention, particularly around error handling. I don't know how to best handle probably non-fatal errors at init time and I would like to be able to proceed if we run across them. I opened this PR to get some discussion on what we want out of this.

This implementation is similar to _walk_normal() but since the extensions aren't in a properly importable package we have to actually use the finder returned by walk_packages() to load the source files. That means the module is actually imported at that point and when _load_component() uses importlib it finds the module already loaded and everything works.

I wanted to keep this PR small but there are other features that could be added to it I bring up below.

Error handling

I assume if we fail to load a third party extension we should try to ignore it and continue on. That currently isn't the case, even the _on_walk_error() method that I am re-using raises an error which is never handled. I am not sure how to best proceed though, just log an error?

• when os.mkdir() fails, because we have seen that before, ✓
• and when importing the extensions, because I assume we don't want third party extensions to crash the browser when their dependancies aren't met or something, ✓
• and when running the module init hooks for the same reason. ✓
• and when running the interceptors? There is no log set up there yet... and it is handled by utils.prevent_exceptions

Experimental feature warning thing ✓

Shall I just log an all caps thing? Or I could make it only be active if the --debug flag is passed. It'll be easily missed if you have --debug on though. I supposed I could also just not autocreate the extensions folder.

Future work

Support loading folders ✓

If I remove the if ispkg: continue check the finder line actually succeeds in loading the directory (if it has a __init__.py) but after we yield it drops back into the walk_packages implemention after its own yield where it calls __import__(name) which fails because it presumably tries to load the parent package which isn't real. It even fails with the current implementation if there is a package folder in the extensions folder ever though we aren't importing it.
It may be as simple as ignoring errors for packages, we could check in sys.modules if the package itself got imported right.

It was failing with "No module named 'extensions'" because I had set the import prefix to "extensions.". Once I changed it to something properly namespaced under a real module (qutebrowser.extensions.third_party.) it just works.

Having a non-existant module (like third_party) in the (fake) module path means you cannot import the extension modules (which I don't see a reason to support). You can still get them from sys.modules though (and importlib). Even making that a package in the source tree doesn't let you import them (it seems to look for child modules as attributes of their parents rather than just looking for the whole heirarchy in sys.modules). I think I'll just leave it out and not support importing third party modules. They are in _module_infos anyway. Actually you can still do from qutebrowser.extensions.third_party import whatever, cool.

Unloading/Re-loading modules

The config-source command has been useful for developing external things so far. Both when developing and when adding things developed in a separate basedir into my main browser instance when I have got something stable without having to restart the whole instance. Currently loaded modules are stored in the _module_infos global list. Since they are modules though they do have unique names so we could support reloading (we might have to delete them out of sys.modules too though). Reloading modules does have the obvious possible problem of the outgoing copy of those modules having done things like register listeners. Although the config change filter in the extension API does avoid this so if continue with that pattern it would be less of a problem. We could also add a hook.unload() decorator where the extension could clean up.
The alternative is to just load new extensions although I would prefer to be able to reload them if it doesn't turn out to be too fraught.

Put behind a config item (added 2024-02-06)

And surface that this is turned on in :version and :report.

Consider how we could make versioning a part of this (added 2024-02-06)

Ideally extensions would declare a version they were compatible with and if they were running against a too new qutebrowser version they would complain/fail/require extra work to enable.

---

This change is 

[jgkamat]

toofar writes:

### Error handling I assume if we fail to load a third party extension we should try to ignore it and continue on. That currently isn't the case, even the `_on_walk_error()` method that I am re-using raises an error which is never handled. I am not sure how to best proceed though, just log an error? * when `os.mkdir()` fails, because we have seen that before, * and when importing the extensions, because I assume we don't want third party extensions to crash the browser when there dependancies aren't met or something, * and when running the module init hooks for the same reason.

configfiles.py has the logic for handling errors in config.py, I'm guessing we should do something almost exactly the same so that normally, an error in an extension will just pop an error (and ideally keep loading the rest of the extensions)

### Experimental feature warning thing Shall I just log an all caps thing? Or I could make it only be active if the `--debug` flag is passed.

This should probably log always when it loads a module for now.

## Future work ### Support loading folders If I remove the `if ispkg: continue` check the finder line actually succeeds in loading the directory (if it has a `__init__.py`) but after we yield it drops back into the `walk_packages` implemention after its own yield where it calls `__import__(name)` which fails because it presumably tries to load the parent package which isn't real. It even fails with the current implementation if there is a package folder in the extensions folder ever though we aren't importing it. It may be as simple as ignoring errors for packages, we could check in sys.modules if the package itself got imported right.

I'm not sure how to handle this. I'm not very familiar with the package walker, but how does it handle alternate top level modules in packages? If two packages define a 'tests' module, what will happen? Unfortunately making the tests folder a module is needed to get pytest to run properly without args. I'm guessing right now, it only loads single files (that would be a bummer for larger extensions, so it would be nice to have it fixed).

### Unloading/Re-loading modules The `config-source` command has been useful for developing external things so far. Both when developing and when adding things developed in a separate basedir into my main browser instance when I have got something stable without having to restart the whole instance. Currently loaded modules are stored in the `_module_infos` global list. Since they are modules though they do have unique names so we could support reloading (we might have to delete them out of `sys.modules` too though). Reloading modules does have the obvious possible problem of the outgoing copy of those modules having done things like register listeners. Although the config change filter in the extension API does avoid this so if continue with that pattern it would be less of a problem. We could also add a `hook.unload()` decorator where the extension could clean up. The alternative is to just load new extensions although I would prefer to be able to reload them if it doesn't turn out to be too fraught.

Since we're using the python import system, couldn't we use importlib.reload to reload modules? Since the hooks work, people should be using hooks for all their initialization, and make their modules properly reloadable. This isn't flawless, for example, if someone registers a interceptor function, it won't be updated (I think?).

[ninewise]

Does this line do anything? If so, maybe document it, because it does not look like it does anything...

[toofar]

Yes it imports the module and saves it into sys.modules as a side effect. I will add a comment and remove the left hand side.

[toofar]

I'm guessing right now, it only loads single files

Yep. I think we should get bigger ones working too but I would rather do that as a separate PR. For jmatrix for now I put the import time init stuff into an init() method and added an extension file that just imported the jmatrix qutebrowser integration and had a @hook.init() decorated method that called jmatrix's one (because hook.init doesn't work outside of the top level extension modules).

couldn't we use importlib.reload

Ah, I was looking for that. It looks like it throws an error if the module's parent isn't in sys.modules, I may have to actually make the extensions folder look like a real package. I feel like that will make it more fragile with respect to user changes but if it gets us nice things it might be worth it.

how does it handle alternate top level modules in packages? If two packages define a 'tests' module, what will happen?

It uses fully specified dotted names. Currently prefixed by the prefix we pass. So extensions.foo.tests and extensions.bar.tests. If it actually imported whole directory structures, which it doesn't due to extensions not being a real package.

[The-Compiler]

FWIW reloading Python modules is a can of worms - that's definitely something I'd rather defer to some later stage of extension support.

[toofar]

Oh also, @The-Compiler, on #30 it says plugins should be able to

Register new commands (with capital letter, like vim)

Is the leading captial letter something you still want to mandate?

[The-Compiler]

Is the leading captial letter something you still want to mandate?

I'm not sure. As long as extensions aren't official I don't have to decide yet, though 😉

[toofar]

That's all the stuff I wanted to get done for this pass (mainly support loading directories). I added some exception handling to _load_component which will hide (but log) errors in the init hooks of internal components as well. The other hook in the API, for interceptors, is already covered by utils.prevent_exceptions(), I didn't want to use that all over the place to separate out errors in third party code and in our third party code loading code. Maybe we could make it a context manager too.

[The-Compiler]

Thanks! Looks good overall, just some small suggestions.

[The-Compiler]

any() takes a list of values and checks whether any of those is boolean-True. It happens to work here because any([]) == False and any(['foo', 'bar']) == any([True, True]) == True, but just

Suggested change

	if any(os.listdir(ext_dir)):
	if os.listdir(ext_dir):

(relying on the list to be either empty or not) is far less confusing 😉

[The-Compiler]

Suggested change

	# sys.modules readif for _load_component.
	# sys.modules ready for _load_component.

I think?

[The-Compiler]

Suggested change

	"Failed importing extension: {}".format(name[len(prefix):])
	"Exception while importing extension: {}".format(name[len(prefix):])

(or "Failed to import")

[The-Compiler]

Suggested change

	"Failed initializing extension: {}".format(mod.__file__)
	"Exception while initializing extension: {}".format(mod.__file__)

[The-Compiler]

This doesn't catch exceptions in the if option is None: part above - I guess the whole block should be in the try: instead?

[The-Compiler]

This seems a bit hacky for getting the name - why not add that information to ModuleInfo or so?

[toofar]

I made this extension to reload extensions to make hacking easier. Previously I was developing them in config.py because that supports reloading and then moving them to extensions. I also made one to reload autoconfig because it makes sharing config between basedirs easier. I tested what would happen if a naughty extension connects to a PyQt signal by connecting to config.instance.changed and printing the curried module ID and they just keep getting printed, even after a gc run, how suspicious, ah, that is just if the connectee isn't a bound method it seems. If they are on an object they stopped getting called.

from qutebrowser.api import cmdutils

@cmdutils.register()
def reload_extensions():
    """Attempt to reload any third party extensions."""
    mod_prefix = "qutebrowser.extensions.third_party"

    # registered commands, including their args
    from qutebrowser.misc import objects
    todel = [
        cmd.name for cmd in objects.commands.values()
        if cmd.handler.__module__.startswith(mod_prefix)
    ]
    for name in todel:
        del objects.commands[name]

    # interceptor functions, could probably del by value too
    from qutebrowser.extensions.interceptors import _interceptors
    todel = [
        idx for idx, func in enumerate(_interceptors)
        if func.__module__.startswith(mod_prefix)
    ]
    for idx in reversed(todel):
        del _interceptors[idx]

    # extension infos with hooks
    from qutebrowser.extensions.loader import _module_infos, load_extensions
    todel = [
        idx for idx, modinfo in enumerate(_module_infos)
        if modinfo.module.__name__.startswith(mod_prefix)
    ]
    for idx in reversed(todel):
        del _module_infos[idx]

    # remove from sys.modules to make sure walk_extensions reloads them
    # I didn't test importlib.reload(), it might not work because these aren't real modules.
    import sys
    todel = [name for name in sys.modules.keys() if name.startswith(mod_prefix)]
    for name in todel:
        del sys.modules[name]

    load_extensions()

[toofar]

TODO:

Deprecated since version 3.4: The recommended API for loading a module is exec_module() (and create_module()). Loaders should implement it instead of load_module(). The import machinery takes care of all the other responsibilities of load_module() when exec_module() is implemented.

[toofar]

I've pushed a commit that should work with python 3.12. Although I'm not convinced it's the right way to do dynamic imports like this. Mainly because I have to add stuff to sys.modules myself which feels wrong. Although there are other examples on the internet doing the same thing. See the commit message for more context and ideas.

I forgot to put it in the commit message but this was the stack trace before I was adding it to sys.modules myself:

  File "/home/user/projects/qutebrowser/qutebrowser/app.py", line 141, in init
    loader.load_extensions()
  File "/home/user/projects/qutebrowser/qutebrowser/extensions/loader.py", line 88, in load_extensions
    _load_component(info, skip_hooks=skip_hooks)
  File "/home/user/projects/qutebrowser/qutebrowser/extensions/loader.py", line 198, in _load_component
    info.spec.loader.exec_module(mod)
  File "<frozen importlib._bootstrap_external>", line 940, in exec_module
  File "<frozen importlib._bootstrap>", line 241, in _call_with_frames_removed
  File "/tmp/qute-basdir-extensions/data/extensions/jmatrix.py", line 6, in <module>
    @hook.init()
     ^^^^^^^^^^^
  File "/home/user/projects/qutebrowser/qutebrowser/api/hook.py", line 37, in __call__
    info = _add_module_info(func)
           ^^^^^^^^^^^^^^^^^^^^^^
  File "/home/user/projects/qutebrowser/qutebrowser/api/hook.py", line 18, in _add_module_info
    module = importlib.import_module(func.__module__)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/importlib/__init__.py", line 126, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<frozen importlib._bootstrap>", line 1204, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1176, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1126, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 241, in _call_with_frames_removed
  File "<frozen importlib._bootstrap>", line 1204, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1176, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1140, in _find_and_load_unlocked
ModuleNotFoundError: No module named 'qutebrowser.extensions.third_party'