Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Console] Reset question validator attempts only for actual stdin (bis)#37286
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| if (!\defined('STDIN')) { | ||
| $inputStream =$this->inputStream ?:fopen('php://stdin','r'); | ||
| if ('php://stdin' !== (stream_get_meta_data($inputStream)['url'] ??null)) { |
nicolas-grekasJun 15, 2020 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
actually this is not needed to fix the failure, but making the behavior of this method global looks suspicious to me so here is a local check
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.
note that this check works even if$inputStream === STDIN
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.
Are you sure withurl here?
>>> stream_get_meta_data(STDIN)['url']PHP Notice: Undefined index: url in phar://eval()'d code on line 1=> null>>> stream_get_meta_data(STDIN)=> [ "timed_out" => false, "blocked" => true, "eof" => false, "wrapper_type" => "PHP", "stream_type" => "STDIO", "mode" => "rb", "unread_bytes" => 0, "seekable" => false, "uri" => "php://stdin", ]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.
Pretty much yes:https://3v4l.org/njZZV
What's your runtime?
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.
That 3v4l snippet follows behaviour I observe on my PC, it looks to beuri, noturl
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.
🤦
| if (\function_exists('stream_isatty')) { | ||
| returnstream_isatty(fopen('php://input','r')); | ||
| returnstream_isatty($inputStream); |
nicolas-grekasJun 15, 2020 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
this was my mistake: usingphp://stdin here instead is enough to fix the failure
… (weaverryan)This PR was squashed before being merged into the 1.0-dev branch.Discussion----------using stable-dev in tests to get latest fixes from SymfonyThis avoids a behavior change insymfony/console@6f533d9...d2c9f77 fromsymfony/symfony#37286I'll create a PR to revert this after merge, which we will merge once the issue is resolved.Commits-------1309c01 updating test in one other spotccfd248 forcing stable ORM in tests6b512b8 making migration test work in all versionsb5a5a11 phpcs6af124a Guaranteeing that dev versions will get dev dependenciesfd89d5d Forcing "stable" for 7.173f9ecc updating test for new version of migrations0334718 Using stable-dev as default version in tests
… (weaverryan)This PR was squashed before being merged into the 1.0-dev branch.Discussion----------using stable-dev in tests to get latest fixes from SymfonyThis avoids a behavior change insymfony/console@6f533d9...d2c9f77 fromsymfony/symfony#37286I'll create a PR to revert this after merge, which we will merge once the issue is resolved.Commits-------1309c01 updating test in one other spotccfd248 forcing stable ORM in tests6b512b8 making migration test work in all versionsb5a5a11 phpcs6af124a Guaranteeing that dev versions will get dev dependenciesfd89d5d Forcing "stable" for 7.173f9ecc updating test for new version of migrations0334718 Using stable-dev as default version in tests
#37160fails, this should fix it by looking at the actual input stream.