Implement WP admin UI for plugin dependencies

[aristath]

This patch adds the ability to define plugin dependencies using a Requires Plugins header.

<?php
/**
 * Plugin Name: Sample
 * Requires Plugins: woocommerce, gutenberg
 */

When we try to activate a plugin with the above, the user sees this:

• There is a new notice on the top of the screen, urging users to install and activate the dependencies
• The plugin with unmet dependencies doesn't actually get activated, but instead gets added to a queue (database option) and will only be activated once the dependencies are met.
• The plugin with unmet dependencies gets a notification added to its row, explaining why the plugin is not activated.
• The "activate" link is replaced with a "Cancel activation request" link. If users click on that link, the plugin is removed from the queue and its dependencies notifications are dismissed.

When a dependency is installed, it gets a notice that the plugin is a dependency, and also the "deactivate" link gets removed from the UI:

Trac ticket: https://core.trac.wordpress.org/ticket/22316

---

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

[Chrico]

You should use ComPoser and the composer.json for that.

---

Edit: To give a bit more input: Yes, ComPoser does only allow to resolve dependencies - which means "they are installed", but what it not does is ensuring that those dependencies (Plugins, Themes, ..) are actually activated.
Since composer.json is already there and a well-known (and by WordPress not accepted) standard in the PHP community, you could/should simply extend this file by using the extra-field to define whatever you want.

So as first step you can define in require / require-dev what needs to be installed in which version, and in a second step you can build on top some custom WordPress-specific logic via extra.

I would not "restrict" this just to "Plugin A in version X is installed/activated". Common things which are needed are:

1. Which Plugin needs to be activated
2. Which Theme needs to be activated
3. Specific values (constants, env vars, ...) or options are set - like "Multisite"/"Singlesite"
4. ... ?

This could lead to a structure like:

{
   /* snip */
  "extra": {
      "wordpress": {
          "requires": {
              "activated:" [ "list-of-plugins-activated", "or-even-a-theme" ]
              "env": { /* not sure about the naming here right now */
                  "MULTISITE": true
              }
          }
      }

  }
}
`` 

[aristath]

composer.json is a different beast. Composer handles packages, and it does that brilliantly. But plugin-dependencies are not, and should not, be treated the same as packages.
If I need plugin dependencies on my plugin but don't use composer, then a composer.json file is meaningless. It's a file specific to an implementation that my plugin doesn't use, and therefore using that name for plugin dependencies is probably not the best choice for all developers.
A dedicated dependencies.json file is unambiguous, and allows developers to use simpler syntax, without nesting things inside extra. It's only an extra if I'm using composer, otherwise there's nothing extra about it.
Additionally, composer is one of many package managers, and package managers come and go. It would be unwise to tie our naming convention for something that has nothing to do with packages, to a specific package-managing tool that may or may not exist in the future when some better tool comes along.

[Chrico]

Composer handles packages, and it does that brilliantly. But plugin-dependencies are not, and should not, be treated the same as packages.

Sry, that's not true. Composer handles dependencies, not (only) packages. You can even define PHP-versions or extensions. Why should "plugin-dependencies" (or Theme, MU-Plugin, php-extension, php-version, ...) not be treated as dependencies to your website or even to your Plugin?

---

If I need plugin dependencies on my plugin but don't use composer, then a composer.json file is meaningless.

A Composer file is never meaningless. It provides a lot of information which could be useful in many cases.

---

A dedicated dependencies.json file is unambiguous, and allows developers to use simpler syntax, without nesting things inside extra.

True, it's simpler, but it also rebuilds what is already there and does not cover everything i posted. If that extra-field is too much, you could even add wordpress to the root-level of the composer.json:

{
    "name": "vendor/my-plugin",
    "description: "....",
    /* snip */
    "wordpress": {
        ...
    }
}

---

Additionally, composer is one of many package managers, and package managers come and go. It would be unwise to tie our naming convention for something that has nothing to do with packages, ...

You know more "PHP package managers"? 🤔 I mean.....yeah, there was Pear https://pear.php.net/ some long long long time ago and https://www.phpclasses.org/ ...but i guess (hope) you're not talking about those..?

