Problem/Motivation
Disclaimer: This was found and fixed by an LLM. It also helped write this issue summary.
ConsoleCompilerPass::process() uses Definition::setTags() to register auto-discovered #[AsCommand] services:
$definition
->setAutowired(TRUE)
->setPublic(TRUE)
->setTags(['console.command' => []]);
setTags(['console.command' => []]) stores the tag as an empty array of instances ([]). Definition::addTag('console.command') stores it correctly as [[]] - an array containing one instance.
In symfony/console 8.0.x, AddConsoleCommandPass::process() iterates per-service and reads command names directly from the #[AsCommand]attribute. An empty tag instance list is harmless - the command is still registered.
In symfony/console 8.1, AddConsoleCommandPass::process() was refactored to iterate per-tag-instance:
foreach ($container->findTaggedServiceIds('console.command', true) as $id => $tags) {
foreach ($tags as $tag) { // $tags is [] — inner loop never runs
$commandServices[$id]...
}
}
With setTags(['console.command' => []]), $tags is [], the inner loop never executes, and the service is silently dropped from the command map. Auto-discovered #[AsCommand] commands are absent from vendor/bin/dr list with no error or warning.
Steps to reproduce
- Run the tests with Symfony 8.1: https://git.drupalcode.org/project/drupal/-/pipelines/869562
- Apply the fix and run again: https://git.drupalcode.org/project/drupal/-/pipelines/869568/
- Revert the upgrade and ensure it still passes: https://git.drupalcode.org/project/drupal/-/pipelines/869575
Proposed resolution
- ->setTags(['console.command' => []]);
+ ->addTag('console.command');
addTag() is compatible with both 8.0.x and 8.1.x.
Remaining tasks
See if this is still needed after #3606744: Invokable commands dont show their usages in help and #3588847: Update to Symfony 8.1
User interface changes
Introduced terminology
API changes
Data model changes
Release notes snippet
Issue fork drupal-3607060
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:
- 3607060-console-8.1
changes, plain diff MR !16165
Comments
Comment #3
mstrelan commentedComment #4
mstrelan commentedRun #1 with Symfony 8.1: https://git.drupalcode.org/project/drupal/-/pipelines/869562
This has 47 test failures. Cspell also fails due to the generated composer.json, opened #3607061: Don't spellcheck composer.json
Run #2 with Symfony 8.1 and the fix: https://git.drupalcode.org/project/drupal/-/pipelines/869568/
No test failures, lots of deprecations. Not in scope here.
Run #3 with just the fix: https://git.drupalcode.org/project/drupal/-/pipelines/869575
All green
Comment #5
moshe weitzman commentedI'm not sure if this is a dupe of the recently submitted #3606744: Invokable commands dont show their usages in help
Comment #6
mstrelan commentedI don't know either, but the diff here is much smaller. I guess that issue should be tested against symfony/console 8.1.
Comment #7
smustgrave commented@moshe and @mstrelan based on the comments should this be closed a duplicate and credit moved over to #3606744: Invokable commands dont show their usages in help ?
Comment #8
mstrelan commentedThat other issue needs to be verified against symfony 8.1. If tests pass then we don't need this.
Comment #9
moshe weitzman commentedThat testing will happen naturally in #3588847: Update to Symfony 8.1. So you can close this IMO.
Comment #10
smustgrave commentedNot comfortable making that call so will let @mstrealn do it. Am saving credit for the work done here.
Comment #11
mstrelan commentedMy problem is if I use https://github.com/amateescu/ddev-drupal-dev and run
ddev composer updateI getsymfony/console:8.1and things are broken now.Comment #12
mstrelan commentedOK I read the docs and have set
"pin-core-lock": trueso now I don't get console 8.1. Marking as postponed just in case and updated remaining tasks with the two relevant tickets.Comment #13
longwaveThis change was needed over in #3588847: Update to Symfony 8.1, I merged in this MR and credited @mstrelan for the fix. This can probably be closed now?
Comment #14
moshe weitzman commentedYes.