All plugins using sub-directives implemented with Dispatcher broken

[erikw]

First, thanks for this great dotfiles manager! I recently migrated from my own >10y old semi-homebrewn solution with many clumsy shell scripts. Dotbot made it all much simpler for me! However there's currently a bug preventing some plugins from working...

As first reported at https://github.com/wonderbeyond/dotbot-if/issues/1#issuecomment-1532139808, it seems like all plugins that allow using arbitrary sub-expressions broke as of refactorings introduced in https://github.com/anishathalye/dotbot/commit/b5499c7dc5b300462f3ce1c2a3d9b7a76233b39b.

Let's take for example the dotbot-if plugin that allows doing things like

- if:
    cond: 'true'
    met:
    - shell:
      - echo The condition was true

The met: allows any dotbot directive to be run, hereshell was used as the examples.

However as of the changes in https://github.com/anishathalye/dotbot/commit/b5499c7dc5b300462f3ce1c2a3d9b7a76233b39b where the Dispatcher was modified to no longer load all plugins by itself (used to be done with the helper _load_plugins), plugins relying on using the Dispatcher to parse and handle sub-directives does no longer work as other plugins are not loaded anymore. The plugins array is now an argument that will be passed in. The problem is that the plugins using the Dispatcher does not know all the other plugins to be loaded as this was done before in an outer context not available here in the plugin. The example above will result in

$ ./install -v
Action shell not handled

==> Some tasks were not executed successfully

as reported in https://github.com/wonderbeyond/dotbot-if/issues/1. Please see this issue for details on how the mentioned refactor broke plugins using the Dispatcher.

Now we could say that the problem is not dotbot but the plugins, and maybe it is so that all plugins should update. But to what, what method should plugins use to parse sub-directive now? However as this did work before and no new major breaking version of dotbot was released, I would consider the problem being in dotbot and not the plugins right now.

I could be very wrong in my analysis, and maybe there's already a way in which plugins like dotbot-if could execute sub-directives with the Dispatcher or another class. Let's figure this out together, as plugins like if is kind of essential for dotbot!

[kurtmckee]

Cross-posted to wonderbeyond/dotbot-if#1

@erikw Thanks for pinging me on this, and sorry for the late reply!

I made the changes in the mentioned commit so that it was possible to test dotbot in a single pytest run. Previously, the unit tests were implemented as bash scripts running in a virtual environment managed by vagrant. This design had the benefit of separating dotbot tests from the user's actual filesystem, but precluded any possibility of testing dotbot on platforms other than Linux.

I modified the plugin loading code as a part of a significant effort to move dotbot's test suite to pytest. This new plugin-loading design had the benefit that plugins wouldn't remain permanently loaded between dotbot runs, which allowed the test suite to better exercise dotbot's failure/fallback codepaths when a directive wasn't implemented by a plugin. However, it appears to have broken some functionality that plugins were relying on. 😞

I'll take some time to refamiliarize myself with the plugin loading and the code paths these plugins were relying on. My hope is that there's a simple way to add the lost functionality back in and still maintain isolation of plugin loading between tests (or, possibly, force plugins to unload during test teardown perhaps).

[erikw]

Yeah I looked at the refactoring a bit and I think it's all good reasons. Just unfortunate side-effect. I'm sure we can find a way to have the improved test setup, while still empowering plugins to do cool stuff!

Thanks for looking in to it. I currently have commented out some if directives in my dotbot config, but I look forward to be able to use them later!

[kurtmckee]

I think that #343 will fix this issue.