I kind of understand what you mean with "tie it to Composer" (i mean, everything is tied to something, right? 🙃 ), but in the end it's just a JSON file which is also used by Composer. It's kind of open to extend it, while at the same time, using something which is well-known and accepted all over the world. Yes, we still would build on top of an excisting eco-system - which can be used, but hasn't to be used. The main benefit is, that it already provides more then enough information to work with. The wheel does not need to be reinvented :)

[gmazzap]

It's all about the version

The biggest problem Composer solves is not what to install, but the version constraint.

This implementation "simplifies" that problem by making the version the "minimum required version".

That means that if a plugin requires 11.0 and the other 12.0 this system will only accept 12.0+.

But if 12.0 introduced breaking changes, the plugin that is requiring 11.0 will probably break.

This means that this PR relies on the fact that everyone writes endlessly backward compatible code, like core (in theory) does, and that is simply not the reality (nor it's desirable IMO because it only brings technical debt, as core has proved in all these years).

That said, the PR only checks if the plugins are active, do not retrieve or download them, or anything like that. That means that this PR could survive side-by-side with Composer (and for what it's worth, if merged, that's how I'll use it) as they solve two different problems.

That said, I think that the version constraint should be removed: it is not capable of guarantee anything, and will probably block a future implementation that could handle it better, considering that if this PR will be merged WP will want to maintain compatibility with the JSON schema it requires.

Industry standard?

If it is really wanted to have a version constraint, why not using a constraint definition that all the package managers, of all languages, are using? That is, instead of "version": "11.0", it could be "version": ">=11.0".

It does not mean that WordPress at this point will have to support all the possible version constraints, it would only mean that door is kept open.

I still think it would be better to remove version support at all until a better solution to the version problem is found, but at least using an industry standard for version requirements would be an improvement.

What about updates?

Have you considered what happens in the case of dependencies' version change upon update?

Scenario:

1. Plugin "Foo", is installed and activated in version "1.0" and it requires plugin "Bar" in version "1.0".
2. Plugin "Bar" in version "1.0" is installed and activated, so everthing is fine, and both stay active
3. Plugin "Foo" gets updated to "1.1" and in this new version it requires plugin "Bar" in version "2.0".

What happens? The requirements are not satisfied anymore. What WordPress will do?

If WordPress does nothing, besides introducing an inconsistent behavior, breakages will happen, because if the plugin "Foo" decided to require a newer version of "Bar" there must be a reason.

If WordPress would disable both "Foo" and "Bar" that could cause huge troubles to the website. Imagine one of those plugins is in reality WooCommerce, and that site is an ecommerce site: it would not commerce anymore.

Dependencies are hard. A complex problem does not become simple just because we approach it with a simple solution.

Removing the handling of versions from this PR we'll leave that problem aside. It will not be solved, but this PR does not solve it either, but still adds complexity (and it will create troubles when a more fitting solution would be attempted).

Schema is too simple

Moreover, again for the ability to evolve in the future, I think the configuration schema right now is optimistically too simple: what will happen if the same thing would be needed for themes? What if in the future Gutenberg blocks will be published in a separate registry and we'd want to declare dependencies for them?

Something like this:

{
    "plugin-dependencies": [
        {
                "slug": "gutenberg",
                "name": "Gutenberg"
        },
        {
                "slug": "woocommerce",
                "name": "WooCommerce"
        }
    ]
}

Will keep the possibility to evolve the file without breaking backward compatibility with its schema.

---

Now that I provided feedback about the PR, let me add a few more considerations:

• people in the WordPress space are using Composer already (johnpbloch/wordpress and roots/wordpress have, combined, around 7.5 million downloads, and that is only a small portion of the Composer usage in WordPress, considering that the great majority uses Composer only at package level and not at website level (an example that might be familiar to you)
• as others have pointed out for many years now composer.json would provide industry-standard machine-readable information to those that want to consume it, without requiring WordPress to do anything (just to mention the first stuping thing, GitHub would recognize the license and show it)
• The argument about not wanting to couple with Composer because "package managers come and go" is, permit me the expression, a bit "bizarre", considering that, unlike in Javascript, Composer is the only package manager for PHP, and that is used by the entire PHP ecosystem (only excluding WordPress core). If something else will come in the future (and that's a big if) it will need to be compliant with Composer to have even a minimal chance to get adopted, in the exact same way when Yarn appeared in the scene it needed to be compatible with npm.
• WordPress is already relying on package managers, and have published more than a hundred packages in a proprietary packages registry. Why the PHP in WordPress has always to be treated so differently than Javascript?

That for me means that what this PR is doing, could have done on top of Composer, as other CMS have done, but this PR goes in the direction of doing it the "WordPress way". Sad, but not surprising.

Imagine a world where this would be a plugin's composer.json:

{
    "name": "acme/my-awesome-plugin",
    "description": "My plugin is awesome",
    "license": "GPLv3",
    "extra": {
        "wordpress": {
            "plugin-dependencies": [
                    {
                        "slug": "gutenberg",
                        "name": "Gutenberg"
                    },
                    {
                        "slug": "woocommerce",
                        "name": "WooCommerce"
                    }
            ]
        }
    }
}

The code currently in place in this PR could still be used as is, only the JSON source would be different.

WordPress would not need to support (at least not for now) all the Composer properties, e.g. any require could be ignored. But the people who use Composer could have additional properties (like require), then use Composer on their own to install the dependencies, and rely on WordPress to do what this PR does.

Of course, using "extra" property in composer.json instead of a separate file is not a requirement to follow the approach of compatibility with Composer (that could be obtained even using the dependencies.json file this PR supports), but using composer.json would be a push for the introduction of Composer support (even if partial support), reducing the distance from WordPress to the wider PHP community, all of that requiring literally zero additional effort comparing to what this PR is requiring.

And that's why I hope that, at least, the version constraint is removed from this PR, it will keep the door open to let Composer handle version constraints, something that Composer is very good at doing.

[tomjn]

I don't understand the need for this, composer already solves all these problems, as well as others you've not accounted for. The justification rests on faulty premises. WP CLI already uses composer internally for package management.

Even then, why not use a line in the plugin header?

<?php
/**
 * Plugin Name:  Foo
 * Requres Plugins: Bar, Gutenberg
 * Requires PHP: 7.2.0
 *

We already have code and precedents for this covering PHP and WordPress versions, and we have a widespread syntax for version handling that composer npm yarn bundler and many other package managers already use, e.g. gutenberg@~10.0.

Not to mention that dependencies.json is a very generic name, and the file would be pointless if plugin header data is moved into a wp-plugin.json in the future. It also doesn't make sense to split dependency data across the plugin header and a separate file, e.g. PHP dependency and WP dependency in one place, plugins in another. It also doesn't account for theme dependencies

---

Composer Notes

composer.json is a different beast. Composer handles packages, and it does that brilliantly. But plugin-dependencies are not, and should not, be treated the same as packages.

This is demonstrably false. There are lots of plugins out there that do exactly this. There are dedicated package types for plugins and themes, many people install WordPress itself this way.

If I need plugin dependencies on my plugin but don't use composer, then a composer.json file is meaningless. It's a file specific to an implementation that my plugin doesn't use, and therefore using that name for plugin dependencies is probably not the best choice for all developers.

A dedicated dependencies.json file is unambiguous, and allows developers to use simpler syntax, without nesting things inside extra. It's only an extra if I'm using composer, otherwise there's nothing extra about it.

I do not see how this would be different if the file was renamed to dependencies.json. This is a self-defeating argument.
Core itself can handle the activation, extra is just for automation that WP would have handled itself here. Otherwise there are no advantages to dependencies.json that aren't also covered by composer.json.

Additionally, composer is one of many package managers, and package managers come and go. It would be unwise to tie our naming convention for something that has nothing to do with packages, to a specific package-managing tool that may or may not exist in the future when some better tool comes along.

Composer is almost a decade old and has no major competitors, and overwhelming backing from the wider PHP community and many people in the WP community. PEAR is the only meaningful alternative as composers direct predecessor, and widely considered legacy.

---

I believe there are some fundamental misunderstandings about what Composer is:

• Composer is not a popular niche tool with numerous rivals, the closest is PEAR for historical reasons ( composers predecessor ).
• Composer is not a package manager for libraries, it's a dependency management tool. You can install WordPress, themes, and plugins using Composer without ever touching libraries
• Composer doesn't have to be run as a CLI tool, it can and does get run in browser requests though this is rarer
• By reading composer.json you:
  • get instant support out of the box or several plugins that already implement this via composer
  • automatically get free CLI support for those who need it via the composer tool
  • can warn users who haven't installed non-WP composer dependencies that extra steps are needed, avoiding a common mistake when grabbing plugins from github
  • can use battle hardened tooling and libraries from the wider PHP community
• WP CLI already uses composer internally so there is precedent and prior art within the WordPress project itselff

People seem to have weird ultra specific ideas about what a package is that don't reflect reality

---

If the goal here is just to focus on activation, why was a line in the plugin header not considered?

[Mte90]

I think that now we have 2 proposals that deserves a better evaluation:

• Composer support, for those plugin dependencies using the extra parameter. This as explained can help all the devs already using composer but also open to others this technology. At the same time we are talking of a json file that often is already included and maybe in the future can be used also for other things like the Tide project to do automatic scans of the code based on the dependence used. Maybe it will open the doors to add in the repository information extracted from the file itself like if has unit tests because has phpunit or codeception as dev-dependence.
• Plugin header, this probably should get the priority as everyone use it and it is the most simple and standard way to do things (like with the new recent parameters like PHP version or woocommerce version minimum required). The issue with that is that is not so dev friendly and code friendly itself. As now this require from WP a search for php files in every plugin folder that include it, parse it and get the information from it. This is not performance friendly from a side (instead to look for a specific json file and using PHP native methods to read them) and it is something very WordPress way to do things (that is just used in this technology and not others).

To me the best solution is to use composer, that opens the doors to a lot of things and also can be a decision changer and move the WP developers to discover the new PHP stuff from the glorious php 5.2 release. It offers a lot of opportunity without creating another new custom standard (whatever it is) and not create problems to already is using it but just open new situation to evolve the very old way of develop plugins and move forward the backward WP world.

[tomjn]

As now this require from WP a search for php files in every plugin folder that include it, parse it and get the information from it.

We already do this in order to get plugin name/slug/version and other dependencies during this process, and have existing tested code to parse the header. The plugin header suggestion would be no slower than what we have right now for just plugin slugs

[tomjn]

I would also note there's the scope for malpractice by including the plugin name as a local field. A plugin may claim to install Yoast SEO when the plugin slug is for a completely unrelated plugin

[Mte90]

As now this require from WP a search for php files in every plugin folder that include it, parse it and get the information from it.

We already do this in order to get plugin name/slug/version and other dependencies during this process, and have existing tested code to parse the header. The plugin header suggestion would be no slower than what we have right now for just plugin slugs

Sure, we have already the code for that. Mine was just 2 cents about that we are doing something that is not so dev/code friendly and just in the WP ecosystem.

I would also note there's the scope for malpractice by including the plugin name as a local field. A plugin may claim to install Yoast SEO when the plugin slug is for a completely unrelated plugin

This is another issue that I think was fixed also in a recent WP version to let use a textdomain of a plugin in the repo but without getting updates alert and so on. In any case it is a problem, maybe it should as reference only the WP repository and not other sources like Packagist or GitHub to avoid any risks.

[aristath]

Thank you all for the feedback! Apparently, my comment above struck a nerve with many. I am not a composer expert, and obviously, people use it for far more than I could imagine.

As per the suggestions above, I changed the implementation and it now uses the plugin headers:

<?php
/**
 * Plugin Name: Sample
 * Requires Plugins: woocommerce, gutenberg
 */

It's simple and it works.
This change also means that for simplification purposes, version checks were removed and now plugin authors just add comma-separated slugs.

I updated the OP to reflect these changes

[Chrico]

As per the suggestions above, I changed the implementation and it now uses the plugin headers:

this will not work, since , is a valid character for Plugin Name:

<?php
/**
 * Plugin Name: My, super, duper, plugin
 */

$pluginData = get_plugin_data(__FILE__);

will result in:

[aristath]

In the Requires Plugins header we define the plugin slug, not its name. Plugin names are not constant and can change at any time. Plugin slugs however are unique and can never, ever change - which is why they are the safer option.

[Chrico]

The Plugin slug can change as well. Aside from that, basically the same: Folder/file-name which is used for plugin slug can contain , or whitespace as a valid char as well. :) You either need to use

/**
 * Requires Plugins: "slug,1", " slug 2", "slug, 3"
*/

or a seperator which is not allowed as file-/folder-name, like |:

/**
 * Requires Plugins: slug,1|slug 2|slug, 3
*/

But this makes it an even bigger mess...aside from that, what about Themes? A Theme can require Plugins as well. And vise versa.

If only there was a file format that makes it all easier ...something which is widely adopted and used...hm...... (SCNR 😬 )

[aristath]

The Plugin slug can change as well.

If the slug of a plugin changes, WP throws an error that the installed plugin could not be found and was deactivated. If a slug changes, then it's considered a different plugin.

Folder/file-name which is used for plugin slug can contain , or whitespace as a valid char as well. :)

I don't think I've ever seen a plugin with a slug like that... Is it even allowed in the w.org repository?

what about Themes? A Theme can require Plugins as well. And vise versa.

Themes on w.org are not allowed to require a plugin. They can recommend a plugin, but not require it.

[gmazzap]

@aristath thanks for the change.

The plugin header is indeed a better approach than dependencies.json, and thanks for @tomjn for suggesting (and I hope that wp-plugin.json Tom mentioned will see the light at some point).

The only note is that, as @Chrico pointed out, commas and spaces are valid file/folder names, which means they could be used in valid plugin slugs.

You can try yourself: create a file named my, plugin.php with a plugin header in it, and try to activate it.

I checked wp.org guidelines, and there's nothing telling you should not use command and/or spaces in plugin folder/file names.

This is an edge case, but IMO before this PR is merged:

• you should first check that wp.org has no plugin with their folder/main plugin file containing commas and/or spaces.
• a note should be added in plugin guidelines that says that comma/spaces are not allowed in plugin folder/main plugin file.

Alternatively, you can use a different approach of using a different separator/format.

One possible approach could be that if the plugins slug contains special characters like commas and/or spaces it must be in between quotes.

So in the great majority of cases a thing like the following will be fine:

Requires Plugins: woocommerce, gutenberg

But for special cases:

Requires Plugins: woocommerce, gutenberg, "some, weird, plugin, slug"

This will surely make the parsing a bit more complex, but not more than adding a single regex. I can help with that if you want.

[aristath]

This will surely make the parsing a bit more complex, but not more than adding a single regex. I can help with that if you want.

@gmazzap Sure! My regex skills are limited, so if you can help with that, it would be awesome 👍

[gmazzap]

@aristath the following should work, but needs testing ;)

/**
 * A plugin slug can contain spaces/commas, not only by spaces/commas.
 *
 * @param string $slug
 * @return bool
 */
function is_valid_plugin_slug($slug)
{
    return trim($slug, ", \t\n\r\0\x0B") !== '';
}

/**
 * @param array $data Data coming from get_plugin_data
 * @return array list of plugin slugs
 */
function extract_required_plugins_from_header($required)
{
    if (!$required || (substr_count($required, '"') < 2)) {
        return $required
            ? array_filter(array_map('trim', explode(',', str_replace('"', '', $required))))
            : [];
    }

    $required = str_replace('""', '', $required);
    preg_match_all('#"([^"]+)"#', $required, $matches);

    $slugs = array_filter(
        array_map('trim', explode(',', strtr($required, array_fill_keys($matches[0], ''))))
    );

    return array_merge(array_filter($matches[1], 'is_valid_plugin_slug'), $slugs);
}

It accounts for edge cases like a single quote, quotes with nothing inside, and via the is_valid_plugin_slug function, plugins slugs made only by spaces/commas.

Sorry the code is not using WP code styling, I'm not really used to those.

[carolinan]

The wordpress.org themes team are open for allowing themes to require plugins, should the meta team change their mind about allowing it, and as long as solutions for the theme previewers can be agreed on.
https://meta.trac.wordpress.org/ticket/3863

[tomjn]

Wouldn't str_getcsv be easier for handling the parsing?

$data = '"slug,1", slug 2, test, "slug, 3"';

$plugins = str_getcsv( $data );
var_dump( $plugins );

array(4) {
  [0]=>
  string(6) "slug,1"
  [1]=>
  string(7) " slug 2"
  [2]=>
  string(5) " test"
  [3]=>
  string(7) "slug, 3"
}

Quoted, unquoted, single word values, etc all handled by native PHP. Just needs whitespace trimming afterwards

[aristath]

@tomjn That's perfect! It didn't even cross my mind... Tested and pushed, works brilliantly

[gmazzap]

That's very good @tomjn!

Still a few edge cases to consider, maybe, and whitespace trimming can't be done, because the whole point of having quotes is to be able to allow spaces (and commas) in plugin slugs, because they are valid characters.

The filtering by is_valid_plugin_slug is still needed IMO.

See https://3v4l.org/S7263

And even that is not perfect. Because the whitespace trimming should be done when outside quotes, but not when in quotes.

[aristath]

Hmmm I think that may be too much of an edge case... A foldername of a plugin ('cause that's what a slug is) with extra whitespace in the beginning or the end is clearly a mistake/typo and should be fixed 🤔

[tomjn]

a user is far more likely to mentioon foobar and put spaces and commas between them then wonder why it failed than they are to have a folder that starts or ends in a space

[gmazzap]

I use to write unit tests for my code, and that simply don't pass unit tests (not even with space trimming) :)

https://3v4l.org/Uudsc

And in the case someone has a space at the end of a file name, that is maybe a typo, but they are not gonna change it, because once published the filename has to stay as-is.

[tomjn]

It's also not possible to have a plugin with the folder name " foo ". I created such a plugin and when activated WP says the plugin file does not exist:

The scenario we're concerned about cannot happen

[aristath]

The slug is not about filenames, but the main plugin folder-name. All plugins on .org go through a review and I believe these get automatically trimmed?

[gmazzap]

The slug is not about filenames, but the main plugin folder-name

Do you know that plugins can be a single file?
You can have a plugin with a single foo .php file, and it'll be fine.

I believe these get automatically trimmed?

Ask who manages the repo :)

