Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Determine minimum PHP version for WordPress core #9

Closed
wants to merge 1 commit into from

Conversation

tangrufus
Copy link

This implementation breaks for non-stable versions, i.e: beta and rc.
https://github.com/ItinerisLtd/wordpress/blob/1d34138b86fb5500480c97fba159b30afbe2a87d/src/MinPhpVersion.php would be a more robust alternative.
However, it requires introducing composer to the build system.

See:

@austinpray
Copy link
Contributor

Sweet, can test when I get home later.

@roots/everyone so I can merge this and it will only affect future releases. However, this runs the risk of pulling the rug out from some people. Consider:

  • user is on php 5.6.10
  • installs roots/wordpress@5.2 successfully
  • roots/wordpress@5.2.1 releases
  • install will now fail because they are on the wrong php version

Do we just recommend people fake it with composer.platform.php?

https://getcomposer.org/doc/06-config.md#platform

Is there some way we can get composer to show this recommendation?

@tangrufus
Copy link
Author

tangrufus commented May 25, 2019

Question: Does this make old relreases immutable?

if (gitTagExists($version)) {


I can merge this and it will only affect future releases. However, this runs the risk of pulling the rug out from some people.

  • make a composer plugin that echo warning messages
  • require the composer plugin in 5.2 and 5.2.1

However, this is not ideal. The new 5.2 and 5.2.1 might never be downloaded because of composer caches.

Or, make this change and hope nobody installs 5.2 and 5.2.1 with incorrect PHP

- version_compare($version, '5.2', '<')
+ version_compare($version, '5.2.2', '<')

@austinpray
Copy link
Contributor

Question: Does this make old relreases immutable?

Yep the updater script is idempotent cause of this line. We def want to avoid undefined behavior with cache status/hash collisions.

@tangrufus
Copy link
Author

tangrufus commented May 25, 2019

make a composer plugin that echo warning messages

This is a prototype: https://github.com/ItinerisLtd/roots-wordpress-announcement


After a second thought, composer generate checksum of the zip file. As the zip file doesn't include composer.json. We might be lucky this time.

However, we need to test what happen if we change 5.2 and 5.2.1 tags on packagist.org.


Alternatively, we could require the composer plugin in roots/wordpress-core-installer. Thus, keeping 5.2 and 5.2.1 unchanged.

bin/build-branch.php Outdated Show resolved Hide resolved
This implementation breaks for non-stable versions, i.e: `beta` and `rc`.
https://github.com/ItinerisLtd/wordpress/blob/1d34138b86fb5500480c97fba159b30afbe2a87d/src/MinPhpVersion.php would be a more robust alternative.
However, it requires introducing `composer` to the build system.

See:
- https://wordpress.org/news/2019/04/minimum-php-version-update/
- http://displaywp.com/wordpress-minimum-php-version/
@swalkinshaw
Copy link
Member

https://github.com/roots/wordpress-packager is taking care of this now and will be reflected in the new packages.

@swalkinshaw swalkinshaw closed this May 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants