Bedrock Installation Paths with WP-CLI Adapter

[maiorano84]

Using the latest version of Wordmove with Bedrock, there appears to be a strange conflict between how Wordmove resolves its paths and how paths are being sent to WP CLI to determine the Core Wordpress installation.

After some experimentation, I've narrowed down this behavior to a few scenarios specific to Bedrock that might be worth looking into.

For reference, the basic directory structure of Bedrock can be represented as follows:

.
├── config
│   ├── environments
│   └── application.php
├── web
│   ├── app
│   │   ├── plugins
│   │   ├── themes
│   │   └── uploads
│   ├── wp
│   │   ├── wp-admin
│   │   ├── wp-content
│   │   └── wp-includes
│   └── wp-config.php
├── composer.json
├── wp-cli.yml
└── .env

Scenario 1 - Uploads Push/Pull

All commands are executed using wordmove pull -e dev -u

Configuration Test 1: Absolute paths and Wordpress root - Fails
When using this type of configuration against a Bedrock installation, no uploads are recognized and the command completes without downloading images.

local:
  vhost: http://localhost
  wordpress_path: /var/www/html/web/wp

  paths:
    uploads: "/var/www/html/web/app/uploads"

dev:
  vhost: https://dev.domain.com
  wordpress_path: /path/to/root/web/wp

  paths:
    uploads: "/path/to/root/web/app/uploads"

Configuration Test 2: Relative paths and Wordpress root - Fails
A similar setup using relative paths also completes without downloading images:

local:
  vhost: http://localhost
  wordpress_path: /var/www/html/web/wp

  paths:
    uploads: "../app/uploads"

dev:
  vhost: https://dev.domain.com
  wordpress_path: /path/to/root/web/wp

  paths:
    uploads: "../app/uploads"

Configuration Test 3: Relative paths and Document root - Succeeds
When pointing the wordpress_path one level back to Document root however, the following setup succeeds and all images are downloaded without issue

local:
  vhost: http://localhost
  wordpress_path: /var/www/html/web # Not the Wordpress path

  paths:
    uploads: "app/uploads"

dev:
  vhost: https://dev.domain.com
  wordpress_path: /path/to/root/web # Not the Wordpress path

  paths:
    uploads: "app/uploads"

Scenario 2 - Database Push/Pull

All commands are executed using wordmove pull -e dev -d

Configuration Test 1: Absolute paths and Wordpress root - Fails
Ordinarily I would expect this to succeed, but the SQL Dump execution fails due to the Wordpress path being concatenated with the wp_content path, resulting in an error:

Can't create/write to file '/var/www/html/web/wp/var/www/html/web/wp/wp-content/local-backup-1587063366.sql'

local:
  vhost: http://localhost
  wordpress_path: /var/www/html/web/wp

  paths:
    wp_content: "/var/www/html/web/wp/wp-content"

dev:
  vhost: https://dev.domain.com
  wordpress_path: /path/to/root/web/wp

  paths:
    wp_content: "/path/to/root/web/wp/wp-content"

Configuration Test 2: Relative paths and Wordpress root - Succeeds
This command succeeds without issue. It is understood that the wp_content path can be left out in this case, as this is the default value of this path.

local:
  vhost: http://localhost
  wordpress_path: /var/www/html/web/wp

  paths:
    wp_content: "wp-content"

dev:
  vhost: https://dev.domain.com
  wordpress_path: /path/to/root/web/wp

  paths:
    wp_content: "wp-content"

Configuration Test 3: Relative paths and Document root - Fails
This is where SQL Dump will succeed, but WP CLI fails with the following error:

Error: This does not seem to be a WordPress installation.
Pass --path=path/to/wordpress or run wp core download

local:
  vhost: http://localhost
  wordpress_path: /var/www/html/web

  paths:
    wp_content: "wp/wp-content"

dev:
  vhost: https://dev.domain.com
  wordpress_path: /path/to/root/web

  paths:
    wp_content: "wp/wp-content"

The Problem

Running either wordmove pull -e dev -d or wordmove pull -e dev -u in isolation each has a working configuration when WP CLI is the selected adapter.

Unfortunately, no combination of settings would allow us to run the following command with success:

wordmove pull -e dev -du

