Skip to content

Clarify --xdebug warning message #2298

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

Closed
wants to merge 1 commit into from
Closed

Clarify --xdebug warning message #2298

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Command/CommandHelper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ public static function begin(
if ($allowXdebug && !XdebugHandler::isXdebugActive()) {
$errorOutput->getStyle()->note('You are running with "--xdebug" enabled, but the Xdebug PHP extension is not active. The process will not halt at breakpoints.');
} elseif (!$allowXdebug && XdebugHandler::isXdebugActive()) {
$errorOutput->getStyle()->note('The Xdebug PHP extension is active, but "--xdebug" is not used. This may slow down performance and the process will not halt at breakpoints.');
$errorOutput->getStyle()->note('The Xdebug PHP extension is active, but "--xdebug" is not used. Using "--xdebug" may slow down performance, but will allow you to debug PHPStan using breakpoints.');
Copy link
Contributor

@staabm staabm Mar 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the point of this message is, that php processes may slow down because xdebug beeing loaded.

phpstan doesn't get slowser because of --xdebug
Copy link
Contributor Author

@AJenbo AJenbo Mar 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't xdebug handler prevent that by disabling the extension?

My understanding of --xdebug is that it prevents xdebug handler from doing so.

The current phrasing makes it sound like --xdebug will improve performance which I don't think is the case either.
Copy link
Contributor

@staabm staabm Mar 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When xdebug is loaded things get a lot slower - e.g. when running make tests.

If you don't run with --xdebug you get a slow process without any benefits - hence the warning.
Copy link
Contributor Author

@AJenbo AJenbo Mar 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The whole point of XdebugHandler is to restart the process without Xdebug exactly to avoid this slow down, even if it is installed. By testing for XdebugHandler::isXdebugActive() before $xdebug->check(); this message is being shown when it's not relevant.

Here are some numbers from a fairly large project.
php phpstan: 117 sec
php+xdebug phpstan: 110 sec
php+xdebug phpstan --xdebug: 264 sec

Maybe you had PHPSTAN_ALLOW_XDEBUG=1 set in your enviroment when testing this so that Xdebug was left active? This would cause Xdebug to still be loaded and result in the behavior you describe (https://github.com/composer/xdebug-handler#basic-usage).

Edit: see #2299 for a fix for that issue.
Copy link
Contributor Author

@AJenbo AJenbo Mar 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see the confusing nature of the message has been pointed out by others as well: #1878 (comment), issue/WI-69996

How about this phrasing instead?

The Xdebug PHP extension is active, but "--xdebug" is not used. Consider disabling Xdebug when not debugging PHPStan, to avoid performance issues.

I took out and the process will not halt at breakpoints to keep the message relatively short as users are unlikely to have set any breakpoints if they where unaware that Xdebug is loaded.

If neither of these versions seem suitable, could you please help me rephrase it? It sounds like the message might be related to how make tests works internally, something I'm not familiar with.
The warning is causing confusion for end-users in its current form. I have noticed that some people are setting up their CI to run slower due to misunderstanding the message and thinking that using --xdebug will actually improve performance when Xdebug is active. Your help with resolving this issue would be greatly appreciated. Thank you
}

if (!$allowXdebug) {
Expand Down