#46938 closed enhancement (fixed)
Add Requires WP and Requires PHP headers for plugin compatibility tests
Reported by: | afragen | Owned by: | SergeyBiryukov |
---|---|---|---|
Milestone: | 5.3 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Site Health | Keywords: | servehappy has-patch needs-testing commit |
Focuses: | Cc: |
Description
Per #43992 comment:47 the second iteration of plugin activation checking should consider taking into account plugins that do not have valid readme.txt
files but instead have added additional plugin headers, Requires WP
and Requires PHP
to the main plugin file.
Precedence is given to headers from the readme.txt
file.
This is most common in premium plugins and plugins hosted elsewhere, like GitHub.
This is a likely enough scenario that we should test for it.
Attachments (2)
Change History (34)
This ticket was mentioned in Slack in #core-php by afragen. View the logs.
5 years ago
#3
@
5 years ago
Not sure how good of an idea it is to add support for these to the plugin headers. It's a bit odd to be able to support these in two places.
#4
@
5 years ago
@swissspidy it’s really a fallback if there aren’t settings in a readme.txt
file.
Many plugins already declare these. https://wpdirectory.net/search/01D8HY8CV3HB0ZBZYGVZ2V6E8K
#6
follow-up:
↓ 7
@
5 years ago
Many plugins already declare these
So they are technically doing_it_wrong at the moment, no?
This might also be useful for mu-plugins.
mu-plugins can't be activated or deactivated. They are always on. If someone adds an mu-plugin, they probably know what they're doing.
#7
in reply to:
↑ 6
@
5 years ago
Replying to swissspidy:
Many plugins already declare these
So they are technically doing_it_wrong at the moment, no?
I’m not clear how adding a header comment is doing_it_wrong. I have personally used these headers in most of my plugins and check for them for precisely this purpose in GitHub Updater.
I think the decision is do we wish to create a method of testing plugins that are not in wp.org or do we not. I obviously have a preference.
Perhaps a few more core committers could give an up or down opinion.
This might also be useful for mu-plugins.
mu-plugins can't be activated or deactivated. They are always on. If someone adds an mu-plugin, they probably know what they're doing.
Absolutely correct.
Thanks @swissspidy for your insight. It is greatly appreciated.
This ticket was mentioned in Slack in #core-php by afragen. View the logs.
5 years ago
This ticket was mentioned in Slack in #core-php by afragen. View the logs.
5 years ago
This ticket was mentioned in Slack in #core-php by afragen. View the logs.
5 years ago
This ticket was mentioned in Slack in #core by afragen. View the logs.
5 years ago
#13
follow-up:
↓ 15
@
5 years ago
How will this impact non-WordPress.org hosted plugins that don't include a readme.txt?
Are we now going to require that ALL plugins use our headers?
#14
follow-ups:
↓ 16
↓ 21
@
5 years ago
Realistically, I'm surprised that we added parsing of the readme.txt for these in the first place. They should have been in the plugin's main PHP file all along.
Historically, the readme.txt for plugins has only been used by the WordPress.org directory. This provided a clear separation. WordPress reads the PHP file, the directory reads the readme.txt file for other things not in the PHP file's headers.
So, my 2 cents is to instead add reading these headers to the plugin's PHP file, and give precedence to that instead of the readme.txt file.
With a possible long term goal being to undo this mistake, and eliminate reliance on the readme.txt file from core itself. Readme.txt files are *not* required files for plugins, and never have been. We cannot rely on them being present.
#15
in reply to:
↑ 13
@
5 years ago
Replying to Ipstenu:
How will this impact non-WordPress.org hosted plugins that don't include a readme.txt?
It will provide a mechanism for non-wp.org hosted plugins to be properly validated for plugin activation based upon the compatibility requirements listed in the plugin headers.
Are we now going to require that ALL plugins use our headers?
I think in time we may choose to require some definitive statement in the plugin regarding WP and PHP compatibility. Whether that occurs in the plugin headers and/or a readme.txt hasn’t been determined.
#16
in reply to:
↑ 14
@
5 years ago
Replying to Otto42:
Realistically, I'm surprised that we added parsing of the readme.txt for these in the first place. They should have been in the plugin's main PHP file all along.
Historically, the readme.txt for plugins has only been used by the WordPress.org directory. This provided a clear separation. WordPress reads the PHP file, the directory reads the readme.txt file for other things not in the PHP file's headers.
So, my 2 cents is to instead add reading these headers to the plugin's PHP file, and give precedence to that instead of the readme.txt file.
I initially wrote the patch giving the precedence to the currently available information. It should be easy to switch.
With a possible long term goal being to undo this mistake, and eliminate reliance on the readme.txt file from core itself. Readme.txt files are *not* required files for plugins, and never have been. We cannot rely on them being present.
While readme.txt files aren’t required; neither are the plugin headers. My thought is that as long as the compatibility information exists in one of these places it should be utilized and in time required.
#17
follow-up:
↓ 18
@
5 years ago
I think in time we may choose to require some definitive statement in the plugin regarding WP and PHP compatibility. Whether that occurs in the plugin headers and/or a readme.txt hasn’t been determined.
That isn't what I meant. Let me rephrase :)
Are we going to require all plugins, be they hosted on WordPress.org or not, to have PHP compat in headers in order to be activated?
The subject of the ticket is "Prevent activation of incompatible plugins without valid readme.txt" (bolding mine)
I took that to mean "If a plugin doesn't have valid headers, do not activate." And if that's the case, I am firmly against this call. It would be a terrible experience for users. Non-org plugins don't have readmes. Home grown plugins often don't. We would do more harm there.
I don't have a horse in the race for readme or php files, though based on what Otto said, PHP sounds smarter in the long run. Especially since you have to have the headers if you want your plugin to look nice in the plugins listing :)
#18
in reply to:
↑ 17
@
5 years ago
Replying to Ipstenu:
I think in time we may choose to require some definitive statement in the plugin regarding WP and PHP compatibility. Whether that occurs in the plugin headers and/or a readme.txt hasn’t been determined.
That isn't what I meant. Let me rephrase :)
Are we going to require all plugins, be they hosted on WordPress.org or not, to have PHP compat in headers in order to be activated?
No, we aren’t ready to make this a mandatory requirement. At some future point I can see this requirement to aid in more completely preventing plugin PHP fatals for PHP incompatibility.
The subject of the ticket is "Prevent activation of incompatible plugins without valid readme.txt" (bolding mine)
I took that to mean "If a plugin doesn't have valid headers, do not activate." And if that's the case, I am firmly against this call. It would be a terrible experience for users. Non-org plugins don't have readmes. Home grown plugins often don't. We would do more harm there.
I see this as an either/or choice. The PHP compatibility headers would be either in a readme.txt
or the plugin headers. At this time there is no requirement for compatibility headers anywhere. As such if none exist a plugin is allowed to install, update, and activate.
The current is_php_version_compatible()
and is_wp_version_compatible()
are true if those values evaluate to true via the version_compare
or if they don’t exist or are empty. This title was a bit of future guessing that someday a WP and PHP requirement might actually be required and the best (IMO) method of allowing this for the most users is the ability to have this information in the plugin headers.
I don't have a horse in the race for readme or php files, though based on what Otto said, PHP sounds smarter in the long run. Especially since you have to have the headers if you want your plugin to look nice in the plugins listing :)
The patch only seeks to allow for the Requires WP
and Requires PHP
headers to be utilized by the compatibility tests. Nothing more.
I do agree that a larger discussion about the requirement of such headers for plugin activation, etc. should be had at some point in the future. Currently I think we shouldn’t penalize anyone for not having the compatibility requirements noted. It is my opinion that both methods should continue to be acceptable now, and in the future, as data in the readme.txt
and in plugin headers is a well established practice for WordPress plugins.
@Ipstenu @Otto42 I really appreciate your opinions and expertise in this area. Thank you for all you do for us in the Plugin Directory.
#19
@
5 years ago
I see this as an either/or choice. The PHP compatibility headers would be either in a readme.txt --
I'm not asking about the PHP compat headers at all at this moment. I'm asking for clarification about the intent of this ticket, becuase based on the name, I think you've crossed your streams. :)
I think you want to rename this trac ticket, because I get the impression you're meaning "Check PHP Compat before activation" and instead you said "Don't let plugins with bad readme headers activate." Those aren't at all the same thing.
To be clear:
- Should we check PHP compatibility before activation? Yes
- Should we prevent activation if compat is invalid (i.e. you have PHP 5.6 and the plugin says 7.1)? Yes
- Should we prevent activation is the PHP compat header is missing? NO
- Should we prevent activation if the readme.txt has mucky headers or is missing? NO
The patch only seeks to allow for the Requires WP and Requires PHP headers to be utilized by the compatibility tests. Nothing more.
Then yeah you absolutely need to rename this ticket, because it's very misleading. I recommend "Add Requires WP and Requires PHP headers to plugin compatibility tests"
And remember that lack of those headers, regardless of where they are (readme, php, a json file, don't care), means we warn but still permit activation.
#20
@
5 years ago
- Summary changed from Prevent activation of incompatible plugins without valid readme.txt to Add Requires WP and Requires PHP headers for plugin compatibility tests
Thanks @Ipstenu
And remember that lack of those headers, regardless of where they are (readme, php, a json file, don't care), means we warn but still permit activation.
We actually don’t warn we simply permit activation, but yes.
#21
in reply to:
↑ 14
;
follow-up:
↓ 26
@
5 years ago
Replying to Otto42:
Realistically, I'm surprised that we added parsing of the readme.txt for these in the first place. They should have been in the plugin's main PHP file all along.
Historically, the readme.txt for plugins has only been used by the WordPress.org directory. This provided a clear separation. WordPress reads the PHP file, the directory reads the readme.txt file for other things not in the PHP file's headers.
Thanks for the context. Just to clarify, the Requires PHP
header was introduced in #meta2952 for the directory first, so it made more sense to have it defined in the same place as Requires at least
, which only exists in readme.txt
. Otherwise it would be confusing and inconsistent to define requirement headers in two different places, one in readme.txt
and one in the plugin's main PHP file, especially if each only works in that specific place.
So, my 2 cents is to instead add reading these headers to the plugin's PHP file, and give precedence to that instead of the readme.txt file.
If that's the preferred way forward, sounds good. Plugin Directory would need the same change as well for consistency.
This ticket was mentioned in Slack in #core-php by afragen. View the logs.
5 years ago
#24
@
5 years ago
- Keywords commit added; dev-feedback removed
For consistency with the readme.txt
headers, I think Requires WP
should be Requires at least
, so that both headers (Requires at least
and Requires PHP
) could be supported in both places (readme.txt
and plugin file). Looks good otherwise.
#26
in reply to:
↑ 21
@
5 years ago
Replying to SergeyBiryukov:
So, my 2 cents is to instead add reading these headers to the plugin's PHP file, and give precedence to that instead of the readme.txt file.
If that's the preferred way forward, sounds good. Plugin Directory would need the same change as well for consistency.
Created #meta4514 for the Plugin Directory.
#27
@
5 years ago
- Owner set to SergeyBiryukov
- Resolution set to fixed
- Status changed from new to closed
In 45546:
test for new plugin compatibility headers