More robust vendor directory detection #2153
Conversation
| } | ||
|
|
||
| $vendorDir = realpath($vendorDir); |
There was a problem hiding this comment.
The use of realpath may interfere with running phpDocumentor's PHAR distribution. Realpath will break out of a PHAR file and consult the host filesystem. This is the reason why the original code did not use realpath to clean up the path :)
There was a problem hiding this comment.
I didn't know that, thanks. I just added realpath to ease the debug with more readability.
Thanks a lot
There was a problem hiding this comment.
No problem, I know this because I did the exact same thing when I originally built this class ;)
There was a problem hiding this comment.
I see you rebased the PR but the realpath is still there; do you still have to make the change or did something get lost in the rebase?
There was a problem hiding this comment.
I just wanted to see the failing tests with realpath rebased on master (because your approach for integration tests interests me) but Behat's tests are passing loacally (80 scenarios (80 passed) 283 steps (283 passed)).
I'm surprised they are not run automatically with this PR and I get a warning/error in the Github's actions of my commit:
- https://github.com/jclaveau/phpDocumentor/runs/366043339
- https://github.com/jclaveau/phpDocumentor/runs/366043344
So I wonder if
- Behat's is wright locally but wrong in Github's actions?
- The opposit.
- The tests you speak about are still not merged in master?
There was a problem hiding this comment.
The failing build you are seeing is using the phar which has been built in a step before we execute behat. As @mvriel already said your changes are breaking the phar and not the plain build which is also passing in your linked build.
Does that make it more clear what is going on?
We are still searching for a solution to build the pr on this project as well. So we can see the results in the pr. Main issue is that we don't want to build on pr and push but only one of them
There was a problem hiding this comment.
Thanks for the explanations. I removed the realpath() call
| * @param string $url | ||
| * @param string|bool The cleaned url or false if it doesn't exist | ||
| */ | ||
| function realpath(string $url) |
There was a problem hiding this comment.
This is unnecessary since realpath should not be used; the vfs filesystem has the same issues as the phar filesystem; with realpath you break out of virtual filesystem
|
Can you rebase master into this PR? Master contains a more robust build setup that will also test things like the PHAR distribution |
|
Thanks! |
|
Thanks for the review and all your work! |
This PR detects the vendor directory based on the presence of autoload.php. This allows the use of COMPOSER_VENDOR_DIR environment variable of Composer when installed as dependency