In any case, even with trimmed, if you want to do a proper parsing you have to skip things like ",", "'", '.

If you can decide to ignore edge cases, and that is a possibility I guess, you can just use explode: plugins with commas/spaces are edge cases anyway.

But if you decide to handle edge cases, you should handle them IMO.

[tomjn]

Core itself appears to do trimming, in the case of " foo " it strips the first space out

[aristath]

My big concern would be where we have double messaging coloring:

Thank you @karmatosed for the feedback!

I fixed the double-coloring and now it looks like the screenshot below:

In addition to the above fix, if the plugin can't be deactivated, the checkbox next to the row is disabled because the plugin can't be deleted/deactivated.

[dd32]

The Plugin slug can change as well. Aside from that, basically the same: Folder/file-name which is used for plugin slug can contain , or whitespace as a valid char as well. :) You either need to use

/**
 * Requires Plugins: "slug,1", " slug 2", "slug, 3"
*/

I would call this over-engineering, A edge-case of a plugin having a comma or whitespace within it's folder name is really never going to happen in any manner that this functionality would be used.

The primary use-case of this will be for WordPress.org-hosted plugins, which are forced to have "slugs" - which in the WordPress world means ^[a-z0-9-]$. If a plugin slug slug, 1 is not supported/parsed, I would deem that extremely acceptable if it simplifies code and implementation.

