#22316 closed enhancement (fixed)
Plugin Dependencies (Yet Another Plugin Dependencies Ticket)
Reported by: | Viper007Bond | Owned by: | afragen |
---|---|---|---|
Milestone: | 6.5 | Priority: | normal |
Severity: | normal | Version: | 3.4.2 |
Component: | Upgrade/Install | Keywords: | has-patch has-unit-tests has-dev-note |
Focuses: | Cc: |
Description
Previously: #10190 #11308 #13296 and I'm sure many more
It's been a few years since we looked at plugin dependencies and this still seems to be a feature people really, really want, especially for shared functionality that isn't a plugin in itself. For example a PHP library that isn't popular enough to be in core but is popular enough to be bundled in multiple plugins.
A bunch of us sat down and talked about this at this year's WordPress Community Summit and there was a lot of enthusiasm for this type of functionality.
We didn't know about the existing tickets at the time but the general summary of what we came up with was this:
- Plugins list WP.org slugs of their dependencies in their
readme.txt
, or perhaps better their plugin's header.
- When you go to install a plugin via the plugin directory UI in the admin area, the WP.org API returns a list of dependencies along with the data about the plugin being installed. WP would say like "these following dependencies will also be installed". This means it's seamless to the user -- they install a plugin and the other plugin(s) that are needed get installed too.
- No versioning support. It's too complicated and what if one plugin wants an older version of a dependency than another plugin does? If your plugin is listing another as a dependency, then it's your job to make sure it stays compatible with the latest version of the dependency. On the flip side, hopefully plugins that get listed as dependencies are made to be forwards and backwards compatible.
- Probably not allowing the disabling of plugins that are dependencies while their dependents are active. This seems better than disabling the dependents when the dependency is disabled ("why did Foo get disabled? I only disabled Bar!").
- On plugin re-activation or on activation of a plugin uploaded via FTP, make sure it's dependencies are already installed. If not, offer to install them. If installed but disabled, just enable them for the user.
So while the previous tickets were closed as wontfix
in the past, I think this is worth taking another look at. A lot of planning and thought will be required though to get this right.
Attachments (14)
Change History (348)
#19
@
12 years ago
On the topic of versioning dependencies, I discussed this at length with a friend of mine who is a PHP/Java developer and we came up with this: โhttps://github.com/bubba-h57/PHP-Library-Version-Framework. Using namespaces plus a factory was the only viable way we could find to fix the issue of class version dependencies, but that can't even be considered until WordPress support PHP 5.3 and most plugin devs won't be able to implement something like it.
While I think header comments are nice, I think a more robust API should be created for handling this, especially when you start getting into the issue of forcing activation/deactivation with dependencies. Maybe header comments could be used to initialize the API, and from there plugin dev could utilize a class with hooks/filters.
External plugin dependencies (e.g. commercial plugins) should also at least be considered should something like this make it into core. There doesn't need to be a specific way to handle external items but at least some hooks or filters to allow commercial plugins to make use of it.
I'd be more than happy to contribute to the planning and development process of this. It's why I created TGM Plugin Activation in the first place. :-)
#22
follow-up:
โย 29
@
12 years ago
Replying to griffinjt:
While I think header comments are nice, I think a more robust API should be created for handling this, especially when you start getting into the issue of forcing activation/deactivation with dependencies.
Header comments are preffered because they can be parsed without activating the plugin first.
On the issue of forcing activation, when you specify a dependency, you say "I _need_ this to be active".
If you don't need it to be active, it's not a dependency; it's a nice-to-have, which should be specified using a separate header or some other method.
The issue is: is it safe for plugin X be auto installed and activated? For example, you wouldn't want BuddyPress to be auto-installed and activated, since it changes your site in a 100 ways.
But, I think this should be specified by the library, with a header like "Provides: metabox". Besides providing a stable name for the library, it could also signal that it's safe to auto-activate.
#23
@
12 years ago
We want to make sure that we think about circular dependencies as well, just to make sure we don't get stuck in infinite loops.
In terms of working out a dependency tree, Composer โported openSUSE's Libzypp dependency solver, which seems complex but might be a good start to actually doing that if we strip it down (and de-PHP5.3 it).
#25
follow-up:
โย 27
@
12 years ago
Replying to griffinjt:
On the topic of versioning dependencies, I discussed this at length with a friend of mine who is a PHP/Java developer and we came up with this: โhttps://github.com/bubba-h57/PHP-Library-Version-Framework. Using namespaces plus a factory was the only viable way we could find to fix the issue of class version dependencies, but that can't even be considered until WordPress support PHP 5.3 and most plugin devs won't be able to implement something like it.
We discussed this and decided that we'd only implement it at a basic level, since resolving multiple versions is going to be a whole new layer of complexity. It then becomes the plugin's problem to ensure that they maintain forwards- and backwards-compatibility.
External plugin dependencies (e.g. commercial plugins) should also at least be considered should something like this make it into core. There doesn't need to be a specific way to handle external items but at least some hooks or filters to allow commercial plugins to make use of it.
I think as long as there are filters for this, it should be fine. i.e. Depends: abc def othersite:ghi
would work, I think.
#26
@
12 years ago
Replying to scribu:
Header comments are preffered because they can be parsed without activating the plugin first.
On the issue of forcing activation, when you specify a dependency, you say "I _need_ this to be active".
If you don't need it to be active, it's not a dependency; it's a nice-to-have, which should be specified using a separate header or some other method.
You are correct, and I see the benefit of header comments now.
The issue is: is it safe for plugin X be auto installed and activated? For example, you wouldn't want BuddyPress to be auto-installed and activated, since it changes your site in a 100 ways.
But, I think this should be specified by the library, with a header like "Provides: metabox". Besides providing a stable name for the library, it could also signal that it's safe to auto-activate.
+1 to this
#27
in reply to:
โย 25
@
12 years ago
Replying to rmccue:
I think as long as there are filters for this, it should be fine. i.e.
Depends: abc def othersite:ghi
would work, I think.
That would be perfect. Nothing drastic, but at least a way to commercial plugin devs to take advantage of it, kind of like how you can tap into plugins_api for updates outside of the repo.
#28
follow-up:
โย 32
@
12 years ago
- Cc mike@โฆ added
Ironically I brought up โa discussion on wp-hackers yesterday to address this issue, but proposed "libraries" for this use case because of the past pushback to adding plugin dependency support into core.
One aspect of the library proposal is that libraries would be hidden from users and I think it's critical that if address this. Plugins add a lot of conceptual overhead for the user and we all know that many blog posts and peer-to-peer discussions at WordPress meetup groups tell users "limit the number of plugins you use."
Imagine a plugin that uses four (4) other plugins as dependencies; if a user installs that plugin now they have five (5) active visible plugins and their anxiety shoots through the roof. This will cause plugin and theme developers to avoid using many or any dependencies even if modularity is the best solution otherwise because they know uninformed users will tell their peers "Avoid that theme/plugin; it adds too many other plugins" even when the code of adding those plugins are essential to accomplish the task.
Drupal has had module dependencies for when I started using it in 2008 and the conceptual overload for users was a problem with it too.
An example of this type of dependent plugin would be an API client SDK for a company like MailChimp or FreshBooks. Why does the user need to see that a plugin that wraps one of those APIs is active? Would it not be much better to simply to have them see the plugin that uses it?
Proposal: Consider doing one of the following:
- Allow plugins to market themselves as a library and if marked can't be activated by themselves and would be invisible to the user.
- Display dependent plugins only when the admin user clicks a "Show dependent plugins" link in the plugin list.
- Implement the library proposal from wp-hackers instead.
Another question to ponder is: "How much overhead does each plugin add vs. using require() or a class auto-loader?" Will 100 plugins being activated slow down a site vs. require()
ing 100 PHP files? If so, we may seriously consider a "library" concept instead of or in addition to plugin dependencies.
#29
in reply to:
โย 22
@
12 years ago
Replying to scribu:
But, I think this should be specified by the library, with a header like "Provides: metabox". Besides providing a stable name for the library, it could also signal that it's safe to auto-activate.
This would be nice because it would require officially delineating a list of "features" that a plugin could add. That could then be used as metadata on WordPress.org plugin search as well as other locations.
#32
in reply to:
โย 28
;
follow-up:
โย 33
@
12 years ago
Replying to MikeSchinkel:
Another question to ponder is: "How much overhead does each plugin add vs. using require() or a class auto-loader?" Will 100 plugins being activated slow down a site vs.
require()
ing 100 PHP files? If so, we may seriously consider a "library" concept instead of or in addition to plugin dependencies.
Why don't you test and see? ;)
Active plugins being loaded are just done via an <code>include()</code> or <code>require()</code>. There's basically no difference. For that matter the include itself has a trivial amount of overhead.
As the saying goes, it's quality not quantity. You could run 500 plugins if you wanted to.
But this is off-topic from this ticket. :)
#33
in reply to:
โย 32
@
12 years ago
Replying to Viper007Bond:
Why don't you test and see? ;)
Isn't one of the benefits a community provides being to multiply the time and effort of its participants? It seems like a foolish waste of time to set up a benchmark when one question can potentially leverage the experience of someone else who has already evaluated.
Active plugins being loaded are just done via an <code>include()</code> or <code>require()</code>. There's basically no difference. For that matter the include itself has a trivial amount of overhead.
As the saying goes, it's quality not quantity. You could run 500 plugins if you wanted to.
My memory was that more happened during plugin load than it does (I just reviewed the code.) OTOH running 500 plugins would do 500 include()
s for every page load whereas having the ability to load on demand would be useful for PHP files that provide supporting functionality to only selected URLs.
Maybe a "Load" header for "on-demand" or "always" would be useful and then a "require_plugin()" function that would load if not already loaded?
But this is off-topic from this ticket. :)
Really? Seems a discussion of potential ramifications of dependencies would be on-topic.
#34
follow-up:
โย 35
@
12 years ago
The performance concern is valid. I've heard tales of ruby gems having 20+ dependencies, most of which weren't warranted.
We should be able to do on demand loading, especially if we use the WP_Dependencies class, but it's something we can easily add in a v2.0 iteration.
#35
in reply to:
โย 34
@
12 years ago
Replying to scribu:
We should be able to do on demand loading, especially if we use the WP_Dependencies class, but it's something we can easily add in a v2.0 iteration.
That's cool, it'll be less of a problem early on. Just as long as we don't for some reason paint ourselves into a corner that won't let us address later.
#37
follow-ups:
โย 38
โย 40
โย 41
@
12 years ago
- Cc dougal@โฆ added
From what I've seen in the Drupal space, dependencies help encourage devs to collaborate on common APIs that they can all use for their own modules. For example, a CDN caching API plugin -- One author can build an S3 plugin on top of it, another dev could build a MaxCDN plugin, or Rackspace Cloud, or whatever. Or a base Twitter API module could be available to build upon. Can you image how much repeated code/effort has gone into the bajillion Twitter plugins currently in the directory?
#38
in reply to:
โย 37
@
12 years ago
Major +1 on this discussion, heavily needed and +1 on @dougal's specific notes:
via dougal:
dependencies help encourage devs to collaborate on common APIs that they can all use for their own modules
#40
in reply to:
โย 37
@
12 years ago
Replying to dougal:
Dependencies help encourage devs to collaborate on common APIs that they can all use for their own modules. ... Can you image how much repeated code/effort has gone into the bajillion Twitter plugins currently in the directory?
+1000.
#41
in reply to:
โย 37
@
12 years ago
Replying to dougal:
dependencies help encourage devs to collaborate on common APIs that they can all use for their own modules.... Can you image how much repeated code/effort has gone into the bajillion Twitter plugins currently in the directory?
I want this for the twitter factor alone. But on top of that, I want to see how much of a shift there can be toward looking at the whole plugin sphere as potentially interrelated (hookable, contributable...) and not just each as its own separate little world.
#43
@
12 years ago
Something else to consider about dependent plugins which I'd like to pose as somewhat rhetorical questions:
- Are plugins that themes and other plugins depend on mostly things an end-user would install independently, or are they mostly things that an end-user would likely never install on their own?
- If the former, what are some examples? But if the latter would it make sense to treat these "depended on" plugins differently than existing plugins?
Specifically, if these "depended on" plugins are mostly API/libraries would it not make sense to consider (also) pulling them from GitHub instead of the WordPress plugin repository where end-users go to search for plugins? GitHub is emerging as the place developers post code to share and collaborate (note: I personally prefer Hg for version control, but GitHub has no peer in enabling collaboration.) The primary benefit to GitHub IMO is the fork/pull model that allows developers to more easily collaborate.
The current plugin repository makes it very difficult to collaborate (I believe that issue was discussed at the summit, directly or indirectly regarding abandoned plugins?) If API/libraries are pulled from GitHub then these emergent APIs will likely be fewer in number but much better in depth and robustness. Rather than have every developer create yet another API/library for the same purpose it will be easier to make an enhancement to an existing API/library because of forking, and easier to get the original developer to accept the enhancement because of pull requests.
Which brings up a corollary, should API/library plugins even be included in the WordPress plugin repository where end-users go looking for plugins to install?
#51
@
12 years ago
In the scope of the discussion, are we also talking about letting themes be dependent on named plugins via a method in core? I've seen โhttp://tgmpluginactivation.com/ fill something of a need for that already -- but I see that as a much larger use case. We could potentially abstract out a lot of functionality code from themes, and into plugins, and make them more interchangeable. And the code to do it for both plugins and themes having the dependencies should cross-pollinate nicely.
#52
@
12 years ago
- Cc mike@โฆ added
The workflow for TMG Plugin Activation is one I think could work for a broad pool of WordPress users. It has pre-install notices and makes all installed plugins visible to users, no hidden libs or invisible activations.
As for the plugin-proliferation that any dependency manager might expose, we already have that and worse. Having many named and shared plugins/libs seems better than having just a few plugins where each has it's own copy of jQueryUI + different lightboxes + overlapping OAuth libs. The overlapping code means even more total code is loaded and there's a greater risk of errors. Even a basic dependency manager would help resolve that.
I don't like the idea of "Provides: metabox" though. Not because of circular dependencies, but because until the WP.org repository resolves issues regarding social/pull coding and ownership transitions we'll be better served to have as little overlap as possible between plugins. If there is overlap we would need to provide a way to filter/set which of many provided libs is used by each of many different plugins.
What might be making this more complex for WordPress is that plugins are so often standalone data, logic and view bundles. That's a bit by necessity of just getting things working, but as a community we also don't recognize good, base plugins enough. An example is the Twitter plugin comment: if people want to compete with 20 other Twitter plugins on features or UX that's *great*. The more of that we have the better for finding what works. ...but if each competing plugin writes their own and/or bundles OAuth code, that's the problem I think we need to solve here. There should be a whole set of OAuth plugins, and a whole other set of Twitter/Facebook/sharing plugins that don't have their own OAuth code.
#53
@
12 years ago
It may be worth looking at the Composer JSON schema, โhttps://github.com/composer/composer/blob/master/res/composer-schema.json
The info provided there might not all fit in our Header comments, but if the WP.org plugin repo made the rest of it available then dependency/library management in core or as a plugin would be a lot easier. It might also help non-WP.org repos to be more functional. They already exist, but tend to be hacked together and tied to the owner's own plugin "frameworks." Why not support them, but raise the bar for interop a bit?
#54
@
12 years ago
- Cc info@โฆ added
In response to @MikeSchinkel, I would certainly hope that theme dependencies on plugins are included in the scope of this ticket, and I also think that's where you would see a lot of plugins that end users would install on their own. Tons of themes bundle widgets, shortcodes, custom post types, etc. that are also already offered by plugins. Getting that stuff out of themes and into plugins would be a big win for theme developers (and users).
However, it raises the issue of whether this ticket would address themes/plugins that support integration with a plugin but don't require it. I love that modularized approach that would lead to lighter-weight sites and cleaner admins, but integrating with a plugin is definitely different than depending on it.
#55
@
12 years ago
- Cc andrew@โฆ added
re: visual overhead
is there any reason there couldn't be a 'dp-plugins' (for dependencies) similar to the mu-plugins folder for these things to be stored? Adding another tab on the top row (next to 'must use' or whatever). This will allow them to stay contained, be useful, and not overload / scare a user when seeing a bunch of new plugins.
#56
@
12 years ago
- Cc jltallon@โฆ added
Plugin *and* Theme dependencies:
We have found ourselves needing certain functionalities from (activated) plugins for themes to work properly (modular code), specially w.r.t. Custom Post Types: the CPT in a plugin, with some code to help use it, while the display logic is in the theme itself.
I hereby propose extending the functionality to theme headers too. Versioned dependencies are a must if we want to enable code sharing.
For a good example on what *definitively* works, look at Debian. We can use the same format, too:
-- cpt-shared-3/cpt-shared-plugin.php --
/* Plugin: cpt-shared Version: 3 */
-- cpt-foo-plugin-1.0/foo-plugin.php --
/* Plugin: CPT FOO Functionality Version: 1.0 Provides: cpt-foo Depends: cpt-shared (>= 2) */
-- my-super-duper-theme/functions.php --
/* Theme: My Super Duper Theme 1 Depends: cpt-foo (>= 1.0) */
#57
@
12 years ago
Shared library / on-demand loading functionality:
I would certainly split this issue into a separate issue.
What we have done (I'm ready to share the code if wanted) is to implement our shared functionality into a series of plugins:
- A shared "framework" plugin, which registers an autoloader and registers a base namespace where most *base* functionality resides (i.e. base and utility/shared classes)
- Functionality-providing plugins, which depend on the framework, each providing a certain set of functionalities (usually within classes derived from the interfaces contained into the framework, so that one can mix and match components)
- The plugin-dependencies plugin (v1.2 by Scribu + patches) to try and avoid an inconsistent mess
Hence, the "libraries" versioning here is not based on particular class versions (which has the potential of becoming messy) but of *library* versions (just like it is done with sonames)
E.g.: "library" A v1.2 provides versions x, y and z of classes Foo, Bar and Baz respectively. So another plugin depending on this shared functionality needs only "Depends: library (>= 1.2)"
#58
@
12 years ago
SUMMARY PROPOSAL:
- plugin-dependencies (w/ versioned dependencies and theme dependencies) into core
- Class Loader (we can certainly even use the UniversalClassLoader from Symfony here) into core
- "Library" plugins have one index.php file which just registers their (sub-)namespace with the ClassLoader. All later loading is demand-based. The declare "Provides:" and "Version:" ร la Debian
- Functionality plugins work as usual (perfect backwards compatibility), though they can (and should, IMHO) use "libraries" when possible. They declare dependencies if at all posible, so that we can fail gracefully if pre-conditions are not met (as opposed to just WSOD'ing)
- Themes can optionally depend on certain functionalities to be present (via de same "Depends" mechanism). The theme chooser UI would need to be refreshed to include some feedback for this, though.
Ideally, and with the auto-loader within core, quite some of WordPress' itself can be demand-loaded: most of the admin (e.g. WP_Table/WP_List), RSS functionality, image processing, etc.
The performance boost can indeed be huge (we have benchmarked the performance gain of doing this with plugins, I expect the gain for WP itself to be even more noticeable)
#65
@
11 years ago
- Cc herb@โฆ added
I've been delivering plugins with plugin dependency logic since April 2012. I developed the "full" solution in the oik base plugin and provided lazy code in the dependent plugins.
Since there's no core API, each plugin that has a dependency needs to deliver the same basic code. I tried to develop a solution that worked during activation; but that was a nightmare. I plumped for responding to "admin_notices".
add_action( "admin_notices", "oiku_activation" ); /** * Implement the "admin_notices" action for oik-user */ function oiku_activation() { static $plugin_basename = null; if ( !$plugin_basename ) { $plugin_basename = plugin_basename(__FILE__); add_action( "after_plugin_row_" . $plugin_basename, __FUNCTION__ ); require_once( "admin/oik-activation.php" ); } $depends = "oik:2.0-alpha.0329,oik-fields:1.18.0325"; oik_plugin_lazy_activation( __FILE__, $depends, "oik_plugin_plugin_inactive" ); }
See โhttp://plugins.svn.wordpress.org/oik-nivo-slider/trunk/admin/oik-activation.php
This solution works for dependent plugins which are: missing, inactive, not at the required minimum version. With the right API built into core the above code could reduce to a one line filter.
Note: plugins which are dependent on others wait for the appropriate "plugin loaded" signal before attempting to use those plugins APIs.
The challenge I now face is where the missing or out-of-date dependent plugin's repository isn't wordpress.org. e.g. oik-fields. I have a similar challenge when it comes to determining which jQuery libraries are available!
This ticket was mentioned in โSlack in #themereview by greenshady. โView the logs.
9 years ago
This ticket was mentioned in โSlack in #core by swissspidy. โView the logs.
9 years ago
#74
follow-up:
โย 86
@
7 years ago
I will add my thoughts, probably they are worth to be said (I dont talk about "Plugin dependency") :
There are many frameworks (like Visual-Composer, Gantry or many other PHP libraries and frameworks).
On WordPress repository, maybe thousands of themes/plugins use the same package...
Thus while I have 20 plugins and 5 themes installed on my site, i have same library installed 5-10 times (in individual plugin/theme folders).
What if WordPress created a flexible repository for Frameworks/libraries, that can be called from plugins/themes easily. For example, in "my-xyz-plugin".
<?php /* Plugin Name: My XYZ Plugin .... */ wp_include_framework('guzzle', '4.15'); ... ... ?>
so, the "wp_include_framework" should be native WP function, which will do the following (phseudo-code):
function wp_include_framework($name, $version){ check if folder doesnt exist if(!is_dir(ABSPATH.'/wp-content/frameworks/guzzle/4.15/')){ download_and_unzip("svn.wordpress.org/frameworks/guzzle/4.15.zip"); } include_once(ABSPATH.'/wp-content/frameworks/guzzle/4.15/loader.php'); }
So, advantages:
- when framework is updated by vendor, then all of the plugin(or theme) developers dont have to download them by theselves.
- plugin developers only submit 1 changed line in the code
(i.e. 4.16)
without submitting whole library to SVN (which obviously causes overload of WP SVN LOGS & REVISION SYSTEM) - the
SVN.WORDPRESS.ORG/FRAMEWORKS
(or whatever it will be) will be much more visible place, where volunteers can monitor&fix security issues. - WP repository will need take less space :) (maybe, not a big deal, but ...)
- I dont know what "cons" does that idea has.
This was the key of the success of Composer, right?
p.s. if someone need to explicitly include the library into own plugin/theme folder, he can still do that of course.
#75
@
7 years ago
p.s. I have to add:
1) similar should be done for *JS* libraries too.
I think that will make things more standartised...
2) once in a while, wp-cron-job can loop thought active plugins and delete all folders in wp-content/frameworks/ which are not used in any active plugins by the function wp_include_framework.
#76
@
7 years ago
I had a free weekend some time ago, so I built a proof-of-concept plugin that illustrates a simple way, how plugins could declare some dependencies via plugin headers: โhttps://github.com/ideag/tinylibraries
#77
follow-ups:
โย 78
โย 80
@
7 years ago
With my opinion, plugin dependency in "plugin header" is the most undesired way.
I think "dependencies" list should be dynamic (dynamic function is much flexible), within plugins own variable, like:
//========= with PHP libraries =======// foreach (array("LibraryXYZ", "libraryABC") as $e) { wp_include_framework($e) }; //========= with JS libraries =======// foreach (array("jquery-ui", "fancybox", "bootstrap") as $e) { wp_ensure_to_download_library_if_not_exists($e) }; ... <script src="<?php echo $WP_FrameWork_URLS_Global_Array['jquery-ui']['js']; ?>" <link rel="stylesheet ......="<?php echo $WP_FrameWork_URLS_Global_Array['bootstrap']['css']; ?>"
I think that shared JS libraries will solve MUCH JS conflicts caused plugins (which incorrectly enqueue same js/jquery scripts)
#78
in reply to:
โย 77
;
follow-up:
โย 79
@
7 years ago
Replying to tazotodua:
With my opinion, plugin dependency in "plugin header" is the most undesired way.
I think "dependencies" list should be dynamic (dynamic function is much flexible),
The reason I went for more static approach, is that I do not want to have any footprint in runtime. If I'm resolving dependencies on activation/deactivation, that means I do not need to run any additional calls on pageload, frontend or wp-admin side.
#79
in reply to:
โย 78
;
follow-up:
โย 82
@
7 years ago
Replying to ideag:
In the terms of what you said, that is worse, because the activated plugin, during update (if in new update a new library is added) wont be called with "activation" hook, thus, it will break with update.
So, as @nacin advised, it should be checked on every new pageload (but while is_admin()), saving the state as transient or option.
#80
in reply to:
โย 77
;
follow-up:
โย 81
@
7 years ago
Replying to ideag:
I had a free weekend some time ago, so I built a proof-of-concept plugin that illustrates a simple way, how plugins could declare some dependencies via plugin headers: โhttps://github.com/ideag/tinylibraries
Replying to tazotodua:
With my opinion, plugin dependency in "plugin header" is the most undesired way.
I think "dependencies" list should be dynamic (dynamic function is much flexible), within plugins own variable, like:
My two cents: Plugin Header is the way to go and then WordPress core should just "handle it."
@ideag: That said, how does TinyLibraries handle incompatible versions? Let's say I have Foo plugin and Bar plugin and they both use Baz library, but Foo uses 1.3 and Bar uses 2.5. Assuming โSemVer then you'd have a situation where either Foo or Bar would be run on the site but not Foo and Bar at the same time because their different versions of Baz would conflict.
I wrote a library 5 years ago(!) called โImperative whose goal was to allow inclusion of a library that would (theoretically) never change and thus any plugin could load it and be confident they had the right version. In it is a register_library()
method that allows a plugin to register the libraries it needs.
Imperative uses the rules of SemVer which means that if Foo plugin uses Baz library 1.3
and and Bar plugin uses Baz library 1.7
then Imperative would load only 1.7
which should theoretically satisfy both Foo and Bar. But if a new version of Bar came alone with Baz library 2.0
then Imperative would tell the user they could not activate Baz unless they disable Foo. It also suggests to contact the developer of Foo to get them to upgrade to Baz 2.0.
But what I found out was that Imperative was rather fragile because WordPress did not have enough hooks and was very inconsistent in how core works with plugins in the areas that Imperative needed to deal with and that the core team was not interested in addressing any of the issues so I gave up on developing plugins (and focused on the individually more profitable approach of developing complex websites for clients.)
Even so I would still love to see dependencies addressed in core. But to do so would require dealing with incompatible library versions if they are to be shared among plugins and themes. Although I fear it will never happen.
#81
in reply to:
โย 80
;
follow-up:
โย 83
@
7 years ago
Replying to MikeSchinkel:
My two cents: Plugin Header is the way to go and then WordPress core should just "handle it."
@ideag: That said, how does TinyLibraries handle incompatible versions? Let's say I have Foo plugin and Bar plugin and they both use Baz library, but Foo uses 1.3 and Bar uses 2.5. Assuming โSemVer then you'd have a situation where either Foo or Bar would be run on the site but not Foo and Bar at the same time because their different versions of Baz would conflict.
I completely agree, that this should be a part of the Core. My plugin is just an attempt to display one way how this could be handled.
Regarding versioning, I think WordPress should always update libraries to the latest stable version. If a new version of the library comes out, WordPress would update it the same way it updates itself, themes, plugins and translations. And plugin developers would need to stay up to date with that. The same way they have to stay up to date with core API's and Core version of jQuery, etc. I know that's a bit idealistic, but I think this would be good for the ecosystem as a whole. And Core would not need to deal with the mess of 12 different plugins requiring 12 different versions of the same library.
#82
in reply to:
โย 79
@
7 years ago
Replying to tazotodua:
Replying to ideag:
In the terms of what you said, that is worse, because the activated plugin, during update (if in new update a new library is added) wont be called with "activation" hook, thus, it will break with update.
Are you talking about situations where update happens not via WP Admin?
#83
in reply to:
โย 81
;
follow-up:
โย 84
@
7 years ago
Replying to ideag:
Regarding versioning, I think WordPress should always update libraries to the latest stable version. If a new version of the library comes out, WordPress would update it the same way it updates itself, themes, plugins and translations. And plugin developers would need to stay up to date with that.
While I agree that it is theoretically ideal I don't think such a strategy could ever be practical.
One of WordPress' core principles is to minimize problems for end users, and part of that means no breaking things that the end users have no expertise to fix. So if a plugin requires version 1.x of a library and core were to force install 2.x of the library -- which by definition would include a breaking change -- then the end user would feel the pain and not have the knowledge or skill to fix the problem (it might require major code changes in the plugin or theme!)
In the case of the plugin or theme developer they could be using an older version of a library that is perfectly acceptable for their needs and for which there are no security concerns. Or maybe they are using an older version because the older one is better! For example it is possible that v2.x of a library requires twice the memory of v1.x and thus the developer chooses to stick with the earlier version of the library. Or the newer version could have bugs the older version does not have.
And let's say all plugins on the site use v1.x out of choice so it would be a very bad idea to force them both to a breaking change in v2.x just because it is a newer version.
Plugins and theme choices are the domain of the end user but libraries are the domain of the developer so I think whatever would happen with libraries would need to be 100% invisible to end users and library support should cater to the needs of developers, not end-users. Forcing upgrades could potentially cause problems for both without certain benefit in many cases.
Bottom line, whatever is done, if anything is done, it has to be pragmatic and have almost zero possibility of negatively impacting end users.
Anyway, #jmtcw
P.S.The more I think about it the more I wonder if we shouldn't use add our own namespaces to libraries so that plugins could use different versions of the same libraries and just handle it that way?
#84
in reply to:
โย 83
@
7 years ago
I am worried, why no-one read my suggestion?
I will repeat once again, what if the libraries were requested by version (from the parent plugin), like:
require_library('library-name', '1.32');
thus, WP can create the structure of frameworks, like this:
wp-content/framework/library-name/1.32
wp-content/framework/library-name/1.31
....
wp-content/framework/library-name/latest <---- this could always be a latest version
and once in a while, WP should check all plugins/themes, and not-requested libraries/frameworks should be removed from site.
#86
in reply to:
โย 74
@
5 years ago
Replying to tazotodua:
There are many frameworks (like Visual-Composer, Gantry or many other PHP libraries and frameworks).
On WordPress repository, maybe thousands of themes/plugins use the same package...
Thus while I have 20 plugins and 5 themes installed on my site, i have same library installed 5-10 times (in individual plugin/theme folders).
#88
@
4 years ago
Here's some perspective about issues I've had when developing WordPress plugins that use Composer to manage dependencies.
Many plugin developers use Composer to manage PHP dependencies in their plugins, however, this may cause hard-to-debug issues if two plugins use the same library. Because of how Composer works, it will only load one version of that dependency at a time. Imagine the following scenario:
- Plugin Developer 1 creates a plugin named Foo that uses version 2.0 of a library Bar
- Library Bar developer releases version 3.0 of their library which renames function get_baz() to get_qux().
- Plugin Developer 2 creates a plugin name Quuz that uses version 3.0 of the library Bar
An user installs both plugins at the same time. Now their site crashes because one of the plugins calls a missing function.
There are some ways developers have worked around this, for example using โhttps://github.com/coenjacobs/mozart , โhttps://github.com/TypistTech/imposter-plugin or โhttps://github.com/humbug/php-scoper which add an extra unique namespace to files defined as Composer dependencies but these tools are error-prone and don't work if the library checks dependencies in runtime.
I think it could be great to have some sort of way to fix this in the core. According to the Composer developers, Composer was never meant to be used on plugin level, only at the project level where each plugin would be its own dependency and Composer could calculate any dependency conflicts run time.
I think two ways to fix this would be:
- Install WordPress (and its plugins) using Composer like Drupal does โhttps://www.drupal.org/docs/extending-drupal/installing-modules . I think this would be the best way to fix this but it's not really that great if the majority of WordPress installations are on shared hosting and can't run Composer. Drupal has a thing named Ludwig โhttps://www.drupal.org/project/ludwig which works from the Drupal admin interface and allows you to manually download and install the required dependencies.
- Distribute libraries on WordPress.org plugin directory. This used to an option but now libraries are explicitly disallowed in the plugin directory โhttps://make.wordpress.org/plugins/2016/03/01/please-do-not-submit-frameworks/ . If libraries would be distributed on WordPress.org, you could at least check if the library is the correct version in your plugin before using it. There's also the issue of ownership, who will upload the libraries and keep them up to date?
Without any solution to this plugin developers who want to use libraries will have to use workarounds or simply hope that users do not install libraries with different versions. Please note that this issue is not unique to Composer, you can run into the same problem even if two plugins require the library manually.
Some more info about this issue is in this blog post: โhttps://deliciousbrains.com/php-scoper-namespace-composer-depencies/
This ticket was mentioned in โPR #1547 on โWordPress/wordpress-develop by โaristath.
3 years ago
#89
- Keywords has-patch added
This patch adds the ability to define plugin dependencies using a dependencies.json
file.
With this implementation, a plugin can add a dependencies.json
file in its root folder. Example of such a file:
{{{json
[
{
"slug": "gutenberg",
"name": "Gutenberg",
"version": "11.0"
},
{
"slug": "woocommerce",
"name": "WooCommerce"
}
]
}}}
so it's an array of objects.
Each item in the array must include a slug
and a name
. The slug
is used to install the plugin, and the name
is used to properly word the requirements notifications.
Optionally they can define a version
, which is the minimum required version.
By using the "version" arg as a minimum-required version we don't have issues with versions conflicts and over-complicating things. All plugins should at all times be at their latest version. Having a minimum-required version will simply prevent a plugin from being activated if one of its dependencies is older than the minimum-required.
When we try to activate a plugin with the above JSON file, 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
#90
@
3 years ago
- Milestone changed from Awaiting Review to 5.9
Moving to the milestone to discuss โPR #1547 by @aristath.
In my testing, it looks good as a proof of concept and seems like a viable approach to solve plugin dependencies.
Any feedback welcome :)
โChrico commented on โPR #1547:
3 years ago
#91
You should use ComPoser and the composer.json
for that.
โaristath commented on โPR #1547:
3 years ago
#92
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 commented on โPR #1547:
3 years ago
#93
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:
`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..?
Even it would tie it to Composer (i mean, everything is tied to something, right?), we still would build on top of an excisting eco-system which can be used, but hasn't to be used and already provides enough information to work with.
โgmazzap commented on โPR #1547:
3 years ago
#94
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 functionality relies on the fact that everyone writes endlessly backward compatible code, like core (in theory) does, and that is simply not the reality (nor is desirable IMO because only brings technical debt, as core as 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 system _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.
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 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 industry standards for the version requirements would be an improvement.
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:
{{{json
{
"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
:
{{{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 source of JSON 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 type 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, but it 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 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 commented on โPR #1547:
3 years ago
#95
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.
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._
Composer handles dependences, and plugins are packages. _Many people already use composer to install plugins as well as plugin dependencies_, dependencies that are themselves plugins. There's even an official package type for WordPress themes and WP plugins. Not all packages are libraries.
Many people install WordPress itself as a composer package.
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.
I do not see how this would be different if the file was renamed to dependencies.json
.
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.
Core itself can handle the activation, extra
is just for automation. Otherwise there are no advantages to dependencies.json
that aren't already 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 viable competitors, and overwhelming backing from the wider PHP community and many people in the WP community.
---
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 ). Composers closest rival is likely the .org plugin repo, which it supports via WPackagist.
- 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
- get instant support out of the box or several plugins that already implement this via
- 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.
---
_If the goal here is just to focus on activation, why was a line in the plugin header not considered?_
#96
@
3 years ago
Noting that my comment updates were not carried over by PRBot. Specifically
Even then, why not use a line in the plugin header?
<?php /** * Plugin Name: Foo * Requires Plugins: Bar, Gutenberg * Requires PHP: 7.2.0 * etc...
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
.
This also avoids the future issue of a plugin.json
being introduced that duplicates this further should the plugin information be moved out of the PHP header, or issues is a JS tool uses the generic dependencies.json
filename.
โMte90 commented on โPR #1547:
3 years ago
#97
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).
![](โhttps://imgs.xkcd.com/comics/standards.png)
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 commented on โPR #1547:
3 years ago
#98
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 commented on โPR #1547:
3 years ago
#99
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 commented on โPR #1547:
3 years ago
#100
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 commented on โPR #1547:
3 years ago
#101
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 commented on โPR #1547:
3 years ago
#102
โaristath commented on โPR #1547:
3 years ago
#103
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 commented on โPR #1547:
3 years ago
#104
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. :)
โaristath commented on โPR #1547:
3 years ago
#105
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 commented on โPR #1547:
3 years ago
#106
@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, butIMO before this PR is merged:
- you should first check that wp.org has no plugin with thier 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 commented on โPR #1547:
3 years ago
#107
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 :+1:
โgmazzap commented on โPR #1547:
3 years ago
#108
@aristath the following should work, but needs testing ;)
{{{php
/
- 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($data)
{
$required = empty($dataRequires Plugins?) ? : $dataRequires Plugins?;
Optimization: not worth further processing if not even a quotes couple is there.
if (!$required (substr_count($required, '"') < 2)) { return $required ? wp_parse_list(str_replace('"', , $required)) : [];
}
Remove quotes with nothing inside: that break the regex.
$required = str_replace('""', , $required);
preg_match_all('#"(["]+)"#', $required, $matches);
Remove from the header the slugs already parsed, then parse the rest.
$slugs = wp_parse_list(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 commented on โPR #1547:
3 years ago
#109
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 commented on โPR #1547:
3 years ago
#110
Wouldn't str_getcsv
be easier for handling the parsing?
{{{php
$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 commented on โPR #1547:
3 years ago
#111
@tomjn That's perfect! It didn't even cross my mind... Tested and pushed, works brilliantly
โgmazzap commented on โPR #1547:
3 years ago
#112
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.
โaristath commented on โPR #1547:
3 years ago
#113
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 :thinking:
โtomjn commented on โPR #1547:
3 years ago
#114
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 commented on โPR #1547:
3 years ago
#115
I use to write unit tests for my code, and that simply don't pass unit tests (not even with space trimming) :)
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 commented on โPR #1547:
3 years ago
#116
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:
<img width="593" alt="Screenshot 2021-08-09 at 12 47 20" src="โhttps://user-images.githubusercontent.com/58855/128701531-42ee2787-269c-48e0-b282-d4da116d6d44.png">
The scenario we're concerned about cannot happen
โaristath commented on โPR #1547:
3 years ago
#117
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 commented on โPR #1547:
3 years ago
#118
The slug is not about filenames, but the main plugin folder-name
Do you know that plugins can be a single file name?
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 si 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 commented on โPR #1547:
3 years ago
#119
Core itself appears to do trimming, in the case of " foo "
it strips the first space out
โgmazzap commented on โPR #1547:
3 years ago
#120
@tomjn only for folders, and only for leading spaces. Check my previous comment. a plugin with a single file named foo .php
is fine for WP.
โgmazzap commented on โPR #1547:
3 years ago
#121
The point is for me:
Do you want to support edge cases of plugins named
, foo .php
?
No -> use explode
Yes -> use something like what I posted.
โaristath commented on โPR #1547:
3 years ago
#122
single-file plugins cannot be installed though... A user can create them, use them etc, but these are one-off plugins (with the exception of Hello Dolly
for historical reasons). When WordPress Core installs a plugin, it always creates a folder for it.
โtomjn commented on โPR #1547:
3 years ago
#123
I can confirm that a single file plugin that begins with a space ( " foo .php"
) fails to activate with the same issues as folders. This edge case is not currently handled by core
โgmazzap commented on โPR #1547:
3 years ago
#124
@tomjn I've just installed a plugin named , foo .php
and it works perfectly.
โgmazzap commented on โPR #1547:
3 years ago
#125
โgmazzap commented on โPR #1547:
3 years ago
#126
Again, I don't think it is *extremely important* to support the edge cases... but if you don't want to, then just use explode
.
โaristath commented on โPR #1547:
3 years ago
#127
I believe if we get something in core, we can handle edge-cases as they come, and improve the implementation as we go. There is no "perfect" solution, but the current solution covers most cases. Since single-file plugins are not installable, they cannot be used as dependencies anyway, so not a concern IMO.
It is possible to depend on a single-file plugin in a custom-built, one-off plugin for a custom site, but in that case, it's saner to just use a correct filename for the single-file plugin :shrug:
โaristath commented on โPR #1547:
3 years ago
#128
I believe if we get something in core, we can handle edge-cases as they come, and improve the implementation as we go. There is no "perfect" solution, but the current solution covers most cases. Since single-file plugins are not installable, they cannot be used as dependencies anyway, so not a concern IMO.
It is possible to depend on a single-file plugin in a custom-built, one-off plugin for a custom site, but in that case, it's saner to just use a correct filename for the single-file plugin :shrug:
โgmazzap commented on โPR #1547:
3 years ago
#129
Again, then use explode
as you did.
The idea of str_getcsv
was proposed to handle quotes. And quotes were proposed to handle single-file plugins with edge case naming.
If you don't want to handle those, then use explode
. It's faster, it's simpler.
โtomjn commented on โPR #1547:
3 years ago
#130
I think it's reasonable to require plugins that start or end in spaces to be wrapped in strings. Otherwise we'll need code to test if it really does include spaces or if the developer separated them for whitespace purposes, and I can see some mismatched expectations happening there
explode
doesn't cover the user of other special characters such as ,
โSergeyBiryukov commented on โPR #1547:
3 years ago
#131
As it looks like there might have been some initial confusion about the PR, just wanted to clarify that we're not building a PHP package manager, the PR is mostly focused on WP admin plugin UI improvements to account for dependencies. The title is now updated to reflect that ๐
โkraftner commented on โPR #1547:
3 years ago
#132
@SergeyBiryukov Maybe now, but I'm afraid this is a slippery slope that might lead to exactly creating a half-baked package manager in the long term when this partial solution shows as incomplete and is then slowly expanded fix after fix.
To summarize what I said on the parallel uproar on Twitter:
Do it right or don't do it at all. Composer or nothing. Everything else complicates an already terribly complicated and messy situation further.
โSergeyBiryukov commented on โPR #1547:
3 years ago
#133
As for plugins having spaces or commas in their slugs, I think that is quite an edge case that does not necessarily has to be solved in this initial implementation, and can be solved it later if needed.
I don't have any strong objections against keeping str_getcsv()
, other than it would be the only instance in WordPress core.
For now, explode( ',' ... )
along with trim()
would probably also work just fine.
โgmazzap commented on โPR #1547:
3 years ago
#134
@tomjn when I say explode, I mean: array_filter(array_map('trim', explode(',', $data)))
.
If you use str_getcsv
you'll still need to do array_filter(array_map('trim', str_getcsv($data)))
.
What you won't cover in both cases is:
- single plugin files that have trailing spaces in the slug
- probable result of a typo, e.g.: a spare quote (
foo, "
) or empty double quotes (foo, ""
), a comma in quotes (foo, ","
), and other edge cases
What you'll cover with str_getcsv
but _not_ with explode
:
- plugin slug which contains comma/spaces "in the middle"
Do you think it is worth using str_getcsv
just for that? If so, then go :)
โgmazzap commented on โPR #1547:
3 years ago
#135
@kraftner even if Composer support would be added, Composer can't do anything about _activating_ plugins, but only put plugins in place.
This means that a WP proprietary system _on top_ of Composer is needed anyway to handle that kind of stuff.
If Composer support would be there, there would likely be a WordPress Composer plugin, that would handle this kind of stuff, taking configuration from a wp-plugin.json
or something like that. That is the "desired direction".
That's why I was _so_ against the original dependencies.json
and the definition of dependencies versions (which have nothing to do with plugin activation mechanism): that would be very against that "desired direction".
Having a plugin header don't think will be against that: I assume at some point WordPress will want to have Javascript-only plugins (๐) and at that point, I can imagine plugin headers will be migrated to a wp-plugin.json
or something like that.
And that file will be readable by Composer and by a WP Composer plugin...
โgmazzap commented on โPR #1547:
3 years ago
#136
@kraftner even if Composer support would be added, Composer can't do anything about _activating_ plugins, but only put plugins in place.
This means that a WP proprietary system _on top_ of Composer is needed anyway to handle that kind of stuff.
If Composer support would be there, there would likely be a WordPress Composer plugin, that would handle this kind of stuff, taking configuration from a wp-plugin.json
or something like that. That is the "desired direction".
That's why I was _so_ against the original dependencies.json
and the definition of dependencies versions (which have nothing to do with plugin activation mechanism): that would be very against that "desired direction".
Having a plugin header don't think will be against that: I assume at some point WordPress will want to have Javascript-only plugins (๐) and at that point, I can imagine plugin headers will be migrated to a wp-plugin.json
or something like that.
And that file will be readable by Composer and by a WP Composer plugin...
โtomjn commented on โPR #1547:
3 years ago
#137
I think then we can agree:
- this is for plugin activation and installation
- specifically plugins hosted on .org
- plugins with spaces and commas in their names aren't on .org, but this edge case can be handled in iterations
- we're moving forward with a plugin header, not a dedicated JSON file
I think any discussion of using composer.json
or a dedicated JSON file for plugin headers is an interesting discussion, but one for a new ticket/PR. We should focus on the task of activating and declaring a plugin depends on another plugin within the scope of .org. 3rd party sources and repositories are out of scope.
โkraftner commented on โPR #1547:
3 years ago
#138
@gmazzap Yes, but as you've โperfectly pointed out before there are plenty of issues with versions and alike when just handling the activation use case.
That is why I think it either needs to be handled fully (version conflicts, installation, activation,...) or not at all. With nothing, like now, it is at least obvious that these things are on you and you better be careful. Half a solution projects this feeling of "Great, WP now handles dependencies. Brain off." From the point of view of the user as well as from the point of view of a dev who might skip checking for a plugin dependency now with the wrong assumption of WP handling this.
To talk in pictures: When standing on a cliff I think the raw edge is much safer than a railing that looks nice but snaps as soon as I lean against it.
(I am aware this might be โmy personal disdain for "Move fast and break things" and releasing alpha-grade "solutions" with the arguments of "better than nothing" and "iterative approach". I can see how this is a fundamental believe not everyone agrees with, still I felt like this needs to be at least mentioned here.)
โChrico commented on โPR #1547:
3 years ago
#139
this is for plugin activation and installation
no! installation is not activation :D Installation (resolving dependencies) is what Composer does. This PR should focus _only_ on activation.
plugins with spaces and commas in their names aren't on .org, but this edge case can be handled in iterations
You can, but you shouldn't, ignore that. You need to handle all valid chars for file and folder namse for plugin/theme slugs. You cannot simply ignore them, because: , foo .php
!== ,foo .php
!== ,foo.php
!== foo .php
. You either need to sanitze _now_ those slugs to ensure that those "duplications" are not existent, or you need to support those "special cases". All in all: write Unit Tests! :)
โtomjn commented on โPR #1547:
3 years ago
#140
Lets not forget this PR is for this trac ticket https://core.trac.wordpress.org/ticket/22316 which _does_ cover installation, citing installation of a plugin from wp.org explicitly
โgmazzap commented on โPR #1547:
3 years ago
#141
So this is also for installation? @tomjn
The description in here does not mention that.
That is a so bad decision IMO.
Among the "let's invent here a package manager" that @kraftner was pointing (and now I get why he said that) as soon as I write a custom plugin from my client that happen to have same slug of plugin in wp.org things will explode.
Not to mention that a plugin can force on my site a plugin I don't know nothing about?
I don't want any of clients' sites to ever having the deal with this.
I have client that want security & privacy assessment for every single line of code that is added to their website, this would mean that code can land on their websites without nobody being able to review... (I wonder how hostings like wp.com VIP Go that do pre-deployment code review will like this).
Please reconsider this. Please stop this.
Or at very least introduce an opt-out mechanism: a constant, a filter, whatever.
WP does not want Composer? Too bad, we use it anyway, and it is like having a Ferrari and being forced to drive a scrap.
โtomjn commented on โPR #1547:
3 years ago
#142
There are already opt outs and filters that were added in 5.8 for that
exact situation, and the context is .org plugin installation. This PR is
not the ticket, and comments here are not the complete discussion. Refer to
Trac for that discussion and the context.
The purpose of the ticket and PR is not to implement a composer competitor
or a package manager, but to facilitate .org repo plugins that have
dependencies on .org.
As a PR this is not a place to argue for the tickets scope and nature to be
fundamentally changed. Implementing composer is a different task.
โtomjn commented on โPR #1547:
3 years ago
#143
Also as someone who did WP.com VIP and VIP Go review I can say with
relative certainty that they have UI based plugin installation disabled.
This would be a non-issue as it relates to a feature those platforms and
most other enterprise WP platforms do not use or enable.
โgmazzap commented on โPR #1547:
3 years ago
#144
The purpose of the ticket and PR is not to implement a composer competitor or a package manager, but to facilitate .org repo plugins that have dependencies on .org.
That is exactly a dependency-solving issue, which is exactly what a "dependency manager" (Composer) does. So to me sounds like: _"the purpose of this ticket is not to implement a Composer competitor, but a WordPress-specific Composer alternative"_.
Which s as equally bad IMO.
There are already opt outs and filters that were added in 5.8 for that exact situation
Can you point me to those filters?
can say with relative certainty that they have UI based plugin installation disabled.
How that is relevant? If I commit a plugin that depends on one or more plugins, once that plugin is online it will install the other plugins (if I understood correctly), but the PR that caused the deployment did not include the required plugins, hence those landed online without review.
Or am I missing something?
โtomjn commented on โPR #1547:
3 years ago
#145
That is exactly a dependency-solving issue, which is exactly what a "dependency manager" (Composer) does. So to me sounds like: "the purpose of this ticket is not to implement a Composer competitor, but a WordPress-specific Composer alternative".
We already have our own system, implementing minimal dependencies is not the same as replacing the entire .org plugin and theme system with composer. That is a huge undertaking that should be a separate issue.
Keep in mind that the plugin header system can be integrated into WPackagist, improving support in existing Composer based setups.
Can you point me to those filters?
At least one of them:
โhttps://make.wordpress.org/core/2021/06/29/introducing-update-uri-plugin-header-in-wordpress-5-8/
.org plugins having slugs that also match a local plugins name, and other naming conflicts would not be a new problem.
The proposed header is unique to .org plugins, this is not a generic package manager with arbitrary dependencies. Nobody has specified that git repos, zip URLs, subversion tags, and other resources as a dependency in a plugin header. The suggestion that a non .org plugin may be declared in this plugin header does not fit the proposed feature on the trac ticket.
How that is relevant?
You mentioned it. The feature this PR and the ticket is focused on simply does not apply to those platforms, and would be disabled by those platforms if it did. If a user of those platforms were to commit such a plugin without its dependencies I do not see how it would be any different before or after this features implementation. Even if it were implemented today using a composer.json
without a build process and vendor
folder.
but the PR that caused the deployment did not include the required plugins, hence those landed online without review.
Assuming there was no review and no testing, this could happen but that is already the case and this feature would not change that for those platforms, nor is it within the scope. composer
would not change this either unless there was some kind of CI step which could catch this issue, which would be the job of the platform.
โSergeyBiryukov commented on โPR #1547:
3 years ago
#146
If I commit a plugin that depends on one or more plugins, once that plugin is online it will install the other plugins (if I understood correctly), but the PR that caused the deployment did not include the required plugins, hence those landed online without review.
Or am I missing something?
No, that does not happen automatically.
As shown on the screenshot from the PR description, if a plugin requires a few other plugins that are not currently installed or activated, installing and activating them is an explicit manual action for clarity.
โChrico commented on โPR #1547:
3 years ago
#147
As shown on the screenshot from the PR description, if a plugin requires a few other plugins that are not currently installed or activated, installing and activating them is an explicit manual action for clarity.
But this is kind of "manual" dependency resolving by trying to install (download) until it breaks. If Plugin A requires Plugin B, then you need to install Plugin B, Plugin B requires C and D and C requires F and J and D requires A - but not in the version you currently have it installed...at some point it just breaks.
โgmazzap commented on โPR #1547:
3 years ago
#148
I feel like I've spent too much time on this issue, but anyway can't help but have to answer one again.
@SergeyBiryukov is saying that this PR is only about checking the _activation_ of plugins' dependencies.
@tomjn is saying that the track ticket this PR refers to is about _installing_ plugins' dependencies.
The difference is not trivial.
In the first case, WordPress is not acting as a dependency manager, not being able to install dependencies.
In the second case, WordPress is acting as a dependency manager. A pretty broken one TBH.
A _proper_ dependency manager takes care of installing the proper version of dependencies, takes into account updates, and many other things. The fact that dependencies are limited to .org plugins doesn't change it.
Let's imagine that WP will install .org plugin dependencies based on a plugin header:
- What will happen when that plugin is (auto-)updated and changes its requirements?
- What will happen if
DISALLOW_FILE_MODS
is true and so WordPress itself is not supposed to change the file system? - What happens if a dependency is removed/suspended from the wp.org repo? Is there a mechanism to check which other plugins are depending on it?
- How do we want to deal with abuses? That is plugins vendors to force the installations of all their plugins for marketing purposes? Assuming the wp.org plugins review team will check if there's a real dependency, what will stop vendors to use a single plugin function from other plugins for no real reason but just to claim it is a "real" dependency?
- What will happen if the "main" plugin has
Requires PHP: 5.6
, but the latest version of dependencies hasRequires PHP: 7.0
andRequires PHP: 7.1
? Imagine that when the main plugin introduced the dependencies they had no that requirements, but they got updated and now they have.
These are only a few of the open questions that would be needed to answer before introducing such "automatic installation" of dependencies, and I'm pretty sure some of the answers will never be satisfactory. Because handling dependencies is a complex problem, and a trivial dependency manager doesn't make dependency management trivial.
Answering those questions in the best possible way would mean building a full-fledged dependency management system, something that really makes no sense.
So, please, leave the dependencies installation alone.
If a plugin _A_ depends on plugins _B_ and _C_, WordPress can surely check if those plugins are installed and activated, if so will continue with plugin _A_ installation, otherwise will stop it and show a notification. _Eventually_ providing a link that will trigger a one-click installation of _B_ and _C_ (if DISALLOW_FILE_MODS
is false).
If the dependencies are not there/not activated it will be up to the site owner to evaluate which version of the dependencies to install and *how* to install: UI, WP CLI, Composer... WP should not be concerned, just like is not concerned right now.
This is what this PR will do (as far as I understood), and I see no problem with that.
---
To answer @tomjn:
At least one of them:
Having a filter that prevents conflicts between a local and a wp.org plugin is nice, thank you, but the opt-out mechanism I want is something that prevents WordPress to automatically install dependencies, if that becomes a thing. But I hope that setting DISALLOW_FILE_MODS
to false will already do it.
We already have our own system, implementing minimal dependencies is not the same as replacing the entire .org plugin and theme system with composer.
Nobody wants to replace .org plugin and theme system. Those are SVN repositories and Composer works with SVN repositories. Writing a script that creates a Composer repository from the wp.org repos is trivial.
Keep in mind that the plugin header system can be integrated into WPackagist, improving support in existing Composer based setups.
First of all, WPackagist is a proprietary thing, maintained for free by a company that has said doesn't have interests in maintaining it. An "official" WordPress WPackagist equivalent will need to provide a composer.json
for plugins/themes generating one if not provided by the user, and will need to ship an official Composer installer. And have something to deal with translation packages.
And I've personally written Composer plugins that โread plugins headers to extend Composer functionality.
But plugins headers are not fit for the scope, not being able to store structured data, will never be able to support the needs of a dependency management system. Unless we want to start writing JSON in the headers...
And I'm waiting for the moment when it will be realized that Gutenberg extensions can be written without the need of any PHP, and people will write plugin headers just to make those extensions installable. How long before someone will say _"let's enable js-only plugins and get over plugins headers"_. I'm sure that from there to a wp-plugin.json
the step will be extremely short considering that pushing JS is the only change-driver these days in WP.
I do not see how it would be any different before or after this features implementation. Even if it were implemented today using a composer.json without a build process and vendor folder.
That's exactly the point. Using Composer you can have a build process that pulls vendors and runs static analysis and tests. If dependencies are installed by WordPress, to obtain the same result you need a working installation of WordPress, something that is a few orders of magnitude harder than running composer install && composer lint && composer tests
in a CI.
โkraftner commented on โPR #1547:
3 years ago
#149
But this is kind of "manual" dependency resolving by trying to install (download) until it breaks. If Plugin A requires Plugin B, then you need to install Plugin B.
Plugin B requires C and D and C requires F and J and D requires A - but not in the version you currently have it installed...at some point it just breaks.
And then this will be bad UX and it will be improved upon incrementally and very quick this is *exactly* โthe slippery slope I am talking about. Use composer or stop it right here before it can do any harm, please, pretty please!
โSergeyBiryukov commented on โPR #1547:
3 years ago
#150
Plugin B requires C and D and C requires F and J and D requires A - but not in the version you currently have it installed...
That's why we had the ability to specify a minimum required version in the initial implementation, though it was removed per some earlier feedback here.
@SergeyBiryukov is saying that this PR is only about checking the _activation_ of plugins' dependencies.
No, I'm not saying that :) I'm saying that both installing and activating is an explicit manual action in this implementation and does not happen automatically, for clarity and transparency. This way the user is aware of any dependencies and can make the choice to either install and activate them too, or cancel the initial activation request.
Let's imagine that WP will install .org plugin dependencies based on a plugin header:
I think these are all very valid questions that can be addressed individually.
โgmazzap commented on โPR #1547:
3 years ago
#151
And then this will be bad UX and it will be improved upon incrementally and very quick
Plugins that require plugins are not a new thing. Right now, if you try to install a WooCommerce or a Gravity Forms extension without those plugins being active, you got a notice, and the plugin is not activated.
If WP is willing to do that on behalf of single plugins owners (which end up in different types of notifications and inconsistent behavior) I don't see that as a huge issue.
The fact that declaring dependencies becomes easier will increase this practice? I bet. But to be honest, as long as there's an opt-out mehanism that makes me (who use Composer and have DISALLOW_FILE_MODS
to false) don't have to deal with that thing... I'm fine.
And once again, even if Composer support is added, WordPress will still need to handle these kinds of things, because even being able to declare in Composer that acme/woocommerce-extension
depends on woocommerce/woocommerce,
would not exempt WordPress to _activate_ WooCommerce when the extension is activated.
If this kind of mechanism is built now, without Composer, I don't see a problem because in an imaginary future where Composer support is added, that mechanism can be kept as-is.
โgmazzap commented on โPR #1547:
3 years ago
#152
I'm saying that both installing and activating is an explicit manual action in this implementation and does not happen automatically, for clarity and transparency.
@SergeyBiryukov Manual installation and activation are already possible :) So the _new_ things this PR does are:
- check the dependencies
- inform the user about unmet dependencies
what happens after that (manual installation and activation) is not a new thing. And in my personal opinion, if the work done on this ticket limits to that, I see no issues.
What I am only asking is to stop there, and don't do any additional step in the direction of "dependency management", or make sure that the steps in that direction involve Composer.
โkraftner commented on โPR #1547:
3 years ago
#153
And then this will be bad UX and it will be improved upon incrementally and very quick
Plugins that require plugins are not a new thing. Right now, if you try to install a WooCommerce or a Gravity Forms extension without those plugins being active, you got a notice, and the plugin is not activated.
If WP is willing to do that on behalf of single plugins owners (which end up in different types of notifications and inconsistent behavior) I don't see that as a huge issue.
The fact that declaring dependencies becomes easier will increase this practice? I bet. But to be honest, as long as there's an opt-out mehanism that makes me (who use Composer and have
DISALLOW_FILE_MODS
to false) don't have to deal with that thing... I'm fine.
And once again, even if Composer support is added, WordPress will still need to handle these kinds of things, because even being able to declare in Composer that
acme/woocommerce-extension
depends onwoocommerce/woocommerce,
would not exempt WordPress to _activate_ WooCommerce when the extension is activated.
If this kind of mechanism is built now, without Composer, I don't see a problem because in an imaginary future where Composer support is added, that mechanism can be kept as-is.
Because devs now know WP does nothing about it so they handle it on their own. If WP now suggests it *does* handle this I bet it will get sloppy pretty fast in reality since the responsibility is now assumed to be on WP. So *if* WP does anything in this realm it should be done better than the home-grown solutions of plugins right now. And my opinion is that that would need to include full dependency resolution which in turn mandates composer.
โgmazzap commented on โPR #1547:
3 years ago
#154
@kraftner I agree that if this PR is merged the next step will be _"how about installing the dependencies"_?
And at that point, only two possibilities will be there: homegrown a package manager or use Composer. And I hope who will take that decision will be savvy enough to don't attempt building a package manager.
So this PR, if merged, will be a push in the direction of dependencies management, let's just hope it's not a push into a ravine.
โtomjn commented on โPR #1547:
3 years ago
#155
My expectation is that if I install Foo, and it depends on Bar, that WP will tell me that and offer to install Bar via the UI, then to activate it, or both at the same time.
I think the installation of dependencies is being conflated with the automatic installation of dependencies.
I also think these issues are being introduced which the ticket did not provide scope for:
- unreviewed plugins that are not on .org
- dependencies of dependencies
- circular dependency detection
I would also make a final note on the subject of composer. If everybody aligned and WP added full native support for composer in every possible way, bundling composer internally, using it to install plugins themes and any of their library dependencies, autoloaders etc we would still need to operate and maintain the wp.org system, and it would still something we'd want basic dependencies for. If this PR introduced a WP package system then that package system was introduced more than a decade ago and predates composer. The WP.org plugin repo isn't going away, and it's not a replacement for composer
.
โaristath commented on โPR #1547:
3 years ago
#156
I think there was a misunderstanding in this ticket.... We're not trying to build a package manager here. I'll use WooCommerce for this example because it's something most people are familiar with.
What we need, is a way to show users that a WooCommerce addon they installed will not work unless they also install WooCommerce. A way to guide them through that installation and activation, and a way to prevent the activation of the addon unless WooCommerce is also installed, in order to avoid errors on the site.
That's all this PR is trying to achieve, that's what the focus was, and from reading core tickets that's what most plugin authors have been asking for for years.
Building a package manager the way it is presented above would be an exercise in futility and yes, if that's what we wanted to achieve then composer would be the way to go. But that's not what we're trying to achieve here... All that "plugin dependencies" means here, is that "Plugin A needs plugin B in order to function, so please install plugin B before activating plugin A". That's it.
โaristath commented on โPR #1547:
3 years ago
#157
unreviewed plugins that are not on .org
Since the implementation uses the plugin headers in order to provide a list of slugs, the WordPress API is used for the installation of these plugins. Now, if a plugin hooks in the updater and changes the requests in order to install an unreviewed plugin from a 3rd-party site, there's nothing we can do about it. It's not something new, and they can do that already if they want to. Nothing changes.
dependencies of dependencies
If plugin A requires plugin B and plugin B requires plugin C, then when activating plugin A there is a prompt to activate plugin B. When plugin B gets activated, there is a prompt to install plugin C. Ideally when activating plugin A there would be a prompt to install both plugins B & C, but that may require a tweak in the .org API, or at least an extra ping in the plugins API to get the dependencies of dependencies. It's possible, but it's something that can only be explored after there are a couple of plugins on .org with dependencies defined so we can further iterate.
circular dependency detection
I have not tested that, but it should be easy to test and fix if we decide to proceed.
โtomjn commented on โPR #1547:
3 years ago
#158
I have not tested that, but it should be easy to test and fix if we decide to proceed.
Given that this is user UI driven, if we have a circular dependency they'd have to install the next plugin in the chain at each step, when they come around full circle they'd find the plugin already installed and WP would be satisfied. I'm unsure about activation
โaristath commented on โPR #1547:
3 years ago
#159
Given that this is user UI driven, if we have a circular dependency they'd have to install the next plugin in the chain at each step, when they come around full circle they'd find the plugin already installed and WP would be satisfied. I'm unsure about activation
Yeah, the activation part is what will need a couple of tweaks... Plugin A is not actually activated until its dependencies are met, which requires plugin B to be activated, but it can't be activated because its dependencies are not met since plugin A is not active. But it's an easy fix :+1:
โkraftner commented on โPR #1547:
3 years ago
#160
@aristath @tomjn You're *already* talking about a dependency resolution algorithm with conflict management and the added "benefit" of a "mechanical turk" aka user to sidestep automation, aren't you?
โgmazzap commented on โPR #1547:
3 years ago
#161
@tomjn
The WP.org plugin repo isn't going away, and it's not a replacement for composer.
I never said that, and I think nobody wants that.
And TBH I don't think that is something that makes sense.
The WP.org plugin repo is a repository, Composer is a package manager that searches repositories, resolves dependencies, and then installs packages from the repositories.
Composer can't do anything if there's no repository. And nobody is expecting that WP will move to Packagist as a repository registry. (At least, I'm not).
Composer have also a mechanism to install packages (aka download from the repo and place them in the proper folder). That mechanism can be extended/overridden via custom installers and is likely what WP will need to do if it wants to integrate Composer.
Nobody thinks that the installation via UI .org plugins/themes will ever go away. But I don't see why the two can't survive side by side. And I am aware that would require effort. In WP code, but not only in code.
---
@aristath
That's all this PR is trying to achieve, that's what the focus was, and from reading core tickets that's what most plugin authors have been asking for for years.
Yep, and as I said multiple times I'm see nothing wrong with that. But what I and others are saying is that as soon as this will be added, the "desire" to have those dependencies installed automatically will surely raise, and we want to avoid doing that using a homegrown solution.
That "desire" will raise very soon when people will start using this mechanism and dependencies of dependencies, of dependencies... will be a thing, and the users will need 30 clicks to install a plugin.
On top of that, what I personally would like to see is an opt-out mechanism from all of that. And considering the whole process, as you describe it, is based on UI, please make sure that when plugin management via UI is disabled (e.g. when DISALLOW_FILE_MODS
is true) the changes in this PR don't apply.
If plugin A requires plugin B and plugin B requires plugin C, then when activating plugin A there is a prompt to activate plugin B. When plugin B gets activated, there is a prompt to install plugin C.
When you say "activate" you mean "install and activate" (assuming is not installed), correct?
But please be sure that before merging you test the UX in the scenario in which A requires B, and B requires C, D, E, F, G, H, I, L, M, and _each_ of those requires _a few_ dependencies, and so on.
And also please consider what happens when one or more of those dependencies _can't_ be installed, because requires a PHP version that is not available, or can't be found in the wp.org API (removed/suspended).
And also please consider the case one or more of the dependencies is _Network only_ but the user is in the site-specific plugin screen.
And also please consider the case one of the dependencies as a fatal error and so is forcefully disabled.
_"That's it"_ makes it look simpler than it is, IMO.
โaristath commented on โPR #1547:
3 years ago
#162
Yep, and as I said multiple times I'm see nothing wrong with that. But what I and others are saying is that as soon as this will be added, the "desire" to have those dependencies installed automatically will surely raise, and we want to avoid doing that using a homegrown solution.
Automatically installing and/or activating something without the user's explicit consent is a big no-no for this implementation. We all like automation, but we can't know why the user has chosen to have a plugin deactivated. Maybe that plugin when activated breaks their entire site and they spent a week debugging it. We cannot in good conscience auto-activate something without their permission.
That "desire" will raise very soon when people will start using this mechanism and dependencies of dependencies, of dependencies... will be a thing, and the users will need 30 clicks to install a plugin.
I agree. But let's get a basic implementation first and not get ahead of ourselves... A dependency of a dependency of a dependency, or a plugin with 10 dependencies are highly unlikely. In most scenarios I can think of, a plugin shouldn't have to require more that 1-2 plugins in order to function. But even if we do get to a point of deeply-nested dependencies and plugins that require 10 plugins, clicking 30 times is easier than searching the repo for the dependencies manually and installing them one by one.
Is it perfect? Of course not, nothing is! But it's better than what we have now, it's a start, and it's something we can iterate on as we go. But we can't iterate on something that currently doesn't exist. The current problem is that there is no way to define dependencies or guide users on what they have to do in order to get their plugin to work. If and when we come across deep-nested dependencies, we can fix the issue then. Maybe it will never happen... No reason to over-engineer a solution that may never be needed when the current solution - though imperfect - will still work and guide users.
On top of that, what I personally would like to see is an opt-out mechanism from all of that.
Opt-out of what exactly? The notification that will be shown, urging you to activate WooCommerce when you install a plugin that depends on WooCommerce? Or opt-out from the fact that a woocommerce addon will not be enabled unless woo is also installed & activated?
And considering the whole process, as you describe it, is based on UI, please make sure that when plugin management via UI is disabled (e.g. when
DISALLOW_FILE_MODS
is true) the changes in this PR don't apply.
I don't understand... If DISALLOW_FILE_MODS
is true, then you manage everything via the CLI or some other way - in which case you are responsible as an admin for everything. So you are responsible for installing Woo when you install an addon. What exactly from this PR would you like to see not apply in this case? Can you please elaborate?
And also please consider what happens when one or more of those dependencies can't be installed, because requires a PHP version that is not available, or can't be found in the wp.org API (removed/suspended).
If a dependency can't be activated, then the plugins that depend on it will also not be activated. And that makes perfect sense... If Woo for some reason can't work or be installed on my site, then what use is an activated woo addon?
And also please consider the case one or more of the dependencies is Network only but the user is in the site-specific plugin screen.
Excellent point, thank you!
And also please consider the case one of the dependencies as a fatal error and so is forcefully disabled.
If WooCommerce is forcefully deactivated, then its dependencies will also be deactivated since the plugin they depend on is no longer active - and therefore their dependencies are not met. When that happens, they get added to a "pending plugin activations" queue (using a pending_plugin_activations
option). That makes the notices show up in the dashboard guiding users to activate Woo, and as soon as the user resolves their fatalities and the plugin is reactivated, all woo addons that were previously active will be auto-activated. The user has the option to cancel that at any point by clicking on the "Cancel activation request" link in the addons that are pending activation.
โgmazzap commented on โPR #1547:
3 years ago
#163
@aristath
Automatically installing and/or activating something without the user's explicit consent is a big no-no for this implementation.
๐๐ป
But even if we do get to a point of deeply-nested dependencies and plugins that require 10 plugins, clicking 30 times is easier than searching the repo for the dependencies manually and installing them one by one.
People *will* use this for marketing purposes. So a plugin that requires 3 plugins will not be that hard to find. And if each of those 3 will require one additional plugin (and I stop there) it's 6 plugins. 2 clicks each (install and update) + 2 clicks for the "main" plugin and we're 14 clicks. That is not that unlikely.
It is better than searching the repo for the dependencies manually? Yes.
Can we expect this will cause _more_ dependencies to be created? I'm quite sure.
Will people ask to automate this process? I'm ready to bet.
Opt-out of what exactly? The notification that will be shown, urging you to activate WooCommerce when you install a plugin that depends on WooCommerce? Or opt-out from the fact that a woocommerce addon will not be enabled unless woo is also installed & activated?
Considering I manage dependencies via Composer, I don't even login into WordPress most of the time. So, ideally, if I had the possibility to have the _current_ behavior (no notification, no preventing installation, no anything this PR contains) I would take that possibility. And if there's a fatal error because the WooCommerce addon is activated without WooCommerce, that will help me to spot the issue and add WooCommerce to my composer.json
.
What exactly from this PR would you like to see not apply in this case? Can you please elaborate?
See above. I would like the Requires plugins
header to be ignored at all. If things will break, will break. No notifications will be there, and so on. If the notification will be there... I'll not see it most of the time, but will not harm. But "preventing installation" because of unmet dependencies is something I would like to opt-out of.
But assuming a situation when a user can't install plugins from the UI, and they are seeing a notice that says:
_"You need to install and activate WooCommerce to use X, click here to do it"_
Then they "click there", and they get an error. So at least, make sure no input is presented to users to install a plugin if they can't do it.
And if you present an input that says:
_"You need to install and activate WooCommerce to use X"_
(without "click here"...) do you think it is useful to tell users they have to do something they can't do?
and therefore their dependencies are not met. When that happens, they get added to a "pending plugin activations" queue (using a pending_plugin_activations option).
Simple case A requires B that requires C.
C can't be installed for some reason. That will make _two_ pending installations right? One for _A_ and one for _B_. If the problem I have with _C_ is not something I can/want fix, I will need to cancel both pending installations, right?
And if things get a bit more complex, that means it is possible to have multiple "pending installations" that need to be canceled?
It would be possible that all nested requirements refer to the "root" installation request and canceling that all the other are canceded? In the prev example, canceling the request for A, would cancel the request also for B.
โaristath commented on โPR #1547:
3 years ago
#164
People will use this for marketing purposes.
If they do, they'll be _doing_it_wrong
. A "requirement" is what it says: A requirement. Not a recommendation or a "good to have" item. For better or worse we can't prevent people from doing the wrong thing, but then again this is open-source. If they want to abuse this like they have been abusing all other APIs (like the admin-notifications) then they will.
ideally, if I had the possibility to have the current behavior (no notification, no preventing installation, no anything this PR contains) I would take that possibility.
Done. Added a check for a DISALLOW_PLUGIN_DEPENDENCIES
constant.
make sure no input is presented to users to install a plugin if they can't do it.
The code already includes current_user_can
checks. As per your suggestion, I added an extra check for DISALLOW_FILE_MODS
.
โgmazzap commented on โPR #1547:
3 years ago
#165
If they want to abuse this like they have been abusing all other APIs (like the admin-notifications) then they will.
That's what said, they will :) And that will disrupt users experience. Just like admin notifications.
If you could back in time when admin notifications API was added, knowing what you know now, would you try to make the abuse less user-impacting? Would you at least factor-in the abuse you know will happen into your decision making process?
---
Added a check for a DISALLOW_PLUGIN_DEPENDENCIES constant.
I added an extra check for DISALLOW_FILE_MODS.
Thank you.
---
@aristath I was thinking something. I see that when a package is a dependency, it can't be disabled/unistalled.
Now imagine the situation in which _A_ requires _B_, _B_ requires _C_ and _C_ requires _A_. Or any other circular dependency situation, even just _A_ requires _B_, _B_ requires _A_.
Because _B_ is a dependency of _A_ it can't be disabled. And because _B_ is a dependency of _A_ it can't be disabled either.
Does it mean that in case of circular dependencies people will need to keep all the plugins forever?
โaristath commented on โPR #1547:
3 years ago
#166
Now imagine the situation in which A requires B, B requires C and C requires A. Or any other circular dependency situation, even just A requires B, B requires A.
Because B is a dependency of A it can't be disabled. And because B is a dependency of A it can't be disabled either.
Does it mean that in case of circular dependencies people will need to keep all the plugins forever?
Circular dependencies are not yet handled here, this is the missing piece IMO and I'm currently working on it :heart:
โaristath commented on โPR #1547:
3 years ago
#167
Update: Circular dependencies are now properly handled.
#168
@
3 years ago
I'm really surprised to have read all this and see no mention of [afragen/wp-dependency-installer](โhttps://github.com/afragen/wp-dependency-installer) which addresses what the recent PR is addressing, although the PR's approach of using the plugin's header is neater. Here's an [example implementation](โhttps://github.com/BrianHenryIE/woo-manage-fraud-orders/blob/address/src/Admin/class-dependencies.php).
I've admonished a few plugin authors for writing is_plugin_active()
checks that end in die
, so to some extent this is an improvement.
I certainly welcome a dismissable admin notice saying ~"WooCommerce is required, without it plugin-x will not work correctly, you should consider activating it."
But I don't think there's a good reason to deny activating the plugin when e.g. WooCommerce is not active. The correct approach is to hook only to appropriate actions of the parent, and some sparse function checks. I know this isn't always available, I just today opened [an issue with Action Scheduler](โhttps://github.com/woocommerce/action-scheduler/issues/749) with a feature request for better actions to facilitate this!
If I want to install and activate a plugin, I should be able to activate, e.g. EasyPost shipping plugin, without being told I MUST also activate WooCommerce. I use that example because I use a third party EasyPost plugin for purchasing shipping labels โ my plugin automates it, it provides the UI I need. Admittedly I am using and do need WooCommerce, but it's not hard to imagine another e-commerce plugin either forking WooCommerce or trying to work upon a compatible set of actions and filters to give itself access to the breath of relevant plugins that are already written.
In the simplest case, I create a plugin which is really just a facade on WooCommerce (which is probably in my /vendor/ directory) such that all the WooCommerce classes are available, most of the functionality is being used, but my plugin's slug is brian-commerce instead. All WooCommerce's plugins CAN and SHOULD work, but if they have REQUIRED rather than SUGGESTED WooCommerce, then they will never work.
I think this is antithetical to the philosophy of open source, as WordPress says in "Our Bill of Rights": The freedom to run the program, for any purpose.
So, I do like the PR, but it should just suggest, not enforce.
Composer is a different topic!
#169
@
3 years ago
Sorry to come late to this. I can only see the discussion of required plugins to have before activation. There is nothing about incompatible plugins (though I may have missed it).
I have a plug-in that is a drop-in replacement for another that was last updated 9 years ago - and uses the same options file, etc.
I really wouldn't want my plugin to be automatically activated (obviously it can be installed) if the other one is currently active.
[I see the job of my plugin to only define my incompatibilities, believing that it would be their job to state their incompatibilies if any. In my case I don't have any, but in the general case, this can give rise to a set of blockers.]
#170
@
3 years ago
Plugin Dependency Thoughts
Activation
We should not be concerned whether a plugin is active or not if it doesn't have dependencies installed and active. It is the plugin developers responsibility to ensure their code gracefully degrades and does not fatal if active without its dependencies.
Version of dependencies
We should install the most recent release version of all plugin dependencies and allow them to be updated.
A JSON file is not Composer
A JSON config file, or a coded array that resides within the plugin, is more efficient and allows for future expansion if needed.
While a header that contains the slug of dependencies has the value of simplicity it has greater drawbacks. Header parsing is required and there will likely be an increased support burden associated when unsophisticated devs use incorrect data in their header.
Plugin dependency inclusion should be easy, but it's not foolproof and it should require an understanding by developers.
Headers also don't allow for recommended vs required dependencies, unless there are 2 different headers.
Using a JSON file or coded array is more performant as it also requires a small bit of code to run the dependency installer. This avoids the looping over every plugin and parsing headers, only then to run code on the results.
Additionally, having the developer run a small piece of initiation code allows this solution to work for themes without additional code.
Recommended vs Required
I think it's a non-starter to automatically install and activate a plugin without user interaction. However, messaging and function can be a bit different.
Required dependencies should have an indication in the plugin row that they are dependencies and what plugin they are dependencies for.
My personal thought is that once required plugins are installed or activated, they cannot be deactivated or deleted until the plugin that requires the dependency is deactivated. This avoid the issue of a user not remembering why a specific plugin is present and active, assuming they don't notice the other indicators. This can be easily removed from the example.
Hidden problem
Some sort of time dismissible notice is required and not yet available in core.
Solutions
I've been developing both a plugin dependency installer and helped with development of a time dismissible admin notice for a while.
โhttps://github.com/afragen/wp-dependency-installer
โhttps://github.com/w3guy/persist-admin-notices-dismissal
I have created an opinionated first pass for a feature plugin in โhttps://github.com/afragen/plugin-dependency-feature
It uses a modified version of the wp-dependency-installer
. It will need to have many other features removed to be acceptable to core.
Feel free to change the JSON file to add your own required and recommended dependencies. Refer to the โwp-dependency-installer wiki for information on setup etc.
Non dot org dependencies
I realize that plugins are not allowed to install plugins from outside of dot org, but we should at least ask whether this is acceptable to be in core. Those features exist in the wp-dependency-installer
code but can easily be removed.
โafragen commented on โPR #1547:
3 years ago
#171
I have added my thoughts to the trac ticket. Putting a link here for completeness.
This ticket was mentioned in โSlack in #core-auto-updates by afragen. โView the logs.
3 years ago
โpbiron commented on โPR #1547:
3 years ago
#174
I'm just starting to review this PR. Initial impressions are that it is mostly working as I expect it to.
The first thing that jumps out at me tho, is that I think the notices should be changed a little (but in styling and content).
The follow screenshot is how I would expect them to be:
- notices appear to be "within" the plugin's row (no box-shadow/border on top and with margins left and right), just like update available notices are
- notices for plugins that are dependencies of another active plugin should be "warnings" instead of "infos"
- the notice text for plugins that are dependencies of another active plugin should explicitly say that the plugin can't be activated
The last 2 points are to help the user realize why the Deactivate row action isn't present
@aristath If you agree that notices would be better as above, what do you suggest is the best way for incorporate the necessary changes? I can push changes to your fork so you can update the PR.
โpbiron commented on โPR #1547:
3 years ago
#175
Another thing I've noticed is that if I try to activate a plugin that depends on another one (which is installed, but not active), I correctly get the "Plugin X depends on plugin Y to be activated. Activate plugin" admin notice.
But I also get the "Plugin activated." admin notice, which should not appear in this case.
I haven't yet investigated how easy it will be to suppress that "success" admin notice, just want to note it here so I don't forgot to mention it :-)
#176
@
3 years ago
I've mostly converted โPlugin Dependency Feature to what should be a core patch.
When I get the opportunity I will add a PR. In the meantime feel free to kick the plugin around.
โaristath commented on โPR #1547:
3 years ago
#177
what do you suggest is the best way for incorporate the necessary changes? I can push changes to your fork so you can update the PR.
@pbiron @dingo-d I added you as collaborators in the PR so you should be able to push changes after accepting the invitation :+1:
โaristath commented on โPR #1547:
3 years ago
#178
notices appear to be "within" the plugin's row (no box-shadow/border on top and with margins left and right), just like update available notices are
Makes sense. Took a bit of tweaking but I believe it's OK now.
notices for plugins that are dependencies of another active plugin should be "warnings" instead of "infos"
I'm not sure about that... :thinking:
I'm thinking of scenarios like for example a woocommerce addon that requires woocommerce. In that case, woocommerce can be activated on its own, and works fine. The addon however requires woo. So the notice in woocommerce's row can be an info since it's technically not a "warning"... The notice in the addon however is a warning
the notice text for plugins that are dependencies of another active plugin should explicitly say that the plugin can't be deactivated
Good point. Fixed in โhttps://github.com/WordPress/wordpress-develop/pull/1547/commits/412d94f05cfe9d74b87dc3b38ee69f9e077a269e
โspacedmonkey commented on โPR #1547:
3 years ago
#179
How does this work in multisite and network activated plugins?
โpbiron commented on โPR #1547:
3 years ago
#180
How does this work in multisite and network activated plugins?
that's on my list of things to test :-)
#181
@
3 years ago
One thing that we have to be aware of is that many plugins that support "addons", have their own UI (in addition to the core plugins screen) where such addons can be installed/activated/deactivated.
The above 2 screenshots show 2 such screens. The first one is for Gravity Forms, the 2nd is for Gravity Perks (which is a Gravity Forms addon that supports it's own addons :-).
I think it completely out of scope for core to try to "police" such additional ways of installing/activating/deactivating plugins. And the current PR respects that (i.e., it does NOT propose to change the behavior of โactivate_plugin() (and cousins).
I'm just mentioning this here so that someone doesn't suggest that this ticket should somehow address that problem :-)
This ticket was mentioned in โPR #1674 on โWordPress/wordpress-develop by โafragen.
3 years ago
#182
This PR based on โhttps://github.com/afragen/wp-dependency-installer and โhttps://github.com/collizo4sky/persist-admin-notices-dismissal.
Need to load the new dependency installer class in all plugins and themes where dependencies exist.
require_once ABSPATH . '/wp-admin/includes/class-wp-plugin-dependency-installer.php';
Uses either a JSON config file or an array of data passed in code.
JSON config file named wp-dependencies.json
resides at the root of the plugin or theme.
Call as follows. WP_Plugin_Dependency_Installer::instance( __DIR__ )->run();
{{{json
[
{
"name": "Query Monitor",
"slug": "query-monitor/query-monitor.php",
"uri": "โhttps://wordpress.org/plugins/query-monitor/",
"required": false
},
{
"name": "WooCommerce",
"slug": "woocommerce/woocommerce.php",
"uri": "โhttps://wordpress.org/plugins/woocommerce/",
"required": true
}
]
}}}
Array example
Call as follows. WP_Plugin_Dependency_Installer::instance( __DIR__ )->register( $config )->run();
{{{php
$config = array(
array(
'name' => 'Hello Dolly',
'slug' => 'hello-dolly/hello.php',
'uri' => 'โhttps://wordpress.org/plugins/hello-dolly',
'required' => true,
),
);
}}}
Example images. Plugin Dependency Feature (modified to use patch) and TwentyNineteen theme both have dependencies. Theme only has dependency for Hello Dolly
.
<img width="857" alt="screenshot_89" src="โhttps://user-images.githubusercontent.com/1296790/133168512-b0a935ac-d460-4e9a-86a0-050a9afaeeb2.png">
Clicking on the Install link for Hello Dolly shows the following.
<img width="853" alt="screenshot_90" src="โhttps://user-images.githubusercontent.com/1296790/133168577-1b9c32e1-af83-48cf-8108-4e6890a4f3bd.png">
Page reload
<img width="860" alt="screenshot_91" src="โhttps://user-images.githubusercontent.com/1296790/133168602-4d2ba06c-c45d-4f8a-a6fc-f83853485480.png">
After clicking on Activate link for Hello Dolly
<img width="852" alt="screenshot_92" src="โhttps://user-images.githubusercontent.com/1296790/133168667-9b89890b-72dc-4cf6-8a1f-0ef3dbb9de42.png">
Page reload
<img width="854" alt="screenshot_93" src="โhttps://user-images.githubusercontent.com/1296790/133168745-0f9265bb-2e3e-4a5b-ba24-763d63b6c1b6.png">
Query Monitor installed via link
<img width="839" alt="screenshot_94" src="โhttps://user-images.githubusercontent.com/1296790/133168812-ea6ea9e9-d917-454a-9b69-e875c5b641a0.png">
Feature Plugin
โhttps://github.com/afragen/plugin-dependency-feature
Trac ticket: https://core.trac.wordpress.org/ticket/22316
This ticket was mentioned in โSlack in #core-auto-updates by afragen. โView the logs.
3 years ago
This ticket was mentioned in โSlack in #hosting-community by javier. โView the logs.
3 years ago
This ticket was mentioned in โSlack in #hosting-community by jadonn. โView the logs.
3 years ago
#186
@
3 years ago
Would adding a hook for plugin devs to utilize to send plugin data to Site Health be a reasonable solution? That way WP core doesn't have to aggregate plugin data, but rather puts the onus on plugin (and theme) devs to provide for their users, while also offering a path to do so.
This ticket was mentioned in โSlack in #core by afragen. โView the logs.
3 years ago
โafragen commented on โPR #1674:
3 years ago
#188
โtomjn commented on โPR #1674:
3 years ago
#189
There's discussion in the other PR that's relevant here:
- There was a lot of discussion around
composer
and that a dependency .json file is redundant, lots of people were opposed to the idea - A dependency JSON file would be obsolete and an additional tech debt burden if a
wp-plugin.json
orwp-theme.json
file was introduced - Precedents already exist for using the plugin header to define plugin slugs, and the other PR adopted such a field for defining .org plugin slugs
- A URI implies that 3rd party plugins can be specified, but the tickets scope is confined to .org repo plugins
- including the name and URL in the JSON file opens this up to malicious actions, for example:
[ { "name": "Super Safe Plugin Monitor", "slug": "site-deleter-plugin/delete-site.php", "uri": "https://wordpress.org/plugins/safe-plugin/", "required": true } ]
โafragen commented on โPR #1674:
3 years ago
#190
@tomjn i did read all of the discussion on the other ticket. Respectfully, I disagree with the concept that having a JSON configuration file means "we should just use composer". There are huge differences and the ability to use a JSON config file, or in this PR, an associative array, provides for much more flexibility.
I believe that using a plugin header is much more restrictive and less performant. There is no need to loop through every installed plugin just to find the appropriate header and act on it.
Notice that this PR can also be used by themes.
There is no ability to designate a recommended plugin in a header. There is here.
Entering data into the configuration that does not conform to an actual plugin in the dot org repository will result in an installation failure. The PR specifically is restricted to dot org plugins. There is no ability to successfully designate an plugin that is not hosted on dot org.
@tomjn install the test plugin. Change the configuration and see what happens.
โtomjn commented on โPR #1674:
3 years ago
#191
Respectfully, I disagree with the concept that having a JSON configuration file means "we should just use composer".
That is not what I said, a dependency file duplicates composer functionality. The consensus on the other PR was that if we add a JSON file it should be general replacement for the plugin header in the future.
There is no ability to designate a recommended plugin in a header. There is here.
Recommendations were not a part of the tickets scope.
I believe that using a plugin header is much more restrictive and less performant. There is no need to loop through every installed plugin just to find the appropriate header and act on it.
WP would already do this, and caches the result, it's necessary in order to identify which file is the plugin file and which isn't. There is no performance cost that isn't already being paid.
The PR specifically is restricted to dot org plugins.
This does not limit the scope of abuse or mismatched expectations.
โaristath commented on โPR #1547:
3 years ago
#192
Summing up what this PR does when plugin A depends on plugin B:
- When both plugins A & B are deactivated, plugin A has an inline notice (info) with the message
Plugin "A" depends on the following plugin(s): B
. - When the user clicks the "Activate" link on plugin A, the following happens:
- If plugin B is not installed, there's an admin notice (warning) added with the text
Plugin "A" depends on plugin "B" to be installed. Install and activate "B"
. "Install and activate B" is a link. - If plugin B is installed, there's an admin notice (warning) added with the text
Plugin "A" depends on plugin "B" to be activated. Activate plugin
. "Activate plugin" is a link. - Plugin A does NOT get activated until all its dependencies are met. Instead, it gets added in a queue and the "Activate" link inline the plugin row gets replaced with a "Cancel activation request" link. Additionally, plugin A gets an inline notice (warning) with the following message:
Plugin "A" has unmet dependencies. Once all required plugins are installed the plugin will be automatically activated. Alternatively you can cancel the activation of this plugin by clicking on the "cancel activation request" link above.
- Once plugin B gets activated, plugin A gets auto-activated because the user previously requested it. The "Deactivate" and "Delete" actions are removed from plugin B and a notice (info) gets added to that row with the message
Plugin B cannot be deactivated because it is a dependency for the "A" plugin.
- If plugin B is not installed, there's an admin notice (warning) added with the text
This PR does NOT force-activate anything, it simply guides users to install and/or activate dependencies.
This PR does not handle "recommended" plugins. "Requiring" and "recommending" a plugin are completely different scenarios and this is a simple implementation to avoid errors when a dependency is not installed. Recommended plugins can be used for marketing and complementary purposes and I am not sure Core would be the right place to do something like that.
A valid scenario would be for example installing a plugin addon, without the "root" plugin being installed. If I install a woocommerce addon, or a contact-form addon, or any plugin addon without woocommerce/contact-form/whatever activated, then without a dependencies implementation the following can (and frequently do) happen:
- The addon depeloper hasn't properly tested what happens when the parent plugin is not activated and the result can be PHP warnings/errors etc (because there's no chance anyone would install woo-payment-gateway-X without Woo installed, right?)
- There are no errors, but there's an active plugin that does nothing at all except run on every page request, looking for its parent
- The addon developer reinvents the wheel every time and they throw notices to the user that they should install the parent plugin
- There is no proper feedback and the user doesn't know why the plugin they installed does nothing
โideag commented on โPR #1674:
3 years ago
#193
One thing I'd like to point out about JSON files is that they will be publicly accessible in plugin directories and will contain information about other plugins in the system and maybe even their versions. In plugin header that information is automatically protected from outside access, with JSON, there would need to be an additional .htaccess rule to restrict that.
โafragen commented on โPR #1674:
3 years ago
#194
@tomjn I will revise the PR to remove recommended plugins. It is out of scope.
Respectfully, I disagree with the concept that having a JSON configuration file means "we should just use composer".
That is not what I said, a dependency file duplicates composer functionality. The consensus on the other PR was that if we add a JSON file it should be general replacement for the plugin header in the future.
Creating a JSON replacement for all plugin headers is way out of scope and not like to happen.
Why does it matter if a JSON file duplicated composer functionality? Composer isn't used for core.
โafragen commented on โPR #1674:
3 years ago
#195
@ideag the only data in the JSON file is data related to plugins in dot org. How is that sensitive.
Given that we actively encourage keeping plugins up to date, installing an old, outdated version seems unwise.
โtomjn commented on โPR #1674:
3 years ago
#196
Why does it matter if a JSON file duplicated composer functionality? Composer isn't used for core.
There was an _enormous_ discussion about this in the other ticket, initially started precisely because core does not use composer. A wp-dependency.json
file is reinventing the wheel that is composer.json
. It would make more sense to just implement composer.json
, or, implement a wp-plugin.json
. Both are outside the scope of the ticket. There are lots of reasons not to use a dependency JSON file in this case, and lots of existing precedents supporting the plugin header approach.
โdingo-d commented on โPR #1547:
3 years ago
#197
I've created a small fix PR that adds some code improvements and cleanup: โhttps://github.com/aristath/wordpress-develop/pull/2
โafragen commented on โPR #1674:
3 years ago
#198
Honestly it's not simply a choice between composer or a header. I really did read the other discussion and the moment using a JSON config was mentioned the discussion went to that duplicates composer functionality.
It's not reinventing composer. The point that it does we should transition to using composer. That time is not now.
Composer isn't used it core, it's a non-starter. That doesn't mean a configuration file cannot be used.
We all admit that a configuration file is more flexible for future use. Why not just start there?
โtomjn commented on โPR #1674:
3 years ago
#199
As a part of a wp-plugin.json
yes, as a wp-dependency.json
file no. Right now the only non-problematic value in the file is the plugin slug, and no benefits have been outlined for putting it in a separate file.
There's also nothing to distinguish the dependency between a plugin and a theme other than the file extension on the end of the slug value. The slug also doesn't map to .org plugin slugs.
โafragen commented on โPR #1674:
3 years ago
#200
As a part of a
wp-plugin.json
yes, as awp-dependency.json
file no. Right now the only non-problematic value in the file is the plugin slug, and no benefits have been outlined for putting it in a separate file.
The slug is used internally, I could change it to the basename( $slug )
, what is usually thought of as the slug, woocommerce
or query-monitor
.
There's also nothing to distinguish the dependency between a plugin and a theme other than the file extension on the end of the slug value. The slug also doesn't map to .org plugin slugs.
The slug isn't to tell the difference between a plugin and a theme.
Have you tried actually running the code? Do you have actual issues with it or is this really all about preferences?
I really don't mean to sound snarky, it's just been a long day fighting Darwin in the trauma center.
โaristath commented on โPR #1547:
3 years ago
#201
@dingo-d Thank you for the cleanup & improvements! Merged :+1:
โkarmatosed commented on โPR #1547:
3 years ago
#202
I'd like to offer some design help here so jumping in to offer that. Just looking back at a few comments I have a little bit of feedback. I am settling into understanding here so please correct my course if I am off track.
- I think what is being added makes a lot of sense based on what exists.
- I think the notice probably falls into 'information' so that color use makes absolute sense.
- Similar the warning does.
My big concern would be where we have double messaging coloring:
I wonder if we need to embed the notices? Typically this doesn't happen and the interface really looks not too awesome with that.
Beyond those thoughts, I think looking at deciding what should or shouldn't be seen right away might be useful. For example could a simple notice of dependency show on the plugin listing? I suggest this as I can see at a scale otherwise it could get a lot. I think that might be useful in later versions though.
โafragen commented on โPR #1674:
3 years ago
#203
Latest commit removes JSON in favor of config array. @tomjn I use the slug (woocommerce/woocommerce.php) to target the plugin row.
Having some issues with the dismissible notice giving me a console error that dismissible_notice.nonce
is undefined. ๐
โafragen commented on โPR #1674:
3 years ago
#204
๐ JS should be working again.
โafragen commented on โPR1674:
3 years ago
#205
State of the โPR1674
Core aspects of #22316
- Designate plugin dependencies for plugins
- Notify the user or the plugin dependency
- No versioning, latest version of dependencies are installed from dot org
- Not allowing a dependency to be deleted or disabled once it is installed and/or activated
- On re-activation of a plugin with dependencies, ensure user is notified that dependencies exist
Uses a configuration array and command to initialize. slug
is used to target the plugin row to deactivate the ability to delete or deactivate installed activated dependencies and to designate the dot org download link. This PR uses a class instance so that multiple configuration sources may be properly used.
This PR does not try to control plugin activation. Every plugin developer should be responsible and ensure that their plugin degrades gracefully if a required dependency is not present and active. This is the current state of core and I believe it is out of scope to address this particular aspect. This PR does not force anything. If the user does not wish to install or activate a dependency all they must do is dismiss the admin notice and it will not reappear for a specified period of time.
Circular dependencies should not be an issue if plugin devs gracefully degrade their plugins. If they don't, this is an issue with the plugin requiring the dependency. This issue exists today.
Dependent plugins may be in either other plugins, themes, or both. In this example Hello Dolly
is a required plugin for both Plugin Dependency Feature and TwentyNineteen.
Admin Notices designate what is requiring the dependency and a link to install or activate the dependency. If the same dependency is in a theme, that is listed as themes load first.
After the dependency is installed an admin notice to activate it will display. The dependent plugin will show itself as a Required Plugin and list what plugins or themes it is Required by
If a dependency is not in dot org, though it can have an install link, it will result in a failure.
All admin notices are dismissible. The default is for 14 days, but this can be modified by a filter for anywhere from 1 day to forever. All that must be done is click the dismiss button. Notice the WooCommerce notice is no longer present.
Once activated a dependent plugin cannot be deactivated or deleted until the dependent plugin or theme is deactivated.
In this case the theme was changed and the plugin deactivated.
The additional benefit of this PR is that it includes code for time dismissible admin notices that may be used for admin notices outside of this scope.
To test, either apply the PR or download and activate the โPlugin Dependency Feature plugin.
Please let me know your thoughts on this PR.
โaristath commented on โPR #1547:
3 years ago
#206
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.
โBrianHenryIE commented on โPR #1674:
3 years ago
#207
Request: filter.
I don't like the instant install without any explanation of why the plugin is needed or any way to assess the required plugin.
Before I discovered your dependency library I had started to write a similar solution, where the admin notice brought the user to the plugins install page and opened the modal for the plugin
wp-admin/plugin-install.php?s=WooCommerce&tab=search&type=term&open-plugin-details-modal=woocommerce
// Open the plugin details modal on plugin-install.php. $(function() { let url = location; let searchParams = new URLSearchParams(url.search); var pluginName = searchParams.get('open-plugin-details-modal'); if(pluginName != null) { // Should maybe be hooked to jQuery( document.body ).trigger( 'post-load' ); setTimeout( function() { jQuery('.plugin-card-' + pluginName + ' .open-plugin-details-modal').click(); }, 2500); } });
Similarly, I would redirect users to the WooCommerce store to purchase missing plugins.
$shipment_tracking_url = 'https://woocommerce.com/products/shipment-tracking/';
$shipment_tracking_url = add_query_arg(
array(
'wccom-site' => site_url(),
'wccom-back' => rawurlencode( '/wp-admin/admin.php?page=' . $this->get_plugin_name() ),
'wccom-woo-version' => WC_VERSION,
'wccom-connect-nonce' => wp_create_nonce( 'connect' ),
),
$shipment_tracking_url
);
So it would be great to have a filer on the action link. Something ~class-wp-plugin-dependency-installer.php:470
like apply_filters( 'plugin_dependency_action_url', $action, $notice );
would allow me to extend your code to include my changes above.
โafragen commented on โPR #1674:
3 years ago
#208
I'm definitely in favor of filters. But what I understand your ask is to be able to modify what is installed and from where. Honestly this is out of scope for core as it could be used to install plugins it in dot org.
@BrianHenryIE there is a solution in afragen/wp-dependency-installer to install from elsewhere. My plan is to update that code with some of the improvements here.
โdd32 commented on โPR #1674:
3 years ago
#209
Need to load the new dependency installer class in all plugins and themes where dependencies exist.
require_once ABSPATH . '/wp-admin/includes/class-wp-plugin-dependency-installer.php';
It would make more sense to not have that call in the plugin, but have it always loaded in the proper contexts for plugins to call it - for example, it wouldn't make sense to call it / load it on front-end requests.
Plugins would then either a) run on a certain hook or b) check if the class is loaded before calling it.
Uses either a JSON config file or an array of data passed in code.
Having an array of passed in data in code has two main disadvantages to me:
a) Can't show information prior to the plugin being activated (or after it's deactivated)
b) Can't be easily detected by the WordPress.org plugin directory
[ { "name": "Query Monitor", "slug": "query-monitor/query-monitor.php", "uri": "https://wordpress.org/plugins/query-monitor/", "required": false }, { "name": "WooCommerce", "slug": "woocommerce/woocommerce.php", "uri": "https://wordpress.org/plugins/woocommerce/", "required": true } ]
First two things that I see here:
required
beingfalse
kind of suggests that this is not a plugin dependencies, but rather aplugin suggestion
. I can see that being abused byACME Blocks
suggestingACME SEO
andACME CRM
plugins very quickly. (Note: I recognise that Themes are where this is probably wanted)slug
should really be just that - a slug,woocommerce
and should not reference the filename. The plugin filename can theoretically change, as well as, it being installed in a different location (for example,mu-plugins/woo/woocommerce.php
). That's really something that would be only in a 0.01% of user type thing, but core does know the proper slugs of all the loaded plugins via the update api calls (I can't recall the transient that the slugs are stored in - it's not the update transient).
Example images. Plugin Dependency Feature (modified to use patch) and TwentyNineteen theme both have dependencies. Theme only has dependency for
Hello Dolly
.
Themes are the obvious use-case for suggested plugins, as currently WordPress.org-hosted themes cannot require a plugin, but can suggest it.
It might make sense to have support for it in core, but have the WordPress.org directories reject suggested plugins for plugins, and reject required plugins for themes. But it kind of makes me hesitate as to whether it should be supported at all in core due to the abuse that pro/premium plugins could do.. but I guess they do whatever they want anyway.
โdd32 commented on โPR #1547:
3 years ago
#210
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.
#211
@
3 years ago
- Keywords early added
As the code stands, I think it's a good proof of concept but I don't think this is ready to be considered for 5.9. The discussion on the approach has been sporadic for the last four years so moving from proof of concept PR to core seems a little rushed.
This is what I would like to see happen before this is considered for merge:
- Feature discussion on this ticket rather than a PR. The code has come before a revised discussion on the feature so getting the architecture down before the proof-of-concept moves forward will help.
- Agenda a discussion on the feature in a Core dev-chat. The only discussion I've seen is an announcement of the PR rather than discussing the feature
- A feature plugin: this is a problem WP has been sitting on for nine years, so there's no need to get it in to Core in the next three months.
During manual testing of the feature branch, I've found a few things that will need a decision:
- should the bootstrapping of WP take in to account dependency order for the
require
-ing of plugins - how should external/commercial plugin repositories be accounted for (there's a suggestion above of a
provider:
prefix) - if a namespace prefix is used, should wordpress.org's plugin repo use one too
It would also be good to get further design team involvement in the notifications. My testing threw three dependency notifications and an activation notification which was already becoming a little overwhelming.
โaristath commented on โPR #1547:
3 years ago
#212
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.
โafragen commented on โPR #1674:
3 years ago
#213
@dd32 as always I appreciate your insight.
Need to load the new dependency installer class in all plugins and themes where dependencies exist.
require_once ABSPATH . '/wp-admin/includes/class-wp-plugin-dependency-installer.php';
It would make more sense to not have that call in the plugin, but have it always loaded in the proper contexts for plugins to call it - for example, it wouldn't make sense to call it / load it on front-end requests.
Plugins would then either a) run on a certain hook or b) check if the class is loaded before calling it.
Last commit pre-loads relevant classes. No longer any need to require
this class in plugins/themes.
Uses either a JSON config file or an array of data passed in code.
Having an array of passed in data in code has two main disadvantages to me:
a) Can't show information prior to the plugin being activated (or after it's deactivated)
b) Can't be easily detected by the WordPress.org plugin directory
Is there a need to show dependencies prior to the dependent plugin/theme being activated? The user can easily look at the plugin/theme code to discern this, alternatively the developer can/should put this information in the plugin/theme readme description.
{{{json
[
{
"name": "Query Monitor",
"slug": "query-monitor/query-monitor.php",
"uri": "โhttps://wordpress.org/plugins/query-monitor/",
"required": false
},
{
"name": "WooCommerce",
"slug": "woocommerce/woocommerce.php",
"uri": "โhttps://wordpress.org/plugins/woocommerce/",
"required": true
}
]
}}}
First two things that I see here:
required
beingfalse
kind of suggests that this is not a plugin dependencies, but rather aplugin suggestion
. I can see that being abused byACME Blocks
suggestingACME SEO
andACME CRM
plugins very quickly. (Note: I recognise that Themes are where this is probably wanted)slug
should really be just that - a slug,woocommerce
and should not reference the filename. The plugin filename can theoretically change, as well as, it being installed in a different location (for example,mu-plugins/woo/woocommerce.php
). That's really something that would be only in a 0.01% of user type thing, but core does know the proper slugs of all the loaded plugins via the update api calls (I can't recall the transient that the slugs are stored in - it's not the update transient).
The array has been vastly simplified and only contains, name
and slug
. Thought core plugins are referenced in arrays by their slug/file_name
. I can easily change this if it's confusing. Do you have a suggestion?
Example images. Plugin Dependency Feature (modified to use patch) and TwentyNineteen theme both have dependencies. Theme only has dependency for
Hello Dolly
.
Themes are the obvious use-case for suggested plugins, as currently WordPress.org-hosted themes cannot require a plugin, but can suggest it.
In this PR, required plugins are really only suggestions as there is not automatic installation/activation. The user can always dismiss the notice of the requirement.
It might make sense to have support for it in core, but have the WordPress.org directories reject suggested plugins for plugins, and reject required plugins for themes. But it kind of makes me hesitate as to whether it should be supported at all in core due to the abuse that pro/premium plugins could do.. but I guess they do whatever they want anyway.
There are no longer _recommended_ plugins, only _required_ plugins. Required plugins only come from dot org.
This ticket was mentioned in โSlack in #core-auto-updates by francina. โView the logs.
3 years ago
โSergeyBiryukov commented on โPR #1674:
3 years ago
#215
Is there a need to show dependencies prior to the dependent plugin/theme being activated? The user can easily look at the plugin/theme code to discern this, alternatively the developer can/should put this information in the plugin/theme readme description.
I think there is some benefit in having the dependencies displayed in a standardized way in advance, before activation and without looking at the code, so that the user knows what to expect when they attempt activating the plugin or theme.
โafragen commented on โPR #1674:
3 years ago
#216
@SergeyBiryukov is there a current standardized way?
The View details iframe or something else?
How is this currently being accomplished prior to plugin activation?
โafragen commented on โPR #1674:
3 years ago
#217
@SergeyBiryukov without attempting to activate a plugin, if the dependencies were in a JSON file it might be possible to read that, should it exist, and create some additional plugin group for dependencies.
Unfortunately there will be those who immediately discard this idea solely based on it using a JSON file. ๐ฅ
โtomjn commented on โPR #1674:
3 years ago
#218
@afragen displaying the slug would be an immediate improvement. I know you want a JSON file so you can specify a pretty name
field but that would introduce security consequences, and it's simply not necessary.
I also oppose the snark, and the mischaracterisation.
I am happy for a JSON file, but it would be a plugin JSON file, e.g. plugin.json
that was a replacement for a plugins header. Such a file would mean a wp-dependencies.json
would be a temporary stopgap and additional technical debt. This combined with the fact that the only field the JSON file added that didn't have issues was the plugin slug, the use of a dedicated file could be whittled down to [ "woocommerce", "jetpack" ]
which could be included in the plugiin header without the additional overhead/complexity of loading and processing another file.
I don't see why it can't be a future enhancement, or why it's necessary for this PR. There's no _need_, it's a preference.
But ignoring that, the file is dangerous and misleading, lets pick it apart line by line:
{{{json
[
{
"name": "Query Monitor",
}}}
there is no enforcement, or updating. If a plugin changes hands this can and does change. Slugs do not change, adding bitrot. This field is superfluous. On top of that malicious actors can lie. What if we put Yoast SEO in here, and the user mistakenly believe it's been uninstalled and clicks it to reinstall, not realising it's actually for an unrelated plugin that has upsells.
{{{json
"slug": "query-monitor/query-monitor.php",
}}}
this implies a โhttps://wordpress.org/plugins/query-monitor/query-monitor.php/, this is meant to be a .org plugin slug, not a plugin active main file to load. Also, plugins get refactored, if Query monitor ever switched to query-monitor/plugin.php
this would break. The query-monitor
part never changes though.
{{{json
}}}
this field has an ambiguous meaning, if it's exposed to the user it could be abused, but also lead to mismatched expectation, e.g. is this the .org URL because that's the QM site? Or because they thought it was the install URL? What if I put a link to cheap betting $$$? Can't this field be generated based on the .org slug? What happens if I put a paypal support link here? Or my personal website?
{{{json
"required": false
}}}
This is great, I can now list all my other products and spam you with free advertisements powered by core itself, and on every update I'll tweak the URI and name so it catches the user.
The format is ripe for abuse and malicious intervention. I expect plugins on .org that go through review will get caught, but aside from bitrot, every 3rd party plugin, every product on code canyon, is going to add this JSON file with a long list of recommendations of questionable value.
This is the future this file format will give us:
{{{json
[
{
"name": "WooCommerce",
"slug": "woocommerce/woocommerce.php",
"uri": "โhttps://wordpress.org/plugins/woocommerce/",
"required": true
},
{
"name": "Hypercharge Plugin Utilities",
"slug": "dependency-marketing/campaigns.php",
"uri": "โhttp://wp-plugin-analytics.com",
"required": true
},
{
"name": "WooCommerce Membership Pro Offer Code MEMBERS20",
"slug": "woocommerce-members",
"uri": "https:/woosellers.tk/memberships",
"required": false,
"campaign-tracking-id": "GK92-18ED"
},
{
"name": "WC Product Gallery Plus",
"slug": "wc-product-gallery",
"uri": "https:/woosellers.tk/product-gallery",
"required": false,
"campaign-tracking-id": "XK92-18SD"
},
{
"name": "Star Sponsor: Turbopack, turbocharge your site",
"slug": "jetpack",
"uri": "https:/turbopack.com",
"required": false,
"campaign-tracking-id": "LLCJ-18SD"
},
.....
]
}}}
โtomjn commented on โPR #1674:
3 years ago
#219
If you really want a JSON file, a plugin.json
file that mirrors the theme.json
added by FSE would be the best approach, and have its own ticket. I would advise that it use a single array of slugs as used by wordpress.org to define dependencies to avoid the security and abuse issues detailed above. This also opens the door to some rather pleasant integrations with FSE, global styles, and code reuse.
But such a file is not necessary to implement the ticket, nor is it within the scope. It would need a new ticket and a new PR.
As for the displaying of dependencies in advance, I would suggest the slug, as a clickable hyperlink that when clicked opens the same modal dialog that the plugin UI already uses to show the wp.org page when searching for plugins from WP Admin:
<img width="966" alt="Screenshot 2021-09-22 at 00 47 47" src="โhttps://user-images.githubusercontent.com/58855/134262125-6723f704-9d5f-4004-9790-4b81a2dfa934.png">
This allows the user to see what they're installing, not what the file "claims" it's installing, as well as a link to click through and see more detail and an up to date name. It even gives them a well tested install UX they're familiar with.
โtomjn commented on โPR #1674:
3 years ago
#220
I would also note, that plugin headers are loaded by get_plugin_data
. This function is called by get_plugins
which goes through every file looking for a plugin header, and loading it. The results are cached, so no additional performance cost is paid. This happens regardless of wether you use or do not use a JSON file, and it must be done to keep plugin compatibility and identify the main plugin file. Any claims of improved performance by using a JSON file for this are unfounded.
โhttps://developer.wordpress.org/reference/functions/get_plugins/
โpeterwilsoncc commented on โPR #1547:
3 years ago
#221
@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.
โpeterwilsoncc commented on โPR #1674:
3 years ago
#222
Two PRs with similar names are confusing.
I left a review on the other PR about 24 hours ago, some of which applies here for UI considerations. โhttps://github.com/WordPress/wordpress-develop/pull/1547#pullrequestreview-759260691
I also posted a comment on the ticket at around the same time detailing a way forward and requesting further architecture discussions before continuing to code up the PR. I think it might be best to hold off on cutting further code until some of these discussions are had.
โafragen commented on โPR #1674:
3 years ago
#223
@tomjn I apologize. If we ever get the opportunity to meet, the beers are on me.
The mostly current state of the code in this PR is outlined in โhttps://github.com/WordPress/wordpress-develop/pull/1674#issuecomment-922550637.
Fundamentally @aristath and I are working to the same goal with slightly different implementations and slightly different UI. Honestly I started along my path due to it's similarity to my other project for installing plugin dependencies.
Currently there is no JSON, only an array. There are only 2 elements to the array. If you don't like the variable names we can discuss that. But certainly there is no security risk to anything in those arrays. If the developer want to misrepresent the name of a plugin, they can. Not sure why they would but they can. In fact, the malicious entry I added to test your previous suggestion, results in a "failure to download". No harm, but a foul.
If a dependency has changed its file path the developer requiring the dependency should be aware and fix it. I use the <dir><file name>
so that I can more easily target the plugin row without cycling through get_plugins()
. For the other project it simply made more sense.
Plugins, once active, have full reign over the site. I know you know this; I repeat it only to say that an improperly coded plugin can and may cause damage. The plugin developer is responsible for the code they release. We all know of plugins that have changed developers and have become something other than what they were initially. The community finds out and people move on. If the developer isn't maintaining their plugin that's problematic on many different levels.
Can we discuss something other than how the dependencies are defined? I give up. I see that the preferred method defining dependencies will be a plugin header. This still leaves lots to discuss.
- Should we concern ourselves with plugins that don't degrade gracefully when dependencies are missing? My feeling is no. It's a near impossible task to fix and adds unnecessary complexity.
- What's the best method to inform user's of a dependency, an admin notice, a dependencies tab in the plugin page, something else?
- How do we display the dependency information in the plugin row? Both Ari and I have different approaches.
There are weekly meetings Tues 1500 UTC for #core-auto-updates. I encourage everyone to join us.
โSergeyBiryukov commented on โPR #1547:
3 years ago
#224
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 commented on โPR #1547:
3 years ago
#225
@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 commented on โPR #1547:
3 years ago
#226
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 commented on โPR #1547:
3 years ago
#227
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.
#228
@
3 years ago
So I had a thought. Instead of all the admin notices why not add a tab to the Plugins > Add New
menu.
This uses plugin headers.
@karmatosed thoughts?
This ticket was mentioned in โSlack in #core-auto-updates by afragen. โView the logs.
3 years ago
This ticket was mentioned in โSlack in #core by audrasjb. โView the logs.
3 years ago
This ticket was mentioned in โSlack in #core-auto-updates by afragen. โView the logs.
3 years ago
This ticket was mentioned in โPR #1724 on โWordPress/wordpress-develop by โafragen.
3 years ago
#233
This PR adds the ability to define plugin dependencies using a Requires Plugin
header.
{{{php
/
- Plugin Name: Test Dependencies
- Author: Andy Fragen
- Description: Test for Required Plugins header.
- Version: 0.1
- Required Plugins: gutenberg, woocommerce */
}}}
If any installed plugin contains a Required Plugins
header with correct data. The user will see an admin notice with a link to a new Dependencies tab in the Install Plugins page.
The Dependencies tab will contain a listing of all plugins that have been designated as plugin dependencies. From this tab the user may easily install/activate any plugin dependency. The listed plugin cards contain information reporting the plugins that require each specific dependency.
The admin notice will continue to display until all plugin dependencies have been installed.
The plugins page will display information in the plugin row of installed plugin dependencies showing what plugins require them.
Installed plugin dependencies will be unable to be deleted. Once activated they will be unable to be deactivated.
This PR creates a tab in the plugins install ( Plugins > Add New ) page as this seems to be the logical area to install a plugin. Specifically it does not display uninstalled plugin dependencies in the Installed Plugins page. Doing so would seem illogical. Whether the tab is in the Plugins page or in the Plugins Install page it is a filter.
This PR makes no attempt to ensure that a plugin with dependencies behaves correctly if any of those dependencies
are not installed. That is the responsibility of the plugin developer.
### Disagreements
- Creating a tab in the Install Plugins page instead of in the Plugins page
- Scroll directly to the uninstalled plugins in the dependencies tab.
Trac ticket: https://core.trac.wordpress.org/ticket/22316
โafragen commented on โPR #1724:
3 years ago
#234
In this example there are 2 unactivated plugins, Test Dependencies and Test Dependencies 2. These both have defined Required Plugins
headers defined.
These headers are sanitized to only allow valid dot org plugin slugs. An example of one of these headers is
Required Plugins: gutenberg, hello-dolly, "junk-list , here for test", 435_bad
An admin notice is present to let the users know there are still unmet plugin dependencies. Once all plugin dependencies have been installed the admin notice will not display.
The plugin row displays information for plugin dependencies showing the plugins that require each specific plugin dependency.
The Dependencies tab displays plugin cards for all plugin dependencies. Each plugin card displays information as to which installed plugin is asking for the dependency.
This ticket was mentioned in โSlack in #core by afragen. โView the logs.
3 years ago
#236
follow-up:
โย 237
@
3 years ago
I added some comments in the PR @afragen :)
I think it's too technical to speak about plugin header. I'd suggest replacing the following string:
These suggestions are based on the `Required Plugins` header of installed plugins.
With something like this:
These suggestions are based on the dependencies required by already installed plugins.
#237
in reply to:
โย 236
@
3 years ago
Replying to audrasjb:
I added some comments in the PR @afragen :)
I think it's too technical to speak about plugin header. I'd suggest replacing the following string:
These suggestions are based on the `Required Plugins` header of installed plugins.With something like this:
These suggestions are based on the dependencies required by already installed plugins.
Updated and tweaked a little ๐
โafragen commented on โPR #1724:
3 years ago
#238
Simplified Dependencies tab header message as suggested in https://core.trac.wordpress.org/ticket/22316#comment:236
This ticket was mentioned in โSlack in #core-auto-updates by afragen. โView the logs.
3 years ago
This ticket was mentioned in โSlack in #core by afragen. โView the logs.
3 years ago
This ticket was mentioned in โSlack in #core by audrasjb. โView the logs.
3 years ago
โObliviousHarmony commented on โPR #1547:
3 years ago
#242
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!
โafragen commented on โPR #1724:
3 years ago
#243
Updated to use active voice for header as Requires Plugins
. Consistent with other Requires headers.
This ticket was mentioned in โSlack in #core by afragen. โView the logs.
3 years ago
#245
@
3 years ago
- Milestone changed from 5.9 to Future Release
With a few days left before 5.9 feature freeze (Nov 9th), moving this to a Future Release
. Please don't let this punt out of 5.9 derail progress on this impactful feature. Punting is not a "no".
Moving it to a featured plugin gives this feature a sandbox for experimentation, discussion, feedback, extensive testing, discovery of edge cases, and more. And when it's ready, pull this ticket into a milestone and welcome it to Core.
โafragen commented on โPR #1724:
3 years ago
#246
Added automatic plugin deactivation when required plugins are not installed and active.
This ticket was mentioned in โSlack in #core-auto-updates by afragen. โView the logs.
3 years ago
This ticket was mentioned in โSlack in #core-auto-updates by afragen. โView the logs.
3 years ago
This ticket was mentioned in โSlack in #core-auto-updates by afragen. โView the logs.
3 years ago
This ticket was mentioned in โSlack in #core by afragen. โView the logs.
3 years ago
This ticket was mentioned in โSlack in #core by afragen. โView the logs.
3 years ago
This ticket was mentioned in โSlack in #core by marybaum. โView the logs.
3 years ago
โafragen commented on โPR #1724:
3 years ago
#253
There is a feature plugin with this PR at โhttps://github.com/afragen/plugin-dependencies-tab
The plugin will receive updates via โGit Updater
โafragen commented on โPR #1724:
3 years ago
#256
This ticket was mentioned in โSlack in #core by afragen. โView the logs.
3 years ago
โpaaljoachim commented on โPR #1547:
3 years ago
#260
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 a good 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 commented on โPR #1724:
3 years ago
#261
I have now tested both the PR's.
PR 1. Single line notification for each dependency.
โhttps://github.com/WordPress/wordpress-develop/pull/1547#issuecomment-1083689256
Adds this example message: Plugin "Sample" depends on plugin "gutenberg" to be installed. Install and activate "gutenberg"
There are 4 lines with notifications. I felt flooded by all the notifications. I received a one line per plugin overview.
PR 2. This PR. Adds a new method Dependencies tab in the Add New plugins screen.
Uploading the sample plugin which includes dependencies. I see this:
It does not flood the screen with notifications. There is only one line. It mentions that I need to go to the Dependencies install page which then brings me over here:
I can easily see which plugins are needed to be installed for the "Sample" plugin to be activated.
This flow feels like a method that has always existed in WordPress core but that I have never noticed it before. It flows well. The UI is easy to understand. It does not flood me with notifications. This PR feels a lot tighter intertwined with core compared to the other PR. I also have in the back of my mind that we are also in general working on showing less notifications in the top which takes up a lot of visual space.
Also looking at the information included below the Sample plugin:
1- PR.
Plugin "Sample" has unmet dependencies. Once all required plugins are installed the plugin will be automatically activated. Alternatively you can cancel the activation of this plugin by clicking on the "cancel activation request" link above.
---> The above text is confusing as one can just skip activating the plugin. But here it seems that one will need to click "Cancel activation request" to make the top notifications go away.
2- This PR has a much more subtile approach: Requires: WooCommerce, Gutenberg
---> It just mentions which plugins are required to be installed and activated before one is able to activate the plugin.
1-PR feels like an older method.
2-This PR feels tighter and adds more subtile touch.
โpaaljoachim commented on โPR #1547:
3 years ago
#262
I have compared this PR with โhttps://github.com/WordPress/wordpress-develop/pull/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 commented on โPR #1547:
3 years ago
#263
@paaljoachim Are you able to add some notes to the feature project repo? I think this is the best ticket โhttps://github.com/WordPress/wp-plugin-dependencies/issues/3
โpeterwilsoncc commented on โPR #1547:
3 years ago
#264
Closing this off as it was decided to go down the feature project route, the repo is โhttps://github.com/WordPress/wp-plugin-dependencies
โpeterwilsoncc commented on โPR #1724:
3 years ago
#265
Closing this off as it was decided to go down the feature project route, the repo is โhttps://github.com/WordPress/wp-plugin-dependencies
This ticket was mentioned in โSlack in #core by afragen. โView the logs.
2 years ago
This ticket was mentioned in โSlack in #core by afragen. โView the logs.
2 years ago
This ticket was mentioned in โSlack in #core by afragen. โView the logs.
2 years ago
This ticket was mentioned in โPR #3032 on โWordPress/wordpress-develop by โafragen.
2 years ago
#269
- Keywords has-unit-tests added
A โFeature Project for Plugin Dependencies has been created. Along with this a โFeature Plugin has also been created.
Testing either via this PR or the feature plugin will show similar/identical results.
Trac ticket: https://core.trac.wordpress.org/ticket/22316
This ticket was mentioned in โSlack in #meta by costdev. โView the logs.
2 years ago
#271
@
2 years ago
It's complicated, but a major unlock. What would be a way for us to test this out in core before fully committing to supporting it forever?
#272
@
2 years ago
Matt,
We have a feature plugin โPlugin Dependencies that allows for users to do a nearly complete test of the feature. I say nearly complete as we have also supplied examples for how to use this with premium plugins too.
I will also refer to the 2 Make posts. โPlugin Dependencies Feature Project and โCall for Testing: Plugin Dependencies.
Ping me anywhere. The team working on this is happy to answer any questions that you or anyone else may have.
This ticket was mentioned in โSlack in #core by audrasjb. โView the logs.
21 months ago
#275
@
21 months ago
- Milestone changed from Future Release to 6.3
As per today's Old Tickets Triage session, let's try to implement this early in 6.3 development cycle (ideally right after 6.2 is branched). There's no point to keep it as a standalone plugin forever.
This ticket was mentioned in โSlack in #core by abhanonstopnews. โView the logs.
21 months ago
#277
follow-up:
โย 279
@
21 months ago
- Keywords 2nd-opinion added
Generally I like the approach of having "manual" plugins and themes dependencies (manual in the sense that nothing is installed or activated automatically, instead there are messages making it easy to install the requirements).
Having some "second thoughts" about the UI and workflow here.
- Not sure the global "Dependencies" tab is a good idea here. The other tabs are for popular, recommended, favorite, etc. plugins that are selected by the plugins repository. The tabs are (mostly) shortcuts when searching for new or replacement plugins. Dependencies are predetermined and required. A "Dependencies" tab doesn't belong there imho.
- Thinking that perhaps the dependencies will be shown separately for each theme/plugin. Seems this can be in the plugin's "Install/View details" dialog (popup/card). Installing dependencies should be from there too imho.
- It is unclear how incompatibilities between different versions of themes/plugins and their dependencies can be handled.
#278
@
21 months ago
@azaozz Not sure the global "Dependencies" tab is a good idea here. The other tabs are for popular, recommended, favorite, etc. plugins that are selected by the plugins repository. The tabs are (mostly) shortcuts when searching for new or replacement plugins. Dependencies are predetermined and required. A "Dependencies" tab doesn't belong there imho.
While it's true that one difference is that the plugins shown in the Dependencies tab are required, both the Dependencies and Favorites tabs contain a somewhat pre-determined list of plugins - One determined via the "Requires Plugins" headers, one determined by the user (plugins the user has marked as favorites).
In that sense, these tabs are not only used as search filters, but as lists of plugins curated through a number of paths, and the Dependencies tab provides shortcuts to install/activate a curated list of required plugins.
Thinking that perhaps the dependencies will be shown separately for each theme/plugin. Seems this can be in the plugin's "Install/View details" dialog (popup/card). Installing dependencies should be from there too imho.
Interesting thought. I took a look at the View details dialog to think about this in situ. I'm not sure where we could put dependency installation/activation links that would be easily consumed by users. On the other hand, the Dependencies tab provides an experience that's consistent with the other Install plugin tabs. It also doesn't overload users with too much additional information, and only shows the user what they need to handle the task of installing/activating dependencies.
It is unclear how incompatibilities between different versions of themes/plugins and their dependencies can be handled.
In the event that old versions of Plugin A and Plugin A addon 1 are installed, and a site owner installs and is able to activate Plugin A addon 2 that requires the latest version of Plugin A, Core already acts on a fatal error upon activation and forces Plugin A addon 2 to deactivate. If the error came later, then the handling of this is a site owner matter, as it is today.
For this initial phase of Plugin Dependencies, I think it's important that we don't explore versioning, as there's a fine line to tread between Plugin Dependencies, and Composer in WP. We could do with some time to discuss such possibilities and consider whether there's a viable path forward for some level of version handling in Core.
#279
in reply to:
โย 277
@
21 months ago
Replying to azaozz:
Thinking that perhaps the dependencies will be shown separately for each theme/plugin.
I think โPR 1547 (has screenshots) went in that direction with admin notices and inline notifications for dependencies. Would that approach be worth a second look?
I don't have a strong opinion on either approach at this time, but the current PR with the Dependencies tab appears to have more traction and wider testing. Some design feedback on what would be a better UX for various cases (one plugin with one dependency, multiple plugins with multiple dependencies, unmet dependencies, etc.) would be great here :)
#281
@
21 months ago
Replying to azaozz:
Generally I like the approach of having "manual" plugins and themes dependencies (manual in the sense that nothing is installed or activated automatically, instead there are messages making it easy to install the requirements).
Having some "second thoughts" about the UI and workflow here.
- Not sure the global "Dependencies" tab is a good idea here. The other tabs are for popular, recommended, favorite, etc. plugins that are selected by the plugins repository. The tabs are (mostly) shortcuts when searching for new or replacement plugins. Dependencies are predetermined and required. A "Dependencies" tab doesn't belong there imho.
The workflow is about installing plugins. Where else would we do that but on the Add Plugins page?
The Dependencies tab is really just a view (or prefilled search) that groups these plugins, just like any other view. It provides for a single source/location to view all the plugin dependencies for every installed plugin. The current implementation does display the plugin that requires the dependency within the plugin card.
- Thinking that perhaps the dependencies will be shown separately for each theme/plugin. Seems this can be in the plugin's "Install/View details" dialog (popup/card). Installing dependencies should be from there too imho.
While it may be possible and beneficial to display the plugin dependencies in the "View details" modal I've never seen an example of installing a secondary plugin from this modal, only the primary plugin. It seems that this would be an entirely new workflow for plugin installation.
I can see this as part of a phase 2 where the Plugins API also returns the Requires Plugins
header content and this is displayed in the modal's meta data. Not sure about how the display will look. But it's doable.
- It is unclear how incompatibilities between different versions of themes/plugins and their dependencies can be handled.
Version checking was discussed much in the ticket and felt to be out of scope at this time. Plugin Dependencies was not looking to create a "Composer for WordPress", which seemed to be where the discussion always led.
Plugin incompatibilities are handled the same way they are currently handled by core, which is to say by the user. If a new plugin were to cause a fatal on activation, core does not activate it. If the auto-update would cause a fatal on activation we are working on a solution to that within the Rollback feature plugin.
I think testing the feature plugin and utilizing the test plugins in the plugins test-plugins
folder is a great way to see the UI and workflow. I encourage everyone to do this.
#282
in reply to:
โย 280
@
21 months ago
Replying to afragen:
see https://core.trac.wordpress.org/ticket/22316#comment:260
https://core.trac.wordpress.org/ticket/22316#comment:261
https://core.trac.wordpress.org/ticket/22316#comment:262
Thanks! These comments have the design feedback I was looking for. It appears that the current PR with the Dependencies tab aims to reduce cognitive load from having too many notifications and offers a more straightforward workflow instead. Sounds good to me :)
This ticket was mentioned in โSlack in #core by abhanonstopnews. โView the logs.
21 months ago
This ticket was mentioned in โSlack in #core-upgrade-install by afragen. โView the logs.
20 months ago
This ticket was mentioned in โSlack in #core-upgrade-install by afragen. โView the logs.
20 months ago
This ticket was mentioned in โSlack in #core by afragen. โView the logs.
19 months ago
This ticket was mentioned in โSlack in #core by afragen. โView the logs.
18 months ago
#288
@
18 months ago
- Keywords changes-requested added
Testing the UI/UX and workflow of the current โPlugin Dependencies feature plugin, thinking couple of parts of the workflow should be changed and few parts need improvements.
- Installing a plugin that requires a dependency. The installation is allowed despite that the requirements for the plugin are not met.
- The first thing the user sees after installing and trying to activate is an error: "The [plugin_name] plugin(s) have been deactivated. There are uninstalled or inactive dependencies...". That's not a good workflow.
- This feels a bit like a "clickbait": "Yea, go ahead, install this plugin!". But after the plugin is installed: "Nonono, you cannot use (activate) this plugin. You have to install that other plugin first!". This is bad UX. Dependencies will have to be installed before a dependent plugin is allowed to be installed. (I know that's a bit harder to do from coding point of view, but the current workflow is really not acceptable imho.)
- The current workflow depends on a warning message in order to work. The next step after installing a plugin with unmet requirements is to show a WP Warning message at the top of
wp-admin/plugins.php
screen (the Plugins list table) in order to redirect the user to another screen where the missing dependency is shown. Depending on an error condition (a WP Warning message) in order for the workflow to work is not logical imho. That's not good UX too. This also seems redundant as the dependency can be installed from the dependent plugin's row in the list table. - After a plugin with unmet requirements (missing dependencies) is installed, the row in the plugin list table has no indication about the unmet requirements. The only indication that something is wrong there is the greyed-out "Cannot Activate" which seems pretty confusing (i.e. the user has a feeling they did something wrong).
- As I mentioned in my earlier comment the separate "Dependencies" screen tab that is under the Add New screen doesn't seem to make sense.
- It is not needed as dependencies can be installed from the listing the dependent plugin in the plugin's list table.
- It doesn't make sense to have such screen as it is out of context. It seems better to enhance the listing of the dependent plugin in the plugin's list table where dependencies will be in context. That would go a long way in making the whole functionality easier to grasp for many users.
- Instead of a new tab on the Add New screen, it seems more useful to have two "filtered views" in the Plugins list table. One for plugins that require a dependency, another for plugins that are used as a dependency.
My recommendations are s follow:
- Change the workflow so plugins that have unmet requirements cannot be installed.
- I looked around for anything that resembles this "Dependencies" UI/UX but couldn't find a good example. The closest one is when trying to install an app that requires some operating system part/library. In these cases the installation in not allowed until the requirements are met. That makes the most sense.
- For plugins with dependencies: list the requirements on the plugin "tile" and in the "Install" modal (on the Add New screen). Not mentioning that a plugin requires other plugins in order to work is a pretty bad omission imho.
- Use the plugin's tile and install modal not only to show but also to install missing dependencies.
- Other enhancements:
- Enhance the plugin's row in the list table to clearly show error conditions if a dependency is erased outside of WP or missing for some reason.
- Remove the Add New/Dependencies tab/screen. It will not be needed when the workflow is fixed to require dependencies before a plugin can be installed.
- Add two new "filtered views" to the plugins list table (next to "All", "Inactive", "Update Available ", etc.). Can't think of good names right now but perhaps something like "Dependent plugins" and/or "Plugin dependencies".
#289
follow-up:
โย 294
@
18 months ago
Thanks for the feedback @azaozz
For plugins with dependencies: list the requirements on the plugin "tile" and in the "Install" modal (on the Add New screen). Not mentioning that a plugin requires other plugins in order to work is a pretty bad omission imho.
The requirements are currently listed on the plugin tile. I believe listing them in the modal's contents would need to be done via wordpress.org consuming the Requires Plugins
header. I may be incorrect on this though.
Use the plugin's tile and install modal not only to show but also to install missing dependencies.
Could you post a mockup of how you're imagining the modal part of this? I'm concerned that this may overload users with a lot of information to install/activate each dependency all within the tight space of the modal. I appreciate that you may not be on the design team, but nevertheless it would be helpful to see a visual repesentation of your thoughts to start things off on exploring this option (otherwise potential design work on this may be way off base).
I looked around for anything that resembles this "Dependencies" UI/UX but couldn't find a good example. The closest one is when trying to install an app that requires some operating system part/library. In these cases the installation in not allowed until the requirements are met. That makes the most sense.
For themes that require plugins and where the plugins are manually installed, these are usually installed after installing the theme, via a screen like Theme Settings > Required Plugins
. However, depending on the mockup of the modal-based dependency installation/activation, installing/activating dependencies before the dependent may work fine even if it's a less familiar UX for users.
#290
follow-up:
โย 295
@
18 months ago
Oh, another thought @azaozz - If dependents can't be installed without their dependencies, what does this mean for uploading plugins to the site? Will we need to detect if the dependencies are installed and if not, block the upload, Or would users be allowed to upload dependents without their dependencies? - Whichever it is, how might this affect a consistent UX for installing/activating dependents and their dependencies?
#291
follow-up:
โย 293
@
18 months ago
On install, these dependencies should be handeled in about the same way as the minimum required version restrictions (WP/PHP).
And we can't assume a plugin with unmet dependencies will never be installed. There are other ways. So this must also be checked before allowing activation (UI and API level), plus, maybe, even after activation, forcing an immediate deactivation.
#292
@
18 months ago
Thanks for your input @knutsp! My response below is also written for those less familiar with the feature plugin and PR, so apologies for anything I mention that you're already aware of.
On install, these dependencies should be handeled in about the same way as the minimum required version restrictions (WP/PHP).
This is a concern of mine about the suggested change to the UX to prevent installation of a dependent without its dependencies - particularly (but not exclusively) for manual uploads.
We would need to unzip the archive, parse the Requires Plugins
header, detect whether all dependencies are installed, and if not, block the install and delete the uploaded ZIP and unzipped archive. So each dependency would need to be manually installed in the exact right order before allowing a dependent's "Install" button to be clicked or allowing a manual upload of the dependent. While this is something we as developers are used to, it may be much less familiar for general users. Installing the dependent that they want, then being told to install the extras it needs, may be a smoother process for them.
The feature plugin and PR currently allow the installation of a dependent without its dependencies, but in order to activate it, the dependencies will need to be installed and activated, which is what I believe you mention after:
And we can't assume a plugin with unmet dependencies will never be installed. There are other ways. So this must also be checked before allowing activation (UI and API level), plus, maybe, even after activation, forcing an immediate deactivation.
The feature plugin and PR currently prevent activation of a dependent plugin if its dependencies are not installed and activated.
In addition, the feature plugin and PR currently deactivate any dependent plugin should its dependencies suddenly become unavailable (such as if a dependency's directory is manually deleted).
#293
in reply to:
โย 291
@
18 months ago
Replying to knutsp:
On install, these dependencies should be handeled in about the same way as the minimum required version restrictions (WP/PHP).
And we can't assume a plugin with unmet dependencies will never be installed. There are other ways. So this must also be checked before allowing activation (UI and API level), plus, maybe, even after activation, forcing an immediate deactivation.
If you haven't tried the Plugin Dependencies feature plugin, you should. Look through the readme and the test-plugins
folder too.
Testing the plugin should answer most questions.
#294
in reply to:
โย 289
;
follow-up:
โย 306
@
18 months ago
Replying to costdev:
The requirements are currently listed on the plugin tile.
Uh, I've used the โPlugin Dependencies plugin from the repo. Perhaps it is a bit older.
I believe listing them in the modal's contents would need to be done via wordpress.org consuming the
Requires Plugins
header. I may be incorrect on this though.
Yes, it may be added through the wp.org API, but probably better to add it from the local code. Thinking it may be easier to "see" the required plugins status (for example "installed but not activated", etc.) and show the proper links or buttons. Also thinking the modal can have a bit expanded "about dependencies" section with a bit more explanations, etc.
Could you post a mockup of how you're imagining the modal part of this? I'm concerned that this may overload users with a lot of information to install/activate each dependency all within the tight space of the modal.
Yea, the exact UI changes to the modal are somewhat TBD. I'll try to make some sort of a mockup. As a minimum thinking that the modal would have a "Requirements" section with a good inline description that lists all of the requirements. That list should include the status (installed, activated), "More Info" link and an "Install" button when needed for each required plugin.
As far as I see the UI would probably be best to include the "info" for the "main" plugin and all of its dependencies in the same modal. This would make the modal taller, but would list everything required in one place which would make it easier for the users to understand and follow, and to be able to "make an informed decision" without needing to navigate away, close/reopen other modals, etc. To make the UI look better this can use an "accordion" type tabs or similar.
An alternative UI may be to "paginate" the modal (so it has "Previous" and "Next" buttons) however thinking that won't be as nice.
I tried making an example mockup of a "tile" for a plugin with dependencies, see above.
For themes that require plugins and where the plugins are manually installed, these are usually installed after installing the theme, via a screen like
Theme Settings > Required Plugins
. However, depending on the mockup of the modal-based dependency installation/activation, installing/activating dependencies before the dependent may work fine even if it's a less familiar UX for users.
Thinking this depends on whether the plugins are "hard" requirement for the theme to work or are optional. I agree that if they are "hard requirement" it would be better to add the same "Requirements" section as for plugins.
#295
in reply to:
โย 290
@
18 months ago
Replying to costdev:
If dependents can't be installed without their dependencies, what does this mean for uploading plugins to the site?
Thinking that dependencies should be listed on the screen after the zip is uploaded (where the version info is shown). That list should include the status of the dependency (installed, activated), and block the installation if any are missing or not activated. The list can also include the "More Info" links to open the modal for the dependency plugin where the plugin can be installed and/or activated.
Then if all dependencies are installed and activated from the modals, the UI can be "unblocked" so the uploaded plugin/theme can be installed (this would need a bit of JS).
But maybe better to take it one step an a time. Maybe improve/change/fix the "main" workflow first, then look at the "upload a zip" workflow and implement the same features.
This ticket was mentioned in โSlack in #core-upgrade-install by pbiron. โView the logs.
18 months ago
This ticket was mentioned in โSlack in #core-upgrade-install by costdev. โView the logs.
18 months ago
This ticket was mentioned in โSlack in #core-upgrade-install by afragen. โView the logs.
17 months ago
This ticket was mentioned in โSlack in #core by mukeshpanchal27. โView the logs.
17 months ago
#300
@
17 months ago
- Keywords changes-requested removed
- Milestone changed from 6.3 to 6.4
@karmatosed has kindly offered to take a look and provide design feedback on โthe issue on the feature plugin repo which should be done in the next few weeks. (Thank you!)
While design feedback and iteration will be happening during the Beta period, let's move this ticket to 6.4 and we should be able to land this during alpha.
Removing changes-requested
for now as the UX has gone through iteration and the status of this can be revisited after design feedback has been received and acted upon.
This ticket was mentioned in โSlack in #core-upgrade-install by costdev. โView the logs.
17 months ago
This ticket was mentioned in โSlack in #core-upgrade-install by afragen. โView the logs.
16 months ago
โ@afragen commented on โPR #3032:
16 months ago
#303
After #design has been completed the PR must be updated.
โ@afragen commented on โPR #3032:
16 months ago
#304
After #design has been completed the PR must be updated.
This ticket was mentioned in โSlack in #core-upgrade-install by karmatosed. โView the logs.
16 months ago
#306
in reply to:
โย 294
@
16 months ago
Replying to azaozz:
The below is what I'm currently working on as additional information to a plugin card. The dependency can be installed from the More details modal or the modal button will indicate if it is already installed.
This ticket was mentioned in โSlack in #core-upgrade-install by afragen. โView the logs.
16 months ago
#309
@
14 months ago
As we discussed with @afragen in Slack, seems the one thing left here is to "move" the functionality of the code in โthe PR to where it belongs in core. This would probably include removing all of the JS that "tweaks" the current WP UI, and moving the functionality to PHP. Also perhaps โmost hooks uses should be removed in favor of adding (new?) hooks where needed, or expanding the use/params and documentation of existing hooks.
#310
@
14 months ago
@azaozz I've updated the PR as best as I could but I'll need help. The JS "tweaks" aren't as easy to convert as we may think as the data is not available in the plugin row where the changes are made.
#311
@
14 months ago
Patch needs refresh, I believe, we don't need an element
in phpcs.xml.dist anymore.
Due to the amount of people who are watching this ticket, it could have been great to have it in 6.4, but we are getting closer to Beta 1, and it looks like the patch still needs some work to be done and tested as well, and this is a big patch...
This ticket was mentioned in โSlack in #core by azaozz. โView the logs.
14 months ago
This ticket was mentioned in โSlack in #core by oglekler. โView the logs.
13 months ago
#315
in reply to:
โย 314
@
13 months ago
Replying to afragen:
Uh, the PR/patch was starting to look really good after the latest changes by @costdev. Lets try to make sure this is ready for commit earlier in 6.5 so there is plenty of time for testing by plugin and theme developers, and any feedback is addressed well before beta.
This ticket was mentioned in โSlack in #core-upgrade-install by afragen. โView the logs.
12 months ago
This ticket was mentioned in โSlack in #core-upgrade-install by pbiron. โView the logs.
11 months ago
#318
follow-up:
โย 319
@
9 months ago
Update: The initial stage of this feature is close to commit-ready. We're working to add some final touches and should hopefully be doing a final review in the coming days.
#319
in reply to:
โย 318
@
9 months ago
- Keywords 2nd-opinion removed
Replying to costdev:
The initial stage of this feature is close to commit-ready.
Yep, the PR is looking quite good. Code, UI, UX all seem to work well. Lets "soak" it overnight and commit tomorrow?
โ@costdev commented on โPR #3032:
9 months ago
#321
Thanks for the PR! Committed in r57545.
#322
@
9 months ago
- Keywords dev-feedback needs-design-feedback removed
For reference, intensive synchronous discussion was carried out recently in Slack and can be read โhere.
Thanks to all who worked on this feature!
If you have a bug report or an enhancement request, please open a new ticket.
#323
follow-up:
โย 324
@
9 months ago
Two quick questions:
1
How can I test the plugins card UI in the 'Add Plugins' admin page? I see that [57545] changed something in the plugin cards that may be relevant for accessibility. However, to my understanding there would be the need for a plugin with dependencies already present in the wo.org plugins directory to be able to test the cards UI?
2
I guess the 'Plugin Dependencies' plugin should be removed from the wp.org directory soon? So far, it is still available in the 'Beta Testing' tab. Installing it and attempting to activate if triggers a fatal error, which is expected since the related code is now in Core.
#324
in reply to:
โย 323
;
follow-up:
โย 325
@
9 months ago
Replying to afercia:
Two quick questions:
1
How can I test the plugins card UI in the 'Add Plugins' admin page? I see that [57545] changed something in the plugin cards that may be relevant for accessibility. However, to my understanding there would be the need for a plugin with dependencies already present in the wo.org plugins directory to be able to test the cards UI?
Check for WPSSO
in Add Plugins search and you will find several examples.
2
I guess the 'Plugin Dependencies' plugin should be removed from the wp.org directory soon? So far, it is still available in the 'Beta Testing' tab. Installing it and attempting to activate if triggers a fatal error, which is expected since the related code is now in Core.
The Plugin Dependencies plugin has a kill switch built in for 6.5-beta1.
#326
@
8 months ago
Test Report
Description
- Bulk actions are not handling dependencies. For example, bulk deactivation should deactivate dependent plugins first before deactivating dependencies, and work in another direction if it is bulk activation. In fact, dependencies just cannot be selected.
- A plugin should be blocked from Activation if it has missing dependencies.
Otherwise, we get:
- Can we have an error message in the admin and not like wp_die() dead end?
- Notice that there is no name for the plugin that requires dependencies. In this case, it is โMy Hello Dollyโ. With a lot of plugins installed and some of them deactivated (as this is usually happening with projects with uneven history), it can be very confusing.
- If plugin was previously removed and due to this deactivated, and then added back again, the Note message is not matching its current state
- There are no check on the back end for dependencies during deactivation:
- Activate My Car plugin. Open another window and activate the My Car Trailer plugin. Go to the previous tab and deactivate the My Car plugin - it is successfully deactivated.
- Plugin deletion also has no check on the back end if with a valid link it can be deleted even if it has dependency.
- The plugin cannot be selected for a bulk action, but it can be deactivated. If it can be deactivated, it can be selected for a bulk action.
- The Note does not match the current possibilities either.
- The Note does not match the current state.
Environment
- WordPress: 6.5-beta2
- PHP: 8.1.9
- Server: Apache
- Database: mysqli (Server: 10.8.4-MariaDB / Client: mysqlnd 8.1.9)
- Browser: Chrome 121.0.0.0
- OS: Windows 10/11
- Theme: Twenty Twenty-Four 1.0
- MU Plugins: None activated
- Plugins:
- Test Reports 1.1.0
- WordPress Beta Tester 3.5.5
I'm working on a project where we've built an implementation of plugin dependencies on top of scribu's solution. It includes versioning support, so that you can require a minimum or maximum version of a given plugin. We've also got support for auto-installing dependent plugins.
See โhttps://github.com/cuny-academic-commons/commons-in-a-box/blob/master/admin/plugins-loader.php for the basic dependency stuff + versioning, and โhttps://github.com/cuny-academic-commons/commons-in-a-box/blob/master/admin/plugin-install.php for the plugin installation/upgrade dependency logic.