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

Require PHP 7.2 or newer #743

Merged
merged 24 commits into from
Oct 24, 2022
Merged

Conversation

Art4
Copy link
Contributor

@Art4 Art4 commented Aug 30, 2022

Hi all 👋

This PR increases the required PHP version to 7.2 or newer. I propose this PR for SimplePie 1.8 (see Roadmap in #731).

Increasing the PHP version to 7.2 will give us some new features:

  • Return type declarations
  • Strict typing (declare(strict_types=1);)
  • Anonymous classes
  • Nullable types
  • Void functions
  • Symmetric array destructuring (already used in this PR)
  • Class constant visibility (already used in this PR)
  • Abstract method overriding
  • Parameter type widening

It will allow us to support PSR-18 HTTP client (see #520)

Based on the download numbers from packagist.org (see below) I would say that it is save to increase the PHP version to 7.2. But I would also like to hear feedback from projects like WordPress (ping @rmccue) and FressRSS (ping @Alkarex).

Downloads for all SimplePie versions

Over 96% of all downloads are on PHP 7.2 or newer.

grafik

Downloads for SimplePie version 1.6

Looking only at SimplePie 1.6 over 98% of all downloads are on PHP 7.2 or newer.

grafik

@Art4 Art4 mentioned this pull request Aug 30, 2022
48 tasks
@Alkarex
Copy link
Contributor

Alkarex commented Aug 30, 2022

But I would also like to hear feedback from projects like WordPress (ping @rmccue) and FressRSS (ping @Alkarex).

We are about to release FreshRSS 1.20.x supporting PHP 7.0 but from FreshRSS 1.21+ we will require PHP 7.2+

We are more or less tracking Debian and Ubuntu LTS (Debian 9 Stretch supported PHP 7.0 until June 2022)
FreshRSS/FreshRSS#3321 (comment)

@Art4
Copy link
Contributor Author

Art4 commented Aug 30, 2022

P.S. What about WordPress? It is shipping SimplePie, right? And still supporting PHP 5.6.20+

Yes, I would very much appreciate feedback from WordPress. Maybe @jrfnl could help us?

@jrfnl
Copy link
Contributor

jrfnl commented Aug 30, 2022

Thank you very much for the ping @Art4 !

You are 100% correct, SimplePie is shipped with every download of WordPress and this will not show in the Packagist's stats as WP does not manage its non-dev dependencies via Packagist, nor does it manage its distribution via Packagist.

While there are always discussions on-going in the WordPress community about dropping support for certain PHP versions on the lower end (and I won't deny that I'm regularly an instigator of those 😇), at this time, I expect no movement away from the PHP 5.6 minimum in the near future.

WordPress generally bases these type of decisions on real-life stats of WordPress installs. The sum-total of the users on PHP versions for which support will be dropped needs to be less than 5% of all WordPress users.

If we look at the current stats, you can see that PHP 5.6 still represents 5.8% of the user base, PHP 7.0 3,1% and PHP 7.1 2.2%, which comes to a sum-total of > 11% of the users.

Please keep in mind that 1% of users for WordPress is > 1 million +/- 0.5 million sites (I'd need to ask around to get a more precise number), so blocking the upgrade path for millions and millions of WP sites is always something which needs very careful consideration.

Having said all that, and though I cannot speak for the WordPress project as such, I do believe it would be greatly appreciated if the minimum supported PHP version for SimplePie would remain at PHP 5.6 for a while longer.

All the same, SimplePie is an independent project, so if you do decide to up the minimum supported PHP version, WordPress would just have to deal with it until the WP minimum supported PHP version will go up.

In practice this would mean:

  • Backporting security fixes (if any).
  • Backporting PHP cross-version compatibility fixes.

Speaking personally and from a maintainer point of view, I do think it would be better for a version drop like this to go into a major release (2.0), not a minor (1.8). As said before, not all downloads/installs go via Composer, so including this in a major will more clearly signal that people need to pay close attention to the changelog when upgrading.

Hope this helps and gives some perspective.

P.S.: I've also pinged the maintainers of the WordPress "External Libraries" component with a link to this ticket, so they may pitch in with an opinion later.

@Art4
Copy link
Contributor Author

Art4 commented Aug 30, 2022

Thanks for the detailed explanations @jrfnl.

I can't speak for the SimplePie team, but based on the roadmap (#731) I can imagine the 1.7 branch as the LTS branch for PHP 5.6 support. If WordPress switches to namespaced classes (src folder), backports of bugfixes will be much easier.

In version 1.8 we could bump to PHP 7.2+ and add alternatives to the cache and HTTP client implementations. However, I could also imagine just bumping to PHP 7.0+ instead of 7.2+. This would complicate the upgrade process, but it would allow WordPress to move to a newer version once PHP 6.5 support is dropped.

With 1.9 we could add warnings about deprecated code as a helpful upgrade layer, and with version 2.0 we will remove all deprecated code.

Or to summarize all this in a table:

Version/branch Status requires First release Latest patch
1.3.x ❌ Unsupported (eol) PHP 5.2.0+ 1.3 (2012-07-07) 1.3.3 (2021-12-24)
1.4.x ❌ Unsupported (eol) PHP 5.3.0+ 1.4 (2016-04-26) 1.4.3 (2016-11-27)
1.5.x ❌ Unsupported (eol) PHP 5.6.0+ 1.5 (2017-04-17) 1.5.8 (2021-12-21)
1.6.x ✅ Supported PHP 5.6.0+ 1.6.0 (2022-04-21) 1.6.0 (2022-04-21)
1.7.x 🏗️ WIP (Switch to PSR-4, LTS) PHP 5.6.0+
1.8.x 🏗️ WIP (PSR-16/18 support) PHP 7.0+ or 7.2+
1.9.x 💭 Add deprecation warnings PHP 7.0+ or 7.2+
2.0.x 💭 Remove deprecated code PHP 7.0+ or 7.2+

@jtojnar
Copy link
Contributor

jtojnar commented Aug 31, 2022

I do not think raising minimum PHP version in minor version would be compatible with semver. It should be done in 2.0.0.

Deprecations should be added in minor versions according to point 7 of semver.

So maybe 1.8 + 1.9 should just become 2.0. And 2.0 → 3.0. And 2.0 does not need to have a support window, it can just serve as a migration step.

@Art4
Copy link
Contributor Author

Art4 commented Aug 31, 2022

I do not think raising minimum PHP version in minor version would be compatible with semver. It should be done in 2.0.0.

Deprecations should be added in minor versions according to point 7 of semver.

Increasing the minimum PHP version is not a deprecation, but a change in dependencies. Just as requiring another library as a new dependency is not a breaking change either. Before installing the new version of a library, one must always check if one meets the requirements of the new version. If not, one have to keep the old version. Composer does this automatically and will not allow you to install the new version if the new requirements are not met.

This is also clarified in semver.

So maybe 1.8 + 1.9 should just become 2.0. And 2.0 → 3.0. And 2.0 does not need to have a support window, it can just serve as a migration step.

My proposal is that version 1.9 will be this migration step.

@skyzyx
Copy link
Member

skyzyx commented Sep 6, 2022

@jtojnar said:

I do not think raising minimum PHP version in minor version would be compatible with semver. It should be done in 2.0.0.

Deprecations should be added in minor versions according to point 7 of semver.

SemVer is for defining how a software contract changes over time. However, it does not cover what is in-bounds/out-of-bounds for that software contract. The fact is: SimplePie needs a software contract to go along with SemVer versioning to define exactly this.

My opinion is that since PHP versions go EOL annually, it doesn't make sense to have a major version bump every year. SimplePie 1.0 went live in 2006(?) and supported PHP 4.3. We've dropped several versions of PHP since then.

Also, old versions of SimplePie still work. If you are still running an EOL PHP runtime, then you can run an old version of SimplePie. The version you have won't suddenly stop working.

If it were up to me, SimplePie would not consider EOL compatibility in new features or refactorings. PHP 7.4 goes EOL this November/December and only 8.x will be left in supported status. I think it's extremely generous that this PR supports back to PHP 7.2.

To agree with @Art4: We should be dropping support for old versions of PHP annually. If WordPress wants to support PHP 5.6, then they can. But it'll be an older version of SimplePie. While I genuinely do appreciate reaching out to affected software projects about compatibility so that everyone can work together, I disagree that those projects should hold SimplePie back.

If only we had something like Babel.js for PHP

@skyzyx skyzyx requested a review from mblaney September 6, 2022 19:33
@jtojnar
Copy link
Contributor

jtojnar commented Sep 6, 2022

SemVer is for defining how a software contract changes over time. However, it does not cover what is in-bounds/out-of-bounds for that software contract. The fact is: SimplePie needs a software contract to go along with SemVer versioning to define exactly this.

Good point. In an absence of such contract, I was basing the suggestion on best practices from the Node ecosystem, where bumping the major version occurs when a support for a Node dependency is dropped. For PHP, we would need to apply this even for any change in dependency bounds in composer.json, since PHP unlike NPM does not support multiple versions of the same library in the dependency tree (unless the symbols are renamed). That probably is not reasonable.

If only we had something like Babel.js for PHP

We do 😉 It works pretty well for me for various refactoring tasks (e.g. j0k3r/graby#282, RSS-Bridge/rss-bridge#2910) and there are people using it to develop in PHP 8.1 and transpile the artifacts downwards.

@mblaney
Copy link
Member

mblaney commented Sep 22, 2022

I think it is important to consider WordPress a special case, but the current version of SimplePie included with it is stable, which means WordPress users aren't asking for updates (like has happened previously). To me that means it's ok to make our own decisions and I tend to agree with dropping support for the older versions of PHP.

The recent changes to SimplePie have been great and mostly aimed at modernizing the code base. Seems reasonable given the direction taken that older versions of PHP are dropped so that newer features of the language can be used.

@Alkarex
Copy link
Contributor

Alkarex commented Sep 22, 2022

But so far, SimplePie is only maintaining a single branch, right?
What would be the workflow for e.g. WordPress or others interested in stability rather than fancy new language features, to get new features and security fixes? Fork SimplePie and backport relevant changes?

@Art4
Copy link
Contributor Author

Art4 commented Sep 22, 2022

@Alkarex good question. I would suggest introducing a new branching workflow, which is then also described in the README.md. If you think about my suggestion above with the table, the branches could look like this:

  • v1.7-lts (LTS for PHP 5.6)
  • v1.x (Other 1.x versions like 1.8.0, 1.9.0, etc)
  • v2.x (All 2.x versions like 2.0.0, etc.)

Bug fixes or security fixes are created in the latest branch (v1.x or v2.x) and can then be backported to the v1.7-lts branch.

The current master branch will then no longer be needed.

@Art4
Copy link
Contributor Author

Art4 commented Sep 29, 2022

If we look at the current stats, you can see that PHP 5.6 still represents 5.8% of the user base, PHP 7.0 3,1% and PHP 7.1 2.2%, which comes to a sum-total of > 11% of the users.

Today after 30 days PHP 5.6 has 5,5%, PHP 7.0 has 3,0% and PHP 7.1 has 2,1% user base, which comes to a sum-total of 10,6%. Based on these falling numbers, I would guess that the LTS branch will not be needed for very long.

Update 2022-11-17: Today after 79 days PHP 5.6 has 5,2%, PHP 7.0 has 2,8% and PHP 7.1 has 1,9% user base, which comes to a sum-total of 9,9%.

@Art4
Copy link
Contributor Author

Art4 commented Sep 30, 2022

@mblaney What do you think about the idea of a new branching strategy? Now that version 1.7.0 is released, it would be the perfect time to do so. The roadmap for that might look like this:

  1. Rename branch master to v1.x (v1.x will be the new default branch)
  2. Create new branch v1.7-lts from v1.x branch.
  3. Edit README.markdown to explain new branching strategy. I can create a PR for this.
  4. Merge this PR into v1.x branch
  5. Add note in v1.7-lts branch about LTS. I can create a PR for this.

@Art4 Art4 requested a review from mblaney October 4, 2022 09:01
@Art4 Art4 marked this pull request as ready for review October 4, 2022 09:01
@Art4 Art4 mentioned this pull request Oct 12, 2022
@Art4
Copy link
Contributor Author

Art4 commented Oct 12, 2022

I moved the the discussion and necessary changes for a new branching strategy to #751.

@mblaney
Copy link
Member

mblaney commented Oct 23, 2022

this looks good to merge now?

@Art4
Copy link
Contributor Author

Art4 commented Oct 23, 2022

this looks good to merge now?

After the merges of the other PRs I will have to take a look again for more changes.

@Art4 Art4 marked this pull request as draft October 23, 2022 04:33
@Art4 Art4 marked this pull request as ready for review October 24, 2022 07:52
@Art4
Copy link
Contributor Author

Art4 commented Oct 24, 2022

this looks good to merge now?

@mblaney This PR is now ready for merge.

@mblaney mblaney merged commit d0552b0 into simplepie:master Oct 24, 2022
@Art4 Art4 deleted the require-min-php-version-72 branch October 24, 2022 09:23
jtojnar added a commit to fossar/selfoss that referenced this pull request Nov 3, 2022
This will be required for the upcoming SimplePie version:
simplepie/simplepie#743

Debian 9 Stretch requiring PHP 7.0 became unsupported in June 2022:
FreshRSS/FreshRSS#3321 (comment)

Changelogs summary:

 - symfony/debug removed (installed version was v3.4.47)

 - paragonie/random_compat removed (installed version was v2.0.21)

 - symfony/polyfill-mbstring updated from v1.19.0 to v1.26.0
   See changes: symfony/polyfill-mbstring@v1.19.0...v1.26.0
   Release notes: https://github.com/symfony/polyfill-mbstring/releases/tag/v1.26.0

 - doctrine/lexer updated from 1.0.2 to 1.2.3
   See changes: doctrine/lexer@1.0.2...1.2.3
   Release notes: https://github.com/doctrine/lexer/releases/tag/1.2.3

 - symfony/polyfill-php72 updated from v1.19.0 to v1.26.0
   See changes: symfony/polyfill-php72@v1.19.0...v1.26.0
   Release notes: https://github.com/symfony/polyfill-php72/releases/tag/v1.26.0

 - symfony/polyfill-intl-normalizer updated from v1.19.0 to v1.26.0
   See changes: symfony/polyfill-intl-normalizer@v1.19.0...v1.26.0
   Release notes: https://github.com/symfony/polyfill-intl-normalizer/releases/tag/v1.26.0

 - symfony/polyfill-intl-idn updated from v1.19.0 to v1.26.0
   See changes: symfony/polyfill-intl-idn@v1.19.0...v1.26.0
   Release notes: https://github.com/symfony/polyfill-intl-idn/releases/tag/v1.26.0

 - psr/container installed in version 1.1.1
   Release notes: https://github.com/php-fig/container/releases/tag/1.1.1

 - symfony/service-contracts installed in version v1.1.13
   Release notes: https://github.com/symfony/service-contracts/releases/tag/v1.1.13

 - symfony/stopwatch updated from v3.4.47 to v4.4.46
   See changes: symfony/stopwatch@v3.4.47...v4.4.46
   Release notes: https://github.com/symfony/stopwatch/releases/tag/v4.4.46

 - symfony/polyfill-php80 installed in version v1.26.0
   Release notes: https://github.com/symfony/polyfill-php80/releases/tag/v1.26.0

 - symfony/process updated from v3.4.47 to v4.4.44
   See changes: symfony/process@v3.4.47...v4.4.44
   Release notes: https://github.com/symfony/process/releases/tag/v4.4.44

 - symfony/polyfill-php70 updated from v1.19.0 to v1.20.0
   See changes: symfony/polyfill-php70@v1.19.0...v1.20.0
   Release notes: https://github.com/symfony/polyfill-php70/releases/tag/v1.20.0

 - symfony/options-resolver updated from v3.4.47 to v4.4.44
   See changes: symfony/options-resolver@v3.4.47...v4.4.44
   Release notes: https://github.com/symfony/options-resolver/releases/tag/v4.4.44

 - symfony/finder updated from v3.4.47 to v4.4.44
   See changes: symfony/finder@v3.4.47...v4.4.44
   Release notes: https://github.com/symfony/finder/releases/tag/v4.4.44

 - symfony/polyfill-ctype updated from v1.19.0 to v1.26.0
   See changes: symfony/polyfill-ctype@v1.19.0...v1.26.0
   Release notes: https://github.com/symfony/polyfill-ctype/releases/tag/v1.26.0

 - symfony/filesystem updated from v3.4.47 to v4.4.42
   See changes: symfony/filesystem@v3.4.47...v4.4.42
   Release notes: https://github.com/symfony/filesystem/releases/tag/v4.4.42

 - symfony/event-dispatcher-contracts installed in version v1.1.13
   Release notes: https://github.com/symfony/event-dispatcher-contracts/releases/tag/v1.1.13

 - symfony/event-dispatcher updated from v3.4.47 to v4.4.44
   See changes: symfony/event-dispatcher@v3.4.47...v4.4.44
   Release notes: https://github.com/symfony/event-dispatcher/releases/tag/v4.4.44

 - symfony/polyfill-php73 installed in version v1.26.0
   Release notes: https://github.com/symfony/polyfill-php73/releases/tag/v1.26.0

 - symfony/console updated from v3.4.47 to v4.4.48
   See changes: symfony/console@v3.4.47...v4.4.48
   Release notes: https://github.com/symfony/console/releases/tag/v4.4.48

 - psr/cache installed in version 1.0.1
   Release notes: https://github.com/php-fig/cache/releases/tag/1.0.1

 - doctrine/annotations updated from v1.4.0 to 1.13.3
   See changes: doctrine/annotations@v1.4.0...1.13.3
   Release notes: https://github.com/doctrine/annotations/releases/tag/1.13.3

 - php-http/message updated from 1.7.2 to 1.13.0
   See changes: php-http/message@1.7.2...1.13.0
   Release notes: https://github.com/php-http/message/releases/tag/1.13.0

 - php-http/promise updated from v1.0.0 to 1.1.0
   See changes: php-http/promise@v1.0.0...1.1.0
   Release notes: https://github.com/php-http/promise/releases/tag/1.1.0

 - php-http/discovery updated from 1.6.1 to 1.14.3
   See changes: php-http/discovery@1.6.1...1.14.3
   Release notes: https://github.com/php-http/discovery/releases/tag/1.14.3

 - j0k3r/graby-site-config updated from 1.0.157 to 1.0.159
   See changes: j0k3r/graby-site-config@1.0.157...1.0.159
   Release notes: https://github.com/j0k3r/graby-site-config/releases/tag/1.0.159

 - fossar/htmlawed updated from 1.2.8 to 1.3.1
   See changes: fossar/HTMLawed@1.2.8...1.3.1
   Release notes: https://github.com/fossar/HTMLawed/releases/tag/1.3.1

 - symfony/deprecation-contracts installed in version v2.5.2
   Release notes: https://github.com/symfony/deprecation-contracts/releases/tag/v2.5.2

 - symfony/phpunit-bridge updated from v5.2.12 to v5.4.14
   See changes: symfony/phpunit-bridge@v5.2.12...v5.4.14
   Release notes: https://github.com/symfony/phpunit-bridge/releases/tag/v5.4.14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants