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

wp-env: Add Xdebug support #20636

Closed
rheinardkorf opened this issue Mar 4, 2020 · 20 comments
Closed

wp-env: Add Xdebug support #20636

rheinardkorf opened this issue Mar 4, 2020 · 20 comments
Labels
[Tool] Env /packages/env [Type] Enhancement A suggestion for improvement.

Comments

@rheinardkorf
Copy link

Is your feature request related to a problem? Please describe.
The wp-env tool is a great way for developers to contribute to WordPress projects quickly. However, many agencies have workflows that don't use the official WordPress Docker image. An example would be to have a custom WordPress image that include tools like xDebug or some custom scripts that execute upon launch.

Describe the solution you'd like
It would be great if the .wp-env.json file included an option to load alternate images for the list of environments. Overriding the wordpress or wordpress-test images.

Describe alternatives you've considered
Could possibly consider alternate docker-compose.yml files, or modifying the one produced by wp-env.

@noahtallen noahtallen added [Tool] Env /packages/env [Type] Enhancement A suggestion for improvement. labels Mar 4, 2020
@noahtallen
Copy link
Member

noahtallen commented Mar 4, 2020

I think that modifying the image the tool uses is a pretty straightforward/simple change -- could look something like this:

{
  "plugins": [ "." ],
  "image": "foobar",
  "testsImage": "foobar2"
}

Would we want the testsImage to automatically match image if it has not been specified?

And that would ultimately just change the string here:

We are trying to avoid exposing everything Docker has to offer, but I could see this as a simple way to integrate custom Docker stuff with the tool.

cc @noisysocks if you have thoughts?

@youknowriad
Copy link
Contributor

why new config options, why not just reuse the core option?

@noahtallen
Copy link
Member

Right now the core option is just a string for specifying the core source code. Even if you have a custom source there, it would just attach the custom source as a volume mapping to the WordPress root, but it wouldn't change the underlying Docker image

@youknowriad
Copy link
Contributor

I'd also encourage folks to resist adding config options when possible. Config options make things harder and more complex and as stated by the dev note, for complex use-cases, it's almost always better to rely on a custom docker-compose/custom tool.

For instance, the example on the issue is about xDebug. I can see how that can be useful for any WordPress dev environment, so why not just add it by default instead of complexifying the config.

@derekherman
Copy link

I would suggest adding a mechanism for activating/deactivating xDebug so it's installed and off by default, but with a way to activate it during times when you need to do code coverage or debug PHP, for example.

@noahtallen
Copy link
Member

I'd also encourage folks to resist adding config options when possible.

I definitely agree. I was thinking that (in this case) specifying a Docker image might be a way to expose some more powerful features to advanced users without adding any extra burden to the simplicity of the tool. We definitely don't want to expose every Docker setting through tons of extra config options (i.e. specifying Docker env configuration is one thing we didn't want to do). But specifying a custom Docker image might be a way to compromise to unlock more of those features without having to support them ourselves.

However, I do see why adding it wouldn't be all good. A certain docker image may not fit well with the rest of the opinionated things that wp-env does does.

@rheinardkorf
Copy link
Author

rheinardkorf commented Mar 5, 2020

I'd also encourage folks to resist adding config options when possible.

Definitely agree. The reason I considered alt Docker images is because xDebug is not in the core image. So if adding it means modifying some of your build script that is a win. But I'd also encourage the option to be able to turn it on or off.

As for other more advanced uses, if the decision is to not extend wp-env to use alt images its no big deal. Was simply considering it as an option for the sake of adoption.

However, I do see why adding it wouldn't be all good. A certain docker image may not fit well with the rest of the opinionated things that wp-env does does.

Absolutely. But if an alt Docker image is simply an extension of the core image, then all opinionated concerns that wp-env does should still be valid.

@rheinardkorf
Copy link
Author

rheinardkorf commented Mar 5, 2020

If this feature is considered...

Would we want the testsImage to automatically match image if it has not been specified?

If you are changing your primary image you usually have a good reason for doing so. Same would apply for a test image. In most cases wordpress-test (I think this is actually just the wordpress image) would be the more sensible default rather than using the specified primary image as default.

@noisysocks
Copy link
Member

Good discussion here, and thanks for using and testing wp-env! 🙂

I'm against allowing the Docker image to be customised because:

  • It will make developing and troubleshooting wp-env more difficult because we will not be able to make assumptions about what image the user is running.
  • It doesn't align with the tool's goal of being a zero config development environment that's designed for the majority.

Developers that need to use a different Docker image can use their own Docker-based environment, and, this being GPLv2, can even use wp-env as a starting point.

With that said, I agree that having Xdebug enabled in wp-env instances is a good idea worth exploring. This potentially could be done by changing the (non-configurable) Docker image that wp-env uses from wordpress to wordpressdevelop/php.

@andygagnon
Copy link

+1 on enabling Xdebug

@noahtallen
Copy link
Member

Going to rename this issue to track Xdebug support since that was the main goal of the original issue, and since we aren't going to add support for different docker images :)

@noahtallen noahtallen changed the title Adding configurable Docker images to wp-env package. wp-env: Add Xdebug support May 21, 2020
@rheinardkorf
Copy link
Author

@noahtallen that's sufficient for my intention. If I need more I will create custom envs, but this will support a majority of my use cases at least.

@pwkip
Copy link
Contributor

pwkip commented Jul 30, 2020

As @noisysocks mentioned, a possible solution is to change the image from wordpress to wordpressdevelop/php. What is preventing us from doing this?

I'm not sure if I understand the philosophy of docker completely, but since we are going to work with only one image. Apart from adding xdebug, would it make sense to also include PHPUnit and WPCLI in the image?

For example, I use this Dockerfile for development on Linux:

FROM wordpress:latest
LABEL maintainer Jules Colle <jules@bdwm.be>

RUN apt-get -y update; apt-get -y install curl; apt-get -y install wget;
 
# Install WP-CLI in the wordpress container
RUN curl -O https://raw.githubusercontent.com/wp-cli/builds/gh-pages/phar/wp-cli.phar; ls -la; chmod +x ./wp-cli.phar; mv wp-cli.phar /usr/local/bin/wp-cli

# Install MySQL extension, as WP-CLI needs to access to WordPress database
RUN docker-php-ext-install mysqli

# Install PHPUnit in the container
RUN wget https://phar.phpunit.de/phpunit-7.5.9.phar; chmod +x phpunit-7.5.9.phar; mv phpunit-7.5.9.phar /usr/local/bin/phpunit; phpunit --version

# Install Xdebug
RUN pecl install "xdebug" \
    && docker-php-ext-enable xdebug

RUN echo "xdebug.remote_enable=1" >> /usr/local/etc/php/conf.d/xdebug.ini && \
    echo "xdebug.remote_autostart=1" >> /usr/local/etc/php/conf.d/xdebug.ini && \
    echo "xdebug.remote_port=9000" >> /usr/local/etc/php/conf.d/xdebug.ini && \
    echo "xdebug.idekey=VSCODE" >> /usr/local/etc/php/conf.d/xdebug.ini && \
    echo "xdebug.remote_host=172.17.0.1" >> /usr/local/etc/php/conf.d/xdebug.ini && \
    echo "xdebug.remote_connect_back=0" >> /usr/local/etc/php/conf.d/xdebug.ini

My guess is that this will only work on Linux though, cause I believe xdebug.remote_host=172.17.0.1 should be xdebug.remote_host=host.docker.internal on Win or Mac

@pwkip
Copy link
Contributor

pwkip commented Jul 31, 2020

Something else I'm wondering about. If xdebug would be enabled, it would make sense to also know the path to the wordpress environment. wp-env seems to generate a kind of unpredictable path where it installs WP core, for example: ~/wp-env/664deb158176e160406cb70ee818701a/WordPress/ For xdebug to be useful I need to specify this path mapping:
"/var/www/html": "~/wp-env/664deb158176e160406cb70ee818701a/WordPress/", but next time I run wp-env this path might change, right?

