search

inpsyde / vip-composer-plugin

A Composer plugin to ease deployment to wordpress.com VIP servers alongside Composer-based local development.
MIT License
11 stars 0 forks source link

[Bug]: Autoload path of packages with type "wordpress-muplugin" is incorrect #8

Closed remyvv closed 1 year ago

remyvv commented 2 years ago

Description of the bug

When using this plugin on a composer-project with a dependency of type "wordpress-muplugin" the classmap autoloader created by this plugin does not contain the correct paths to autoloaded files within the plugin folder.

Reproduction instructions

  1. Create WordPress VIP project based on https://github.com/inpsyde/vip-go-website-template
  2. Install dependency with autoloaded classes as wordpress-muplugin
  3. Run composer vip --local --git and inspect /vip/.vipgit%hash%/client-mu-plugins/vendor/vip-autoload/autoload_classmap.php
  4. Notice that the path for the mu-plugin is $baseDir . '/vip/client-mu-plugins/%plugin-name%, which is non-existant.

Expected behavior

I expect installing mu-plugins to work with the production autoload as composer handles it locally with it's own autoload.

Environment info

No response

Relevant log output

No response

Additional context

My proposal is to extend the replacement regex here with the client-mu-plugins directory.
This way the autoloader contains the correct reference again.

Code of Conduct

remyvv commented 1 year ago

Example repository

For better illustration of the bug i have created an example repository: https://github.com/remyvv/vip-composer-bug8
This repository contains a project created from the vip-go-website-template, and it installs the WP-Stash as an example of a package with type "wordpress-muplugin".

Hopefully this helps in reducing time for reproduction of this bug.
If it helps i can also provide a PR with my proposed change!

Reproduction instructions with the example repo

  1. Clone repository
  2. Run composer install
  3. Run composer vip --local --git and inspect /vip/.vipgit%hash%/client-mu-plugins/vendor/vip-autoload/autoload_classmap.php
  4. Search for Inpsyde\\WpStash\\WpStash inside the autoload_classmap.php
  5. See the non-existant path: $baseDir . '/vip/client-mu-plugins/wp-stash/src/WpStash.php',
gmazzap commented 1 year ago

@remyvv The problem is that in WordPress MU plugins are single files.

This library sticks with WordPress standards because VIP is a closed environment that don't give us a lot of configuration possibilities, we don't even have access to wp-config or to WordPress core itself.

Not to mention that VIP has its own weirdness, and MU plugins are loaded from a specific "client-mu-plugins" folder, and not from the usual "mu-plugins" folder.

For these reasons we try to use as much as "wordpressy" we can to avoid issues.

When we need something that is multi-file, and is required at network level, we assume to create a library, possibly using WP App Container.

The problem with WP Stash is that it is declaring itself as a MU plugin, which is against WP standards that define MU plugins as single files.

So what you're reporting is not a bug, it is a request to support non-standard MU plugins composed by several files and requiring autoload.

We could make that work, but to be honest, I don't have the time to work on this now.

I'll be glad to review a PR.

remyvv commented 1 year ago

@gmazzap I understand wanting to stick as close to WordPress as possible because of the closed environment at VIP.
However i want to point out that loading mu-plugins consisting of several files is not actually discouraged by VIP or WordPress for that matter, since the documentation of both actually explains how to do this albeit with some caveats:

Plugins that are added to /client-mu-plugins with their own subdirectory must be loaded programmatically with a plugin-loader.php file, placed within the /client-mu-plugins directory.

Only custom plugins with code that needs to be auto-loaded, or code that needs to run earlier in the WordPress load process, should be added to the /client-mu-plugins directory. - https://docs.wpvip.com/technical-references/vip-codebase/client-mu-plugins-directory/

WordPress only looks for PHP files right inside the mu-plugins directory, and (unlike for normal plugins) not for files in subdirectories. You may want to create a proxy PHP loader file inside the mu-plugins directory: - https://wordpress.org/support/article/must-use-plugins/#caveats

Anyway, i have created a PR with my proposed change here: https://github.com/inpsyde/vip-composer-plugin/pull/9

gmazzap commented 1 year ago

@remyvv the point is being "discouraged" the point is that they are not "supported" out of the box.

Meaning that custom support has to be provided. Custom support means you're responsible of loading the file you need. If the MU plugin is self-contained, that is, you load one file and everything else works (including loading multiple files if needed), that is a no brainer However, when you have an autoload then the MU plugin is not self-contained anymore.

Cosnider that we use a MU plugin to require the autoload file, because that's the earliest entry point on VIP.

An autoloaded MU plugin will rely on the _autoloader _MU plugin to be loaded first, and we know by experience that, so far, that is true because of how we name the autoload file, but we have no warranty that'll stay true.

In other words, that might work, but it is fragile,.

That said... What is the purpose of WP Stash on VIP?

VIP has its own object cache implementation based on Memcache, and AFAIK you have no way to access the memcache server configuration and feeds it to WP Stash config.

Moreover, on VIP, filesystem writes are only available to temp directory, and do not survive the current request.

And considering you will not be able to install any software, the only WP Stash driver you will be able to use successfully is "Ephermal", which provides zero benefit in terms of performance.

Actually, considering VIP object cache uses memcache, if you use WP Stash "Ephermal"driver... you'll get a performance penalty.

Am I missing anything?

remyvv commented 1 year ago

@gmazzap I agree that is it fragile, and generally fragility should be avoided.

Don't worry, we are not trying to use WP Stash on VIP hosting. 🙂 WP Stash was just used as an example of a publicly available multifile wordpress-muplugin that also autoloads files through composer.
I wanted to provide an example repository to make it easy to reproduce the bug, and I didn't want to reference private repositories in a public example repository.