[peterwilsoncc]

I've been doing some manual testing with a minimal plugin to test this pull request. The full plugin is:

<?php
/**
 * Plugin Name:       PWCC Depencency checker
 * Description:       Checking the dependency pull request.
 * Version:           1.1.0
 * Requires at least: 5.2
 * Requires PHP:      7.2
 * Author:            Peter Wilson
 * License:           GPL v2 or later
 * License URI:       https://www.gnu.org/licenses/gpl-2.0.html
 * Text Domain:       deps-testing
 * Requires Plugins: user-switching, wp-call-button
 */

trigger_error( 'user switching defined: ' . (class_exists( 'user_switching' ) ? 'yes' : 'no') );
trigger_error( 'call button defined: ' . (defined( 'WP_CALL_BUTTON_VERSION' ) ? 'yes' : 'no') );

In the initial pre-activation state:

• user-switching is installed but deactivated
• wp-call-button is not installed

Notifications when activating

The plugin activated message doesn't make sense in this context, as the plugin is pending activation.

I received an alert for three plugin requirements: "User Switching", "wp-call-button" and "" (ie, an empty plugin name). I can't see the trigger for the bug.

The "" dependency didn't block activation so I was able to continue testing.

Plugin require order

Once activated, the unserialized active_plugins option is:

