drush-ops / drush

Drush is a command-line shell and scripting interface for Drupal, a veritable Swiss Army knife designed to make life easier for those who spend their working hours hacking away at the command prompt.
https://www.drush.org
2.34k stars 1.08k forks source link

Drush 12 Upgrade issues #5694

Closed loopy3025 closed 1 year ago

loopy3025 commented 1 year ago

Describe the bug

We have sites running Drupal 10 and Drush 11 on PHP 8.1. Recently, there was an upgrade to BLT from 13.7.0 to 13.7.1 which includes Drush 12. We've had several issues with this. We have been able to downgrade drush/drush to ^11.0 for the time being, but I was hoping to get to the bottom of this. On our local sites, when we run drush commands, we get deprecation warnings, but the commadns run. When we push the code up to the remote, Acquia pipelines fails as it is unable to execute any Drush commands. Since we are having problems in both environments, I chose to create a bug report here instead of in the issue queues for those two projects.

To Reproduce

Expected behavior

Drush commands run without error or deprecation warnings.

Actual behavior

Local

Locally, we are using Docksal. During any drush command, we get the following deprecation warnings:

PHP 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:///usr/local/bin/drush8/.box/src/RequirementCollection.php on line 14

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:///usr/local/bin/drush8/.box/src/RequirementCollection.php on line 14
PHP 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:///usr/local/bin/drush8/.box/src/RequirementCollection.php on line 19

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:///usr/local/bin/drush8/.box/src/RequirementCollection.php on line 19

It makes me think Drush 12 would prefer it if we installed PHP 8.2. Not sure.

I really wonder why it's looking in usr/local/bin/drush8. Drush 12 is bootstrapping:

Drupal version   : 10.1.1                                
Site URI         : http://oursite.docksal.site          
DB driver        : mysql                                 
DB hostname      : db                                    
DB port          : 3306                                  
DB username      : user                                  
DB name          : default                               
Database         : Connected                             
Drupal bootstrap : Successful                            
Default theme    : ourtheme                                
Admin theme      : claro                                 
PHP binary       : /usr/local/bin/php                    
PHP config       : /usr/local/etc/php/php.ini            
PHP OS           : Linux                                 
PHP version      : 8.1.8                                 
Drush script     : /var/www/vendor/bin/drush             
Drush version    : 12.1.1.0                              
Drush temp       : /tmp                                  
Drush configs    : /var/www/vendor/drush/drush/drush.yml 
                   /var/www/drush/drush.yml              
Install profile  : minimal                               
Drupal root      : /var/www/docroot                      
Site path        : sites/default                         
Files, Public    : sites/default/files                   
Files, Private   : /var/www/files-private                
Files, Temp      : /tmp                   

However, after these warnings, the commands do actually run. I noticed some issues that have a similar error point to the drush launcher being an issue. fin drush --version reports we are using Drush Launcher 0.10.1.

Acquia

We can't get any branches with Drush 12 past pipelines. We get the following error: drupal:sync:db

 [Acquia\Blt\Robo\Tasks\DrushTask] Running /mnt/tmp/local.prod/source/vendor/bin/drush cache-clear drush --no-interaction --ansi &&
 /mnt/tmp/local.prod/source/vendor/bin/drush sql-sync @oursite.test @self --target-dump=/tmp/tmp.target.sql.gz --structure-tables-key=lightweight --create-db --no-interaction --ansi &&
 /mnt/tmp/local.prod/source/vendor/bin/drush cr --no-interaction --ansi &&
 /mnt/tmp/local.prod/source/vendor/bin/drush sql-sanitize --no-interaction --ansi in /mnt/tmp/local.prod/source/docroot

  Command cache-clear was not found. Drush was unable to query the database.   
  As a result, many commands are unavailable. Re-run your command with --debu  
  g to see relevant log messages.                                              

 [Acquia\Blt\Robo\Tasks\DrushTask]  Exit code 1  Time 0.206s

The command failed. This often indicates a problem with your configuration. Review the command output above for more detailed errors, and consider re-running with verbose output for more information.

I was wondering if there's something amiss with our site file. Here's the code for the test environment's alias:

