Late composer installation for plugin libraries

[SOF3]

Motivation

This issue serves as a milder alternative to #2755, which radically changed plugin installation methods.

Compared to #2755, this issue does not aim to resolve issues with NIH class loading and built-in plugin manager (#2504). The sole purpose is to allow automatic installation of library dependencies (not plugin dependencies) to prevent problems of (unwarned) conflicting library versions while making it easier to use libraries in plugins.

Mechanism

This proposal introduces a new optional file composer.json (or composer.lock, depending on the scenario) in the standard plugin structure. The only attribute used in the file would be the require atrribute. (All other attributes would be ignored) The server shall merge all the required packages into a (cachable) temp folder known as ad-hoc vendor and install composer dependencies inside.

The responsibility of the server includes the merging of such dependencies, and the selection/detection of incompatibility of shared libraries.

This shall enable single-instance libraries, which currently requires a plugin format known as "API plugins".

Concerns

Conflict with PMMP dependencies

There are two options:

1. PMMP also loads dependencies via this system. This implies that PMMP must not use any composer packages without loading all plugins first.
2. Append the PMMP dependencies into the ad-hoc vendor such that plugins cannot require these dependencies indirectly. This results in double loading of the same libraries. I am not sure if composer API and PHP autoloading API allow us to overcome potential problems here.

Plugins must be loaded on startup

Do we really need to allow loading plugins after the server starts? I even suggest removing the /reload command. Is it so hard to restart the server?

Packaged performance

To emulate the performance boost currently offered by packaging plugins in phar files, it is possible to lazily package the ad-hoc vendor directory into a phar too.

Editing plugins

Plugins can still be edited, but libraries cannot be edited without the danger of being overwritten. Nonetheless, it is not our responsibility to ensure that plugins (much less only libraries) can be edited. Observing that editing libraries is not prevalent right now, and it is usually unnecessary to edit libraries (because they are usually better-coded than plugins and do not have direct impact on users), this concern is dismissed. Users may still fork the library and produce their own version if they are professional enough; but they won't notice they want to edit a library otherwise anyway.

Libraries must be open-source

This is not true. Composer supports local/VCS-based library requirement.

Arbitrary updates

Plugin approval systems like Poggit will run into problems, because libraries are installed from a source that only its author has access to change.

It is possible for such approval platforms to modify the composer.json such that the library version is locked at a specific version, but this would result in problems when merging multiple plugins using the same library.

Another issue is https://www.google.com/amp/s/www.theregister.co.uk/AMP/2016/03/23/npm_left_pad_chaos/ But let's go the dangerous way of thinking: since npm is doing this for years, it should be ok.

Existing deployment workflow

This is a pure feature addition. Existing workflows do not have to be broken.

Debugging libraries

Consider adding extra include paths as a config in pocketmine.yml or alike.

[ghost]

@SOF3 This sounds much more reasonable than #2755

[lukeeey]

Removing /reload is not useful in testing scenarios. All i have to do to change chat format or adjust some coordinates for something is edit the code then reload and i can test instantly, whereas a full restart means i must leave the game and rejoin.

[dktapps]

@lukeeey pls RTFM and correct that misconception.

[lukeeey]

@dktapps when using DevTools folder plugin loader it seems to. I get the onEnable messages in the console and my code changes apply

[dktapps]

it seems to

here's the problem.

It may appear that some of your code changes are indeed reloaded when you run /reload, but this is not what is actually happening. What may happen is that some classes which were not loaded prior to the reload may get autoloaded, since class autoloading happens on-the-fly. You'll notice that your code will never be "reloaded" twice (unless you change the namespace or something of that nature).

[3zeroed]

Can I reload the plugin code without shutting down the server? This problem has plagued me for a long time.

[dktapps]

You can't. Please stop taking the thread off topic.

[MHIGists]

Why do I think it should be possible for DevTools to reload a folder plugin entirely?

[SOF3]

No, not at all, otherwise it might cause other problems such as problems with statics that you might not 100% be aware of. Live code swap is always dangerous, and even software much more professional than PocketMine like Android is still having trouble implementing proper code swap that does not lead to crashes that otherwise wouldn't happen. Server startup is usually fast enough. If it's slow for you, make sure you're using a phar distribution of PocketMine. If you are referring to slow joining time of the client, you should have a look at plugins like Specter. you might also want to write unit tests for your plugin before testing it with PocketMine code. That is, assuming your plugin is large enough to require unit tests.

[3zeroed]

