Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drush 8 warnings on PHP 8.1 #5196

Closed
danepowell opened this issue Aug 2, 2022 · 20 comments
Closed

Drush 8 warnings on PHP 8.1 #5196

danepowell opened this issue Aug 2, 2022 · 20 comments
Labels

Comments

@danepowell
Copy link
Contributor

Describe the bug
Running Drush 8 in a PHP 8.1 environment leads to deprecation warnings:

Deprecated: Return type of HumbugBox3140\KevinGH\RequirementChecker\RequirementCollection::getIterator() should either be compatible with IteratorAggregate::getIterator(): Traversable, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in phar:///home/ide/project/drush.phar/.box/src/RequirementCollection.php on line 14

Deprecated: Return type of HumbugBox3140\KevinGH\RequirementChecker\RequirementCollection::count() should either be compatible with Countable::count(): int, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in phar:///home/ide/project/drush.phar/.box/src/RequirementCollection.php on line 19

To Reproduce

Run drush using Drush 8 and PHP 8.1

Expected behavior
No deprecation warnings

Actual behavior
Deprecation warnings

Workaround
It's just a warning so it doesn't block anything, but it is annoying

System Configuration

Q A
Drush version? 8.4.11
Drupal version? none
PHP version 8.1
OS? Linux

Additional information
Looks like maybe this was intended to fix it, but it didn't: #4978

@weitzman weitzman added the drush8 label Aug 8, 2022
@mbomb007
Copy link

mbomb007 commented Sep 5, 2022

I get these warnings:

bash-5.1:/var/www/html> drush version
PHP Deprecated:  Return type of DrushBatchContext::offsetSet($name, $value) should either be compatible with ArrayObject::offsetSet(mixed $key, mixed $value): void, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in phar:///usr/local/bin/drush8/includes/batch.inc on line 39

Deprecated: Return type of DrushBatchContext::offsetSet($name, $value) should either be compatible with ArrayObject::offsetSet(mixed $key, mixed $value): void, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in phar:///usr/local/bin/drush8/includes/batch.inc on line 39
 Drush Version   :  8.4.8 

(Never mind, these are fixed in 8.x dev)

@mbomb007
Copy link

https://github.com/drush-ops/drush/blob/8.x/commands/core/drupal/batch_7.inc

Do you wish to run all pending updates? (y/n): y
PHP Deprecated:  Optional parameter $command declared before required parameter $options is implicitly treated as a required parameter in .../vendor/drush/drush/commands/core/drupal/batch_7.inc on line 19

@greg-1-anderson
Copy link
Member

PRs welcome

@mbomb007
Copy link

mbomb007 commented Oct 20, 2022

