laravel / octane

Supercharge your Laravel application's performance.
https://laravel.com/docs/octane
MIT License
3.78k stars 296 forks source link

Require pcntl in composer.json #796

Closed gisostallenberg closed 10 months ago

gisostallenberg commented 10 months ago

Octane Version

v2.2.6

Laravel Version

v10.31.0

PHP Version

8.1.26

What server type are you using?

FrankenPHP

Server Version

1.0.2

Database Driver & Version

No response

Description

Without pcntl you'd get

  Undefined constant "SIGINT"

  at vendor/laravel/octane/src/Commands/Concerns/InteractsWithServers.php:152
    148▕      * Returns the list of signals to subscribe.
    149▕      */
    150▕     public function getSubscribedSignals(): array
    151▕     {
  ➜ 152▕         return [\SIGINT, \SIGTERM];
    153▕     }
    154▕ 

I think octane should require ext-pcntl:*.

Steps To Reproduce

Install octane in an environment where the pcntl extension is not installed and run php artisan octane:start

crynobone commented 10 months ago

Can you make a PR to add this? laravel/horizon also has the same requirements so it should be fine to add this.

nunomaduro commented 10 months ago

@crynobone I would rather see this new extension requirement being included for Octane v3 or something, so existing customers, you are developing Octane on windows for example can still leverage updates on 2.x.

Can you pull request it to master? Thanks! https://github.com/laravel/octane/tree/master

kohenkatz commented 10 months ago

When this is implemented on master, maybe we should document this simple workaround for users on Windows, which I already use to work on projects that have Horizon in them. Set COMPOSER_IGNORE_PLATFORM_REQ=ext-pcntl,ext-posix, and then composer works (everything in the application works except actually running horizon, but that's not needed for development since queues can be run in other ways).