Because I often add some new features or urgently fix some bugs when the server is running normally. So if you don't shut down and restart the server, It's a nice feature to be able to reload the plugin code. I don't want to tell the players that the server has to be restarted. Rejoin the server later.

[SOF3]

It has NEVER been and WILL never be possible to reload plugin code without restarting the server. It is only possible to load absolutely new code, so this has nothing to do with urgent fixed.

This issue is discussing about the removal of the ability to load absolutely new plugins, which rarely happens. (If you are updating a plugin, you must restart anyway) More servers have most of their plugins installed on the first few days, and this late enable ability is rarely useful after that. Since you won't have lots of players on the first few days, restarting the server would not be a big deal.

[dktapps]

yOu cAn'T rElOaD pLugIn sOurCe coDe aT RuNtImE

how many times...

[ghost]

yOu cAn'T rElOaD pLugIn sOurCe coDe aT RuNtImE

how many times...

It's pretty misleading though. /reload could mean anything.

• Reload server
• Reload plugin source
• Reload vending machine

[dktapps]

yes, this is exactly why it's been removed in 4.0.

[SOF3]

The proposal is still facing difficulties with plugin loaders. Suppose a plugin called D registers a new plugin loader that is going to load T. D cannot be loaded without loading libraries required for D. On the other hand, plugin T might also require libraries. However, they cannot be loaded because the composer stage has already been initialized in order to trigger T.

This is a very problematic issue. Fortunately, we don't have many plugins that load other plugins on startup (much less after startup - see #2825). The most popular example is DevTools (others like ZipPluginLoader and AllAPILoader are nonstandard and pretty much hated by some conservative members of the community). So, why don't we disallow late composer installation only for these specific plugins?

Hence, the solution is like this: Add a new plugin load order PluginLoadOrder::NO_COMPOSER (or NO_LIBS, or SEARCHING_PLUGINS). Plugins that provide plugin loaders must use this load order, and they must not request libraries. Plugins loaded by these plugin loaders may still use PluginLoadOrder::NO_COMPOSER.

The pseudocode becomes like this:

plugins := []
for each plugin in "plugins/*.phar"
    push plugin to plugins[plugin.load_order]
while plugins[NO_COMPOSER] is not empty
    plugin := plugins[NO_COMPOSER].take
    load plugin // this may recursively append to plugins[NO_COMPOSER]

merge_libs := []
for each plugin in plugins[STARTUP] + plugins[POSTWORLD]
    append plugin.libs to merge_libs

composer_update merge_libs

for each plugin in plugins[STARTUP]
    load plugin
...
for each plugin in plugins[POSTWORLD]
    load plugin

[JaxkDev]

What if a plugin A requires the lib Z specifically v1.0 however plugin C requires lib Z specifically v2.0, may have read this wrong but looks like you just merge all dependencies assuming they’re all unique or depend on the same lib but compatible version constraints ?

[SOF3]

Then the server should fail to start because plugins are incompatible.

[eternalharvest]

Any update on this?

I agree with @SOF3 that server should fail to start if plugins dependencies conflict. Though I'm not sure about if composer provides functionality merging dependency, it seems the plugin 'composer-merge-plugin' exists.

Please refer to the document below. https://github.com/wikimedia/composer-merge-plugin

We are currently using php library phpseclib and loading it with our own composer loader. But it is not supported by PocketMine-MP itself and Poggit CI. Since currently there are no another plugin using own composer loader except PocketMine itself and phpseclib dependency is simple enough, we have no problem about dependency conflict. But we have some issue about building with DevTools.phar and we can't use Poggit as CI/CD. So, I'm glad if this feature is supported officially.

I also consider about using submodule of git, but it is difficult because git submodule can't refer subdirectory of library repo. And I think git submodule is dangerous because it is not managed by composer. So, it may cause namespace conflict problem because the third-party library is not following namespace prefix of PocketMine plugin. And composer dependency management is powerfull, and it may help install big library which has recursive dependencies.

[SOF3]

Not recursive libraries; that's be impossible. Just re l transitive.

Anyway, point taken, but it turns out that it's not trivial to implement this in the core because resolving dependency versions isn't straightforward when there are multiple plugins. Also consider performance concerns of composer on Windows.

[dktapps]

But we have some issue about building with DevTools.phar

Just add vendor to your --make argument. (and if you're actually booting a server to build your plugin, please stop ...)

and we can't use Poggit as CI/CD.

Whether or not Poggit supports installing composer dependencies is a separate problem from PM itself resolving your dependencies. Don't confuse the issue.