Drush Launcher busted with Drush 12

loopy3025 commented 1 year ago

Upon upgrading to BLT 13.7.1, composer tries upgrading drush to 12.x. Several incompatibilities seem to have cropped up. On the Drush project, it has been suggested that we run Drush commands with vendor/bin/drush instead of drush (REF: https://github.com/drush-ops/drush/issues/5694). This seems to work locally for one-off commands on the command line. This won't work in blt, of course, because those scripts are installed to vendor. As of now, none of our blt projects can successfully build in pipelines if they are upgraded to Drush 12.

To Reproduce

drupal/core: 10.1.1
acquia/blt: 13.7.1
drush/drush: 12.1.1
PHP: 8.1
Platforms:
- Docksal
- Acquia Pipelines

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. As I stated earlier, the folks at the drush/drush project suggest using vendor/bin/drush instead of drush when running commands. This does allow the commands to execute correctly on the command line. This, however, is not so helpful in regards to BLT commands during Pipelines CI...

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.

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 and I'm not sure how I can execute blt commands that have a drush component without using drush launcher.

Workaround

Downgrade to Drush 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

BLT doctor output

BLT Doctor collapsed

Thanks, in advance, for your help.

mikemadison13 commented 1 year ago

i'm not certain this is a BLT bug. if you look at how BLT executes Drush...

https://github.com/acquia/blt/blob/main/src/Robo/Common/Executor.php#L80

$drush_array[] = $this->getConfigValue('composer.bin') . DIRECTORY_SEPARATOR . "drush";

BLT calls a drush task by going into the composer bin and calling vendor/bin/drush. It doesn't ever "just drush."

loopy3025 commented 1 year ago

@mikemadison13 my bad. I guess this is going to have to be an Acquia ticket.

carolpettirossi commented 1 year ago

I'm facing an issue with Drush 12 when I run blt setup where setup strategy=sync. This only happens if my database is clean/empty meaning that Drupal has not been installed yet.

carolpettirossi commented 1 year ago

It looks like blt could remove the drush clear-cache drush as per the maintainer's suggestion: https://drupal.slack.com/archives/C62H9CWQM/p1692636439244739

halisonfernandes commented 1 year ago

As you are running on docksal, might be worth checking this too: https://github.com/docksal/service-cli/issues/304

loopy3025 commented 7 months ago

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

acquia / blt