- Notifications
You must be signed in to change notification settings - Fork 545
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.
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
Conversation
| $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.'); |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
| Please rebase since the code moved. |
| I did one more thing regarding this: 1d4ede1 |
| I closed this PR because I don't think we need to improve this anymore. |
| Agreed, the new message shown since 1d4ede1 makes it much more clear which options is the right one for ones use case. |
I have seen several developers getting confused by this message thinking that --xdebug will result in better performance.
Since it can be read as:
(missing the meaning of the period and surrounding text)
Having xdebug and phpstan installed is common for developers :)