Absolute paths against Wordpress root fails in both scenarios.
Relative paths against Wordpress root fails in Scenario 1 and succeeds in Scenario 2
Relative paths against Document root succeeds in Scenario 1 and fails in Scenario 2

The only way around this at the moment is to use the default adapter and use relative paths against Document root. I would prefer to avoid this if I can.

The Solution

I'm not sure how paths are being resolved, but the SQL Dump output path should probably have a check to determine whether or not absolute vs relative is used, and concatenate only when relative paths are specified.

When it comes to path resolution for rsync, neither absolute paths nor relative paths appear to be honored properly if the wordpress_path setting is one level deeper than the rest of the paths. I'm not even sure that wordpress_path should even have a role in determining the path for uploads, themes, plugins, or languages, but I can't be certain since I don't know what code is driving this behavior.

Lastly, it looks as if WP CLI will fail no matter what if wordpress_path is not pointing to a Wordpress instance. That means that even if we have a wp-cli.yml file in our project root to tell it where Wordpress actually is, it's being ignored in favor of the --path flag and we still see the This does not seem to be a WordPress installation error.

Let me know if any of this is unclear or if you need me to test anything further. Myself and my team have been using Wordmove for years and love everything about it!

Environment

• OS: Alpine Linux (Docker)
• Ruby: 2.7.1p83
• Wordmove: 5.0.2

[maiorano84]

To assist in replicating the issue, an example Docker environment has been created

This is configured with a working configuration using the default adapter, but WP CLI is available to replicate the issue highlighted above.

[nlemoine]

Damn, I just filled an exact similar issue here #591

Sorry about that. I started writing it yesterday and yours was not here.

[maiorano84]

@nlemoine Ha! It's pretty remarkable how close our issue postings are.

[nlemoine]

Indeed!

[alessandro-fazzi]

@nlemoine Ha! It's pretty remarkable how close our issue postings are.

And they are both terrific. I'll take the time to dive-in asap.

[alessandro-fazzi]

Further reading maybe related #479

[alessandro-fazzi]

For sure related reading #513

[alessandro-fazzi]

I've noticed something during a first investigation: taking techical notes and thoughts.

I feel that we have a liar method which is working behind the scenes: https://github.com/welaika/wordmove/blob/master/lib/wordmove/wordpress_directory.rb#L33-L40

What I see is that custom paths are accepted in a relative format (e.g.: ../foo/bar) but this method never really resolves them, returning a simple string ../foo/bar. From this string we have the ever needed complexester (cit) logic which transform this path in an rsync's series of inclusion directories....and well: I'm not sure that those .. would play well with rsync.

What we'd need there should be something like

irb(main):051:0> File.expand_path '../app/uploads', '/var/www/html/web/wp'
=> "/var/www/html/web/app/uploads"

thus a resolved path.

Moreover Wordmove should take in consideration if the custom paths are absolute or relative and control the subsequent behaviours (maybe always resolving to an absolute path?).

Maybe in https://github.com/welaika/wordmove/blob/master/lib/wordmove/deployer/ssh.rb#L98-L105 we should not always pass wordpress_path as base directory for all the operations, but the absolute path to the directory we're working on. Having such a scenario would bring to have these other methods https://github.com/welaika/wordmove/blob/master/lib/wordmove/deployer/ssh.rb#L116-L166 to rely on an absolute path, not a relative one.

Another issue to work on, as pointed out in this bug report, could be to remove the hardcoded --path and let it be configurable. With wordpress_path as default and a false value that would disable the flag letting wp-cli to read it from its yaml? Off by default and totally relying on wp-cli.yml? I don't know atm.

So I'd like to deepen a "bit" - read it: a lot - more on this one.

[maiorano84]

@pioneerskies That sounds like a good start. I think in an effort to avoid duplicate configurations (ie: configuring WP CLI in both movefile.yml as well as wp-cli.yml), would it be possible to perform a preliminary file existence check for wp-cli.yml at wordpress_path before overriding --path with the contents of wordpress_path?

Specifically in Bedrock, I know that I typically keep movefile.yml right next to wp-cli.yml, so WP CLI shouldn't have a problem autodetecting it if --path is left out in the case that wp-cli.yml passes the existence check.

