Update SimplePie to version 1.8.0

[jrf]

A new version of SimplePie has just been released.

This version contains a few enhancements and some bug fixes.

The most notable change, however, is that this release contains a forward-compatibility layer for the change to PSR-4 namespaced classes which is targetted for SimplePie 2.0.0.

With some similarity to Requests - the namespaced versions of the classes are in a different base directory (src) from the original versions (library).

As WP currently only includes the files in the library directory, I would like to suggest to continue doing so for now.
This still makes the forward-compatibility layer available as all files in the library directory now create a class alias to their namespaced version.

Once 2.0.0 has been released, the files included in WP, should be switched to the files from the src directory (which is currently in place mostly to allow for Composer autoloading) and should start using the namespaced names for the SimplePie classes.

I'd recommend for this update to be mentioned in a dev-note, so plugins/themes directly using SimplePie can decide for themselves when they want to change to using the namespaced names for SimplePie classes.

Refs:

• ​https://github.com/simplepie/simplepie/releases/tag/1.6.0
• ​https://github.com/simplepie/simplepie/blob/1.6.0/CHANGELOG.md#160---2022-04-21
• ​https://github.com/simplepie/simplepie/compare/1.5.8...1.6.0

I've done a cursory check of the changes and they look sane to me, but would very much like to invite a second opinion and I'd recommend testing this change (more thoroughly than usually done for upgrades like these).

I'd also like to recommend for a few cursory tests to be added to the WP test suite to ensure that both the PSR-0 as well as the PSR-4 class names load correctly when only including the library directory in WP.

I'd recommend for this update to be applied in WP 6.1 early.

Previous: #36669, #51521, #54659

[costdev]

Adding 2nd-opinion and needs-unit-tests per recommendations in the ticket summary.

[chaion07]

Thanks @jrf for reporting this. We reviewed this ticket during a recent bug-scrub session. Based on the feedback received from the team, we feel that this ticket needs unit testing and/ or, patches before anything can be done for this ticket to move closer to a resolution.

Props: @cu121 & @markparnell

[audrasjb]

Hello there, as per today's bug scrub, just wanted to mention that we'll give this ticket a few days before moving in to Future Release, as it is marked as early and as we're just one month away from 6.1 beta 1 :)

[chaion07]

Thanks @jrf for reporting this. We reviewed this ticket during a ​recent bug-scrub session. We are updating the ticket with the following changes:

1. Updating milestone to Future Release considering no significant change has taken place.

Props to @audrasjb

[jrf]

In the mean time SimplePie 1.7.0 has just been released:

• ​https://github.com/simplepie/simplepie/blob/1.7.0/CHANGELOG.md#170---2022-09-30
• ​https://github.com/simplepie/simplepie/releases/tag/1.7.0
• ​https://github.com/simplepie/simplepie/compare/1.6.0...1.7.0

Note: as the codebase has undergone a code style cleanup, the diff is kind of useless.
For a code review of the 1.7.0 release, we'll probably need to review the individual commits (with the exception of the code style one).

---

Based on the discussion in ​this ticket in the SimplePie repo, the intention is to make 1.7.0 an LTS branch which will keep supporting PHP 5.6 for the foreseeable future.

SimplePie 1.8.0 is slated to drop support for PHP < 7.2.

[audrasjb]

As per today's bug scrub: since this ticket is marked for early commit and as it is still waiting for a patch and testing, let's move it to Future Release.

[hellofromTonya]

Moving this into 6.3.

[audrasjb]

No update during the 6.3 early cycle: moving to milestone 6.4.

[Presskopp]

​https://github.com/simplepie/simplepie/releases/tag/1.8.0

[jrf]

@Presskopp Unfortunately upgrading to 1.8.0 is not an option for WordPress as it raises the minimum supported PHP version to 7.2, while WP still supports PHP 5.6.

​https://github.com/simplepie/simplepie/blob/1.8.0/CHANGELOG.md#180---2023-01-20

[Presskopp]

Ah, yes, sorry I didn't think about that. But on the other hand the support for 5.6 could end soon, see https://core.trac.wordpress.org/ticket/57345#comment:111

[jrf]

@Presskopp I hear you, but I'll believe when I see it...

[Presskopp]