array(3) {
  [0]=>
  string(23) "deps-testing/plugin.php"
  [1]=>
  string(33) "user-switching/user-switching.php"
  [2]=>
  string(33) "wp-call-button/wp-call-button.php"
}

As there's no testing of the dependency order when requiring the plugin, the log messages for the test plugin indicate that the dependencies are unavailable as the plugin is loaded:

[21-Sep-2021 01:19:02 UTC] PHP Notice:  user switching defined: no in /vagrant/content/plugins/deps-testing/plugin.php on line 15
[21-Sep-2021 01:19:02 UTC] PHP Notice:  call button defined: no in /vagrant/content/plugins/deps-testing/plugin.php on line 16

Questions:

• should plugins be required take in to account the dependency tree?
• if so, how would circular dependencies be resolved?

I guess it's fine if the answer to the first question is No but it would be good for it to be decided rather than not considered.

Namespaced dependencies

On the original ticket there is a brief discussion about using a namespace convention for third party plugin repositories (for example automattic:woocommerce-subscriptions).

I am wondering if namespacing everything would be better, for example wordpressorg:user-switching.

---

I've added a few comments inline on the pull request. I am also hesitant about the timing of including this but I'll address that on the ticket.

[aristath]

Thank you @peterwilsoncc for the review and suggestions!

