Skip to content
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

[Process] Fix PhpProcess with phpdbg runtime #16574

Merged
merged 1 commit into from Nov 18, 2015

Conversation

Projects
None yet
4 participants
@nicolas-grekas
Copy link
Member

commented Nov 17, 2015

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets -
License MIT
Doc PR -

With this PR, I'm able to successfully run the test suite of the Process component using
phpdbg -qrr ./phpunit src/Symfony/Component/Process/

$this->assertFalse($process->hasBeenSignaled());
}
public function testProcessWithoutTermSignalIsNotSignaled()

This comment has been minimized.

Copy link
@stof

stof Nov 17, 2015

Member

why are you removing these tests ?

This comment has been minimized.

Copy link
@nicolas-grekas

nicolas-grekas Nov 17, 2015

Author Member

they are exact duplicates if I did not make any mistake. And I moved the only assertion in the below test.

This comment has been minimized.

Copy link
@stof

stof Nov 17, 2015

Member

they are not, because of overwrites changing the behavior of some tests based on sigchild being enabled (see child classes). hasBeenSignaled and getTermSignal need to be tested separately to handle these cases properly.

This comment has been minimized.

Copy link
@nicolas-grekas

nicolas-grekas Nov 17, 2015

Author Member

ok, reverted, thanks for checking

@nicolas-grekas nicolas-grekas force-pushed the nicolas-grekas:phpdbg branch 2 times, most recently from 107fd3e to 7a97093 Nov 17, 2015

@@ -49,7 +52,6 @@ if (!file_exists("$PHPUNIT_DIR/phpunit-$PHPUNIT_VERSION/phpunit") || md5_file(__
$zip->close();
chdir("phpunit-$PHPUNIT_VERSION");
passthru("$COMPOSER remove --no-update symfony/yaml");
passthru("$COMPOSER require --no-update phpunit/phpunit-mock-objects \"<=3.0.0\"");

This comment has been minimized.

Copy link
@nicolas-grekas

nicolas-grekas Nov 18, 2015

Author Member

phpunit-mock-objects 3.0.4 fixes the issue we had

@@ -242,7 +242,7 @@ public function start($callback = null)
if (!$this->useFileHandles && $this->enhanceSigchildCompatibility && $this->isSigchildEnabled()) {
// Workaround for the bug, when PTS functionality is enabled.
// @see : https://bugs.php.net/69442
$ptsWorkaround = fopen('php://fd/0', 'r');
$ptsWorkaround = fopen(__FILE__, 'r');

This comment has been minimized.

Copy link
@nicolas-grekas

nicolas-grekas Nov 18, 2015

Author Member

php://fd/0 is not always working, __FILE__ is just fine

@nicolas-grekas nicolas-grekas force-pushed the nicolas-grekas:phpdbg branch from 7a97093 to 9669238 Nov 18, 2015

@nicolas-grekas nicolas-grekas merged commit 9669238 into symfony:2.3 Nov 18, 2015

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
fabbot.io Your code looks good.
Details

nicolas-grekas added a commit that referenced this pull request Nov 18, 2015

bug #16574 [Process] Fix PhpProcess with phpdbg runtime (nicolas-grekas)
This PR was merged into the 2.3 branch.

Discussion
----------

[Process] Fix PhpProcess with phpdbg runtime

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

With this PR, I'm able to successfully run the test suite of the Process component using
`phpdbg -qrr ./phpunit src/Symfony/Component/Process/`

Commits
-------

9669238 [Process] Fix PhpProcess with phpdbg runtime

@nicolas-grekas nicolas-grekas deleted the nicolas-grekas:phpdbg branch Nov 18, 2015

@jaytaph

This comment has been minimized.

Copy link
Contributor

commented on src/Symfony/Component/Process/PhpExecutableFinder.php in 9669238 Nov 19, 2015

This is causing issues (and seems a BC break). The returned binary in my case is '/opt/remi/php56/root/usr/bin/php'. This will cause the process not to find the binary.

sensiolabs/SensioGeneratorBundle#430 (comment)

This comment has been minimized.

Copy link
Member Author

replied Nov 19, 2015

Can you please try the patch in PR #16599?

This was referenced Nov 23, 2015

This was referenced Nov 30, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.