The tests currently fail when ran against PHP 5.4:

Profiles tests 123 passes, 7 fails, and 1 exception

This needs to be fixed.

CommentFileSizeAuthor
#11 interdiff.3159270.7-11.txt1016 bytestobiasb
#11 3159270-11.backup_migrate.Fixed-compatibility-with-PHP-54.patch652 bytestobiasb
#7 backup_migrate-php_54_update_7310-3159270-7.patch810 bytesjeffschuler

Comments

DamienMcKenna created an issue. See original summary.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Priority: Normal » Major

Unfortunately I don't have a way of running PHP 5.4 locally..

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Issue summary: View changes
damienmckenna’s picture

damienmckenna
Location TN, USA
commented

This will help: https://github.com/drud/ddev-contrib/tree/master/docker-compose-services...

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Issue tags: +PHP 5.4

@DamienMcKenna, which issues are you running into, link to test failures?

damienmckenna’s picture

damienmckenna
Location TN, USA
commented

https://www.drupal.org/pift-ci-job/1766336
The problem is with BmTestUpdate7310.

jeffschuler’s picture

jeffschuler
Location Boulder, Colorado
commented
Status: Active » Needs review
StatusFileSize
new backup_migrate-php_54_update_7310-3159270-7.patch810 bytes

Just this:

-        ->fields(['enabled' => '0'])
+        ->fields(array('enabled' => '0'))
damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Parent issue: » #3159271: Plan for Backup and Migrate 7.x-3.10

That's all it was? 🤦‍♂️

Thanks for finding that, jeffschuler!

tobiasb’s picture

tobiasb
Primary language German
Location Berlin
commented

The short array syntax is supported since >= 5.4.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Needs review » Needs work

So it seems like there's more to it, because the tests still fail against PHP 5.4.

09:21:30 FATAL BmTestUpdate7310: test runner returned a non-zero error code (255).
09:21:37 Profiles tests 123 passes, 7 fails, and 1 exception

tobiasb’s picture

tobiasb
Primary language German
Location Berlin
commented
Status: Needs work » Needs review
StatusFileSize
new 3159270-11.backup_migrate.Fixed-compatibility-with-PHP-54.patch652 bytes
new interdiff.3159270.7-11.txt1016 bytes

Ok phpstorm found the problem. array_column (>=php 5.5) in backup_migrate_update_7310.

solideogloria’s picture

solideogloria commented

You should probably put both changes into a single patch.

tobiasb’s picture

tobiasb
Primary language German
Location Berlin
commented

@solideogloria

There is only one change. The patch for [] -> array() is wrong.

solideogloria’s picture

solideogloria commented

Ah, okay. The module already is PHP 5.4 and later.

solideogloria’s picture

solideogloria commented
solideogloria’s picture

solideogloria commented

There should maybe be a space after the semicolon in the anonymous function. Other than that, it looks good to me, since it follows the recommendation from https://www.php.net/manual/en/function.array-column.php#117229

jeffschuler’s picture

jeffschuler
Location Boulder, Colorado
commented

@tobiasb: Drupal 7's requirements still include PHP 5.3 support, so while I admit that that patch is not the solution to this 5.4-related issue, it's still correct and appropriate.

From #2135291: [Policy, no patch] PHP 5.4 short array syntax coding standards for Drupal 8:
"Short array syntax should not be used in Drupal 7.x or 6.x core or contributed modules."
"Please note, short array syntax is unsupported in versions of PHP prior to 5.4. This means that Drupal 7 core and Drupal 7 contributed projects without an explicit PHP 5.4+ requirement must use long array syntax."

I see no explicit PHP 5.4+ requirement for Backup and Migrate.

jeffschuler’s picture

jeffschuler
Location Boulder, Colorado
commented

Ah, I stand corrected. Req is not on the module page, README, or issue queue, but in the module's .info file:

; Require PHP 5.4 so that some slightly more advanced PHP logic can be used.
; This is for future usage; removing this will allow the module to continue
; working fine for now.
php = 5.4
damienmckenna’s picture

damienmckenna
Location TN, USA
commented

The main reason to require PHP 5.4 is so that we can use traits to greatly simplify the codebase: #2704333: Use Traits to simplify the codebase

There's an awful lot of duplicate code between the destination and source APIs because traits didn't exist in older PHP. With traits we can have one class for handling databases and then import it into the destination and source classes, instead of duplicating code all over the place.

  • DamienMcKenna committed 35c7f5c on 7.x-3.x authored by tobiasb 
    Issue #3159270 by tobiasb, jeffschuler, DamienMcKenna, solideogloria,...
damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Needs review » Fixed

Committed.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

markabur’s picture

markabur commented

FYI that short array syntax is causing a WSOD for a client of mine who updated to the latest backup_migrate on a PHP 5.3 server. I can fix it, but I thought people here might be interested to know PHP 5.3 is still out there needing to be supported.

solideogloria’s picture

solideogloria commented

This module does not support PHP 5.3 or earlier. The .info file specifies that PHP 5.4 is required.

Also, PHP 7.2 and earlier is not supported by PHP, so anyone using it should work to get to a newer version.