The plugin activated message doesn't make sense in this context, as the plugin is pending activation.

I agree. However the way this message gets added makes it difficult to tweak, and I'm still looking for the best way to change the current behavior. For reference that notice gets added in https://github.com/aristath/wordpress-develop/blob/2d7e62d7bbd13dd6468bbb18f0ce4b8d12766775/src/wp-admin/plugins.php#L697-L698
The solution I'm trying to implement for that, is tweaking the validate_plugin_requirements function to account for plugin requirements. But throwing an error in that class shows the familiar WordPress error screen, and obviously in this scenario we can't do that. If you have any ideas on how to address this issue I'm all ears! 👍

should plugins be required take in to account the dependency tree?

You mean change the order of notifications and show them according to the dependency tree? We can do that...

if so, how would circular dependencies be resolved?

Circular dependencies will most likely be cases of doing-it-wrong. They are taken into consideration in the code so in case there is a circular dependency it's still possible to activate the plugins (otherwise a circular dependency would fail requirements as plugin A can't be activated before plugin B is active, and plugin B can't be activated unless plugin A is active which is an obvious irrational loop). Circular dependencies also show a notice in the plugin saying Warning: Circular dependencies detected. Plugin "%s" has unmet dependencies. Please contact the plugin author to report this circular dependencies issue.
As for the order in which notices for circular dependencies will be shown, I don't think it makes any difference... there is no "parent" or "child" in a circular dependency, whichever plugin gets activated first is the parent 🤷