function _drush_backend_batch_process($command = 'batch-process', $args, $options) {

I think the change would need to be made in 3 files:

commands/core/drupal/batch_7.inc:19
commands/core/drupal/batch.inc:20
commands/core/drupal/batch_6.inc:19

The only reference is in includes/batch.inc, which has the parameter defaults all set:

function drush_backend_batch_process($command = 'batch-process', $args = array(), $options = array()) {
  // Command line options to pass to the command.
  $options['u'] = drush_user_get_class()->getCurrentUserAsSingle()->id();

  drush_include_engine('drupal', 'batch');
  _drush_backend_batch_process($command, $args, $options);
}

@mbomb007
Copy link

mbomb007 commented Oct 20, 2022

PRs welcome

I created a PR: #5286

@danepowell
Copy link
Contributor Author

Whatever issue @mbomb007 fixed is different from what I originally reported. Here's the fix for the Box-related errors I reported: #5341

@danheisel
Copy link

There seem to be a few issues related to this incompatibility. I thought it had been fixed but maybe not. It's mostly causing a blockage on Drupal 7 sites using Drush 8 with PHP 8 on Pantheon. Very curious when this will be rolled out since PHP 7 is EOL and Drupal 7 is unable to run any higher version of Drush.

@ivantibezh
Copy link
Contributor

Can we merge changes to the Drush8 branch?
PHP 7 is not supported anymore but Drupal 7 supports PHP 8.1 https://www.drupal.org/docs/7/system-requirements/php-requirements.

@klausi
Copy link
Contributor

klausi commented Jan 10, 2023

Fully agreed with Ivan - Drupal 7 projects will have to move to PHP 8.1+ sooner or later to be on a supported PHP version.

@greg-1-anderson hi, you approved #5341 3 weeks ago - anything missing so that it can be merged for a new Drush 8 release? Thank you!

@nevergone
Copy link
Contributor

PHP 7 is security supported with Linux distributions. Ubuntu 14.04 EOL is April 2024 https://canonical.com/blog/ubuntu-14-04-and-16-04-lifecycle-extended-to-ten-years and by default, Ubuntu 14.04 ships with PHP 5.5.9.

@greg-1-anderson
Copy link
Member

@greg-1-anderson hi, you approved #5341 3 weeks ago

Um, sorry, I was just supposed to merge that. 😊 Done.

@klausi
Copy link
Contributor

klausi commented Jan 20, 2023

Yay, thanks a lot Greg!

I see the tests are failing on the Drush 8.x branch for quite some time, so the next steps I think are

  1. Fix automated tests
  2. Make a new Drush 8 release

@greg-1-anderson
Copy link
Member

The tests on the Drush 8.x branch are now green, but I discovered that various changes in dependencies are making it hard to support global Drush 8 on Drupal 9 (and 10, but I haven't even tried that). Since Drush never formally supported Drupal 9, I simply stopped testing on that version. I did turn on some tests for Dupal 8, but only with PHP 7.4, as there are failures for Drupal 8 with PHP 8. I will have to look to the community to fill these gaps if these configurations are still important. Drush 8 tests are green for Drupal 7 on PHP 5.6 through 8.1, and I will add PHP 8.2 when it is supported in Drual 7.

@greg-1-anderson
Copy link
Member

Next step is to look at some Drush 8 issues and PRs and see which can be resolved quickly, then I will make a release.

@greg-1-anderson
Copy link
Member

I'm satisfied with my sweep through Drush 8 issues and PRs, and will make a release shortly, sometime between this evening and Tuesday, probably.

@klausi
Copy link
Contributor

klausi commented Jan 23, 2023

Great, thanks! In my opinion we should only support Drupal 7 with Drush 8.

Drupal 9 and 10 can use higher drush versions, Drupal 8 is not supported anymore.

@greg-1-anderson
Copy link
Member

Yes, I agree with you philosophically. I am not going to spend a lot of time on the unsupported configurations; hopefully, anyone still stuck there is working to upgrade, as it probably won't keep working forever.

@gitressa
Copy link
Contributor

Thanks for making Drush 8.4.11 and Drupal 7 work with PHP 8.1 @greg-1-anderson. I got the error below after upgrading to PHP 8.2. Downgrading to PHP 8.1 made it go away and everything works fine:

$ drush @website uli
<h1>Additional uncaught exception thrown while handling exception.</h1><h2>Original</h2><p>Error: Undefined constant PDO::MYSQL_ATTR_SSL_CA in Drush\Sql\Sqlmysql-&gt;creds() (line 56 of phar:///usr/local/bin/drush/vendor/composer/../../lib/Drush/Sql/Sqlmysql.php).</p><h2>Additional</h2><p>TYPO3\PharStreamWrapper\Exception: Unexpected file extension in &amp;quot;phar:///usr/local/bin/drush/vendor/composer/../symfony/polyfill-mbstring/Mbstring.php&amp;quot; in Drupal\Core\Security\PharExtensionInterceptor-&gt;assert() (line 38 of /var/www/html/website/public_html/misc/typo3/drupal-security/PharExtensionInterceptor.php).</p><hr />Drush command terminated abnormally due to an unrecoverable error.

[error] Error: Return type of HumbugBox3140\KevinGH\RequirementChecker\RequirementCollection::count() should either be compatible
with Countable::count(): int, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in
phar:///usr/local/bin/drush/.box/src/RequirementCollection.php, line 19

@greg-1-anderson
Copy link
Member

Although I did merge one community-provided PR related to PHP 8.2 warnings, I have not otherwise attempted to support PHP 8.2 on Drush 8 yet. It does look like Drupal 7 supports PHP 8.2 now, so this would be a useful addition. PRs welcome.

@danepowell
Copy link
Contributor Author

This should be resolved in 8.4.12. Let's open new issues if there are unresolved issues with Drush 8 and PHP 8.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

9 participants