search

WyriHaximus / reactphp-child-process-messenger

MIT License
31 stars 14 forks source link

/bin/child-process has absolute path to vendor/autoload.php breaking relocation of source #72

Open lucasnetau opened 3 years ago

lucasnetau commented 3 years ago

The new /bin/child-process file that is generated in v4.0 breaks deployments when mapping to seperate location due to hard-code absolute path to vendor/autoload.php

In development we install composer dependencies on the development machine and then this directory is mapped through to Docker containers through volume mapping. The absolute path to autoload.php in the Docker container is not the same as the absolute path on the development machine (different OS). This causes a Fatal error running our ReactPHP components

What is the purpose of hard-coding the path to autoload.php in a library?

WyriHaximus commented 3 years ago

The assumption, when adding that enhancement to this package, is that source isn't moved around. The thing I didn't consider at the time was volume mapping in Docker containers and the likes. The whole purpose of putting it in there hardcoded (using a composer plugin) is to be 100% sure where to expect the autoloader to be. And not have to guess by iterating over a set of possibilities each time it's started.

However that should not cause issues for you, it was done, rather lazy, with the absolute path of the autoloader as supplied by composer. I'll look at using a relative path instead, that will still ensure the same functionality, but shouldn't cause issues in your use case.

WyriHaximus commented 3 years ago

PR with a fix is up at https://github.com/WyriHaximus/php-composer-update-bin-autoload-path/pull/36

lucasnetau commented 3 years ago

Will the PR with the fix be merged anytime soon?

WyriHaximus commented 3 years ago

In its current state not. In theory, it should work fine, in practice however it doesn't. So, when I faced that issue I went back to the drawing board and got distracted by another drawing board. But there might be a simpler solution I'm going to try later this week and than hopefully we can resolve this.

WyriHaximus commented 3 years ago

@lucasnetau Can you try this for me? Instead of fixing it upstream, I've added a fallback in case the absolute path doesn't work:

composer require wyrihaximus/react-child-process-messenger:"dev-4.x-fallback-autoloader-paths as 4.1.0"

