Action shell not handled

[erikw]

Hey,

using the latest version of this plugin and a doubt installation configuration containing only the partially modified example from this README:

- if:
    cond: 'true'
    met:
    - shell:
      - echo Found Python 3.

then running ./install gives me

$ ./install -v
Action shell not handled

==> Some tasks were not executed successfully

What could be wrong here? I tried other actions than shell but I get the same output.

[erikw]

Using pdb I found that we end up here
https://github.com/anishathalye/dotbot/blob/328bcb32590e5057b09bd27a40bc2fb21385fbf3/dotbot/dispatcher.py#L68-L70

as self._plugins is an empty array in this case :O

Edit.
I noticed that after evaluating the condition in the if: directive and we will execute the met or unmet directive, the function _run_internal()

dotbot-if/if.py

Lines 41 to 46 in 2b4dc56

	dispatcher = Dispatcher(
	self._context.base_directory(),
	only=self._context.options().only,
	skip=self._context.options().skip,
	options=self._context.options(),
	)

will initialize without passing in any plugins. In Dotbot itself, the plugins are collected and passed down to the dispatcher:

https://github.com/anishathalye/dotbot/blob/328bcb32590e5057b09bd27a40bc2fb21385fbf3/dotbot/cli.py#L149-L155
(last line).

So this makes sense: no plugins was passed to Dispatcher() , thus plugins is empty and shell can't be executed in the condition.

I would assume this did work before though. Maybe something in the dotbot Dispatcher code changes recently so that plugins now must be passed explicitly, and before it was automagical?

Edit2.
Yes look at this, with git-log I found that this seems to be the case. The Dispatcher used to load plugins by itself in the helper _load_plugins that was removed here:

anishathalye/dotbot@b5499c7

[erikw]

Just as a PoC, this minimal diff in this plugin enables the Dotbot built-in plugins:

  --- a/if.py
  +++ b/if.py
  @@ -2,6 +2,8 @@ import subprocess

   import dotbot
   from dotbot.dispatcher import Dispatcher
  +from dotbot.plugins import Clean, Create, Link, Shell
  +


   class If(dotbot.Plugin):
  @@ -38,10 +40,12 @@ class If(dotbot.Plugin):


       def _run_internal(self, data):
  +        plugins = [Clean, Create, Link, Shell]
           dispatcher = Dispatcher(
               self._context.base_directory(),
               only=self._context.options().only,
               skip=self._context.options().skip,
               options=self._context.options(),
  +            plugins=plugins
           )
           return dispatcher.dispatch(data)

Now it's possible to use shell inside the met or unmet again!

Of course this is not a full solution, as it does not load any plugins from -p or --plugin-dir specified on the command line. The same logic that happens at
https://github.com/anishathalye/dotbot/blob/328bcb32590e5057b09bd27a40bc2fb21385fbf3/dotbot/cli.py#L121-L133
should happen here too.

Maybe that logic could be extracted to a helper in the Dotbot project, or maybe there's a way for this plugin to access the original dispatcher and re-use or fetch the plugin list from there?

@kurtmckee (tagging as of anishathalye/dotbot@b5499c7) would you know if there's an easy way for a Dotbot plugin to create a Dispatcher with the same plugins as the original Dispatcher created in Dotbot's cli.py:main()? :) Or maybe there's a better way for a plugin to execute sub-actions than creating their own Dispatcher instance 🤔

[kurtmckee]

Cross-posted to anishathalye/dotbot#339

@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).

[kurtmckee]

anishathalye/dotbot#343 should resolve this, and should make this dispatch pattern easier for plugins in the future by exposing a Dispatcher instance with the correct configuration so plugins can easily re-dispatch to other plugins.

[erikw]

@kurtmckee many thanks! I can confirm that with anishathalye/dotbot#343 this plugin dotbot-if is working with the sample config in the issue description here! 🚀

[johnlettman]

Hello @erikw and everyone,

I have opened a pull request on a temporary fix borrowed from ifplatform at PR #2. The approach in ifplatform is fairly robust by using utility functions in dotbot itself to load the module instances. It certainly isn't the most efficient method, as obtaining the already loaded list from dotbot would likely be preferable, but it works until upstream PRs are merged.

Cheers!