Bug #9068: PHP 7.2 compatibility - CUNY Academic Commons - CUNY Graduate Center - Project Tracking System

Actions

Copy link

Bug #9068

closed

PHP 7.2 compatibility

Added by Boone Gorges almost 7 years ago. Updated about 6 years ago.

Status:

Resolved

Priority name:

Normal

Assignee:

Boone Gorges

Category name:

Server

Target version:

1.14.1

Start date:

2018-01-09

Due date:

% Done:

Estimated time:

Deployment actions:

Description

We need to move toward full PHP 7.2 compatibility across our codebase. The target for this is late spring; I'll put this ticket into the 1.13 milestone for the time being.

The number one concern is eliminating fatal errors. Between our current version (5.6.x) and 7.2, there are a number of hard deprecations and removals. See https://secure.php.net/manual/en/migration72.php and the corresponding 7.1 and 7.0 pages for more info.

Much of this work can probably be done with an existing linter, like https://github.com/sstalle/php7cc. The linter will need to be configured to ignore certain parts of the codebase, such as bbPress legacy bundled with BP.

Where possible/convenient/worthwhile, we'll make an attempt to pass compatibility patches to upstream plugin developers. When plugins are abandoned, we can patch locally and call it good enough. In some cases, we may take this opportunity to disable or remove certain plugins/themes. We'll take this on a case-by-case basis.

We'll also likely need a tool that enforces PHP 7.2 compatibility across the codebase after updates. We don't want to patch a plugin only to have a future update to that plugin reintroduce an incompatibility. One idea here is to run a linter in a Travis CI task, which would email us when bad code is committed. But this would require some changes to our deployment process, since we'd need time for the linting to complete.

We should scour the internet to look for people who have done this with large WordPress codebases, as it's likely that we can borrow much or all of others' work.

Let's use this ticket to start gathering ideas.

Related issues

Actions

Copy link

Updated by Matt Gold almost 7 years ago

Thanks for pushing this forward, Boone.

Actions

Copy link

Updated by Boone Gorges almost 7 years ago

I spent some time working on this issue today. Here's what I've done.

1. I adapted an existing script to do the linting. The linting is done using the local version of PHP, so in order to catch PHP 7.2 issues, it must be run against PHP 7.2. The script lives at developer/lint-php.php. https://github.com/cuny-academic-commons/cac/blob/a41d800f3a2f69d030756f5e66a2a08c3563cca7/developer/lint-php.php

2. I configured a CircleCI integration with our GitHub repo. https://circleci.com/gh/cuny-academic-commons/cac/ This integration runs the linter script after each commit to the repo. Failures result in email notifications to me. This will have to be modified once we decide exactly when the linting ought to take place. Maybe only against the master branch, since that's the release branch. I'm using CircleCI because they don't charge to run jobs on a private repo, while Travis does.

3. I've compiled a spreadsheet with the results of the initial linting run: https://docs.google.com/spreadsheets/d/1n2HcDTiCrTDt9wTmMotU9xXxHTin5MNOmPVl_Pwmov0/edit?usp=sharing The ones marked "Abandoned" should just be fixed directly in our repo by one of the dev team, and then checked off in that column. The others require some other sort of action: getting new copies of a premium plugin, etc.

Actions

Copy link

Updated by Boone Gorges over 6 years ago

Category name set to Server

Actions

Copy link

Updated by Boone Gorges over 6 years ago

Target version changed from 1.13 to 1.14

I've fixed all the items in the spreadsheet https://docs.google.com/spreadsheets/d/1n2HcDTiCrTDt9wTmMotU9xXxHTin5MNOmPVl_Pwmov0/edit#gid=0 and our build is now green on CircleCI: https://circleci.com/gh/cuny-academic-commons

We still have some work to do here on processes, so that we can ensure that we don't reintroduce PHP 7.2 incompatibilities.

In the meantime, I'll follow up with Lihua to see where the server stuff plans, and so that we can start coming up with a clear schedule. I'm going to bump this out of the 1.13 milestone, since it's not directly affected by the release.

Actions

Copy link

Updated by Raymond Hoh over 6 years ago

Caught a PHP 7.2 deprecated call in social-paper - https://github.com/cuny-academic-commons/cac/commit/3aba5ecb49cbef06482dc8d91679ef4d88a75cd2

Boone - Does our linter pick up git submodules? It looks like your linter script should work, but I'm not sure.

Actions

Copy link

Updated by Boone Gorges over 6 years ago

Thanks, Ray.

The linter is set up to catch fatals only. I'm sure there are thousands of instances of code that will trigger notices.

But yes, the linter will run on anything that's in the local directory. So as long as the submodules are checked out, they'll be linted. I just added a couple lines to our circleci config to ensure that submodules do get initialized. I'll verify after it's run.

Actions

Copy link