Problem/Motivation
Basically, MarkdownCommands causes this fatal error with Drush 13:
ambientimpact:~/.../Omnipedia/Web$ ddev drush status
AssertionError: Instead of using replacing Drush's logger, use $this->add() on DrushLoggerManager to add a custom logger. See https://github.com/drush-ops/drush/pull/5022 in /var/www/html/vendor/drush/drush/src/Commands/DrushCommands.php on line 76 #0 /var/www/html/vendor/drush/drush/src/Commands/DrushCommands.php(76): assert()
#1 /var/www/html/vendor/drush/drush/src/Runtime/ServiceManager.php(480): Drush\Commands\DrushCommands->logger()
#2 /var/www/html/vendor/drush/drush/src/Boot/DrupalBoot8.php(282): Drush\Runtime\ServiceManager->inflect()
#3 /var/www/html/vendor/drush/drush/src/Boot/DrupalBoot8.php(218): Drush\Boot\DrupalBoot8->addDrupalModuleDrushCommands()
#4 /var/www/html/vendor/drush/drush/src/Boot/BootstrapManager.php(211): Drush\Boot\DrupalBoot8->bootstrapDrupalFull()
#5 /var/www/html/vendor/drush/drush/src/Boot/BootstrapManager.php(397): Drush\Boot\BootstrapManager->doBootstrap()
#6 /var/www/html/vendor/drush/drush/src/Boot/BootstrapManager.php(332): Drush\Boot\BootstrapManager->bootstrapMax()
#7 /var/www/html/vendor/drush/drush/src/Boot/BootstrapManager.php(304): Drush\Boot\BootstrapManager->bootstrapToPhaseIndex()
#8 /var/www/html/vendor/drush/drush/src/Boot/BootstrapHook.php(36): Drush\Boot\BootstrapManager->bootstrapToPhase()
#9 /var/www/html/vendor/consolidation/annotated-command/src/Hooks/Dispatchers/InitializeHookDispatcher.php(44): Drush\Boot\BootstrapHook->initialize()
#10 /var/www/html/vendor/consolidation/annotated-command/src/Hooks/Dispatchers/InitializeHookDispatcher.php(36): Consolidation\AnnotatedCommand\Hooks\Dispatchers\InitializeHookDispatcher->doInitializeHook()
#11 /var/www/html/vendor/consolidation/annotated-command/src/Hooks/Dispatchers/InitializeHookDispatcher.php(29): Consolidation\AnnotatedCommand\Hooks\Dispatchers\InitializeHookDispatcher->callInitializeHook()
#12 /var/www/html/vendor/consolidation/annotated-command/src/CommandProcessor.php(145): Consolidation\AnnotatedCommand\Hooks\Dispatchers\InitializeHookDispatcher->initialize()
#13 /var/www/html/vendor/consolidation/annotated-command/src/AnnotatedCommand.php(376): Consolidation\AnnotatedCommand\CommandProcessor->initializeHook()
#14 /var/www/html/vendor/symfony/console/Command/Command.php(292): Consolidation\AnnotatedCommand\AnnotatedCommand->initialize()
#15 /var/www/html/vendor/symfony/console/Application.php(1096): Symfony\Component\Console\Command\Command->run()
#16 /var/www/html/vendor/symfony/console/Application.php(324): Symfony\Component\Console\Application->doRunCommand()
#17 /var/www/html/vendor/symfony/console/Application.php(175): Symfony\Component\Console\Application->doRun()
#18 /var/www/html/vendor/drush/drush/src/Runtime/Runtime.php(110): Symfony\Component\Console\Application->run()
#19 /var/www/html/vendor/drush/drush/src/Runtime/Runtime.php(40): Drush\Runtime\Runtime->doRun()
#20 /var/www/html/vendor/drush/drush/drush.php(140): Drush\Runtime\Runtime->run()
#21 /var/www/html/vendor/bin/drush.php(119): include('...')
#22 {main}
AssertionError: Instead of using replacing Drush's logger, use $this->add() on DrushLoggerManager to add a custom logger. See https://github.com/drush-ops/drush/pull/5022 in assert() (line 76 of /var/www/html/vendor/drush/drush/src/Commands/DrushCommands.php).
[warning] Drush command terminated abnormally.
Failed to run drush status: exit status 1
Steps to reproduce
Run drush status or any other Drush command; get above error.
Proposed resolution
It took a while to locate exactly what command was triggering the error until I found that MarkdownCommands does indeed override the logger in its constructor: $this->logger = $logger; - given that the entire class was apparently deprecated in 8.x-2.0 and was intended to be removed in 3.0.0 as part of #3103679: [PP-1][markdown] Drupal 9 Support, we might be able to remove it, but it looks like it's tied to some Composer stuff and I'm reluctant to pull on that thread because this would make more sense in a single issue to remove all deprecated that was supposed to be removed in 3.0.0. For now, we should wrap the assignment in a check for the logger() existing on $this and not assign it if it's present:
// Don't replace Drush's logger because it'll cause a fatal error on Drush
// 13 and newer.
//
// @see https://github.com/drush-ops/drush/pull/5022
if (!method_exists($this, 'logger')) {
$this->logger = $logger;
}
Remaining tasks
Do the thing.
User interface changes
None.
API changes
Drush command removed.
Data model changes
None?
| Comment | File | Size | Author |
|---|---|---|---|
| #14 | 3483437-markdown--fix-drush-13.diff | 975 bytes | robloach |
Issue fork markdown-3483437
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
ambient.impactComment #3
ambient.impactComment #4
ambient.impactComment #6
mayeulk commentedHi. Apparently I'm facing this bug with:
$ drush st
Drupal version : 10.3.7
DB driver : pgsql
PHP OS : Linux (Docker container)
PHP version : 8.3.11
Drush version : 13.3.2.0
I had to uninstall markdown module (which was not working anyway), through the admin GUI then with :
composer remove drupal/markdown
composer remove league/commonmark:^1
drush cache:rebuild
Is there a way that I can get both drush and markdown working?
Comment #7
malcomio commentedThere isn't a merge request or a patch, so the status shouldn't be Needs review.
The fork does have some change in it.
Comment #9
malcomio commentedHave created MR 35
Comment #10
malcomio commentedThis change seems to fix the problem as far as I can see.
I've seen different approaches in some other modules though:
https://git.drupalcode.org/project/digital_signage_framework/-/commit/c4...
https://git.drupalcode.org/project/do_username/-/merge_requests/13/diffs
I guess that the good thing about this approach is that it is backwards compatible
Comment #11
aaronmchaleCan confirm after applying patch from MR, also fixes issue for me.
Comment #12
moshe weitzman commentedI dont see the benefit of using a custom logger here. I suggest removing the logger related code from the constructor and create().
Comment #13
orkutmuratyilmazany chance for the merge?
Comment #14
robloachConfirming. Here's the patch from @malcomio's MR for those needing it.
Comment #16
joelpittetThank you all I have merged this into 3.0.x-dev
Comment #19
orkutmuratyilmazthanks for the fix:)