WordPress: Fix wp-cli not working if $WP_CORE is provided

[jdoubleu]

The Problem/Issue/Bug:

The command wp (wp-cli) does not work properly if the $WP_CORE environment variable was not set, because the bash script at /usr/local/bin/wp appends an empty path option "--path=".

Use Case

You might wonder why the $WP_CORE environment variable is not set.

This is a special use case where I am using the default php type along with bedrock. DDEV's WordPress setup is not compatible with bedrock.

In this case the $WP_CORE environment variable is not set automatically. However I still need to run wp (bedrock ships a [.wp-cli.yml config file]($WP_CORE environment variable).

Workaround

Manually provide the $WP_CORE environment variable (see https://ddev.readthedocs.io/en/latest/users/extend/customization-extendibility/#providing-custom-environment-variables-to-a-container).

How this PR Solves The Problem:

The /usr/local/bin/wp script only applies the --path= option to wp-cli if $WP_CORE is not empty.

Manual Testing Instructions:

Automated Testing Overview:

Related Issue Link(s):

Release/Deployment notes:

[rfay]

Hmm, we often use bedrock as a test environment for Wordpress, and it's seemed to be OK in the past. What's not compatible about it?

(I'm not objecting, but want to know about this.)

[jdoubleu]

The typical bedrock folder structure looks like this:

$ tree -a -L 3 -I vendor
.
├── .ddev
│   ├── .gitignore
│   ├── config.yaml
│   ├── db-build
│   │   └── Dockerfile.example
│   └── web-build
│       └── Dockerfile.example
├── .env
├── .env.example
├── .gitignore
├── CHANGELOG.md
├── LICENSE.md
├── README.md
├── composer.json
├── composer.lock
├── config
│   ├── application.php
│   └── environments
│       ├── development.php
│       └── staging.php
├── web
│   ├── app
│   │   ├── mu-plugins
│   │   ├── plugins
│   │   ├── themes
│   │   └── uploads
│   ├── index.php
│   ├── wp
│   │   ├── index.php
│   │   ├── license.txt
│   │   ├── readme.html
│   │   ├── wp-activate.php
│   │   ├── wp-admin
│   │   ├── wp-blog-header.php
│   │   ├── wp-comments-post.php
│   │   ├── wp-config-sample.php
│   │   ├── wp-content
│   │   ├── wp-cron.php
│   │   ├── wp-includes
│   │   ├── wp-links-opml.php
│   │   ├── wp-load.php
│   │   ├── wp-login.php
│   │   ├── wp-mail.php
│   │   ├── wp-settings.php
│   │   ├── wp-signup.php
│   │   ├── wp-trackback.php
│   │   └── xmlrpc.php
│   └── wp-config.php
└── wp-cli.yml

15 directories, 34 files

Thereby the WordPress installation path is: web/wp.

This path is configured in wp-cli.yml:

path: web/wp
server:
  docroot: web

Then, If you run the wp-cli, it would lookup the config and automatically use the correct path.
This is necessary, because both ddev exec and ddev ssh set /var/www/html as working dir.

The problem: Since /usr/local/bin/wp always adds the --path option to the wp-cli command it overwrites the path provided in the config and as command option (compare the following commands).

$ ddev exec wp core version --debug

...
Debug (bootstrap): argv: /usr/local/bin/wp-cli core version --debug --path= (0.303s)
Debug (bootstrap): ABSPATH defined: / (0.306s)
...
Error: This does not seem to be a WordPress install.
Pass --path=`path/to/wordpress` or run `wp core download`.
Failed to execute command wp core version --debug: exit status 1 

$ ddev exec wp core version --path=web/wp --debug

...
Debug (bootstrap): argv: /usr/local/bin/wp-cli core version --path=web/wp --debug --path= (0.325s)
Debug (bootstrap): ABSPATH defined: / (0.306s)
...
Error: This does not seem to be a WordPress install.
Pass --path=`path/to/wordpress` or run `wp core download`.
Failed to execute command wp core version --debug: exit status 1 

This issue does not appear if you run wp directly in the web/ directory (e.g. $ ddev exec 'cd web; wp core version').

[..] we often use bedrock as a test environment for Wordpress [..]

What other commands do you run before and what environment variables are set?

There could be another case I am missing right now, which would explain why it works for you.

[rfay]

I do find myself unable to get bedrock going right now (with a composer install). Seems they have very appropriately moved on to an environment-driven or .env driven setup, and we need to follow that. I don't have time to completely chase it right now, but I'd really rather have bedrock working than anything else, certainly rather than tweaking the container. I'm willing.

Do you have any time to poke around with the wp-config-ddev.php and see if you can get it to work right in ddev environment (with project type wordpress)? Remove the "ddev-generated" line at the top to experiment.

[jdoubleu]

Yes, I can look into it.

Do I understand you right, that you want to prefer bedrock's behaviour when using project type WordPress rather than the current one?

[rfay]

I don't have an agenda on any of this. Just want wordpress in general to work for as many people as possible.

[jdoubleu]

First of all: I created a PoC repository https://github.com/jdoubleu/ddev-pull-1810. Follow the steps to reproduce this error.

I noticed, that the issue also occurs when the project type is wordpress.

Next thing I'm trying to find out is why the $WP_CORE env variable is not set.

[rfay]

I'm pretty sure it doesn't do what you want right now. Also, it will need its own image for testing. Did you figure out what was going on with WP_CORE in the first place?

[jdoubleu]

Did you figure out what was going on with WP_CORE in the first place?

No, I couldn't find any reason why there's a WP_CORE variable.

Neither in the docs, nor the git history. There is no static reference in the code-base, meaning the symbol WP_CORE isn't used anywhere else. This does not mean that it could be set dynamically e.g. set_env(strings.Join("WP_", envName), "/path/to/wp").

So far I know that this script has been added through from the https://github.com/drud/docker.nginx-php-fpm-local repository.

But there's maybe an edge case when this is used so I wouldn't remove it?
Maybe you want to add a deprecation notice and maybe someone will notice it, or leave it with that little switch proposed by this PR.

Thanks for your patience.

[rfay]

No, if we don't know how WP_CORE is being used and know of no reason for it, it should be removed. It's not a WordPress thing? In this coming sprint I'm hoping to give our WordPress support a bit of love, it's been a bit neglected, I'll make sure in there.

[jdoubleu]

It's not a WordPress thing?

It's not a core variable.

There is this thirdparty deployment configuration https://github.com/evertramos/docker-wordpress-letsencrypt which uses a variable like WP_CORE but I think it's just coincidence.

[rfay]

Oh, so the problem is that bedrock and only bedrock uses the WP_CORE environment variable, is that correct, and ddev defines it for WordPress project type to support bedrock?

And your situation is that you're using bedrock with the 'php' app type?

In that case, I'm quite sure the quick path to solving this is just to add WP_CORE as an environment variable using a docker_compose.wpcore.yaml override like this: https://ddev.readthedocs.io/en/stable/users/extend/customization-extendibility/#providing-custom-environment-variables-to-a-container

Right?

I'm sorry I didn't catch on to your basic problem earlier (assuming I understand it now).

[jdoubleu]

Oh, so the problem is that bedrock and only bedrock uses the WP_CORE environment variable, is that correct, and ddev defines it for WordPress project type to support bedrock?

No, bedrock does not use the WP_CORE env var. As far as I know there is no other project which uses it.

DDEV sets it anyways.

And your situation is that you're using bedrock with the 'php' app type?

Yes, I do. However using the wordpress type, bedrock or better said wp-cli, wouldn't work neither (see reproduction repository).

In that case, I'm quite sure the quick path to solving this is just to add WP_CORE as an environment variable using a docker_compose.wpcore.yaml override like this: [..]

This is the current work-around I am using.

But this means, one has to do this additionally task in order to kickstart a new wordpress project.

I'm sorry I didn't catch on to your basic problem earlier (assuming I understand it now).

I am sorry for any misunderstanding.

[rfay]

This is a special use case where I am using the default php type along with bedrock. DDEV's WordPress setup is not compatible with bedrock.

Just an FYI that with #1918 roots/bedrock is not only supported, but it's the suggested technique in the docs. Hoping that you can review that one.

[rfay]

There's no reason for WP_CORE to exist, and I can't even figure out where it comes from. We should actually be using the real DOCROOT for this.

There is one problem with bedrock and https://github.com/jdoubleu/ddev-pull-1810 though: wp-cli uses the existence of version.php to determine whether it's found a WordPress, and bedrock doesn't include version.go in there. So it will never be happy. Discussion at https://wordpress.stackexchange.com/a/244168/173449

However, I think the push here fixes most WordPress setups besides bedrock no matter how they are done.

Please note that I rebased your fork/branch and pushed new commits, so you'll have to git reset --hard origin/ddev

I guess for bedrock, one workaround is just to use wp-cli directly. It's also possible that we should just get rid of this wp wrapper script, as it does nothing (now) except try to provide --path

And THANK YOU so much for the excellent PR, with explanation, a demo repository, everything a maintainer could want. And thanks for your patience.

[rfay]

What do you think @jdoubleu - Should we just make "wp" a straight wrapper on wp-cli, not trying to do anything extra? Or even a symlink to it? Most of what I removed from the wp script came from long-ago days when containers on Windows had to run as root, which they don't any more.

[jdoubleu]

What do you think @jdoubleu - Should we just make "wp" a straight wrapper on wp-cli, not trying to do anything extra? Or even a symlink to it? Most of what I removed from the wp script came from long-ago days when containers on Windows had to run as root, which they don't any more.

I recommend creating a symlink to the original wp-cli script, as you would do installing wp-cli (https://make.wordpress.org/cli/handbook/installing/#recommended-installation) yourself.

If someone downloads the original WordPress core and just spins up some containers with ddev, the wp-cli would work as intended.

If you are using a different file structure you should be already aware, that the wp-cli won't work by default. In that case you would:

1. manually add the --path option, or
2. create a config file wp-cli.yml (https://make.wordpress.org/cli/handbook/config/).

Bedrock already comes with a config file changing the default path.

In my opinion, having additional features in a wp-cli wrapper script, would only complicate things. One should deal with different setups/structures on a higher level on top of ddev. Ddev should only provide the tools, or in this case, the wp-cli. And, as you may have seen, features from a wrapper can conflict with internal (wp-cli) behaviour.

Thanks for having the patience to resolve this issue.

[rfay]

Changed to just a symlink in 1bda954