Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.There is an implementation in migration source plugin core/modules/migrate/src/Plugin/migrate/source/SqlBase.php
which extends SourcePluginBase.php and problem arises for the use of count function
the way it's being implemented(the inheritance concepts) which is not being supported by PHP 7.
The source plugin have the method definition(class SourcePluginBase) in the following way
class SourcePluginBase {
public function count($refereh=FALSE) {
//
}
}
abstract class SqlBase extends SourcePluginBase {
public function count() {
//
}
}
So PHP 7 now interprets the implementation of count() in a strict manner & that count() definition is not recognized from PHP 7 itself.
Patch is coming soon
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | 2911306-7.patch | 5.08 KB | alexpott |
| #7 | 6-7-interdiff.txt | 860 bytes | alexpott |
| #6 | 2911306-6.patch | 5.08 KB | alexpott |
| #2 | 2911306-Migration_module_breaks_PHP7-2.patch | 666 bytes | roynilanjan |
Comments
Comment #2
roynilanjan CreditAttribution: roynilanjan commentedComment #3
roynilanjan CreditAttribution: roynilanjan commentedComment #4
quicksketchThe implementation here looks great, but indentation if off in the code. Could you add another space before each line?
In addition, all of the other classes that extend SourcePluginBase should also be updated to include the $reset parameter, including EmbeddedDataSource and EmptySource.
Comment #5
alexpottI don't see why we need to call the parent at all though. Also as per #2920492: [PHP 7.2] Incompatible declaration of Drupal\migrate\Plugin\migrate\source\SqlBase::count() there are other classes to fix. I'm going to close that issue and add the patch from that one here.
Comment #6
alexpottHere's the patch. This is also a critical bug. It makes migrations impossible on PHP7.2
Comment #7
alexpottBetterer english :)
Comment #8
quietone CreditAttribution: quietone as a volunteer commented@alexpott, thanks for moving the patch and closing the other as a duplicate. Honestly, I wasn't sure which was better to mark as a duplicate. And it was too much for me for me to think about this early in the day, So early that it is dark and the birds are quiet.
Comment #9
mondrakePatch in #7 + patch in #2853503-41: Remove all assert('string') calls from Drupal core because deprecated in PHP 7.2 allow to start fine tests on a TravisCI build with PHP 7.2 for the Imagemagick module.
See #2853503-39: Remove all assert('string') calls from Drupal core because deprecated in PHP 7.2 and https://travis-ci.org/mondrake/imagemagick/jobs/296358802
Now tests there fail because of #2850639: Doctrine incompatibility with PHP 7.2.x: StaticReflectionClass::newInstance($args) should be compatible with ReflectionClass::newInstance(...$args) :(
Comment #10
Gábor HojtsyComment #12
Gábor HojtsyThis makes a ton of sense. Committed to 8.5.x. I don't think this would case any problems on 8.4.x, but it would be nice to get a confirmation from people working on the isse to cherry-pick :) Moving to 8.4.x as RTBC.
Comment #13
alexpottI totally think this is okay for 8.4.x we have good unit test coverage of loading these classes and there is no issue with changing a signature to add a missing param with a default like this.
Comment #14
Gábor HojtsyYeah I think it would only affect those who then tried to extend the empty method I guess, but only the same problem that there is already. It would either work the same odd way pre 7.2 (just one level down) or break one more level down the chain in 7.2.