Adding a PHP version directive to the plugin manifest

[jasonw4331]

Introduction

It has been mentioned before (#3195) that using features from higher php versions on servers using previous versions is an issue. This PR addresses that issue by adding an optional property php to let plugin developers specify the compatible php versions.

Relevant issues

Closes #3195

Follow-up

This PR requires translation for pocketmine.plugin.incompatiblePhpVersion (https://github.com/pmmp/Language/pull/57)

Tests

All of these tests were done using PHP 7.3.14, I added the following properties to the plugin manifest and noted the result:

• php: 7.3 -> Plugin loads
• php: 7.3.14 -> Plugin loads
• php: 7.3.15 -> Plugin doesn't load
• php: 5.4 -> Plugin doesn't load
• Unspecified -> Plugin loads

[jasonw4331]

Is it decided this wont go on stable for feature freeze?

[dktapps]

I think this is OK for stable since the problem it's trying to address is getting more prevalent (PM3 now supports 7.3, 7.4 and 8.0 simultaneously).

However, this can't be merged directly into stable because it's not a bug fix and would cause inconsistent behaviour of plugin loading (a plugin which declares this might load in 3.18.0 ignoring the constraint, but not on 3.18.1 which respected it). Therefore, this might be better suited to 4.0 anyway (or at the earliest 3.19, which would still have the same problem).

[VixikHD]

It is already possible to check php version in extensions field (https://github.com/CzechPMDevs/BuilderTools/blob/default/plugin.yml#L10)

[JaxkDev]

Yes its possible however as you can only specify a minimum OR a maximum (from what I can tell you cant specify multiple constraints) this leads to very different behaviour to this PR, the PR doesn't allow 'major'/'minor' differences. eg php: 7.3 would load in php 7.3.x but not 7.4.x and 8.x.y whereas in the extensions method specifying core: >=7.3 would load in php 7.3 and above including 8.x.y

[SOF3]

I don't think it's meaningful to consider what happens when 8.0 comes. Currently we aren't checking PHP version at all, and after this check, plugins can opt to reject PHP versions that the plugin is known to have problems with. Using minimalistic version_compare() can't be worse than not using it; that was my rationale when I added the extensions directive. It's a total waste of time to think about versions we can't predict about, especially when PHP isn't semver-compliant at all.

[fuyutsuki]

Tbh, I don't think this change is necessary at all. From now on, pmmp will start using 8.0, the latest of the three multi-releases of php, and inevitably older versions of php will be eliminated.
Therefore, it is not the plugin but the server software that specifies the php version, and this PR that specifies the php version on the plugin side is meaningless.

…I think.

[dktapps]

Not really. It's not like PHP 8.0 will be the last version with new features. 8.1 is coming in just a few months with a host of new features also.

[JaxkDev]

Is there anything pending on this ? As it would be great to have a dedicated PHP field so poggit and others can parse the value.

[jasonw4331]

I can change the branch to API 4 if needed

[dktapps]

After further consideration it occurred to me that since people are already using extensions.Core for this, the php field might as well use the same version constraint system as extensions to reduce the amount of code required and improve consistency.

[SOF3]

Alternative opinion: in the future, we should lock all PM API versions to a specific PHP version, where PM explicitly requires that PHP version to work. In that case, only specifying the PM API version should be sufficient and we don't need to mess with all the extensions stuff.

The counterargument is that PHP is actually not server compliant. But right now there are no mechanisms to prevent this anyway (I don't think any plugins would set a minor upper bound unless they're doing something highly syntax specific like php-cs-fixer). So when that happens, just update the plugin to fix it, since PHP BC breaks typically just affect (proclaimed) bad practices anyway.

[jasonw4331]

I think sofe mentioned a better solution, and I no longer believe this PR provides a method any better than the existing extensions.Core directive.

[SOF3]

@jasonwynn10 that said, I don't see this version locking happening any time soon, especially with all the people demanding new language features whilst us lagging to ship up-to-date, production-ready pthreads