On the original ticket there is a brief discussion about using a namespace convention for third party plugin repositories (for example automattic:woocommerce-subscriptions).

Pushed a commit that will get the right plugin slug if using a namespace with :, and added a filter.
The filter will allow developers to hook in there, check if the required plugin has a namespace, and if it does they'll be able to hook in the normal WP installers to change the source etc - just like they've always done in the past.

[peterwilsoncc]

@aristath

You mean change the order of notifications and show them according to the dependency tree? We can do that...

No, I mean during the bootstrapping procedure so dependencies are available before the plugin.

wordpress-develop/src/wp-settings.php

Lines 406 to 420 in d05ecf2

	// Load active plugins.
	foreach ( wp_get_active_and_valid_plugins() as $plugin ) {
	wp_register_plugin_realpath( $plugin );
	include_once $plugin;
	/**
	* Fires once a single activated plugin has loaded.
	*
	* @since 5.1.0
	*
	* @param string $plugin Full path to the plugin's main file.
	*/
	do_action( 'plugin_loaded', $plugin );
	}
	unset( $plugin );

[SergeyBiryukov]

I fixed the double-coloring and now it looks like the screenshot below:

I think highlighting the whole plugin row like that might look inconsistent with other inline notices on the Plugins screen.

Could we go back to just the inline notices here? See some examples of the current notices below, and changeset 51678 for an example of adding one of these notices.

