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.

Sign up for GitHub

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

Jump to bottom

Escape shell command in wp-cli update #5818

Merged
merged 1 commit into from Aug 2, 2023
Merged

Escape shell command in wp-cli update #5818

merged 1 commit into from Aug 2, 2023

Conversation

adamperrry
Copy link
Contributor

Link to original issue.

Link to comment about existing tests.

This fix escapes a command that was causing wp cli update to fail when the $php_binary path contained a space.

@adamperrry adamperrry requested a review from a team as a code owner July 25, 2023 17:19
@swissspidy swissspidy linked an issue Jul 25, 2023 that may be closed by this pull request
Update command doesn't escape php_binary path, update fails when path has spaces #5815
Open
2 tasks
@adamperrry
fix: escape shell command in wp-cli update function
b6a8074 
This allows for paths to the php binary to contain spaces, which was previously breaking the wp cli update command.
@adamperrry adamperrry force-pushed the adamperrry/escape-update-command branch from 8091ef9 to b6a8074 Compare July 25, 2023 17:25
@swissspidy swissspidy changed the title fix: escape shell command in wp-cli update function Escape shell command in wp-cli update Jul 28, 2023
@danielbachhuber danielbachhuber added this to the 2.9.0 milestone Aug 2, 2023
danielbachhuber
danielbachhuber approved these changes Aug 2, 2023
View reviewed changes
Copy link
Member

@danielbachhuber danielbachhuber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will manually verify the update process still works after the nightly Phar is built.

@danielbachhuber danielbachhuber merged commit 2696b5a into wp-cli:main Aug 2, 2023
31 checks passed
@danielbachhuber danielbachhuber mentioned this pull request Aug 2, 2023
Closed
@danielbachhuber danielbachhuber removed this from the 2.9.0 milestone Aug 2, 2023
danielbachhuber added a commit that referenced this pull request Aug 2, 2023
@danielbachhuber
Revert "fix: escape shell command in wp-cli update function (#5818)"
1a34b8d 
This reverts commit 2696b5a.
danielbachhuber added a commit that referenced this pull request Aug 2, 2023
@danielbachhuber
Revert "fix: escape shell command in wp-cli update function (#5818)" (#…
5c4eeaa 
…5822)

This reverts commit 2696b5a.
This was referenced Nov 1, 2023
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danielbachhuber danielbachhuber danielbachhuber approved these changes

@swissspidy swissspidy swissspidy approved these changes

Assignees
No one assigned
Labels
command:cli-update
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update command doesn't escape php_binary path, update fails when path has spaces
3 participants
@adamperrry @danielbachhuber @swissspidy