test:
  host: oursitestg.ssh.prod.acquia-sites.com
  options:
    ac-env: test
    ac-realm: prod
    ac-site: oursite
  root: /var/www/html/oursite.test/docroot
  uri: oursitestg.prod.acquia-sites.com
  user: oursite.test

As far as I can tell, no documentation has said we need to change anything with these files when upgrading from 11.x-12.x

Workaround

Downgrade to 11.x

System Configuration

Q A
Drush version? 12.1.1
Drupal version? 10.1.1
PHP version 8.1
OS? Mac and Acuqia Cloud

Thanks, in advance, for your help.

greg-1-anderson commented 1 year ago

Looks like your configuration is using Drush 8 as a launcher to run Drush 12. Try adjusting your $PATH so that Drush 12 is called directly.

loopy3025 commented 1 year ago

Looks like your configuration is using Drush 8 as a launcher to run Drush 12. Try adjusting your $PATH so that Drush 12 is called directly.

Interesting. I could look into why the container would be doing this, but it does beg the question why it isn't having the same issue with Drush 11.

greg-1-anderson commented 1 year ago

Looks like the deprecation warnings are in the box (.phar maker) libraries of Drush 8. In theory, these are compatible with PHP 8.2.

I'm not sure what the issue is with your Acquia example.

loopy3025 commented 1 year ago

My actual local computer really doesn't handle the drush commands, so $PATH probably won't fix anything. Here's the relevant stuff in my computer's $PATH:

$HOME/./usr/local/bin/drush:$HOME/.composer/vendor/pedraic/humbug_get_contents:

But when you execute drupal with docksal using fin, it does it in the container and uses whatever is in the container. It's SUPPOSED to use whatever you have installed with composer.

I am very curious why it's throwing an error now and never had a problem with Drush 11. It's very strange.

greg-1-anderson commented 1 year ago

In that configuration, my suggestion is for the path inside the container, which I suppose you do not maintain.

loopy3025 commented 1 year ago

I do not. I finally found something that explains the Docksal issue at least: https://github.com/docksal/docksal/issues/1783

drush-ops/drush-launcher ref: https://github.com/drush-ops/drush-launcher/issues/105

And I also noticed on Acquia, they are using a launcher, but the version is just reporting a variable: Drush Launcher Version: @git-version@ but if Drush 12 isn't compatible with any version of drush launcher, then it really won't matter I guess.

weitzman commented 1 year ago

Acquia provides a launcher but you dont have to use it. Use vendor/bin/drush instead. I think everthing is covered in other issues so closing.

loopy3025 commented 1 year ago

Thanks so much for that explanation. I didn't realize that the direction was to literally use vendor/bin/drush on the command line rather than drush. Running fin exec vendor/bin/drush status locally finally produced a response without error.

Is that really how we're supposed to be executing drush commands now? I can't even create an alias for that.

What's more, we cannot re-write the blt commands to use vendor/bin/drush. We'll have to stay with Drush 11 until BLT's drush commands run through ci without error.

greg-1-anderson commented 1 year ago

The maintainer of the image should remove the launcher, and add vendor/bin to the $PATH. This should have been done when they upgraded to Drush 12.

loopy3025 commented 1 year ago

@greg-1-anderson Docksal doesn't actually enforce drush versioning. It leaves that up to the user. But yeah, they are going to have to adjust the image so that it doesn't use the launcher, it seems.

greg-1-anderson commented 1 year ago

They could adjust the PATH such that vendor/bin is near the end, and the Drush launcher appears afterwards.

socketwench commented 1 year ago

Flightdeck maintainer here, we added /var/www/vendor/bin to our $PATH and drush is now being found in our 8.1 and 8.2 containers, but it always claims Drupal doesn't exist, even when sitting in the docroot.

weitzman commented 1 year ago

Please open a new issue showing the command you ran, the dir you are in, and the -vvv log output. Also please show output of composer show drupal/core

socketwench commented 1 year ago

Actually, this thread pointed me in the right direction and we no longer have this issue. The problem is the container still had the Drush launcher globally installed and in $PATH. When the launcher is removed, the composer-local drush works as expected.

loopy3025 commented 8 months ago

I believe this is now fixed if you switch to https://github.com/docksal/service-cli/releases/tag/v3.6.0