search

haraka / Haraka

A fast, highly extensible, and event driven SMTP server
https://haraka.github.io
MIT License
5.09k stars 662 forks source link

backwards compat while moving plugins to npm packages #1658

Closed msimerson closed 8 years ago

msimerson commented 8 years ago

I'm in the process (#1657) of moving the watch plugin to a npm packaged plugin. An issue I'm running into is that I need to leave a "watch/index.js" or "watch.js" behind, so that if someone has "watch" in their config/plugins file and doesn't run 'npm install' after upgrading Haraka, their server doesn't refuse to start because the plugin is missing.

If I do leave something behind, that something is going to get loaded first, because of the ordering of the entries in plugins.js. The solution I'm considering is:

  1. change the plugin path ordering so haraka-plugin-$name is checked first
  2. leave behind a stub plugin, that does nothing other than emit a warning that:
    • the plugin they are using has been moved to npm, and they should run 'npm install' in their Haraka directory.

As soon as they've run npm install, the stub plugin with the warning should never again be run.

Thoughts?

baudehlo commented 8 years ago

change the plugin path ordering so haraka-plugin-$name is checked first

Wouldn't that prevent you being able to load your own watch plugin in $local/plugins/watch.js?

That's one of the nice features of the local folder - easy overriding.

msimerson commented 8 years ago

Wouldn't that prevent you being able to load your own watch plugin in $local/plugins/watch.js?

Oops, my bad on use of the word first. What I meant by first was, "before $dir/plugins/$name/package.json", or in the second search position, which is what I've done in #1657 on this line and suggested in #1649.

Does moving that search path up prevent the ability to load from $local/plugins/watch.js? Not at all. Even if we did set haraka-plugin-$name pattern as the first search path (opinion: that makes no sense until we have more plugins packaged as npm modules than plugin/$name.js plugins), the search ordering is still "all 4 paths in $local" and then "all 4 paths in $installed".

baudehlo commented 8 years ago

OK. This whole plugin loading system proved incredibly fragile to write so I'm very hesitant to change it, but I'm OK with that change as far as you describe it, I think.

On Tue, Oct 11, 2016 at 1:12 PM, Matt Simerson notifications@github.com wrote:

Wouldn't that prevent you being able to load your own watch plugin in $local/plugins/watch.js?

Oops, my bad on use of the word first. What I meant by first was, "before $dir/plugins/$name/package.json", or in the second search position, which is what I've done in #1657 https://github.com/haraka/Haraka/pull/1657 on this line https://github.com/haraka/Haraka/pull/1657/files#diff-779ba29ca31786b5ba28f320e69586b2R55 and suggested in #1649 https://github.com/haraka/Haraka/issues/1649.

Does moving that search path up prevent the ability to load from $local/plugins/watch.js? Not at all. Even if we did set haraka-plugin-$name pattern as the first search path (opinion: that makes no sense until we have more plugins packaged as npm modules than plugin/$name.js plugins), the search ordering is still "all 4 paths in $local" and then "all 4 paths in $installed".

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/haraka/Haraka/issues/1658#issuecomment-252981803, or mute the thread https://github.com/notifications/unsubscribe-auth/AAobYyHErKBMf-P-lLgnUGTjFmIAWEjrks5qy8OTgaJpZM4KTNz8 .

msimerson commented 8 years ago

proved incredibly fragile to write

Understood. I think part of the cloudiness and difficulty was supporting 3 styles of packaged plugins. The fourth I've recently added (haraka-plugin-$name without the prefix) is really just an incremental improvement on # 3). The primary reason for supporting # 2 ($path/$name/package.json) was the watch plugin, and I now think that supporting it was nice (thanks) but also a mistake.

I think you made it half-way to that conclusion when you dropped a package.json file into the watch directory. Instead, we should have packaged up watch and tossed it onto npm. Then plugins can be loaded in exactly two ways: from files in plugins/$name.js and from npm packages installed in the usual way. In $local or $installed dirs. The number of permutations shrinks and plugin loading is a bit more deterministic.