Upgrade of the blog to PHP 8.0 and WP 6.0

Hi,

in the near future (eg, next week), I will proceed with the upgrade of the blog to the version 6.0 of Wordpress (because the older one is no longer supported, if I understand well their page). At the same time, the php version will be upgraded to 8.0.

I have done quick tests on others blogs, and so far most were ok. However, it seems that the OpenID plugin is broken: Issue #10745: podcast.fp.o admin site fails - fedora-infrastructure - Pagure.io so we will need someone to take a look at that.

1 Like

A fix have been submitted for the plugin, so it wait on a review and a release. So the php 8 upgrade is on hold, but the WP one should proceed (on stg first, for the site who have a staging instance)

1 Like

Things are pretty slow right now. So now is probably a good time to do it anyway. Thanks for taking care of this.

1 Like

So:

  • the plugin with my fix was released, and it work
  • now the theme is broken
PHP Fatal error: Uncaught ArgumentCountError: Too few arguments to function WP_Widget::__construct(), 0 passed in /nas/content/live/fedoramagstg/wp-includes/class-wp-widget-factory.php on line 61 and at least 2 expected in /nas/content/live/fedoramagstg/wp-includes/class-wp-widget.php:162

Stack trace:
#0 /nas/content/live/fedoramagstg/wp-includes/class-wp-widget-factory.php(61): WP_Widget->__construct()
#1 /nas/content/live/fedoramagstg/wp-includes/widgets.php(115): WP_Widget_Factory->register('magazine_postse...')
#2 /nas/content/live/fedoramagstg/wp-content/themes/fmag2018-1.02/widgets/magazine-postseries-widget.php(64): register_widget('magazine_postse...')
#3 /nas/content/live/fedoramagstg/wp-content/themes/fmag2018-1.02/functions.php(172): require_once('/nas/content/li...')
#4 /nas/content/live/fedoramagstg/wp-settings.php(555): include('/nas/content/li...')
#5 /nas/content/live/fedoramagstg/wp-config.php(120): require_once('/nas/content/li...')
#6 /nas/content/live/fedoramagstg/wp-load.php(50): require_once('/nas/content/li...')
#7 /nas/content/live/fedoramagstg/wp-blog-header.php(13): require_once('/nas/content/li...')
#8 /nas/content/live/fedoramagstg/index.php(17): require('/nas/content/li...')
#9 {main}
 thrown in /nas/content/live/fedoramagstg/wp-includes/class-wp-widget.php on line 162

I will take a look later, just dropping the stacktrace here so I can keep people updated (and so I can go straight to coding right away, as I need to update the theme anyway)

1 Like

It looks like that magazine-postseries-widget.php was something Ryan wrote over six years ago.

https://pagure.io/fedoramagazine-theme/history/fedoramagazine/widgets/magazine-postseries-widget.php?identifier=master

I added you to the repo so you can commit changes directly. I (or other editors) can approve PRs too if you want to do it that way.

Thanks!

My PHP knowledge is a bit rusty (I mean, the version number doubled since last time I wrote a sizable php program), but I would say the problem is on Tree - fedoramagazine-theme - Pagure.io , who do not seems to fit the signature on: wordpress-develop/class-wp-widget.php at 6.0 · WordPress/wordpress-develop · GitHub

I guess something changed on php 8, and removing $name= is the only fix that come to me after looking at upstream code (like here).

I’m sure your experience with PHP is more extensive than mine. :slightly_smiling_face: The change you suggest looks harmless to me. If it appears to work on a local instance, then I’d say go for it! We can always rollback the theme if we need to.

Thanks!

So, I suspect there is a problem with the theme upgrade. I tested on staging and:

  • I cannot clean the uploaded themes.
  • when I upload a new one, it doesn’t change the version (so not 1.11.1misc)
  • it also seems to not run my updated code

I know @jasonbrooks told me about uploading the theme using git, while there is evidence the theme was updated from within wordpress in the past.

I start to wonder if the theme was updated at all since the move to WPengine (if someone remember the timeline of the move ?).

I’m curious what you mean by “I tested on staging”. I have made theme updates before. But to test them, I ran a local instance of WordPress on my PC as documented here:

WPEngine allow admins to copy environment. We have one called “stg”, and I tested there (and not on the production website).

But the test using the docker image do not test with php 8, so I am not sure if that’s good enough.

OK. I see. Also, I think I see why we haven’t been using staging previously. Apparently it causes some confusion with the JetPack plugin.

Mhh, so likely my fault. I copied the prod environment to stg (for testing), but I guess that stg took over jetpack. Sorry, I forgot about that, and I think it already happened before.

Understood. Thanks again for working on this!

It looks this has been a known problem for at least a year and a half.

Maybe we shouldn’t expect it to get fixed anytime soon and we should just update WordPress without updating PHP for now?

Yeah, I think the trick is to upgrade 1st to WP 6.0 and then to php 8. I have a working instance of php 8 and WP 6 (for next.redhat.com, we host it on the same platform), and I indeed noticed that WP 5.6 + php 8 didn’t work. But 6 work fine but the theme is not the same.

I am still trying to understand how the vendor upgrade system work (cause I say “upgrade” and then, it doesn’t upgrade right away, and sometime, after a while, the instance is upgraded. So far, I am testing on a staging instance, so no fear for the main website.

Ok so I finally managed to get it working. I was experimenting, and I noticed I wasn’t editing the right file. So the constructor is wrong, it need a __construct() function, as explain in a link on the page given by @glb .

I will send a PR for that.

1 Like

@rlengland: Be sure to include Michael’s PR in the theme update that you are working on. I think it should be backwards compatible with the current version of WordPress. Once the theme is updated on the current site, then Michael should be able to cleanly upgrade it to WordPress 6.0 and PHP 8.0.

PR#51: Make the widgets use non deprecated constructors - fedoramagazine-theme - Pagure.io the link. Yeah, it should be compatible. I haven’t tested it fully, but the syntax should be ok. I will do a fuller test later on the test instance, likely next week.

2 Likes

New theme v1.12 has been installed with PR#51 merged.