Make WordPress Core

Opened 6 weeks ago

Closed 11 days ago

#63543 closed enhancement (fixed)

Environment variable WP_CONFIG_PATH should be set on cli container in wordpress-develop env

Reported by: westonruter's profile westonruter Owned by: westonruter's profile westonruter
Milestone: 6.8.2 Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords: has-patch has-test-info fixed-major dev-reviewed
Focuses: Cc:

Description

I'm working on a multi-environment setup where I can have plugins cloned into src/wp-content/plugins which use wp-env for their development/test environments, but also using the core files in wordpress-develop. At the same time that the wp-env environments can be running for these plugins, I intend for the wordpress-develop Docker environment to also be able to run. In order for this to work, there's currently some footwork needed.

The core issue is that wp-env will create a wp-config.php at ABSPATH which it uses for its environments, but the wordpress-develop environment uses the wp-config.php which is one directory above. To work around this, the wp-config.php created by wp-env is modified to add the following at the top:

<?php
// Load the wp-config.php for wordpress-develop when the current request is coming from that environment.
if ( basename( __DIR__ ) === 'src' && file_exists( __DIR__ . '/../wp-config.php' ) ) {
        require_once __DIR__ . '/../wp-config.php';
        return;
}

This does not work in the CLI container, however, because of how WP-CLI reads the config during bootstrapping. An error results when trying to run WP-CLI:

Warning: Constant WPINC already defined in /var/www/src/wp-settings.php on line 16

Fatal error: Cannot redeclare _wp_can_use_pcre_u() (previously declared in /var/www/src/wp-includes/compat.php:33) in /var/www/src/wp-includes/compat.php on line 33

Therefore, there needs to be another way to force WP-CLI to use a different wp-config.php, and the wp_locate_config() function provides this via the WP_CONFIG_PATH environment variable. The remaining issue then is how to pass this environment variable into the CLI container. Adding it to .env is not enough apparently, as it seems it must be added specifically to docker-compose.yml, for example:

diff --git a/docker-compose.yml b/docker-compose.yml
index 48f3abc607..172c406b4b 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -106,6 +106,7 @@ services:
       PHP_FPM_UID: ${PHP_FPM_UID-1000}
       PHP_FPM_GID: ${PHP_FPM_GID-1000}
       HOST_PATH: ${PWD-}/${LOCAL_DIR-src}
+      WP_CONFIG_PATH: ${WP_CONFIG_PATH-/var/www/wp-config.php}
 
     volumes:
       - ./:/var/www

Doing this ensures that the wordpress-develop environment uses the expected wp-config.php in the repo root.

Change History (14)

This ticket was mentioned in PR #8931 on WordPress/wordpress-develop by @westonruter.


6 weeks ago
#1

  • Keywords has-patch added

#2 @westonruter
6 weeks ago

  • Keywords has-patch removed
  • Owner set to westonruter
  • Status changed from new to accepted

#3 @westonruter
6 weeks ago

  • Keywords has-patch added

#4 @SirLouen
6 weeks ago

  • Keywords dev-feedback has-test-info added

Combined Reproduction and Test Report

Description

✅ This report validates that the indicated patch works as expected.

Patch tested: https://github.com/WordPress/wordpress-develop/pull/8931.diff

Environment

  • WordPress: 6.9-alpha-60093-src
  • PHP: 8.2.28
  • Server: nginx/1.27.5
  • Database: mysqli (Server: 8.4.5 / Client: mysqlnd 8.2.28)
  • Browser: Chrome 137.0.0.0
  • OS: Windows 10/11
  • Theme: Twenty Twenty-Five 1.2
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.2.0

Reproduction Steps

  1. Set up the Gutenberg plugin inside wordpress-develop plugins directory with the given instructions
  2. Run npm run env:cli -- wp post create --post_type=post --post_title="Sample" --post_content="Sample" --post_excerpt="Sample"
  3. 🐞 Error reproduced Warning: Constant WPINC already defined in /var/www/src/wp-settings.php on line 21

Expected Result

  • wp-cli runs as normal

Actual Results

  1. ✅ Issue resolved with patch.
> WordPress@6.9.0 env:cli
> node ./tools/local-env/scripts/docker.js run --rm cli wp post create --post_type=post --post_title=Sample --post_content=Sample --post_excerpt=Sample

[+] Creating 2/2
Success: Created post 8.
  1. ✅ Also tested without a Gutenberg running installation (no src/wp-config.php) and working as expected.

Additional Notes

Ngl, I was curious about this setup, so I took the opportunity to test it with the corresponding report.

PS: Although this looks good, I still feel that if this docker env was only meant for GHA, all these "improvements" are not relevant (as theorically it was not relevant to add images for mailhog/ftp server, similarly for testing purposes).

