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

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

Open
lucasnetau opened this issue Jul 9, 2021 · 18 comments · May be fixed by WyriHaximus/php-composer-update-bin-autoload-path#36

Comments

@lucasnetau
Copy link
Contributor

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
Copy link
Owner

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 WyriHaximus added the bug label Jul 9, 2021
WyriHaximus added a commit to WyriHaximus/php-composer-update-bin-autoload-path that referenced this issue Jul 10, 2021
When this package was initially designed it didn't take into account situations where files aren't
moved. But are mounted into a running Docker container for development. By switching to relative
paths both situations, and most logical others are supported.

Resolves: WyriHaximus/reactphp-child-process-messenger#72 (reported by @
lucasnetau)
WyriHaximus added a commit to WyriHaximus/php-composer-update-bin-autoload-path that referenced this issue Jul 10, 2021
When this package was initially designed it didn't take into account situations where files aren't
moved. But are mounted into a running Docker container for development. By switching to relative
paths both situations, and most logical others are supported.

Resolves: WyriHaximus/reactphp-child-process-messenger#72 (reported by @lucasnetau)
@WyriHaximus
Copy link
Owner

PR with a fix is up at WyriHaximus/php-composer-update-bin-autoload-path#36

WyriHaximus added a commit to WyriHaximus/php-composer-update-bin-autoload-path that referenced this issue Jul 10, 2021
When this package was initially designed it didn't take into account situations where files aren't
moved. But are mounted into a running Docker container for development. By switching to relative
paths both situations, and most logical others are supported.

Resolves: WyriHaximus/reactphp-child-process-messenger#72 (reported by @lucasnetau)
@lucasnetau
Copy link
Contributor Author

Will the PR with the fix be merged anytime soon?

@WyriHaximus
Copy link
Owner

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
Copy link
Owner

@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
Copy link
Contributor Author

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
Copy link
Owner

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
Copy link
Contributor Author

lucasnetau commented Sep 3, 2021

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
Copy link
Owner

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
Copy link

voku commented Nov 29, 2021

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
Copy link
Owner

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

@voku
Copy link

voku commented Nov 30, 2021

@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
Copy link
Owner

@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 😂 . Please have a go with dev-4.x-shouldn't-require-vendor.php-but-autoload.php, that should fix it: #78

@voku
Copy link

voku commented Dec 17, 2021

@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
Copy link
Owner

@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
Copy link

voku commented Dec 17, 2021

@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
Copy link
Owner

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

@WyriHaximus
Copy link
Owner

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.

@settings settings bot removed the bug label Jul 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants