Skip to content

[Console] Restore SHELL_VERBOSITY after a command is ran #61033

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.

Sign up for GitHub

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

Jump to bottom
Merged
fabpot merged 1 commit into from
Aug 23, 2025
Merged

[Console] Restore SHELL_VERBOSITY after a command is ran #61033

fabpot merged 1 commit into from
Aug 23, 2025

Conversation

@lyrixx
Copy link
Member

@lyrixx lyrixx commented Jul 4, 2025 •
edited by OskarStark
Loading

Q A
Branch? 7.2
Bug fix? yes
New feature? no
Deprecations? no
Issues
License MIT

Considereing the following code:

require __DIR__.'/vendor/autoload.php';

use Symfony\Component\Console\Application;
use Symfony\Component\Console\Attribute\AsCommand;
use Symfony\Component\Console\Input\ArgvInput;
use Symfony\Component\Console\Output\OutputInterface;

#[AsCommand('a')]
class A
{
    public function __invoke(OutputInterface $output)
    {
        $output->writeln('Command A executed');

        return 0;
    }
}

#[AsCommand('b')]
class B
{
    public function __invoke(OutputInterface $output)
    {
        $output->writeln('Command B executed');

        return 0;
    }
}

#[AsCommand('main')]
class Main
{
    public function __construct(
        private Application $app
    ) {}

    public function __invoke(OutputInterface $output)
    {
        $this->app->setAutoExit(false);

        $this->app->run(new ArgvInput([__FILE__, 'a', '--quiet']));

        $this->app->run(new ArgvInput([__FILE__, 'b']));

        $output->writeln('Main command executed');

        return 0;
    }
}

$app = new Application();
$app->addCommand(new A());
$app->addCommand(new B());
$app->addCommand(new Main($app));

$app->run();

Without this patch,

  • the output of B command is not displayed. It should be!
  • But the output Main command executed is. It should be, it's correct. But it's hard to understand. the current $output has already been configured, and it's not silenced by --quiet yet

@lyrixx lyrixx requested a review from chalasr as a code owner July 4, 2025 08:54
@carsonbot carsonbot added this to the 7.2 milestone Jul 4, 2025
@OskarStark OskarStark changed the title [Console] Restore SHELL_VERBOSITY after a command is ran [Console] Restore SHELL_VERBOSITY after a command is ran Jul 4, 2025
welcoMattic
welcoMattic reviewed Jul 4, 2025
View reviewed changes
@lyrixx lyrixx force-pushed the shell-verbosity branch from 236c06a to 1d0837c Compare July 4, 2025 15:42
GromNaN
GromNaN requested changes Jul 4, 2025
View reviewed changes
@lyrixx lyrixx force-pushed the shell-verbosity branch from 1d0837c to 570e67d Compare July 11, 2025 09:35
@lyrixx lyrixx requested a review from GromNaN July 25, 2025 10:30
@lyrixx lyrixx requested a review from nicolas-grekas July 25, 2025 10:30
@lyrixx
Copy link
Member Author

lyrixx commented Aug 6, 2025

ping @GromNaN & @chalasr

@chalasr
Copy link
Member

chalasr commented Aug 6, 2025

Looks good. Can you think of a test case that would help avoiding regressions?

@lyrixx lyrixx changed the base branch from 7.2 to 7.3 August 6, 2025 16:14
@lyrixx
Copy link
Member Author

lyrixx commented Aug 6, 2025

I added tests + rebased + targeted branch 7.3

@chalasr
Copy link
Member

chalasr commented Aug 6, 2025

👍 push needed to re-trigger the pipeline

@lyrixx lyrixx force-pushed the shell-verbosity branch 4 times, most recently from d53e95b to 3531989 Compare August 7, 2025 08:48
@lyrixx
Copy link
Member Author

lyrixx commented Aug 7, 2025

All green 🎉 Thanks for the review

@xabbuh xabbuh modified the milestones: 7.2, 7.3 Aug 14, 2025
@fabpot
Copy link
Member

fabpot commented Aug 23, 2025

Thank you @lyrixx.

@fabpot fabpot merged commit 7c54399 into symfony:7.3 Aug 23, 2025
11 checks passed
@lyrixx lyrixx deleted the shell-verbosity branch August 25, 2025 08:32
@fabpot fabpot mentioned this pull request Aug 29, 2025
Merged
@lyrixx lyrixx mentioned this pull request Sep 2, 2025
Merged
@ttskch ttskch mentioned this pull request Oct 14, 2025
Merged
nicolas-grekas added a commit that referenced this pull request Oct 14, 2025
@nicolas-grekas
bug #62058 [Console] Ensure SHELL_VERBOSITY is always restored prop…
a63f93a 
…erly (ttskch)

This PR was merged into the 7.3 branch.

Discussion
----------

[Console] Ensure `SHELL_VERBOSITY` is always restored properly

| Q             | A
| ------------- | ---
| Branch?       | 7.3
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | Related to #61033
| License       | MIT

There was a bug where if only `$_SERVER['SHELL_VERBOSITY']` or `$_ENV['SHELL_VERBOSITY']` had a value, they would be erroneously unset after command execution.

For example, in [PHPUnit's default configuration](https://github.com/symfony/recipes/blob/main/phpunit/phpunit/11.1/phpunit.dist.xml#L17), `SHELL_VERBOSITY` is set only to `$_SERVER['SHELL_VERBOSITY']`.

This caused the `SHELL_VERBOSITY` setting to be invalid after [a console command was executed internally](https://github.com/zenstruck/foundry/blob/v2.7.5/src/Test/ResetDatabase.php) using the `ResetDatabase` trait of [zenstruck/foundry](https://github.com/zenstruck/foundry).

Commits
-------

8da524e [Console] ensure `SHELL_VERBOSITY` is always restored properly
@JaZo JaZo mentioned this pull request Oct 20, 2025
Merged
9 tasks
JaZo added a commit to swisnl/laravel-encrypted-data that referenced this pull request Oct 20, 2025
@JaZo
build: set minimum symfony/console version
47ae1e0 
Caused by: symfony/symfony#61033
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fabpot fabpot fabpot approved these changes

@GromNaN GromNaN GromNaN approved these changes

@welcoMattic welcoMattic welcoMattic approved these changes

@chalasr chalasr chalasr approved these changes

@nicolas-grekas nicolas-grekas Awaiting requested review from nicolas-grekas

@xabbuh xabbuh Awaiting requested review from xabbuh

@yceruto yceruto Awaiting requested review from yceruto

@kbond kbond Awaiting requested review from kbond

@dunglas dunglas Awaiting requested review from dunglas

@OskarStark OskarStark Awaiting requested review from OskarStark

@jderusse jderusse Awaiting requested review from jderusse

Assignees

No one assigned

Labels

Bug Console Status: Reviewed

Projects

None yet

Milestone

7.3

Development

Successfully merging this pull request may close these issues.

7 participants

@lyrixx @chalasr @fabpot @GromNaN @welcoMattic @xabbuh @carsonbot