Skip to content

Commit

Permalink
Allow PHP binary to be specified
Browse files Browse the repository at this point in the history
This change allows a binary path to be given to the Process context, which in turn can be specified when constructing a WorkerProcess. The default worker factory now looks for an environment variable or constant named AMP_PHP_BINARY and, if found, uses the value for the PHP binary. Fixes #26 and supersedes #28.
  • Loading branch information
trowski committed Nov 30, 2017
1 parent a1e62d6 commit 7c67f66
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 5 deletions.
5 changes: 3 additions & 2 deletions lib/Context/Process.php
Expand Up @@ -26,10 +26,11 @@ class Process implements Context {
/**
* @param string|array $script Path to PHP script or array with first element as path and following elements options
* to the PHP script (e.g.: ['bin/worker', '-eOptionValue', '-nOptionValue'].
* @param string $binary Path to PHP binary. Defaults to \PHP_BINARY.
* @param string $cwd Working directory.
* @param mixed[] $env Array of environment variables.
*/
public function __construct($script, string $cwd = "", array $env = []) {
public function __construct($script, string $binary = \PHP_BINARY, string $cwd = "", array $env = []) {
$options = [
"html_errors" => "0",
"display_errors" => "0",
Expand All @@ -44,7 +45,7 @@ public function __construct($script, string $cwd = "", array $env = []) {

$options = (\PHP_SAPI === "phpdbg" ? " -b -qrr " : " ") . $this->formatOptions($options);
$separator = \PHP_SAPI === "phpdbg" ? " -- " : " ";

This comment has been minimized.

Copy link
@kelunik

kelunik Nov 30, 2017

Member

These options must be moved as well, otherwise it chooses the PHPDBG options if you run via PHPDBG, but have a AMP_PHP_BINARY specified.

This comment has been minimized.

Copy link
@trowski

trowski Nov 30, 2017

Author Member

What about making the conditional check if $binary does not end with (or perhaps contain) "phpdbg"?

This comment has been minimized.

Copy link
@kelunik

kelunik Nov 30, 2017

Member

I'd probably just avoid the constructor parameter here and move the DefaultWorkerFactory code into the constructor.

$command = \escapeshellarg(\PHP_BINARY) . $options . $separator . $script;
$command = \escapeshellarg($binary) . $options . $separator . $script;

$processOptions = [];

Expand Down
7 changes: 6 additions & 1 deletion lib/Worker/DefaultWorkerFactory.php
Expand Up @@ -22,6 +22,11 @@ public function create(): Worker {
return new WorkerThread;
}

return new WorkerProcess;
$binary = \getenv("AMP_PHP_BINARY");
if (!$binary && \defined("AMP_PHP_BINARY")) {
$binary = \AMP_PHP_BINARY;
}

return new WorkerProcess($binary ?: \PHP_BINARY);
}
}
5 changes: 3 additions & 2 deletions lib/Worker/WorkerProcess.php
Expand Up @@ -9,17 +9,18 @@
*/
class WorkerProcess extends AbstractWorker {
/**
* @param string $binaryPath Path to PHP Binary. Defaults to value of \PHP_BINARY.
* @param string $envClassName Name of class implementing \Amp\Parallel\Worker\Environment to instigate.
* Defaults to \Amp\Parallel\Worker\BasicEnvironment.
* @param mixed[] $env Array of environment variables to pass to the worker. Empty array inherits from the current
* PHP process. See the $env parameter of \Amp\Process\Process::__construct().
*/
public function __construct(string $envClassName = BasicEnvironment::class, array $env = []) {
public function __construct(string $binaryPath = \PHP_BINARY, string $envClassName = BasicEnvironment::class, array $env = []) {
$dir = \dirname(__DIR__, 2) . '/bin';
$script = [
$dir . "/worker",
"-e" . $envClassName,
];
parent::__construct(new Process($script, $dir, $env));
parent::__construct(new Process($script, $binaryPath, $dir, $env));
}
}

0 comments on commit 7c67f66

Please sign in to comment.