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

Members fund testing for the Drupal project. Drupal Association Learn more

Comments

roynilanjan created an issue. See original summary.

roynilanjan’s picture

roynilanjan’s picture

Status: Active » Needs review
quicksketch’s picture

Status: Needs review » Needs work

The 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.

alexpott’s picture

+++ b/core/modules/migrate/src/Plugin/migrate/source/SqlBase.php
@@ -330,8 +330,11 @@
+   // We keep the parent call to utilize source count information.
+   parent::count($refresh);

I 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.

alexpott’s picture

Title: Migration Module Breaks with PHP-7 » Migration Module Breaks with PHP 7.2
Version: 8.3.x-dev » 8.5.x-dev
Priority: Major » Critical
Status: Needs work » Needs review
Issue tags: +PHP 7.2
FileSize
5.08 KB

Here's the patch. This is also a critical bug. It makes migrations impossible on PHP7.2

alexpott’s picture

Betterer english :)

quietone’s picture

@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.

mondrake’s picture

Gábor Hojtsy’s picture

Title: Migration Module Breaks with PHP 7.2 » Migration module breaks with PHP 7.2 due to inherited method signature differences

  • Gábor Hojtsy committed 4e83abc on 8.5.x
    Issue #2911306 by alexpott, roynilanjan, mondrake, quicksketch, quietone...
Gábor Hojtsy’s picture

Version: 8.5.x-dev » 8.4.x-dev

This 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.

alexpott’s picture

I 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.

Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Fixed

Yeah 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.

Status: Fixed » Closed (fixed)

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