[pbiron]

@SergeyBiryukov

I think @karmatosed was originally referring to the case when the plugin is active (and hence has a blue background) and an "info" notice in the same row (which is also blue).

And the revision by @aristath in such cases is to not have the "double blue".

[karmatosed]

I think @karmatosed was originally referring to the case when the plugin is active (and hence has a blue background) and an "info" notice in the same row (which is also blue).

I was. It's a ponder and potentially this is where we might find we're at the edges of this interface in what works. Maybe we can have one information notice 'win'. It's really tricky as I don't think the info in info is at all the right solution here.

[SergeyBiryukov]

I think @karmatosed was originally referring to the case when the plugin is active (and hence has a blue background) and an "info" notice in the same row (which is also blue).

And the revision by @aristath in such cases is to not have the "double blue".

Indeed, thanks for the clarification! Apparently I don't have a lot of active plugins on my test install, and I forgot they have a blue background :) I retract my suggestion in that case, the current UI in the PR seems like a reasonable compromise.

[ObliviousHarmony]

Looking at the proposed solution, I think this would be a great solution for WooCommerce. From my perspective, there are two different kinds of constraints we should consider.

The first kind, like this pull request, is a declaration that a plugin requires another plugin in order to function. This is useful so that every developer who uses an add-on doesn't have to add the same checks, and is something we'd considered adding to WooCommerce Core. I think the backwards compatibility we see in the ecosystem makes the version constraint kind of unnecessary (I know this PR dropped that earlier on), as plugins should be gracefully handling those kinds of incompatibilities already.

The other kind, like Composer, is a version constraint complete with version reconciliation. Keep in mind that plugins have their own dependencies already provided through Composer, and so any useful implementation would reconcile all of those versions too. While it's true that this is likely a growing necessity, at the same time, there are already solutions that those who choose to use Composer can take advantage of.

While I think the first kind is a great quality-of-life improvement for site owners, the other feels like it would create a confusing experience for those who are less technically inclined. A lot of things can go wrong when trying to reconcile all of the plugins that would need to be resolved. It's true that this could be a great way to prevent confusion when getting errors out of incompatible versions, but at the same time, when it goes wrong the process of dealing with these incompatibilities is not very easy.

I would be very happy to see this pull request land!

[paaljoachim]

Testing.

This is what I see:

Regarding the UI.
Plugin "Sample" depends on plugin ....... + name of plugin + link to install/activate the plugin.

To me I first get overwhelmed by all the information, but as I gradually look at it I see the order of having one plugin per line. It creates an alright overview this way.

Information below the Sample plugin is a general information which tells me what I need to know regarding the above notifications.

[paaljoachim]

I have compared this PR with #1724 and the other PR integrates really well into WordPress.
It handles notifications by using a Dependency tab in the Add plugin screen.
The message below the plugin with a dependency is much more subtile and easier to understand.
(I had not initially looked closely at the message in the yellow box in this PR.)

The flow of using a Dependency tab works really well.

[peterwilsoncc]

@paaljoachim Are you able to add some notes to the feature project repo? I think this is the best ticket WordPress/wp-plugin-dependencies#3

[peterwilsoncc]

Closing this off as it was decided to go down the feature project route, the repo is https://github.com/WordPress/wp-plugin-dependencies