-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Console] HHVM read input stream bug #17253
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
Can you add a test to avoid regressions? |
@@ -437,7 +437,7 @@ private function getShell() | ||
*/ | ||
private function readFromInput($stream) | ||
{ | ||
if (STDIN === $stream && function_exists('readline')) { | ||
if (STDIN === $stream && function_exists('readline') && !defined('HHVM_VERSION')) { | ||
$ret = readline(); |
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.
giving the empty string looks more feature-full, isn't it? why prefer fgets on hhvm when readline works?
@nicolas-grekas i think you are right. (i have changed it) |
Please also report it to HHVM as an incompatibility bug. |
@stof done see facebook/hhvm#6731 |
👍 |
Thank you @mbutkereit. |
This PR was squashed before being merged into the 2.8 branch (closes #17253). Discussion ---------- [Console] HHVM read input stream bug | Q | A | ------------- | --- | Bug fix? | [yes] | New feature? | [no] | BC breaks? | [no] | Deprecations? | [no] | Tests pass? | [yes] | License | MIT HHVM readline() function requires a parameter see: https://github.com/facebook/hhvm/blob/master/hphp/runtime/ext/readline/ext_readline.php But the php readline does not require one. ``` In Symfony\Component\Console\Helper\QuestionHelper … readFromInput($stream) … ``` we use readline() without a parameter. HHVM Version: ``` HipHop VM 3.10.1 (rel) Compiler: tags/HHVM-3.10.1-0-g689b4969a141620ee5a282ce0dbf72278c84d44b Repo schema: 6c99ee1f98340f6f3ef397a332583f0e843a627d ``` Docker Container: ``` docker run --rm -it -v `pwd`:`pwd` -w `pwd` brunoric/hhvm:deb hhvm test.php ls ``` Test.php ``` <?php use Symfony\Component\Console\Application; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Question\Question; use Symfony\Component\Console\Helper\QuestionHelper; require_once "vendor/autoload.php"; $console = new Application(); $console ->register('ls') ->setDescription('Try input') ->setCode(function (InputInterface $input, OutputInterface $output) { $question = new Question('Your email address: '); $question->setValidator( function ($answer) { return $answer; } ); $question->setMaxAttempts(5); $helper = new QuestionHelper(); $email = $helper->ask($input, $output, $question); $output->writeln(sprintf('Email Adress: <info>%s</info>', $email)); }) ; $console->run(); ``` composer.json ``` { "require": { "symfony/console": "^3.0" } } ``` Result: No input is possible and you got a warning ``` Your email address: Warning: readline() expects exactly 1 parameter, 0 given in /clitest/vendor/symfony/console/Helper/QuestionHelper.php on line 436 Email Adress: ``` Fix: Add a empty string parameter to readline() or use fgets() for hhvm. Commits ------- e7f17a7 [Console] HHVM read input stream bug
HHVM readline() function requires a parameter see:
https://github.com/facebook/hhvm/blob/master/hphp/runtime/ext/readline/ext_readline.php
But the php readline does not require one.
we use readline() without a parameter.
HHVM Version:
Docker Container:
Test.php
composer.json
Result:
No input is possible and you got a warning
Fix:
Add a empty string parameter to readline() or use fgets() for hhvm.