[2.5][FrameworkBundle] Fix server run in case the router script does not exist #12489
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I'm currently checking what's happening on HHVM (I'm total noob on this platform) |
954a368 to
b964292
Compare
|
HHVM does not support such option ( |
b964292 to
1a79859
Compare
| @@ -137,11 +147,18 @@ private function createPhpProcessBuilder(InputInterface $input, OutputInterface | |||
| if (!file_exists($router)) { | |||
| $output->writeln(sprintf('<error>The given router script "%s" does not exist</error>', $router)); | |||
|
|
|||
| return 1; | |||
| return; | |||
There was a problem hiding this comment.
In current implementation, if createPhpProcessBuilder fails, it returns 1. The line 102 expects a ProcessBuilder :
$builder = $this->createPhpProcessBuilder($input, $output, $env);
I prefer to return null in case no builder is created. I suppose the return 1 is a wrong copy/paste when this code was inside the Command::execute method and where it was valid
|
Thank you @romainneutron. |
fabpot
added a commit
that referenced
this pull request
Nov 16, 2014
…cript does not exist (romainneutron) This PR was merged into the 2.5 branch. Discussion ---------- [2.5][FrameworkBundle] Fix server run in case the router script does not exist | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT I've also added the use of `Process\PhpExecutableFinder` Commits ------- 1a79859 [FrameworkBundle] Fix server run in case the router script does not exist
| $output->writeln('<error>This command is not supported on HHVM.</error>'); | ||
|
|
||
| return 1; | ||
| } |
There was a problem hiding this comment.
this is useless, given that the command is disabled entirely on HHVM, same than on PHP 5.3 (see line 33)
xabbuh
added a commit
to xabbuh/symfony
that referenced
this pull request
Nov 23, 2014
xabbuh
added a commit
to xabbuh/symfony
that referenced
this pull request
Nov 23, 2014
xabbuh
added a commit
to xabbuh/symfony
that referenced
this pull request
Nov 23, 2014
It is already checked in the `isEnabled()` method of the parent `ServerCommand` class if the current PHP build is HHVM and the `server:start` command is never enabled then. Thus, it's not needed to check for HHVM on every command execution. This was pointed out by @stof in symfony#12489 for the `server:run` command.
fabpot
added a commit
that referenced
this pull request
Nov 28, 2014
This PR was merged into the 2.3 branch. Discussion ---------- [FrameworkBundle] backport #12489 | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | Commits ------- 556eff1 backport #12489
fabpot
added a commit
that referenced
this pull request
Nov 28, 2014
* 2.3: [Debug] fix error message on double exception Fix initialized() with aliased services Rename Symfony2 to Symfony backport #12489 Conflicts: CONTRIBUTING.md src/Symfony/Bundle/FrameworkBundle/Command/ServerRunCommand.php src/Symfony/Bundle/TwigBundle/Loader/FilesystemLoader.php
fabpot
added a commit
that referenced
this pull request
Nov 28, 2014
* 2.5: [Debug] fix error message on double exception Fix initialized() with aliased services Rename Symfony2 to Symfony compare version using PHP_VERSION_ID backport #12489 remove an unneeded check
fabpot
added a commit
that referenced
this pull request
Nov 28, 2014
* 2.6: (36 commits) [Debug] fix error message on double exception [Validator] make DateTime objects represented as strings in the violation message. [RFC] [DebugBundle] [HttpKernel] Avoid using container as dependency for DumpListener Upgrade information for the Translation component regarding the new LoggingTranslator class. [WebProfilerBundle] Remove usage of app.request in search bar template Fix initialized() with aliased services fix data type in docblock Rename Symfony2 to Symfony bumped Symfony version to 2.6.0 updated VERSION for 2.6.0-BETA2 updated CHANGELOG for 2.6.0-BETA2 [Debug] fix ENT_SUBSTITUTE usage compare version using PHP_VERSION_ID backport #12489 remove an unneeded check Remove block submit_widget reformat code as suggested by @fabpot Fix typo Make `\Request::get` more performant. properly set request attributes in controller test ...
fabpot
added a commit
that referenced
this pull request
Nov 28, 2014
* 2.7: (36 commits) [Debug] fix error message on double exception [Validator] make DateTime objects represented as strings in the violation message. [RFC] [DebugBundle] [HttpKernel] Avoid using container as dependency for DumpListener Upgrade information for the Translation component regarding the new LoggingTranslator class. [WebProfilerBundle] Remove usage of app.request in search bar template Fix initialized() with aliased services fix data type in docblock Rename Symfony2 to Symfony bumped Symfony version to 2.6.0 updated VERSION for 2.6.0-BETA2 updated CHANGELOG for 2.6.0-BETA2 [Debug] fix ENT_SUBSTITUTE usage compare version using PHP_VERSION_ID backport #12489 remove an unneeded check Remove block submit_widget reformat code as suggested by @fabpot Fix typo Make `\Request::get` more performant. properly set request attributes in controller test ...
fabpot
added a commit
that referenced
this pull request
Nov 29, 2014
This PR was merged into the 2.6 branch. Discussion ---------- [FrameworkBundle] removed unneeded check | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | It is already checked in the `isEnabled()` method of the parent `ServerCommand` class if the current PHP build is HHVM and the `server:start` command is never enabled then. Thus, it's not needed to check for HHVM on every command execution. This was pointed out by @stof in #12489 for the `server:run` command. Commits ------- ce2adfa removed unneeded check
ostrolucky
pushed a commit
to ostrolucky/symfony
that referenced
this pull request
Mar 25, 2018
* 2.3: [Debug] fix error message on double exception Fix initialized() with aliased services Rename Symfony2 to Symfony backport symfony#12489 Conflicts: CONTRIBUTING.md src/Symfony/Bundle/FrameworkBundle/Command/ServerRunCommand.php src/Symfony/Bundle/TwigBundle/Loader/FilesystemLoader.php
ostrolucky
pushed a commit
to ostrolucky/symfony
that referenced
this pull request
Mar 25, 2018
* 2.5: [Debug] fix error message on double exception Fix initialized() with aliased services Rename Symfony2 to Symfony compare version using PHP_VERSION_ID backport symfony#12489 remove an unneeded check
ostrolucky
pushed a commit
to ostrolucky/symfony
that referenced
this pull request
Mar 25, 2018
* 2.6: (36 commits) [Debug] fix error message on double exception [Validator] make DateTime objects represented as strings in the violation message. [RFC] [DebugBundle] [HttpKernel] Avoid using container as dependency for DumpListener Upgrade information for the Translation component regarding the new LoggingTranslator class. [WebProfilerBundle] Remove usage of app.request in search bar template Fix initialized() with aliased services fix data type in docblock Rename Symfony2 to Symfony bumped Symfony version to 2.6.0 updated VERSION for 2.6.0-BETA2 updated CHANGELOG for 2.6.0-BETA2 [Debug] fix ENT_SUBSTITUTE usage compare version using PHP_VERSION_ID backport symfony#12489 remove an unneeded check Remove block submit_widget reformat code as suggested by @fabpot Fix typo Make `\Request::get` more performant. properly set request attributes in controller test ...
ostrolucky
pushed a commit
to ostrolucky/symfony
that referenced
this pull request
Mar 25, 2018
* 2.7: (36 commits) [Debug] fix error message on double exception [Validator] make DateTime objects represented as strings in the violation message. [RFC] [DebugBundle] [HttpKernel] Avoid using container as dependency for DumpListener Upgrade information for the Translation component regarding the new LoggingTranslator class. [WebProfilerBundle] Remove usage of app.request in search bar template Fix initialized() with aliased services fix data type in docblock Rename Symfony2 to Symfony bumped Symfony version to 2.6.0 updated VERSION for 2.6.0-BETA2 updated CHANGELOG for 2.6.0-BETA2 [Debug] fix ENT_SUBSTITUTE usage compare version using PHP_VERSION_ID backport symfony#12489 remove an unneeded check Remove block submit_widget reformat code as suggested by @fabpot Fix typo Make `\Request::get` more performant. properly set request attributes in controller test ...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I've also added the use of
Process\PhpExecutableFinder