D7: Fully support PHP 7.3

Hi,

Here's a first try at the patch fixing this.

As I cannot reproduce the failing tests locally, I'm re-uploading here a patch including a small refactoring and optimizations of the drupal_regenerate_session() function.

Updating the patch.

New and updated patch.

This patch is not ideal per se, as the old session is saved with the new user ID for a fraction of a second before being migrated to the new session ID. But I at least wanted to see what the test results are.

Ok we also need to save and restore the user object, because starting session resets the global user object.

Also, added a way to avoid writing the session.

@mfb since this is the Plan issue for supporting PHP 7.3 in D7, we should fix the session id issue in the sub-issue #3009351: "session_id(): Cannot change session id". Can you move the patch to that issue?

Done :) Looks like testbot is running with PHP 7.3 and MySQL 5.7 - for that you will also need #2981248: MySQL 5.7 incompatibility in system upgrade 7061

New tag per joseph.olstad.

reminder to enable testing for php 7.3 https://www.drupal.org/node/3060/qa

For those interested in PHP7 support, let's not forget the outstanding child issues of #2656548: Fully support PHP 7.0 in Drupal 7.

Well apparently there is no love for #2656548: Fully support PHP 7.0 in Drupal 7, so against my better judgement I've moved the child issues over here as instructed.

There is love no worry :-)

Good day, I'm a little confused, but do I understand that Drupal 7.64 (its core) fully supports PHP 7.3?

@strelkovandreyvalerievich No, you would also need this patch applied: #3025335: session_id() cannot be changed after session is started

@mfb
Thanks, as I understand that most likely in version 7.65 will be out of the box full support 7.3

Fingers crossed the next version could support 7.3?

As for all the PHP 7.x child issues, I would say that since they affect every supported version of PHP, they are just straight up issues, not really child issues of this one per se.

@mfb
I agree you with, however see #2656548: Fully support PHP 7.0 in Drupal 7 for the supposed rational behind this.

Seems like the child issues could just be tagged, or if a parent issue is helpful then we could make one called "PHP 7.x regressions" or something like that. That way PHP 7.3 can be "fully supported" despite any ongoing PHP 7.x issues.

I don't understand the claim that PHP 7.3 can be full supported when issues introduced by PHP 7.0 are still present.

But then again equally I don't accept the claim that PHP 7.0 is fully supported just because the corresponding issue is closed and the news on the front page of Drupal.org says it is.

Tags are too bad nowadays that a meta issue is preferable, but it would essentially be a #2656548: Fully support PHP 7.0 in Drupal 7 part 2, considering that was originally a PHP 7.x issue and was hijacked to be PHP 7.0 specific, but whatever.

I don't think "fully supported" means bug free, it just means it got to a sufficiently bug free state that it could be supported (the meaning of which is essentially, "bugs will be fixed").

At the moment PHP 7.3 has a critical issue re: session regeneration failing so it would be premature to call it "fully supported" but hopefully it's close.

I think this poses two questions, why is it so hard to get this committed when it's for php 7.2 and 7.3 which have been out for quite some time. Also what will happen with Drupal 7 and it's patriots. As the data shows 7 is still pretty popular and the new route on symphony goes completely different route as it is a rewrite.

#3025335: session_id() cannot be changed after session is started is specific to PHP 7.3 - I'm guessing not many are in a rush to upgrade their infrastructure to PHP 7.3, but 🤞 eventually someone will, who also has time to review the patch..

see new patch
I have not tested this before or after.
but the patch looked wrong to me so I changed it.

@joseph.olstad this is now a meta-issue, there is no patch here.

#3025335: session_id() cannot be changed after session is started is where that patch lives now.

Your change isn't needed because $original_session_saving is guaranteed to be set if $old_session_id is set.

ok, ya you're right the change in patch 37 wasn't needed. However I generally prefer to explicitly initialize variables that are going to be used conditionally. Either way works though, php is not strict.

however, looks like the testbot is broken.
errors that have nothing to do with the patch I'm sure.

check for fixes to the testbot
https://www.drupal.org/project/issues/drupalci_testbot

