-
Notifications
You must be signed in to change notification settings - Fork 1k
Append uniq id when extracting file from Phar #4692
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
Conversation
php/utils.php
Outdated
| $fname = basename( $path ); | ||
|
|
||
| $tmp_path = get_temp_dir() . "wp-cli-$fname"; | ||
| $tmp_path = get_temp_dir() . "wp-cli-$fname-" . uniqid( 'wp-cli-extract-from-phar-', true ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be better to do this as
get_temp_dir() . uniqid( "wp-cli-extract-from-phar-", true ) . "-$fname";
to keep the extension the same.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about:
get_temp_dir() . 'wp-cli-' . uniqid( "wp-cli-extract-from-phar-", true ) . "-$fname";
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The wp-cli- bit though is covered by the arg to uniqid()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see:
local ➜ wordpress-develop git:(master) wp shell
wp> $fname = 'router.php';
=> string(10) "router.php"
wp> 'wp-cli-' . uniqid( "wp-cli-extract-from-phar-", true ) . "-$fname";
=> string(66) "wp-cli-wp-cli-extract-from-phar-5a8b0566008283.78459585-router.php"
|
Ok this is failing due to #4613, and this check in
with "Yes! We do!". Probably better to revert #4613 than to modify the check...? |
|
Our own break can be fixed either way. Let's discuss reverting during office hours today. |
Fixes #4676