brianlmoon / net_gearman

A PHP interface for Gearman
84 stars 46 forks source link

PHP 8 Support #48

Closed j03k64 closed 2 years ago

j03k64 commented 3 years ago

PHP 8 Support

This PR adds support for getting this library working on PHP 8. I'm going to leave this open for a bit while I determine if there are other places that need to be addressed as well.

Issue 1

get_resource_type fails with $this->socket passed in on PHP8 since it's expecting a resource instead of a Socket object.

Fix 1

Update Net/Gearman/Connection.php to return early if $this->socket is an instance of Socket (which should be only on PHP8+), otherwise falls back to default logic.

Issue 2

Unable to run functional tests.

Fix 2

j03k64 commented 3 years ago

@brianlmoon thoughts on how to update the unit test? Do a PHP version check like below:

// Net_Gearman_ConnectionTest.php:19
if (version_compare(PHP_VERSION, '8.0.0') >= 0) {
  // PHP 8+ returns a Socket class instead of a resource now
  $this->assertInstanceOf('Socket', $connection->socket);
}
else {
  $this->assertEquals('socket', strtolower(get_resource_type($connection->socket)));
}

or would you prefer something else?

brianlmoon commented 3 years ago

@j03k64 Makes sense to me to do it that way.

j03k64 commented 3 years ago

@brianlmoon I don't have an easy way to test this on a PHP 7.X install. I could figure it out, but if you have something readily available that would be appreciated.

j03k64 commented 3 years ago

@brianlmoon any other comments? (I have this branch in production running on 7.4 fwiw)