@joseph.olstad it looks like the 7.x branch is failing tests on PHP 5.3, because of recently added test re: invalid UTF-8 in file names.

For check_plain function:

If $text is not valid UTF-8, an empty string is returned and, on PHP < 5.4, a warning may be issued depending on server configuration (see https://bugs.php.net/bug.php?id=47494).

The test needs to ignore these warnings on PHP < 5.4

Totally off-topic for PHP 7.3 issue :p but test fix for PHP 5.3: #3047844: [Regression] Tests fail on PHP 5.3

Could someone who has access go ahead and enable automated testing for Drupal 7 on PHP 7.3 @ https://www.drupal.org/node/3060/qa?

We should be ready to wrap this issue up - there is one patch left which is RTBC: #3025335: session_id() cannot be changed after session is started, and automated testing of 7.3 should be enabled. (The rest of the subissues are generic issues re: every supported version of PHP.)

@Pol or @Fabianx , please add php 7.3.x test on commit for automated test coverage....
On every commit a php 7.3.x test should be triggered.

Also, php 7.4.x is about to be released.

I updated https://www.drupal.org/docs/7/system-requirements/php-requirements#php_r... to say "Follow this issue" rather than "Spring 2019"

Hi! This *feels* like a PHP 7.3 compatibility issue, but my "ctrl + F" on this page for the words "continue" and "break" came up empty, so I'm less certain.

Here's the log message:
Warning: "continue" targeting switch is equivalent to "break". Did you mean to use "continue 2"? in require_once() (line 341 of /path/to/drupal/includes/module.inc).

(Drupal 7.67, PHP 7.3.9)

So I say "this feels like a PHP 7.3 compatibility issue..." because when I googled the warning message, all the results I found were about PHP 7.3 compatibility, including this one for Drupal 8.x support of PHP 7.3. Oh and, I'm here b/c this is the "parent issue," obviously, but if there's a different thread I should focus on, just lmk.

Alisonjo315, please open a support request and link it here. Very much doubt that core is at fault in your setup, I am running php 7.3.x on several environments, open the support request , add backtrace output to the support issue, a backtrace output will most likely reveal the true culprit. Google how to backtrace , put the backtrace right before the line of code that crashes. Show us the backtrace in a support issue and please add the link to that issue here.

@joseph.olstad Thanks for the quick reply! I forgot how cumbersome life can be without drush (the site is on weird hosting) -- in any case, while digging deeper, I ended up triggering the same sort of warning message from the Date module, and when I applied that patch, the other error (referencing includes/module.inc from core) disappeared -- yippie!

PHP 7.3 was official released 5. Dec. 2018, and this issue was created 8 Nov. 2018. However this issue still has status "Unassigned", and now it has been 10 months since PHP 7.3 was released, and PHP 7.4 is almost ready. I wish somone in charge of decision making could start working on this for real, not tomorrow, but today. For example as a first step enable automated testing for Drupal 7 on PHP 7.3?

I've been in contact with one of the core maintainers and he expected to take immediate action on 3025335 a week or two ago, perhaps it is time to remind him again of what he was planning to do.

AFAIK there's only one patch that is urgently needed, I've been using it extensively for quite some time now and haven't noticed any issues at all with it.

#3025335: session_id() cannot be changed after session is started

@aommundsen please try the patch in the mentionned issue and provide your feedback there.

@joseph.olstad Thank you for your reply and information. I do not use Drupal myself, but I am running a shared hosting company, and trying to plan for a upgrade of PHP 7.2 to 7.3. However many of our customers is running Drupal 7, therefore I am postponing the upgrade until Drupal 7 has official support for PHP 7.3. It is a frustrating wait, because Drupal 7 is the only thing left holding us back. (Please don't ask about offering multiple major PHP versions to customers, I know many do, but we only offer one PHP version on our shared hosting servers).

ah ok, yes you're an ISP provider, makes sense, I use ISPConfig and it supports multiple php versions. upgrading to php 7.3.x also means upgrading/updating custom modules and contrib modules, not just 'core'

I'm using php 7.3.x though, had to update to latest contrib module releases, in some cases untagged dev releases

It does not seem to me that Drupal prioritize working on adding support for PHP 7.3, as I don't see any activity. I hope I am wrong, and that a new Drupal 7 release wich support PHP 7.3 will be released very soon?

As far I can tell, the trouble seems to be that more volunteers are needed (of course, ideally it would be someone whose work could sponsor their time, so not necessarily a volunteer); there has been no one committing on the 7.x branch aside from the security team. Pol requested commit access which would have been excellent to get things moving, but then resigned :/

Oh, I just realized @mcdruid was added as a committer, #3088557: Volunteering as a D7 core committer (@mcdruid) so there is some hope that commits can happen :)

@mfb , yes mcdruid is our new maintainer. FabianX and mcdruid are planning something. I am currently using php 7.3.x and its noticably quicker than previous versions.

There is no need to add in Drupal 7 in the title, since the version is set to 7.x-dev.

@kiamlaluno, just like there is no need to remove Drupal 7 in the title, because it is a more spesific and better title. It is not helpful when using Google to search for Drupal 7 and PHP 7.3, if all PHP 7.3 issues has the same title without the Drupal version. However, if Drupal developers would care more about pushing a Drupal 7 version with patch for PHP 7.3, then I would care less about insignificant title changes.

Currently we consider upgrading our shared hosting servers to PHP 7.3.x and tell our Drupal 7 customers that we can't take responsibility for this any more, and that they would have to contact Drupal about the issue - as long as Drupal has not released a PHP 7.3 compatible version in a timely manner. We have given Drupal almost one year since the GA release of PHP 7.3.x, and still there is not a release with official support.

We will no longer let Drupal hold us back.

I totally understand Drupal is run by volunteers, and I have spent my fair share of time contributing too.

Drupal 7 is still being used by hundreds of thousand sites, and I find it quite unfair to those who submitted and reviewed the 7.3 compatibility patches. With 7.4 around the corner, we are quite late in this to apply patches.

I personally run PHP 7.3 and MySQL 8 with several D7 patches because I didn't want to be held by this. For those who don't want to patch themselves, we are not playing the role of a responsible open source project.

I'm also a WordPress core contributor, and we already have Drupal 7.4 support tested. This is a software that required PHP 5.2 to work a few weeks back.

I'm not calling any names, but I hope I can out my 2 cents forward and give a friendly encouragement to those are have commit and tag permission to roll a new release.

Thank you.

Thanks for voicing up, expect action soon, it would be nice if the important people had a read of that comment mentionning Wordpress.

Meanwhile, if you are using the patches, please take a look at all the patches you're using for php 7.4.x compatibility and /or php 7.3.x compatibility and report your successes to each of these patches as a nudge to move them along.

One of my top clients right now is running php 7.3.x with Drupal 7 , so far I've only had to use 2 core patches for it to work properly for them. There may be other patches needed but I haven't yet encountered the need.

So far for php 7.3.x compatibility right now I'm using
patch 42 and interdiff 58 for patch 42 from:
#2819375-58: Do not make entries in "cache_form" when viewing forms that use #ajax['callback'] (Drupal 7 port)

and of course the session id patch:
#3025335: session_id() cannot be changed after session is started

for contrib I'm using latest versions mostly tagged releases but I think in one or two cases a dev release of modules, and I must say that contrib is mostly ready for php 7.3.x. What we're waiting on is core.

Importantly, we need to get moving on this so that we can focus on php 7.4.x compatibility so that we can get contrib ready for that.

@Ayesh, I don't really understand what you say about WordPress, because WordPress did not "required PHP 5.2 to work a few weeks back", it would be more like many years ago. I am sure it is only a misunderstanding. :) By the way, talking about WordPress https://wordpress.org/about/requirements/ "PHP version 7.3 or greater." Thats more like it.

Re: comment about WordPress.
The minimum required version for WordPress was 5.2 until a few weeks ago, and WP has a reputation for holding its users back with super long backwards compatibility. Drupal is moving to require PHP 7 (it already requires 5.6 minimum), and it is pretty much ready to face PHP 7.4 as well.

Confirming that this is a meta issue for PHP 7.3 compatibility of core's 7.x branch.

I am trying to identify which issue(s) we definitely need to fix in order to be satisfied that D7 "fully supports" PHP 7.3.

As far as I can tell, this may the only one:

* #3025335: session_id() cannot be changed after session is started

Other issues which may be included (but this needs to be confirmed):

* #2902430: [D7] PHP 7.1 warning: A non-numeric value encountered in theme_pager()
* #2905980: In PHP7.1 - Error - [] operator not supported for strings in l() (line 2507 of /docroot/includes/common.inc)

I've removed a couple of child issues where it didn't seem to be a requirement that they were fixed in order for us to consider PHP 7.3 fully supported by D7.

I've been double-checking whether there's any reason not to enable testing on PHP 7.3 (and 7.4) for the 7.x branch of core (this is mentioned as a task in the IS, and it'd definitely help here). I hope to enable that very soon assuming no obstacles to doing so come up in the next few days.

I'd like to update the IS with a list of the issues which do need to be fixed, to serve as acceptance criteria for this meta issue to be considered fixed.

Are any such issues missing from the list?

As I mentioned in #45 that issue and enabling automated testing should be the only blockers for PHP 7.3 "support," the rest are just generic bugs/issues.

#3025335: session_id() cannot be changed after session is started has been committed and indeed the 7.x branch now passes tests under PHP 7.3, so we've added that to the automated testing config. Hopefully this will help with any more PHP 7.3 bugfixes we need to work on.

I think we can now (tentatively) call Drupal 7 compatible with PHP 7.3. Thank you everyone who helped us get here :)

I'm about to give up on php 7.3.x, I have recommended to my client downgrade to php 7.2.x php 7.3.x is fine except in some edge cases when using views_php (it was for prototyping, not a hard requirement) with the views that have ajax forms provided by the editablefields module.

I've pushed this as far as I can.

There's changes to the behavior of the serialize() function in php 7.3.x that affects also unserialize()

I've explained the issue as best I could and provided an example of a broken serialization , however, I believe the serialization is broken on serialize() prior to the unserialize()

See:
#3093110: D7 - EntityType objects cannot be reliably serialized without DependencySerializationTrait

and
the original issue as reported:
#2274543-108: [7.x-1.x] Remove usage of deprecated create_function()

Note: Red Hat 8.0 will support php 7.2.x for between 10 and 15 years.

re: PHP's broken serialization, the bug was present but silently ignored in older versions; in PHP 7.3 an error is triggered and unserialization fails. Hopefully any contrib modules running into this will be able to develop workarounds.

There's apparently a workaround in D8 using DependencySerializationTrait , I am not sure what that means yet.

ok, sorry for the previous noise and thanks for the very helpful suggestions and background information.

The issue I stumbled on was probably not yet noticed in D8 because the module in question has not yet been ported to D8 (editablefields) which provides convenient ajax forms of entity fields in a view.

So, editablefields was exposing the php 7.3.x issue I was observing in combination with views_php and the opis/closure library (to complicate things) which also serializes closures which were contained in the same view with an ajax form which it all was being serialized by includes/cache.inc , in php 7.2 and previous releases, the serialize error was silent and no one noticed.

Good news is I managed to make a working patch for this module the editablefields module (haven't yet submitted it)

However I also had to make one very small patch to includes/form.inc in order to squelch an unsupported operand types error, however this patch is probably optional, because the editablefields module still actually saves correct ajax data without the patch however if I don't patch there's an annoying popup warning about this.

For now I have a working solution for my client so I will stick to php 7.3.x

full details in #2819375-127: Do not make entries in "cache_form" when viewing forms that use #ajax['callback'] (Drupal 7 port)

edge case caused by a conflict between using the opis/closure library to replace the 'create_function' which was deprecated in combination with the views_php module and the editablefields module. Editablefields works fine if views_php fields are not used in a view with this.

Alternatively, I simply just stopped using views_php for this particular view that contains ajax forms provided by the editablefields module, and implemented a more normal solution. Views_php is more for prototypeing anyway so eventually would have done this anyway.

***EDIT***

Determination prevails
Followup, solution to my edge case bug, to fix, a contrib module patch and also a small core patch for this edge case ( editablefields module )
#3093423: php 7.3.x compatibility, unexpected operands in form_get_cache and too few arguments

***END EDIT***

Just to say
- today messages from Google about a couple of sites not working, server error (5xx)
I checked, and completely blank pages, not even reporting anything for "source"

I couldn't find a reason.
Asked webhost, and just been told it's issue with php 7.3; he changed so that running php 7.1, and all well again.

Install - a multi-site - was updated to Drupal 7.67 soon after this was released.

So for me, Drupal 7 not at all compatible with php 7.3.

@DocMartin thanks for providing feedback.

However, a couple of things:

1) Drupal 7.67 was a security release. There hasn't been a bugfix/maintenance release of D7 since 7.64 in February 2019. As such, most of the changes discussed in this issue which have been committed to the 7.x branch are not included in any current release.

We plan to make the next bugfix release on 2019-12-04 per the D7 release schedule. That will effectively be the first D7 release which is "compatible with PHP 7.3" in terms of the work tracked in this issue.

In the meantime, feel free to test the dev release which would include everything committed since the release you are running.

2) When reporting any problems such as compatibility issues with a specific PHP version, it's pretty much impossible to assist or troubleshoot without any details such as log messages of specific errors.

3) We're talking about D7 core here. Your multi-site likely includes several contrib projects (modules and themes) and possibly also custom code. It's quite possible that even with a release of Drupal core which has fixed compatibility issues, you may encounter problems with those contrib (and custom) projects. Again, specific details such as log / error messages will be required for troubleshooting, and those problems would be dealt with in the issue queues for the relevant projects (assuming they're contrib - if it's custom code, you may need to seek the assistance of a developer if you're not able to troubleshoot yourself).

@DocMartin, Why would you wait until 2019-12-04 before releasing a new D7 release? This is way overdue already, and you should give more priority to get a new release out. It is frankly a embarrassment that Drupal 7 is not compatible with PHP 7.3 as of 2019-11-20.

We've now closed all of the issues listed in #69 and the only child issues which remain open either don't relate to core, or are other meta issues about supporting later versions of PHP.

Are there any remaining tasks here, or can we close this issue?

Noting that doing so would not be equivalent to saying "everything in (core and contrib) D7 relating to PHP 7 is now 100% fixed". For some discussion on that, see e.g. #2756297-42: element_children sort order inconsistent between PHP 5 and PHP 7 [Drupal 7 backport]. It would, however, allow us to identify and move on to the next priority.

@aommundsen, if 2019-12-04 is not soon enough you can simply download the dev release of core as it is ready. The urgency was to get dev ready so that contrib can be tested in the automated testing.

With that said, the vast majority of Contrib is already ready either in a full release or in some cases a dev release. With the odd exception some modules might need a patch for php 7.3.x support however I've gone through already the contrib my clients use and pushed up releases in every project that I was using or asked other contrib maintainers to do so and which they have.

I updated https://www.drupal.org/docs/7/system-requirements/php-requirements to say "Planned for 2019-12-04" - this issue could now be closed.

Oh and... of course there should be a CHANGELOG.txt entry for this, but I'm guessing core maintainers will add these when they make the release?

Yes, we'll update CHANGELOG.txt when we do the new release in a few weeks.

Thanks @mfb, @joseph.olstad, and all of the other contributors who got D7 core working with PHP 7.3!

@mcdruid
With just nothing working, no Drupal logs to be seen.
Didn't think to check once sites working again, w php7.1; nothing in the recent logs right now.

Well, fingers crossed!

If you have access to your webserver error log, then https://www.drupal.org/project/error_log will cause all errors to be printed there, so you can see what's happening even if you have WSOD. See also https://www.drupal.org/project/raven which will send errors to https://sentry.io

@mcdruid, Are you still planning to release a new Drupal 7 release today? I hope so, as we have trouble waiting any longer.

See https://www.drupal.org/project/drupal/releases/7.68