In this case, we could theoretically point wordpress_path to the project root of a Bedrock install, and use the existing paths block to override where appropriate. This seems like it would minimize the need for adding additional configuration options to Wordmove, or mixing WP-CLI configurations with Wordmove's configuration.

As far as paths go, it sounds like using Ruby to first expand relative paths to absolute would be the best and least error-prone approach. The only idea I have in this regard is that you would probably want to expand only in cases where you know that a given path is relative vs absolute.

We know that wordpress_path itself must be absolute, but whether or not the paths inside the paths block are absolute appears to be a matter of debate. The docs appear to push for a relative path setup, but some example configurations ( see: #506 ) also attempt to favor absolute paths when using ERB syntax. Documentation doesn't appear to specify if one or the other (or even some weird mix of both?) should be used, so it might be a matter of either supporting both or just specifying and enforcing which should be used (I would personally favor absolute paths and leave it to the user to not screw it up).

If supporting both, is it as simple as checking each path's first character to determine absolute vs relative? ie: If the first character of a path is a slash, it's considered absolute?

If enforcing absolute/relative-only paths, I don't know if that would be considered a BC break. Not sure how hardcore you are about sticking to semver, but it's something to consider.

These are just some thoughts. Unfortunately, I don't know Ruby well enough to be able to contribute to the codebase, but hopefully some of this makes sense and wouldn't be too terribly difficult to implement.

If there are challenges around any of this, let me know. I'm happy to at least try and help think through this.

[maiorano84]

@pioneerskies I decided to try my hand at a draft PR anyway despite having no background in Ruby. It should cover at least for the wp-cli.yml check. I'll do some digging to see if I can create another PR to cover for the relative vs absolute conundrum, but it's likely that I'll either take too long, or my inexperience with Ruby will lead to a less optimal or naive solution.

[maiorano84]

@pioneerskies It looks like File.expand_path is absolutely what we will need to support both absolute and relative paths from movefile.yml.

Is it as simple as adjusting the path method in lib/wordmove/wordpress_directory.rb?

[nlemoine]

In this case, we could theoretically point wordpress_path to the project root of a Bedrock install, and use the existing paths block to override where appropriate.

I would also go that way first, before making too much changes. Check the presence of a wp-cli.{yml,yaml} file, load it, and check for the presence of a path key. If both are true, don't append the --path option in the command.

WP-CLI will also work fine without --path as long as:

• wordpress_path is the path where index.php lives
• wordpress_path contains a wp-cli.yml file with a path key inside

[alessandro-fazzi]

Keep on annotating things:

preliminary file existence check for wp-cli.yml at wordpress_path [...] I typically keep movefile.yml right next to wp-cli.yml, so WP CLI shouldn't have a problem autodetecting it if --path is left out in the case that wp-cli.yml passes the existence check.

This is eversimplistic: firstly wodmove accepts arbitrary config paths through command line options, so movefile.yml could be anywhere. Secondly: wp-cli too do supports arbitrary wp-cli.yml path. Moreover both softwares will automatically search for configurations from cwd upwards

wp-cli.yml file inside the current working directory (or upwards). ref

Fortunately we have wp cli info --format=json command which returns, e.g.:

{
  "php_binary_path": "/usr/local/Cellar/php@7.2/7.2.28/bin/php",
  "global_config_path": false,
  "project_config_path": "/Users/fuzzy/dev/sshwordmove/wp-cli.yml",
  "wp_cli_dir_path": "phar://wp-cli.phar/vendor/wp-cli/wp-cli",
  "wp_cli_packages_dir_path": "/Users/fuzzy/.wp-cli/packages/",
  "wp_cli_version": "2.4.0",
  "system_os": "Darwin 19.4.0 Darwin Kernel Version 19.4.0: Wed Mar  4 22:28:40 PST 2020; root:xnu-6153.101.6~15/RELEASE_X86_64 x86_64",
  "shell": "/usr/local/bin/fish"
}

avoiding us the responsibility to search for project_config_path: if the key has a value, then we have a config, otherwise the value will be null. And json is obviously easily parsable.

Going further, having this wp-cli.yml

path: /foo/bar

we can also use this one other command to do the magic: wp cli param-dump --with-values that returns (I'll paste just the interesting portion)

{
  "path": {
    "runtime": "=<path>",
    "file": "<path>",
    "synopsis": "",
    "default": null,
    "multiple": false,
    "desc": "Path to the WordPress files.",
    "current": "\/foo\/bar" // <---- here we are!
  }
}

[alessandro-fazzi]

We know that wordpress_path itself must be absolute, but whether or not the paths inside the paths block are absolute appears to be a matter of debate. The docs appear to push for a relative path setup, but some example configurations ( see: #506 ) also attempt to favor absolute paths when using ERB syntax. Documentation doesn't appear to specify if one or the other (or even some weird mix of both?) should be used, so it might be a matter of either supporting both or just specifying and enforcing which should be used (I would personally favor absolute paths and leave it to the user to not screw it up).

When Wordmove was wrote was probably unrealistic to think about setup with the core in a different directory tree than the rest of the "components" (I mean upoads, plugins, etc.).

So yes: these days the documentation itself sounds unclear; actually, given the code we have in current stable, we should state to use relative paths and implement a doctor check about it.

[alessandro-fazzi]

If supporting both, is it as simple as checking each path's first character to determine absolute vs relative? ie: If the first character of a path is a slash, it's considered absolute?

This is as easy as relative? or absolute?

[alessandro-fazzi]

@pioneerskies It looks like File.expand_path is absolutely what we will need to support both absolute and relative paths from movefile.yml.

Is it as simple as adjusting the path method in lib/wordmove/wordpress_directory.rb?

I'd say WordpressDirectory#path, WordpressDirectory#relative_path and the almost unintelligibles Wordmove::Deployer::SSH#push_inlcude_paths, Wordmove::Deployer::SSH#pull_inlcude_paths, Wordmove::Deployer::SSH#push_exclude_paths and Wordmove::Deployer::SSH#pull_exclude_paths as a consequence (unless it would be possible to 100% retain the interface, but it doesn't sounds like that to me)

[alessandro-fazzi]

In this case, we could theoretically point wordpress_path to the project root of a Bedrock install, and use the existing paths block to override where appropriate.

I would also go that way first, before making too much changes. Check the presence of a wp-cli.{yml,yaml} file, load it, and check for the presence of a path key. If both are true, don't append the --path option in the command.

WP-CLI will also work fine without --path as long as:

* `wordpress_path` is the path where `index.php` lives
* `wordpress_path` contains a `wp-cli.yml` file with a `path` key inside

Both your point are invalidated if you run wordmove from a "foreign" directory. There it is a related discussion #407. This means that fallback to --path=#{wordpress_path} it's still required or maybe that we should cd #{wordpress_path} before running this command

[alessandro-fazzi]

@nlemoine obviously your comments are welcome on #597 too :)

[maiorano84]

@pioneerskies

Both your point are invalidated if you run wordmove from a "foreign" directory. There it is a related discussion #407. This means that fallback to --path=#{wordpress_path} it's still required or maybe that we should cd #{wordpress_path} before running this command

You raise an interesting point.

Because WP CLI uses the CWD by default, that changes its behavior and whether or not it can find a wp-cli.yml file on its own. Attempting to defer too much to WP CLI defaults could yield unpredictable behavior that might not seem clear to the user.

Since we're already planning on expanding the wp-cli.yml existence check to determine both that the file exists and that the path key is available, we would have to parse the file no matter what. I think it would make sense to simply set the --path option with the value inside the file - should it exist - or fall back to wordpress_path as a default.

That should fix the CWD problem, and we will know that --path will always be set. No cd call needed.

Does that work?

[alessandro-fazzi]

I think it would make sense to simply set the --path option with the value inside the file - should it exist -

What's the difference between parsing the file and set the --path flag, or just omit the flag if wp has already a foundable wp-cli.yml?

BTW I left some examples in the PR

[maiorano84]

@pioneerskies

Your code examples offered some insight into an approach that I think would ultimately yield the same result I was looking for.

Conceptually, I was planning on using require 'yaml' rather than require 'json' and searching for and parsing the file directly. Your approach instead allows WP CLI to do all that for us and extract a string out of the result, which I think is more reliable and less error-prone.

I'll use your examples and try to expand on my logic in my PR to support that approach.

Thank you for all your ideas!