Replying to jrf:

@Presskopp I hear you, but I'll believe when I see it...

:-) The hope dies last

​https://make.wordpress.org/core/2023/07/05/dropping-support-for-php-5/ we're getting close @jrf

[hellofromTonya]

I'd recommend for this update to be applied in WP 6.1 early.

6.4 is in the middle of the beta cycle with RC1 fast approaching. It's too late for early tasks. Though it's marked as a blessed task, there's work to be done in reviewing the version and preparing tests

I've done a cursory check of the changes and they look sane to me, but would very much like to invite a second opinion and I'd recommend testing this change (more thoroughly than usually done for upgrades like these).

I'd also like to recommend for a few cursory tests to be added to the WP test suite to ensure that both the PSR-0 as well as the PSR-4 class names load correctly when only including the library directory in WP.

I know this ticket has been kicked down the road release after release. Should it go into Future Release or the next milestone 6.5? Moving it to 6.5 might give it visibility, though it hasn't yet. But in the effort to keep dependencies (near) up-to-date, trying for one more major. Punting to 6.5.

[audrasjb]

It’s a bit late in 6.5 cycle to commit an early ticket, moving to Future release

[desrosj]

Just noting that as of [57985], the minimum required version for WordPress has been updated to 7.2.24. This change will be shipped in WordPress 6.6, but beta is only 1 month away (currently scheduled for 4 June). That may be too soon for adequate testing unless there are enough interested contributors with the bandwidth. But it would be great to get a PR started.

Not going to move this to the 6.6 milestone until there's consensus that everyone is comfortabl with that timeline.

[jrf]

AS WP 6.6 has now been released, I'd recommend for a patch to be created based on SimplePie 1.8.0 ASAP for committal early in the 6.7 release cycle to allow for sufficient testing.

• ​https://github.com/simplepie/simplepie/releases/tag/1.8.0
• ​https://github.com/simplepie/simplepie/blob/master/CHANGELOG.md#180---2023-01-20
• Diff with the version currently shipped with WP Core: ​https://github.com/simplepie/simplepie/compare/1.5.8...1.8.0

Trac ticket: https://core.trac.wordpress.org/ticket/55604

The PR only have one file change, There should be more per ​https://github.com/simplepie/simplepie/compare/1.5.8...1.8.0

@mukeshpanchal27 I've added more files, but since the new version uses PSR-4, @peterwilsoncc suggested including the src folder as well. I'm still figuring out how to do that.

@mukeshpanchal27 just following up on the blocker here. Can you please ping someone who can review or help with this PR?

@faisal-alvi I did some very rough experimenting with dropping the simple pie zip in to the existing folder (ie, moving the library files in to the library directory) and replacing inclusion of files with their new autoloader.

It wasn't super successful as WP_Feed_Cache_Transient is now required to implement SimplePie\Cache\Base which is not the case.

IN short, it loosk like there will be quite a bit of messing around require to upgrade to 1.8.0.

[peterwilsoncc]

In a scrub of tickets marked for early attention, it was decided to bump this ticket from the 6.7 milestone.

@faisal03 has made a start on the PR but it looks like it will require quite a lot of wrok to account for the refactor of the library.

I did some testing of potential approaches a while ago but never made it as far as pushing. WP uses WP_Feed_Cache_Transient to cache feeds and Simple Pie now requires any caching implement SimplePie\Cache\Base.

[jrf]

Just noting here that if this ticket is bumped, WP will most definitely not be compatible with PHP 8.4 for the WP 6.7 release.

See #62061 for further analysis and details.

[peterwilsoncc]

@jrf Thank you.

