roots / trellis

WordPress LEMP stack with PHP 8.2, Composer, WP-CLI and more
https://roots.io/trellis/
MIT License
2.51k stars 607 forks source link

Feature Request: Add server env/PHP constant for site WordPress root directory #1362

Closed strarsis closed 2 years ago

strarsis commented 2 years ago

Terms

Summary

Some plugins like WP-Matomo Integration (WP-Piwik) plugin use a standalone PHP script for tracking (due to performance considerations). That standalone PHP tracking file has to include the WordPress core wp-load.php file, it has to assume and hardcode a common WordPress directory structure. But when the WordPress setup is customized (as with a Bedrock WordPress site), the paths will deviate and the wp-load.php cannot be found.

The WP-Matomo Integration (WP-Piwik) plugin now has a new way to explictly set the path to wp-load.php using an additional configuration PHP file that sets a constant with the path to the WordPress root directory. For streamlining this, it would be great if Trellis can add a server environment variable or a PHP constant that points to the WordPress root directory. I don't this would pose a security issue as being able to access the constants and file system would allow searching for the root directory anyway.

Motivation

Why are we doing this?

Some plugins use standalone PHP script files that include WordPress core due to performance considerations (like for tracking/stats).

What use cases does it support?

All cases where plugins are used that allow overriding that path (or has it added after a support request).

What is the expected outcome?

Easier and more robust and streamlined setup for these kinds of plugin on Trellis/Bedrock.

Potential conflicts / foreseeable issues

The path to the WordPress root directory is available as environment variable/constant - but this shouldn't introduce security issues, as having local access wouldn't require knowledge of a path to WordPress root directory anyway. PHP errors/warnings/info are disabled on production anyway.

Additional Context

Discussion about the assumed/hardcoded path to wp-load.php file in WP-Matomo Integration (WP-Piwik) plugin: https://wordpress.org/support/topic/failed-opening-required-wp-load-php/

swalkinshaw commented 2 years ago

I think this makes sense. The easiest way would be to add it to the generate .env file, which can be loaded in any PHP script where it's needed.

Do you want to contribute this change? Shouldn't be too hard if that env idea works

strarsis commented 2 years ago

@swalkinshaw: The right place IMHO would be wordpress_env_defaults. It already contains keys for the WordPress Home URL (wp_home) and the WordPress Site URL (wp_siteurl): https://github.com/roots/trellis/blob/56582d408054f556e8d634a6308730ec17f75068/roles/deploy/vars/main.yml#L6-L8

Those keys are used by default and added to all sites env files, adjusted for each individual site.

ABSPATH is the constant name in WordPress core. A similar key name like wp_abspath would make its purpose more recognizable in .env. Or what about wp_homepath, like the WordPress core function get_home_path? As those env keys are used in $_SERVER, there should be no issues with collisions.

Some key name candidates:

swalkinshaw commented 2 years ago

Oh wait, do you want the path including /wp or not?

ABSPATH points to the /wp subdirectory, so if we don't want /wp, then wp_homepath makes more sense

strarsis commented 2 years ago

This new key wp_homepath: "{{ deploy_helper.new_release_path }}" would contain the path to the release directory (explicit folder, not the current symlink), e.g. /srv/www/example.com/releases/20220209183855.

The path to the WordPress directory wp can be obtained by appending /web/wp to that path: $_SERVER['wp_homepath'] . '/web/wp'

  1. Is a path with the explicit release folder name better than just using current?
  2. Should that new wp_homepath point to the release folder as it does in this example, to ./web directory inside or to ./web/wp (but then wp_homepath makes less sense as you posted)?
swalkinshaw commented 2 years ago

Yeah we should use the real path and avoid the symlink whenever possible to avoid potential issues downstream (like in PHP).

We can just offer both versions:

Then both mirror the WP built in concepts?

strarsis commented 2 years ago

@swalkinshaw:

  wp_homepath: "{{ deploy_helper.new_release_path }}"
  wp_abspath: "{{ deploy_helper.new_release_path }}/web/wp"
WP_ABSPATH='/srv/www/example.com/releases/20220209191906/web/wp'
WP_HOMEPATH='/srv/www/example.com/releases/20220209191906'

(Note: The keys in output are alphabetically sorted)

When this is fine I can create a new PR for this addition?

swalkinshaw commented 2 years ago

Yep looks good to me 👍 The only tricky part is that env defaults is defined in two places:

The latter is used in development during WP install (99% sure that's it). So there's two options:

  1. just do it for deploys only and ignore development (probably not a good idea for dev/prod parity)
  2. add them to the global defaults too, but hardcode it with /current:
wp_abspath: "{{ www_root }}/{{ item.key }}/current/web/wp"
wp_homepath: "{{ www_root }}/{{ item.key }}/current"
strarsis commented 2 years ago

@swalkinshaw: Ah, learned something!

Would these keys, that use item.key in the value be added to group_vars/all/helpers.yml? I tried this and it seems that the main key wordpress_env_defaults is later overridden by another one as the two wp_abspath and wp_homepath keys are missing. 🤔

swalkinshaw commented 2 years ago

They need to be added in both places. For remote servers, the one defined in the deploy role will take precedence. In development, only the one in group_vars/all/helpers.yml should be used.

QWp6t commented 2 years ago

I'm a little confused.

If we're setting these in .env, then how is that getting communicated to a standalone php file?

And if the .env file is being read by the standalone file, then in what scenario would one know the path to .env but not know the path to WordPress?

I'm not strictly opposed to this obviously, but I'm just having trouble understanding how the proposed solution being discussed would actually solve the original problem.

swalkinshaw commented 2 years ago

🤔 that's a good point. It is possible to add env variables through php-fpm config:

env[FOO] = bar

But then the PHP-FPM config is also changed on each deployment 🤷‍♂️ I think I looked into that forever and wanted to avoid it.

With Trellis it's of course possible to customize that config and add anything you want, and reference {{ site_env }} too.

QWp6t commented 2 years ago

I think if we did offer this as a first-party feature, then we would need to make sure to clarify that updating wp_abspath won't actually do anything in phpland.

That is, we would need to make it clear that changing that value should be done in tandem with updating these:

Because of that, I'm less in favor of abspath than some sort of root path being defined in Trellis, but again, I'm not necessarily opposed to any of it.

swalkinshaw commented 2 years ago

@QWp6t has raised some good questions/objections on this so I'm going to close this issue for now. If you have any updates for this @strarsis I'm happy to re-open it again.