Making WordPress.org

Opened 12 months ago

Last modified 3 days ago

#3791 new defect

Consider bumping pre-commit checker on plugins to PHP 7.1

Reported by: Ipstenu Owned by:
Milestone: Priority: high
Component: Plugin Directory Keywords: pending-systems


Currently we have pre-commit checks set for PHP 7.0, which is great ... except people are starting to use PHP 7.1 and up for their code and getting burned by not being able to commit code to SVN becuase of that.

7.0 is only getting security reports through end of 2018, so it's probably time to have a gander at the possibility of bumping to 7.1 (or if we want to be ahead of the game, 7.2...)

Things to consider:

1) Will bumping the checker have an ADVERSE impact on plugins written for 5.2.4?

2) Do we have a way to see what PHP versions plugins use?

3) If 2 is yes, do more plugins use 5.2.4 or 7.1+?

Change History (20)

This ticket was mentioned in Slack in #meta by tellyworth. View the logs.

12 months ago

#2 @tellyworth
11 months ago

  • Keywords neso added

#3 @tellyworth
11 months ago

More WordPress sites use PHP 7.2 than PHP 5.2. I'll see if I can dig up stats on plugin compat.

It seems like the best way to proceed here is to bump the requirement and listen to feedback.

Can anyone suggest a compelling reason not to?

#4 @dd32
11 months ago

1) Will bumping the checker have an ADVERSE impact on plugins written for 5.2.4?

Shouldn't do. When we bumped it to 7.0 that would have added some (but that was okay)

Looking at the PHP 7.1/7.2 backward incompatible changes, I see nothing that should change linting of older code

2) Do we have a way to see what PHP versions plugins use?

We can run manual lookups for that kind of thing, there are roughly 40 plugins whose requires_php is PHP 7.1 or 7.2.

3) If 2 is yes, do more plugins use 5.2.4 or 7.1+?

Literally thousands more plugins are marked as requiring PHP 5.0~5.2.4. I don't think that has any impact here.

I see no reason why the linting for plugin commits shouldn't be increased to PHP 7.2 (which is what we're currently running for web nodes).

#5 @SergeyBiryukov
11 months ago

  • Component changed from General to Plugin Directory

#6 @SergeyBiryukov
5 months ago

  • Priority changed from normal to high

Could we bump the priority for this? See the discussion in https://twitter.com/Josh412/status/1100487567236059136 and https://twitter.com/Rarst/status/1100813140726562816.

Also discussed with @Rarst at WordCamp Nordic 2019.

#7 @Rarst
5 months ago

Do note that stable PHP is at 7.3 by now. In larger PHP community it seems that 7.1 is going to be "common" minimum required version for a while.

Having a hard barrier to use current (not even new anymore) PHP features significantly impairs progress of plugin ecosystem. While back I had counted barely 400+ PHP 7+ plugins. This is abysmally low.

#8 @dd32
5 months ago

  • Keywords pending-systems added; neso removed

Latest system request for this: https://make.wordpress.org/systems/2019/03/18/change-plugins-svn-linting-to-php-7-current-please/

I've requested that we continue to bump it as we roll out new PHP branches to dotorg, however due to the way the dotorg infrastructure is setup that might not be as easily achieved.

#9 @Otto42
5 months ago

We have had problems with the current iteration of the php linter, insofar as it looks at each individual file independently.

If a plugin includes backwards compatibility files in a way where the back compat checks are actually in another file, then the non compatible file, which normally would not be loaded in such an environment, will fail the lint check and thus cannot be committed.

Example: plugin does if ! function exists, then include file. File has that function. Function is a back commit function that exists in PHP 7. File, by itself, fails the lint. But it's never loaded because the plugin doesn't load it in such a case.

This is shockingly common in libraries included by plugins, so the authors don't know why it fails, because it's a library they didn't write.

#10 follow-up: @danielbachhuber
3 months ago

I'm getting this error when I try to commit to solr-power:

svn: E165001: Commit failed (details follow):
svn: E165001: Commit blocked by pre-commit hook (exit code 1) with output:

PHP error in: solr-power/trunk/vendor/symfony/contracts/HttpClient/ChunkInterface.php:

Parse error: syntax error, unexpected '?' in solr-power/trunk/vendor/symfony/contracts/HttpClient/ChunkInterface.php on line 65
Errors parsing solr-power/trunk/vendor/symfony/contracts/HttpClient/ChunkInterface.php

It looks like there might be some PHP 7.3 syntax in the Symfony dependency. Is there a way to skip the pre-commit hook?

#11 in reply to: ↑ 10 @dd32
3 months ago

Replying to danielbachhuber:

Is there a way to skip the pre-commit hook?

Unfortunately not.

#12 @Ipstenu
2 months ago

Is there anyone we can poke about this? It's getting progressively worse and more and more libraries people use are PHP 7.1+

Plus now that we block new submissions until people have used their already approved plugins, we get developers stuck :( The only work-around I can do for them is close the plugin and let them continue on, but it's a terrible experience.

#13 @jvanhengeldationnl
7 weeks ago

Same here. Checking in our plugin is blocked for all our libraries use 7.1+.

#14 @dhuethorst
7 weeks ago

Any updates? Blocked by some libraries using 7.1...

This ticket was mentioned in Slack in #meta by sergey. View the logs.

6 weeks ago

This ticket was mentioned in Slack in #core-php by clorith. View the logs.

5 weeks ago

#17 follow-up: @danielbachhuber
5 weeks ago

@dd32 @Otto42 Is there a known timeline for this being fixed? It would be concerning if this started blocking security updates to plugins (for plugins dependent on a third-party library).

#18 in reply to: ↑ 17 @dd32
5 weeks ago

Replying to danielbachhuber:

Is there a known timeline for this being fixed?

There's no timeline, it'll be fixed when the systems team gets to it and it's reasonable.

#19 @danielbachhuber
3 days ago

For the next person to run into this issue with their Composer dependencies, I thought of a temporary workaround:

"config": {
    "platform": {
        "php": "7.0"

If you haven't seen this before, this tells Composer to behave as though it's updating your dependencies on a PHP 7.0 system. For us, the only bundled change ended up being Downgrading symfony/event-dispatcher (v4.2.8 => v3.3.6), so it's a reasonable compromise. However, I did have to also "phpunit/phpunit": "^6 || ^7".

Lastly, I found https://github.com/JakubOnderka/PHP-Parallel-Lint to be a neat tool for seeing which files failed to lint. If you're on a Mac here are instructions on how to install PHP 7.0 via Homebrew. Once you've done so, linting looks like this:

parallel-lint ./ -p /usr/local/Cellar/php@7.0/7.0.33/bin/php

This ticket was mentioned in Slack in #meta by ipstenu. View the logs.

3 days ago

Note: See TracTickets for help on using tickets.