Add phar_safe_path() function.#4024
Conversation
This function allows to retrieve a path by making sure it is adapted when used within a Phar archive.
php/utils.php
Outdated
| } | ||
|
|
||
| return str_replace( | ||
| PHAR_STREAM_PREFIX . getcwd() . '/', |
There was a problem hiding this comment.
What about my comment from yesterday?
danielbachhuber [1:11 PM] I wonder if we should set a `WP_CLI_PHAR_PATH` in `php/boot-fs.php`
[1:12] PHP can change the working directory, which would make `getcwd()` incorrect
There was a problem hiding this comment.
This function is for generally moving a given path inside of the Phar structure.
The actual paths you want to use depend on the package you're in, and how the packages have been included. So, I'm not sure we can reduce this problem to 1 single constant.
There was a problem hiding this comment.
I want to run some tests now to see how the getcwd() will behave. If this works as expected in all scenarios, then it works across the board for getting rid of all the "three-different-ways-of-getting-a-path" issues.
There was a problem hiding this comment.
The actual paths you want to use depend on the package you're in, and how the packages have been included. So, I'm not sure we can reduce this problem to 1 single constant.
The packages care about the base path for the Phar file. Setting a WP_CLI_PHAR_PATH at the beginning of the Phar execution will provide this value, more consistently than getcwd() can.
If this works as expected in all scenarios, then it works across the board for getting rid of all the "three-different-ways-of-getting-a-path" issues.
What if my script calls chdir()?
There was a problem hiding this comment.
Ah, I understand. So you want to still do the replacement above, but want to catch a constant early on instead of the potentially changing getcwd()? Yes, that makes sense.
There was a problem hiding this comment.
danielbachhuber [10:15 PM]
actually, `php/boot-fs.php` would still move around. we'd want to set it when
running `utils/make-phar.php
We need to still keep this dynamic in nature, not bake it into the Phar in a compiled way. The Phar file can be moved around, so we need to catch the folder early on when running the Phar.
There was a problem hiding this comment.
I currently think that the best place for this constant would be php/wp-cli.php. This comes immediately after php/boot-fs.php / php/boot-phar.php, and I think that there has not yet been a possibility to mess with getcwd() at this point.
There was a problem hiding this comment.
No, as we only need it within a Phar, php/boot-phar.php is better.
Store the `WP_CLI_PHAR_PATH` constant early on as being `getcwd()`, for use in the `phar_safe_path()` function.
| <?php | ||
|
|
||
| // Store the path to the Phar early on for `Utils\phar-safe-path()` function. | ||
| define( 'WP_CLI_PHAR_PATH', getcwd() ); |
There was a problem hiding this comment.
The mechanism works by having something to diff against __DIR__.
__DIR__ => phar:///home/alain/test/wp-cli.phar/php
getcwd() => /home/alain/test
This gives us the bit in front of the phar name, and it even works with different phar names.
There was a problem hiding this comment.
Ah, I should add: the getcwd() produces the path to the Phar, not to a file from within the Phar.
There was a problem hiding this comment.
This gives us the bit in front of the phar name, and it even works with different phar names.
I know. I'm just afraid of getcwd() sometimes producing a different value than we expect.
This function allows to retrieve a path by making sure it is adapted when used within a Phar archive.
This is meant to solve issues like wp-cli/scaffold-command#10
Fixes #3987