As WP_Feed_Cache_Transient will need to be refactored (and possibly others, when I experimented I was hitting fatal errors so didn't even commit locally) and the 1.8 branch will need a new tag, is there anything WP can do in the mean time to reduce the issue.

Do you recall the level of incompatibility for 8.4: is it fatal errors, warnings or depredations/other notices?

[jrf]

Replying to peterwilsoncc:

As WP_Feed_Cache_Transient will need to be refactored (and possibly others, when I experimented I was hitting fatal errors so didn't even commit locally)

@peterwilsoncc Would you like me to have a look at it ? Or maybe you want to set up a call for us to look at this together ?

and the 1.8 branch will need a new tag

The new tag is on the way. My PRs with PHP 8.4 fixes were merged today. The necessary ​backports to the 1.8 branch were done as well. Based on ​https://github.com/simplepie/simplepie/pull/886, I kind of expect a 1.8.1 release some time over the next few days.

is there anything WP can do in the mean time to reduce the issue.

Do you recall the level of incompatibility for 8.4: is it fatal errors, warnings or depredations/other notices?

IIRC it is only deprecation notices, but in three different flavours.

Generally speaking, that is not that much of a problem. If users open tickets about those, we can close them with a "this is being addressed upstream and WP will update once an update is available", but then, we haven't been updating, so that statement would be a bit disingenuous.

The bigger problem, from my point of view, is that SimplePie not being updated to 1.8.1 means we cannot turn on test runs for WP Core against PHP 8.4, as we want tests to fail on PHP deprecation notices to prevent new issues being introduced in the code base, but at this time that would mean the tests would fail on SimplePie.

I suppose we could add a temporary and conditional call to TestCase::markTestSkipped() for those tests when run against PHP 8.4, but we'd really need to remember to remove those again once the update does go through.

For the record, the following two tests in the WP Core test suite would currently fail on the version of SimplePie which is used by WP not being compatible with PHP 8.4:

• WP_Test_REST_Widgets_Controller::test_get_items()
• Tests_Widgets_wpWidgetRss::test_url_happy_path with data set "when url is given"

Trac ticket: https://core.trac.wordpress.org/ticket/55604

[SergeyBiryukov]

Replying to jrf:

For the record, the following two tests in the WP Core test suite would currently fail on the version of SimplePie which is used by WP not being compatible with PHP 8.4:

• WP_Test_REST_Widgets_Controller::test_get_items()
• Tests_Widgets_wpWidgetRss::test_url_happy_path with data set "when url is given"

Spent some time looking into this, and ​PR 7441 is my attempt at updating SimplePie to version 1.8.0, with those tests (and all others) passing.

Replying to jrf:

As WP currently only includes the files in the library directory, I would like to suggest to continue doing so for now.
This still makes the forward-compatibility layer available as all files in the library directory now create a class alias to their namespaced version.

Once 2.0.0 has been released, the files included in WP, should be switched to the files from the src directory (which is currently in place mostly to allow for Composer autoloading) and should start using the namespaced names for the SimplePie classes.

It appears that PSR-4 namespaced classes are used as of 1.7.0, so in my understanding both library and src directories are now necessary. Would that be correct?

[jrf]

Replying to SergeyBiryukov:

Replying to jrf:

For the record, the following two tests in the WP Core test suite would currently fail on the version of SimplePie which is used by WP not being compatible with PHP 8.4:

• WP_Test_REST_Widgets_Controller::test_get_items()
• Tests_Widgets_wpWidgetRss::test_url_happy_path with data set "when url is given"

Spent some time looking into this, and ​PR 7441 is my attempt at updating SimplePie to version 1.8.0...

That's awesome @SergeyBiryukov ! If you don't mind, I won't look at it now, but will try and get to it over the next few days.

..., with those tests (and all others) passing

The tests I mentioned above would only fail on PHP 8.4 (which we're not running in CI yet) and if the PR is based on SimplePie 1.8.0, they should still be failing on PHP 8.4 (until we update to 1.8.1 once it has been released).

Update: I've just ran the tests from your branch and can confirm these tests still fail on PHP 8.4 with multiple deprecations hiding behind each other, so this is as expected for now.

Replying to jrf:

As WP currently only includes the files in the library directory, I would like to suggest to continue doing so for now.
This still makes the forward-compatibility layer available as all files in the library directory now create a class alias to their namespaced version.

Once 2.0.0 has been released, the files included in WP, should be switched to the files from the src directory (which is currently in place mostly to allow for Composer autoloading) and should start using the namespaced names for the SimplePie classes.

It appears that PSR-4 namespaced classes are used as of 1.7.0, so in my understanding both library and src directories are now necessary. Would that be correct?

I think you're right. IIRC when I opened this ticket two and half years ago for SimplePie 1.6.0, the intention was still to do the switch-over in the 2.0 release, but their plans have since changed and this ended up going into the 1.7.0 release.

[jrf]

Also see: ​SimplePie Roadmap proposal (which was opened after the 1.6.0 release)

[peterwilsoncc]

After discussing with @jrf and seeing that @SergeyBiryukov is currently working on a PR, I'm happy for this to go back on to the 6.7 milestone.

The first beta is Tuesday morning Australian time (Monday afternoon in the US) so it will need to be committed prior to that.

I've not caught up on the PR as yet (an need to re-read some of the discussion above, tbh) but aim to do so over the course of the weekend.

[SergeyBiryukov]

Replying to jrf:

The tests I mentioned above would only fail on PHP 8.4 (which we're not running in CI yet) and if the PR is based on SimplePie 1.8.0, they should still be failing on PHP 8.4 (until we update to 1.8.1 once it has been released).

Update: I've just ran the tests from your branch and can confirm these tests still fail on PHP 8.4 with multiple deprecations hiding behind each other, so this is as expected for now.

Thanks for the clarification! For me it was a good indicator that the PR at least seems to work on PHP < 8.4 :) I will do some further testing with the master branch of SimplePie on PHP 8.4.

[jrf]

Replying to SergeyBiryukov:

For me it was a good indicator that the PR at least seems to work on PHP < 8.4 :)

Oh absolutely!

I will do some further testing with the master branch of SimplePie on PHP 8.4.

You'll need the one-dot-eight branch of the SimplePie repo. master is what will become their 2.0 version.

[SergeyBiryukov]

Replying to jrf:

You'll need the one-dot-eight branch of the SimplePie repo. master is what will become their 2.0 version.

Thanks! I've updated the PR to include the latest fixes from the one-dot-eight branch and ran the tests on PHP 8.4, they pass now locally.

@SergeyBiryukov I haven't finished my review completely yet, but have a feeling there will be some changes/updates to be made. Would you prefer I leave a written review with notes or would you be okay with giving me access to your fork and for me to push an extra commit with suggested changes/additions ?

_Practical note: I won't be able to get back to this until Sunday, but it is my highest prio action item for when I get back behind my PC._

Would you prefer I leave a written review with notes or would you be okay with giving me access to your fork and for me to push an extra commit with suggested changes/additions ?

@jrfnl Thanks! Happy to give you access to the fork. I see that “Allow edits and access to secrets by maintainers” is already checked, is there anything else I should do?

@jrfnl Thanks! Happy to give you access to the fork. I see that “Allow edits and access to secrets by maintainers” is already checked, is there anything else I should do?

Not really, I just didn't want to surprise you with a push to your branch without asking for permission.

I've now finished my review. The patch itself is looking good and I'll happily approve it once you've has a chance to look over the additional changes I've now pushed.

There are two issues I'd like to call out (not so much with this patch, but with SimplePie itself/the existing WP customizations):

1. In my third commit I'm syncing the parameter names, types and return types of methods in the WP_Feed_Cache_Transient class to be in line with the SimplePie\Cache\Base interface.

These changes should be covered by tests, but currently aren't as there are no dedicated tests (at all) for the WP native transient based caching for SimplePie.
I've done some preliminary work on creating tests for this, but my tests are currently not even triggering the caching, so I'm doing something wrong (or have discovered a bug, this is yet to be determined).
Either way, the tests won't be ready in time for WP 6.7.0-beta1, but as tests can go in at any time, this should not be a blocker for merging this PR.
_Note: I'm also perfectly happy to let someone else (continue) work on these tests. This doesn't have to depend on me (and my very limited availability at this moment)._

2. The use of the SimplePie\Cache\Base interface is deprecated since SimplePie 1.8.0 in favour of the SimplePie\Cache\DataCache interface, which is PSR16 compliant.

The transient based caching should be refactored to implement the SimplePie\Cache\DataCache interface instead, in combination with using the SimplePie\SimplePie::set_cache() method, but it is too late in the development cycle to do this for WP 6.7, so this will need to be moved to a later release.
There is a problem with this though, which is that the SimplePie\SimplePie::set_cache() method has a type declaration which depends on a new external dependency psr/simple-cache, which means that a call to the SimplePie\SimplePie::set_cache() method would currently cause a fatal error due to this dependency not being included in WP.
In SimplePie, this dependency is current in require-dev as switching to the new caching implementation is not yet enforced.
We need to have a good think/discussion about how to handle this.
The dependency itself is nothing exiting, just three interfaces, but we'd be stuck on version 1.0.1 (from 2018) as v 2.0.0/3.0.0 have a PHP 8.0 minimum requirement.

Oh and I'm still forgetting to mention the following:

The "hooking in" of the custom cache handler in the fetch_feed() method in src/wp-includes/feed.php should also be updated, but considering that the caching will need refactoring anyhow, I excluded this for the "Update references to SimplePie class names" commit for now.

Final note (sorry, I keep forgetting to mention things):

Various files have been moved around in SimplePie and the directory structure of the project has changed. This has been mirrored in this patch without "placeholder" files for the deprecated/removed old file locations.

While this is not a typical way to do this for WP, I'm okay with this for the following reasons:

1. Even before this update, WP - in the src/wp-includes/class-simplepie.php file, already included an autoloader for all files it did not hard require.
2. The src/wp-includes/class-simplepie.php file is the typical entry point for loading SimplePie and plugins/themes not going via that file are _doing it wrong_ anyhow.
3. External dependencies should not be held to the same standards of "no BC breaks" as WP itself.
4. There is a precedence for external dependencies which have had directory layout changes to not provide placeholder files. Think: the update to Requests 2.0.

[jrf]

I've done an extensive code review of the patch proposed by @SergeyBiryukov and have added a few small tweaks to it.

In my opinion, the patch as it is now (after okay on the tweaks by @SergeyBiryukov), can go in today, providing we open a couple of follow-up tickets to be addressed later (tests, refactor of caching).

Better it go in now to benefit from the full beta period for testing, then going in late and issues not being found before the final release.

[SergeyBiryukov]

In 59141:

External Libraries: Update the SimplePie library to version 1.8.0.

The most notable change in this update is that all code is now namespaced and uses PSR-4 classes, though there is a compatibility layer available for extenders using the older class names, so plugin or theme authors directly using SimplePie can decide for themselves when they want to change to using the namespaced names for SimplePie classes.

Note: This commit includes additional fixes for PHP 8.4 compatibility (​PR 875, ​PR 888) from the one-dot-eight branch of SimplePie, which is expected to be ​released as SimplePie 1.8.1 soon.

References:

• ​SimplePie 1.8.0 release notes
• ​SimplePie 1.7.0 release notes
• ​SimplePie 1.6.0 release notes
• ​Full list of changes in SimplePie 1.8.0

Follow-up to [47733], [49176], [52393], [52413].

Props jrf, peterwilsoncc, chaion07, cu121, markparnell, audrasjb, costdev, Presskopp, desrosj, faisal03, mukesh27, SergeyBiryukov.
See #55604.

Thanks @jrfnl! Merged in r59141.

Thank YOU!

Thanks for the PR! This is now resolved in r59141.

[SergeyBiryukov]

In 59142:

External Libraries: Correct the case for wp-includes/SimplePie/src/Gzdecode.php.

The file should be named Gzdecode.php (with the capital G), to avoid autoloading problems on case-sensitive file systems (*nix).

Follow-up to [59141].

Props jrf.
See #55604.

[peterwilsoncc]

I've removed the early keyword to aid with milestone reports while this is kept open for any follow up. If SimplePie 1.8.1 is released in the next few weeks, I think it safe to upgrade as it appears to mainly include PHP compatibility fixes.

[jrf]

Replying to peterwilsoncc:

If SimplePie 1.8.1 is released in the next few weeks, I think it safe to upgrade as it appears to mainly include PHP compatibility fixes.

Thanks @peterwilsoncc. Most of what will be in 1.8.1 has already been included in the patch as was committed, so the update to the official v1.8.1 should be very small.

Progress on getting SimplePie 1.8.1 released can be followed via ​https://github.com/simplepie/simplepie/pull/886

[peterwilsoncc]

The upstream PR for SimplePie 1.8.1 is still in progress. As we only a couple of days from the RC, I'll close this ticket off as fixed.

If the upstream release is done during the next few weeks, let's discuss it's inclusion on another ticket to weigh up the pros and cons.