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

Added WP Cli install task #5

Closed
wants to merge 2 commits into from
Closed

Added WP Cli install task #5

wants to merge 2 commits into from

Conversation

rslanzi
Copy link
Contributor

@rslanzi rslanzi commented Jan 8, 2021

Usefull to not switch from local machine and remote only for install wp-cli.

@gaambo
Copy link
Owner

gaambo commented Jan 8, 2021

Hi,
thanks for your contribution. I tested the changes and wanted to make it a little bit more reusable and configurable and also to be pretty similar to how phpdeployer manages composer-binary installation. I'd like to review and discuss them in this pull request together with you. Could you test if this works with your workflow?

I've added them to a new branch wp-cli-tasks that you can see here: https://github.com/gaambo/deployer-wordpress/tree/wp-cli-tasks
My commit is here: ba2334b

Could you please enable the setting that allows me to push to your master - see GitHub docs. At least that's how I think that should work 😅

@rslanzi
Copy link
Contributor Author

rslanzi commented Jan 8, 2021

Hi!
I've tested the new script but without moving wp-cli in /usr/local/bin/ the locateBinaryPath() function fails.
I agree that using sudo is not the best but I guess there are no alternatives. Or there are? 🤔😉

@rslanzi
Copy link
Contributor Author

rslanzi commented Jan 8, 2021

The reason why I create this PR is related to the possibility to check if WP cli is installed on remote server and, if not (with a fail() task or something similar), install it and the re-run the wp related task.

This because I want create a deploy script that also a pure front-end designer can use to public a new website, without knowing nothing about droplet management. Utopia? Probable 😅

@gaambo
Copy link
Owner

gaambo commented Jan 12, 2021

Oke, I get what you want to do. I'm just thinking about the possibilities, making it configurable but also not to complicated by adding a ton of configuration variables (because the set.php already is huge 😅).
I see a few options:

  1. Allow configuring an installPath for wp-cli (if it's not installed)
    How would this be differend from bin/wp? The default bin/wp would check if wp-cli is installed and return it's path, if not it would install it to the installPath and return that, kinda seems redundant. Also if one overwrites bin/wp the complete "check if installed and if not install"-procedure is gone. Also this doesn't solve the sudo-problem.
  2. Make an extra task install-cli like in your last commit, which also allows passing an installPath via CLI.
    You'd have to manually run this task. Would that work for your idea? It may not be optimal, because you have to tell your frontend-developers to install wpcli - or would you run this deploy-task on the first deployment?
  3. Make a getWPCLIBinary function which does the commandExist/locateBinaryPath and .dep/wp-cli.phar file test - if one of those exist, it returns the path, if not it returns false and add a sudo parameter to installWPCLI. This could be extracted from the bin/wp in set.php in my commit. That would make it easier for you to overwrite bin/wp like so:

utils/wp-cli.php

function getWPCLIBinary() {
    if (commandExist('wp')) {
        return locateBinaryPath('wp');
    }

    $installPath = '{{deploy_path}}/.dep';
    $binaryFile = 'wp-cli.phar';

    if (test("[ -f $installPath/$binaryFile ]")) {
        return "{{bin/php}} $installPath/$binaryFile";
    }
    return false;
}

set.php

set('bin/wp', function() {
    if($path = getWPCLIBinary()) {
        return $path;
    }
    return installWPCLI('/usr/local/bin', 'wp', true);
}

For reference, here's how deployphp handles installing composer (that's how I tried to do it in my last commit): https://github.com/deployphp/deployer/blob/da432e01399489a896e880e9469c01ffad5491e6/recipe/deploy/vendors.php

I'm leaning towards option 3, which makes it easier to overwrite bin/wp, but adds no additional configuration variables. What do you think?
Maybe I'm overcomplicating things here 😅 Just wanna work out way to implement this, which can be used in many different situations.

@rslanzi
Copy link
Contributor Author

rslanzi commented Jan 13, 2021

For solution 2, it wouldn't be a problem to advise a frontend dev to follow a specific task list but, I agree with you that solution 3 is the most reusable and if someone wants to create a task for wp installation they can use the getWPCLIBinary function.

@gaambo
Copy link
Owner

gaambo commented Jan 14, 2021

Please have a look at 282f9c3

By default bin/wp in set.php tests for a globally installed binary and if none is found installs it to {{deploy_path}}/.dep/wp-cli.phar.

You can easily overwrite this configuration for your boilerplates etc. to install it to a global file like so:

set('bin/wp', function() {
    if($path = getWPCLIBinary()) {
        return $path;
    }
    return installWPCLI('/usr/local/bin', 'wp', true);
}

You can also tell your developers to run
dep wp:install-wpcli production -o installPath=/usr/local/bin -o binaryFile=wp -o sudo=true to install it to a global file, then no change of bin/wp is needed.

I think that should handle all cases, what do you think? Could you please try it out :)

@rslanzi
Copy link
Contributor Author

rslanzi commented Jan 19, 2021

Sorry for delay, it was a busy week! I think that sounds good! Reusable and functional! Really a great job!

@gaambo
Copy link
Owner

gaambo commented Jan 21, 2021

Closed for GH-6 and merged in v1.4.0

Thanks for your contribution, input and feedback 👍

@gaambo gaambo closed this Jan 21, 2021
@gaambo gaambo mentioned this pull request Jan 21, 2021
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 this pull request may close these issues.

None yet

2 participants