So I was wondering: maybe a lando image could serve to run parallelly, for example, the mailhog server without having to mess the docker-composer.yml and install.js files?

Last edited 6 weeks ago by SirLouen (previous) (diff)

#5 follow-up: @westonruter
6 weeks ago

  • Summary changed from Environment variable WP_CONFIG_PATH should be to cli container in wordpress-develop env to Environment variable WP_CONFIG_PATH should be set on cli container in wordpress-develop env

For the longest time I've used this Docker environment as my development environment. It's not my understanding that it was ever only intended for GHA. The readme provides instructions on how to use it for local development.

#6 in reply to: ↑ 5 @SirLouen
6 weeks ago

Replying to westonruter:

For the longest time I've used this Docker environment as my development environment.

The only good thing is that these gimmicks don't affect GHA in any regard:

wp_cli( `config create --dbname=wordpress_develop --dbuser=root --dbpass=password --dbhost=mysql --force --config-file=${process.env.LOCAL_DIR}/../wp-config.php` );

Personally, I'm in favour of providing a better and more complete developer experience within the wordpress-develop environment. I like this kind of improvement.

For example, I had a couple of things going on locally, atm:
https://core.trac.wordpress.org/ticket/63135 (finished)
https://core.trac.wordpress.org/ticket/63172 (finished)
https://core.trac.wordpress.org/ticket/63336 (WIP)
https://core.trac.wordpress.org/ticket/63421 (WIP)

Because testing under any of these conditions have been a thing several times for me in the past weeks.

It's not my understanding that it was ever only intended for GHA. The readme provides instructions on how to use it for local development.

The thing here is that I commented about this during a dev-chat and that was the answer. This is why I'm looking how maybe a Lando spinoff could make the trick.

Last edited 6 weeks ago by SirLouen (previous) (diff)

#7 @westonruter
5 weeks ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 60305:

Build/Test Tools: Improve env:install command with better configurability and error handling.

  • Force WP-CLI to use the wp-config.php in the directory above the src directory via the WP_CONFIG_PATH environment variable [read by WP-CLI](https://github.com/wp-cli/wp-cli/blob/2800ad0a66747a826ae4221b2f022f1df6779cb6/php/utils.php#L328-L329) in the wp_locate_config() function.
  • Update the env:install command to write out the config at the repo root instead of writing it inside of the ABSPATH only then to move it one directory up.
  • Fix JSHint issues.
  • Add error handling to when npm run env:install is executed without having first done npm run env:start, in which case the script will end with an exit code of 1 and emit:

Error: It appears the development environment has not been started. Message: Timed out waiting for: tcp:localhost:8000
Did you forget to do 'npm run env:start'?

Fixes #63543.
Props westonruter, jorbin, SirLouen.

#8 follow-up: @westonruter
5 weeks ago

  • Keywords fixed-major added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for 6.8.2 consideration.

#9 in reply to: ↑ 8 @SirLouen
5 weeks ago

Replying to westonruter:

Reopening for 6.8.2 consideration.

Wondering why do you feel you need this in the 6.8 branch?

#10 @westonruter
5 weeks ago

Because if you have the 6.8 branch checked out and you start up the dev environment, it should work consistently as when you are on trunk. This is especially important now as the 6.8 branch is slated to be longer-lived.

This ticket was mentioned in Slack in #core by zunaid321. View the logs.


4 weeks ago

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


11 days ago

#13 @audrasjb
11 days ago

  • Keywords dev-reviewed added; dev-feedback removed

Second committer review: This looks good to be backported to branch 6.8.

#14 @westonruter
11 days ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 60431:

Build/Test Tools: Improve env:install command with better configurability and error handling.

  • Force WP-CLI to use the wp-config.php in the directory above the src directory via the WP_CONFIG_PATH environment variable [read by WP-CLI](https://github.com/wp-cli/wp-cli/blob/2800ad0a66747a826ae4221b2f022f1df6779cb6/php/utils.php#L328-L329) in the wp_locate_config() function.
  • Update the env:install command to write out the config at the repo root instead of writing it inside of the ABSPATH only then to move it one directory up.
  • Fix JSHint issues.
  • Add error handling to when npm run env:install is executed without having first done npm run env:start, in which case the script will end with an exit code of 1 and emit:

Error: It appears the development environment has not been started. Message: Timed out waiting for: tcp:localhost:8000
Did you forget to do 'npm run env:start'?

Reviewed by audrasjb.
Merges [60305] to the 6.8 branch.

Fixes #63543.
Props westonruter, jorbin, SirLouen.

Note: See TracTickets for help on using tickets.