(Have to fix it in this package in the short term, but will fix it in https://github.com/WyriHaximus/php-composer-update-bin-autoload-path to do that out of the box.)

lucasnetau commented 3 years ago

Patch did not work, it's trying to load a find vendor.php under the wyrihaximus path, there is no vendor.php under /vendor in any directory.

PHP Warning:  require_once(/ie/vendor/wyrihaximus/vendor/vendor.php): Failed to open stream: No such file or directory in /ie/vendor/wyrihaximus/react-child-process-messenger/bin/child-process on line 14
PHP Fatal error:  Uncaught Error: Failed opening required '/ie/vendor/wyrihaximus/vendor/vendor.php' (include_path='.:/usr/local/lib/php') in /ie/vendor/wyrihaximus/react-child-process-messenger/bin/child-process:14
Stack trace:
#0 /ie/vendor/wyrihaximus/react-child-process-messenger/bin/child-process(15): {closure}()
#1 {main}
  thrown in /ie/vendor/wyrihaximus/react-child-process-messenger/bin/child-process on line 14

What exactly is the reason for the $binPathAutoloader and $vendorPathAutoloader guessing? What install case cause autoload.php not to be resolved at

dirname(__DIR__, 3) . DIRECTORY_SEPARATOR . 'autoload.php';

WyriHaximus commented 3 years ago

Ow, yeah doh my bad. Just pushed an update.

The use case are packages like this one that comes with a bin. Composer doesn't put the bin of such package when you're developing on the package in vendor/bin as it does with dependencies. Hence creating a package to make it absolute, but that didn't consider for your edge case, hence having to fall back to guess.

lucasnetau commented 3 years ago

No problems, I tested it an it is working for me.

Have you had a look at what some other libraries do? nesbot/carbon defines their binary in composer.json "bin": [ "bin/carbon" ] so that it is symlinked into vendor/bin and then checks three different paths for the autoload.php, symfony/var-dumper does similarly.

WyriHaximus commented 3 years ago

No problems, I tested it an it is working for me.

Cool, will be releasing it later today/tomorrow then.

Have you had a look at what some other libraries do? nesbot/carbon defines their binary in composer.json "bin": [ "bin/carbon" ] so that it is symlinked into vendor/bin and then checks three different paths for the autoload.php, symfony/var-dumper does similarly.

Thought I had it in there as well. So that's a bit weird, but I'll have a test with it soon because that would have been my expectation as well.

voku commented 2 years ago

I don't know if it's the same problem, but I see this error message: Could not open input file: phar:///home/lmoelleken/meerx/Framework/thirdparty/phpdoctor.phar/vendor/wyrihaximus/react-child-process-messenger/bin/child-process when I try to use react/filesystem in a phar file. Any idea how I can fix that on my end?

WyriHaximus commented 2 years ago

@voku Even with the latest version? IIRC I did release that change. Will have a look tonight and get back to you.

voku commented 2 years ago

@voku Even with the latest version? IIRC I did release that change. Will have a look tonight and get back to you.

I tested this version: composer.lock: 4.x-dev ⇒ "7aebab12f12a09f80a36ce6f5d34a4832a5b6fa5"

EDIT:

Now I tested "dev-master ⇉ "04369f7894b88c7e4c7883571c77e308a9510e05" (via "wyrihaximus/react-child-process-messenger": "dev-master as 4.x-dev") and I see this error:

Fatal error: Declaration of React\Filesystem\ChildProcess\Process::create(WyriHaximus\React\ChildProcess\Messenger\Messenger $messenger, React\EventLoop\LoopInterface $loop) must be compatible with WyriHaximus\React\ChildProcess\Messenger\ChildInterface::create(WyriHaximus\React\ChildProcess\Messenger\Messenger $messenger, React\EventLoop\LoopInterface $loop): void in phar:///home/lmoelleken/meerx/Framework/thirdparty/phpdoctor.phar/vendor/react/filesystem/src/ChildProcess/Process.php on line 18

WyriHaximus commented 2 years ago

@voku That's a incompatibility known incompatibility. master (v5) won't be compatible with react/filesystem as it currently is (or well the other way around).

Ok right I'm pretty sure I found it, and feel so stupid now :joy: . Please have a go with dev-4.x-shouldn't-require-vendor.php-but-autoload.php, that should fix it: https://github.com/WyriHaximus/reactphp-child-process-messenger/pull/78

voku commented 2 years ago

@voku That's a incompatibility known incompatibility. master (v5) won't be compatible with react/filesystem as it currently is (or well the other way around).

Ok right I'm pretty sure I found it, and feel so stupid now joy . Please have a go with dev-4.x-shouldn't-require-vendor.php-but-autoload.php, that should fix it: #78

This is still not working, same error message. Maybe I can fix that on my end, by some configuration for box, php-scoper, ...?

WyriHaximus commented 2 years ago

@voku This error? If so, then the autoloading works, but we have another issue on our hands.

Fatal error: Declaration of React\Filesystem\ChildProcess\Process::create(WyriHaximus\React\ChildProcess\Messenger\Messenger $messenger, React\EventLoop\LoopInterface $loop) must be compatible with WyriHaximus\React\ChildProcess\Messenger\ChildInterface::create(WyriHaximus\React\ChildProcess\Messenger\Messenger $messenger, React\EventLoop\LoopInterface $loop): void in phar:///home/lmoelleken/meerx/Framework/thirdparty/phpdoctor.phar/vendor/react/filesystem/src/ChildProcess/Process.php on line 18

voku commented 2 years ago

@voku This error? If so, then the autoloading works, but we have another issue on our hands.

Fatal error: Declaration of React\Filesystem\ChildProcess\Process::create(WyriHaximus\React\ChildProcess\Messenger\Messenger $messenger, React\EventLoop\LoopInterface $loop) must be compatible with WyriHaximus\React\ChildProcess\Messenger\ChildInterface::create(WyriHaximus\React\ChildProcess\Messenger\Messenger $messenger, React\EventLoop\LoopInterface $loop): void in phar:///home/lmoelleken/meerx/Framework/thirdparty/phpdoctor.phar/vendor/react/filesystem/src/ChildProcess/Process.php on line 18

Sorry, no it's still this error Could not open input file: phar:///home/lmoelleken/meerx/Framework/thirdparty/phpdoctor.phar/vendor/wyrihaximus/react-child-process-messenger/bin/child-process and I looked into the phar file and the file exist, so maybe I can't open the file in the phar file? Is this working with phar files at all? ⇉ https://github.com/voku/Simple-PHP-Code-Parser/blob/master/src/voku/SimplePhpParser/Parsers/PhpCodeParser.php#L328

WyriHaximus commented 2 years ago

Honestly? No clue, never build this with phar in mind. Will look into this during the holidays.

WyriHaximus commented 2 years ago

Later than I hoped, but I just started looking into this. And from what I gather now the only way is to ship this package with a Phar that can be read from your Phar, put on the host filesystem, and be executed. Deep under the hood of react/filesystem, through this package, react/child-process is used, and that uses proc_open to spawn the processes requested. Will dig deeper because composer probably does something simmular.