I might be missing something obvious, but is there a way to make sure the wordpress core folder is always the same for a given project? For example while developing a plugin in a directory named my-plugin-name, install WP inside ~/wp-env/my-plugin-name/WordPress/?

@noahtallen
Copy link
Member

Apart from adding xdebug, would it make sense to also include PHPUnit and WPCLI in the image

Phpunit and WPCLI are included in some of the services (i.e. the CLI and phpunit services). But that does raise the question, why not use the same docker image for each service, each with the correct dependencies available?

cc @epiqueras, not sure if you have ideas about that

a kind of unpredictable path where it installs WP core

So the path should be completely the same every time. Basically, it is just md5( path/to/current/working/dir )

const workDirectoryPath = path.resolve(
await getHomeDirectory(),
md5( configPath )

For xdebug to be useful I need to specify this path mapping

IMO it would be nice to have this stuff working within wp-env so that the end user doesn't need to hack around with xdebug and install locations to get it working. But I admit I'm not super familiar with xdebug :)

@leutrimhusaj
Copy link
Contributor

Something else I'm wondering about. If xdebug would be enabled, it would make sense to also know the path to the wordpress environment. wp-env seems to generate a kind of unpredictable path where it installs WP core, for example: ~/wp-env/664deb158176e160406cb70ee818701a/WordPress/ For xdebug to be useful I need to specify this path mapping:
"/var/www/html": "~/wp-env/664deb158176e160406cb70ee818701a/WordPress/", but next time I run wp-env this path might change, right?

I might be missing something obvious, but is there a way to make sure the wordpress core folder is always the same for a given project? For example while developing a plugin in a directory named my-plugin-name, install WP inside ~/wp-env/my-plugin-name/WordPress/?

This would be very nice...i can explain a scenario where it can be very useful, when the wordpress core folder is configurable.

For example when multiple developers are working on the same project and check it out on different project paths on their own system. This would generate an other md5 of the configPath.
Our pathMappings inside the launch.json are checked in and each developer must have its own pathMapping for debugging the wordpress core.

Something like
WP_ENV_NAME="my-wordpress" and WP_ENV_HOME="wp-env-home" can result in
wp-env-home/my-wordpress/docker-compose.yml

WP_ENV_NAME could be used for the docker container name too, if this is wanted.
So the md5 of the configPath should only be used if the WP_ENV_NAME is not set.

This would be useful for xdebug support.

@noahtallen
Copy link
Member

I'm a little confused -- WP_ENV_HOME should already override the md5. Is that not working??

I think in your use case, if debugging core WordPress is necessary, why not map it to a local checkout via relative path? e.g.

{
  "core": "../wordpress-develop"
}

You could also put that in .wp-env.override.json if you need to change the default wordpress source on a case-by-case basis.

If I'm not understand your use case, please let me know :)

@leutrimhusaj
Copy link
Contributor

@noahtallen No, as i understand following block

const workDirectoryPath = path.resolve(
await getHomeDirectory(),
md5( configPath )
);

it would combine the return value of getHomeDirectory, which would be the WP_ENV_HOME
async function getHomeDirectory() {
// Allow user to override download location.
if ( process.env.WP_ENV_HOME ) {
return path.resolve( process.env.WP_ENV_HOME );
}

with the md5 of the config path.

This would result in ./<wp-env-home>/<md5-of-config-path>/.

This is exactly what is explained here
For example, running WP_ENV_HOME="something" wp-env start will download the project files to the directory ./something/$md5_of_project_path (relative to the current directory).

@noahtallen
Copy link
Member

Ah yeah, I see what you mean. Is there a reason to use that behavior rather than just mapping your own core source?

@swissspidy
Copy link
Member

This was fixed by #27346 apparently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Tool] Env /packages/env [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

No branches or pull requests

9 participants