#60504 closed defect (bug) (fixed)
Plugin dependencies: Account for mu-plugin as dependency
Reported by: | johnbillion | Owned by: | costdev |
---|---|---|---|
Milestone: | 6.5 | Priority: | normal |
Severity: | normal | Version: | 6.5 |
Component: | Upgrade/Install | Keywords: | needs-testing dev-feedback has-patch has-unit-tests |
Focuses: | Cc: |
Description
I believe there could be a situation where a plugin on a site gets automatically deactivated despite its dependencies being met by a mu-plugin, and subsequently that plugin cannot be reactivated.
I'm using "WooCommerce" and "WooCommerce Extension" as examples for clarity.
- A site is using WordPress 6.4 or earlier which does not contain the plugin dependencies feature
- The site is using WooCommerce as a mu-plugin
- The site is using WooCommerce Extension as a regular plugin
- WooCommerce Extension gets updated to the latest version which adds the "Requires Plugins" header, and its update goes ahead as expected
- At a later date the site gets updated to WordPress 6.5 which does include the plugin dependencies feature
- WooCommerce Extension now has an unmet dependency and gets automatically deactivated, despite WooCommerce being in place on the site as a mu-plugin
- The site is now missing a plugin that cannot be reactivated because its dependency exists as a mu-plugin
I've not had time to test this properly. There could be safeguards in place to prevent a site from getting to this point (eg by preventing the core update to 6.5), however the general problem of a mu-plugin fulfilling a dependency remains. If a user circumvents a safeguard by manually updating, they'll find themselves in the same situation where WooCommerce Extension gets deactivated and cannot be reactivated.
- What safeguards are in place for this?
- How would a user recover from it regardless? Can they?
- Are mu-plugins accounted for at all within plugin dependencies? Should there be a means of forcibly activating a plugin with unmet dependencies?
- Should plugins be automatically deactivated when their dependencies aren't met? It seems potentially destructive
Related: #60491
Partially related: #60457
Change History (25)
#2
@
10 months ago
- Keywords dev-feedback added
@johnbillion Thanks for opening this ticket!
Indeed, Must-Use plugins were not considered valid dependencies in the Plugin Dependencies feature (going back aways in the discussions). However, you raise a valid issue if a site already has a plugin in its Must-Use plugins that becomes a dependency of a plugin in the Plugins repository.
So, it's possible that said plugins could just be moved to the mu-plugins
folder along with the Must-Use dependency. However, Must-Use plugins may be installed by hosts/developers. Any other plugin can be installed by a site owner/developer/administrator, so that doesn't seem like a feasible approach.
So, currently, it looks like the following things would need to be done, at least:
Core patch
Move the initialization of Plugin Dependencies until after- Plugin Dependencies is no longer initialized in bootstrap.do_action( 'muplugins_loaded' );
so that the Must-Use plugin's "loader" file can tell Core what its filename is via thewp_plugin_dependencies_slug
hook.Ensure all calls to- Theget_mu_plugins()
andget_plugins()
update theplugin_data
option, rather than replace it (as is currently done inget_plugins()
.plugin_data
option no longer exists.- Add a new
$mustuse_plugins
property toWP_Plugin_Dependencies
. - In
WP_Plugin_Dependencies::get_plugins()
, storeget_mu_plugins()
in the new$mustuse_plugins
property, and ensure these are also merged intoself::$plugins
to avoid severalarray_merge( self::get_plugins(), self::$mustuse_plugins )
calls elsewhere. - In
WP_Plugin_Dependencies::sanitize_dependency_slugs()
, add(\.php)?
to the regex. - In
WP_Plugin_Dependencies::has_unmet_dependencies()
make theforeach()
's internal condition:<?php if ( ! isset( self::$mustuse_plugins[ $dependency ] ) && ( false === $dependency_filepath || is_plugin_inactive( $dependency_filepath ) ) ) { return true; }
- In
WP_Plugin_Dependencies::convert_to_slug()
, don't replace'.php'
with''
anymore. Instead, justtrim( $plugin_file, './' )
to remove potential./file.php
or../../../file.php
entries. - Test updates.
- Additional tests to cover Must-Use plugins.
Site owners/Hosts/Developers
- As Core has no way to know the name of the "loader" file used for the Must-Use plugin, add a filter to your loader file to let Core know the filename.
<?php add_filter( 'wp_plugin_dependencies_slug', function ( $slug ) { static $muplugins = array( 'woocommerce' => true, 'wordpress-seo' => true, 'akismet' => true, ); return isset( $muplugins[ $slug ] ) ? basename( __FILE__ ) : $slug; } );
I've done the above and done some manual testing locally, and this seems to work fine, though I can't say I've fully tested the approach in such a short period of time.
This is likely going to need some discussion, so pinging @afragen, @pbiron, @azaozz and @desrosj to get in on this too if they have the time.
@knutsp Can you add some detail about how you think the Text Domain will need to play a part here? Thanks!
#3
@
10 months ago
WordPress VIP loads several plugins as mu-plugins (notably Jetpack and Query Monitor), it could be a good testing platform.
As Core has no way to know the name of the "loader" file used for the Must-Use plugin, add a filter to your loader file to let Core know the filename
Yeah this is going to be a sticking point, I've worked on many sites where there's one wp-content/mu-plugins/loader.php
type file that loads all mu-plugins.
#4
follow-up:
↓ 5
@
10 months ago
@johnbillion a question. Does a plugin like WooCommerce, if loaded as an mu-plugin, show as an active plugin?
Perhaps anyone using this pattern should always use a loader plugin and the plugin would then normally be where it is, in wp-content/plugins
. If the plugin isn't in wp-content/plugins
how is it to be updated? Mu-plugins don't receive updates.
#5
in reply to:
↑ 4
@
10 months ago
Replying to afragen:
Does a plugin like WooCommerce, if loaded as an mu-plugin, show as an active plugin?
The "loader" that you need to place in the root of the wp-content/mu-plugins
directory does. It's listed under the "Must-Use" tab on the Plugins screen. It's not required to have a plugin header, but if it does then the info will appear there.
Mu-plugins are manually managed. They don't get checked for nor receive auto updates, and because the mu-plugin system in WordPress only looks at .php
files in the root of the wp-content/mu-plugins
directory it means that any plugin in a subdirectory needs a corresponding "loader" file which is arbitrary. Examples:
- https://github.com/Automattic/vip-go-mu-plugins/blob/develop/jetpack.php#L546-L589
- https://github.com/Automattic/vip-go-mu-plugins/blob/baa497d021588900a22ca9c7583de908c63d5fba/query-monitor.php#L70-L72
- https://github.com/humanmade/mu-plugins-loader/blob/92eba0ddd09449ecb884acb1301af01fb0e50122/loader.php#L16-L24
#6
@
10 months ago
@johnbillion Yeah this is going to be a sticking point, I've worked on many sites where there's one wp-content/mu-plugins/loader.php type file that loads all mu-plugins.
A filter callback could have several conditions/in_array()
/isset()
in the if
so that if any of the slugs for the MU plugins it loads are found, it passes basename( __FILE__ )
back.
I've updated the example above to include an isset()
.
#7
@
9 months ago
Note: With the changes needed for #60457, 1 and 2 of the Core patch in this comment will no longer be necessary.
I'll get a PR up for the Core patch suggested in that comment as soon as possible.
This ticket was mentioned in PR #6141 on WordPress/wordpress-develop by @costdev.
9 months ago
#8
- Keywords has-patch has-unit-tests added
Previously, Must-Use plugins were not taken into account as dependencies. This meant that dependents could only be installed if their dependencies were installed as "normal" plugins.
This adds support for detecting Must-Use plugins as dependencies.
Note:
Must-Use plugins are loaded by a "loader" file in wp-content/mu-plugins/
, such as woocommerce-loader.php
. WordPress Core does not know that this file is connected to the dependency plugin.
To tell WordPress Core that the loader file should be considered the dependency plugin's "main" file, a filter must be used in the loader file.
Example:
// Load the dependency plugin's file.
require_once __DIR__ . '/woocommerce/woocommerce.php';
add_filter(
'wp_plugin_dependencies_slug',
static function ( $slug ) {
if ( 'woocommerce' === $slug ) {
// Tell WordPress Core to consider this file to be the dependency's "main" plugin file.
$slug = basename( __FILE__ );
}
return $slug;
}
);
#10
follow-up:
↓ 12
@
9 months ago
For a new feature, I don't think requiring developers add code to declare the dependency is overly helpful. The onus should be on WordPress to handle these things rather than plugin developers.
Given I raised this issue on the plugin repo two years ago, it's a little frustrating to be dealing with it in a rush during the beta period.
#11
@
9 months ago
- Keywords 2nd-opinion added; needs-testing has-patch has-unit-tests removed
For others' reference, here's the issue that Peter raised on the Feature Plugin's repository in 2022: https://github.com/WordPress/wp-plugin-dependencies/issues/2
Noting Dion's comment in particular as indicating reasons why Must-Use plugins weren't considered valid dependencies during the feature's development.
Now that this has been raised in Trac, I think it's fine that we discuss the merits/needs of supporting Must-Use plugins again. Even if Must-Use plugins continue not to be supported, at least we'd have considered and reached that decision twice. Adding 2nd-opinion
.
Should there be consensus to support Must-Use plugins as dependencies, PR 6141 is one implementation that demonstrates how Must-Use plugin support could be added and may aid discussion by indicating possible limitations such as the loader file name having no connection to the actual plugin it loads, without the use of the wp_plugin_dependencies_slug
filter demonstrated in the PR's description.
Removing has-patch
, has-unit-tests
and needs-testing
so as not to indicate that Must-Use plugin support has been agreed.
#12
in reply to:
↑ 10
@
9 months ago
- Keywords needs-testing has-patch has-unit-tests added; 2nd-opinion removed
Replying to peterwilsoncc:
For a new feature, I don't think requiring developers add code to declare the dependency is overly helpful. The onus should be on WordPress to handle these things rather than plugin developers.
How do you suggest finding or defining the plugin slug of a directly loaded MU-plugin? Naming the PHP file is only a convention, not a rule (I use index.php as main). Only for those loaded by a loader file, one can assume the directory basename is the slug, as for normal plugins.
Installing and updating MU plugins is not supported by WP Admin UI. You need skills beyond a plain site admin. There will be reasons for chosing to load a publicly available, by UI installable, plugin as a must-use, but then not to for a dependent plugin.
Directly loading complex plugins with subdirectories is not really supported (dirname conflicts) so you need a loader file, and that is written by a programmer, or pretender.
Plugin developers may add a snippet in readme.
To require a loader file with an embedded filter to make it fill it's role as dependency for normal plugins, and when well documented, seems fine to me.
Future: WP comes with a default MU loader file, ready to edit down to a line (like wp-config.php), with filters built in for an array of plugins by [ slug => dirname/filename.php, ].
This ticket was mentioned in Slack in #core-upgrade-install by costdev. View the logs.
9 months ago
#15
@
9 months ago
It seems the issue isn't that some devs use mu-plugins, but that some devs add normal dot org plugins into the mu-plugins folder. This means they must add a loader to require
the main plugin file. This will load the plugin but the plugin will return a false value for is_plugin_active()
, will not properly show as installed, and makes updating the plugins a completely manual process.
This is non-standard behavior even though WordPress has the ability to "make it work". The problem as @johnbillion points out is that mu-plugins are managed manually. The primary issue is that once installed and active in the mu-plugins folder these plugins are no longer capable of receiving updates. This will cause an undue burden on the site admin who must now manually update the plugin and will likely just miss the update as they would receive no indication that an update is available. It makes integration as a plugin dependency difficult, and it may leave a security update missing.
As @knutsp suggested this could be handled by a default MU loader file. @costdev and I have created a MU Loader that only requires the plugin file name (eg. woocommerce/woocommerce.php
) to be entered as an array variable in the loader file. This MU Loader will forcibly load the designated plugin within the "normal plugin directory" and filters the active_plugins
option so it is detected as active.
This isn't a huge barrier to entry for devs as they must already create a loader file. Further, I believe this solves all issues that have been outlined such as returning a truthy value for is_plugin_active()
and showing as installed.
All that is required is the normal installation of the plugin and the addition of the plugin file as an array variable in the MU Loader file. In the case of WooCommerce, simply adding woocommerce/woocommerce.php
is sufficient to load WooCommerce as an mu-plugin. If WooCommerce is declared somewhere as a dependency this is also accounted for and WooCommerce also shows as an active plugin.
The mu-loader.php ensures that the plugin shows as both active and installed making use as a plugin dependency seamlessly and doesn't require any addition modifications to core to use plugins loaded as mu-plugins as dependencies. This can also handle multiple plugins installed as mu-plugins.
Using this type of mu-loader, mu-plugins can work as dependencies without needed to modify core.
This ticket was mentioned in Slack in #core-upgrade-install by afragen. View the logs.
9 months ago
This ticket was mentioned in Slack in #core by costdev. View the logs.
9 months ago
#18
@
9 months ago
At Human Made we have a package for doing the same thing, except it supports defining the list of mu-plugins either via a PHP constant or via a config entry in composer.json
for Composer based projects. Essentially the same thing, minus filtering the active_plugins
value, which I don't yet have an opinion about.
A default mu-plugin loader could be a good idea. Would it need to consider handling conditional logic that is present in, for example, the WordPress VIP mu-plugins? Conditionally defining the array of mu-plugins to load would mean centralising a lot of the logic that is currently separated into multiple mu-plugin loaders.
#19
follow-up:
↓ 20
@
9 months ago
[57658] removed the auto-deactivation of plugins with unmet dependencies.
This now means upon updating to WordPress 6.5 an active plugin that has a dependency that is fulfilled by a mu-plugin will remain active (in the same way an active plugin with a dependency that is not fulfilled will remain active).
It remains that an inactive plugin that has a dependency that is fulfilled by a mu-plugin will be prevented from being activated, as its dependency is considered unfulfilled. Bearing in mind that the introduction of a mu-plugin is a manual process I think this is less of a pressing concern, but it's my opinion that we do need a way of allowing a mu-plugin to be recognised as a dependency.
A default mu-loader could be a good idea but we'd need to decide where its configured list of mu-plugins comes from, it might have to come from another mu-plugin. I'm not a big fan of filtering the list of active_plugins
to lie about what's active as that could come back to bite, especially if other plugins are reading that option and acting upon it.
#20
in reply to:
↑ 19
@
9 months ago
Replying to johnbillion:
It remains that an inactive plugin that has a dependency that is fulfilled by a mu-plugin will be prevented from being activated, as its dependency is considered unfulfilled. Bearing in mind that the introduction of a mu-plugin is a manual process I think this is less of a pressing concern, but it's my opinion that we do need a way of allowing a mu-plugin to be recognised as a dependency.
Given this, I'm inclined to say that we can consider this an edge case acceptable for 6.5. It still needs to be addressed, but because of the already manual nature of mu-plugins
, I think it's a reasonable nuance to leave unaddressed for now.
#21
@
9 months ago
- Milestone changed from 6.5 to 6.6
- Severity changed from critical to normal
Reducing the severity to normal and tentatively punting to 6.6.
As this is a defect (bug)
ticket related to the 6.5 cycle, it can be pulled back into the 6.5 milestone during 6.5 RC if we want to action it during this cycle.
#22
@
9 months ago
- Milestone changed from 6.6 to 6.5
Perhaps we should close this ticket as fixed for 6.5 and open a new one for 6.6. The main concern (auto deactivation of a plugin where its dependencies are met by a mu-plugin) has been fixed.
Tricky, since dependency is annotated by the plugin slug (dir), but MU plugins don't necessary have such, only the name of the main file as unique id (among MU plugins).
Some thinkable solutions, as a start: