Env: Add exec Command

[ObliviousHarmony]

What?

Note: This lacks appropriate documentation updates. Assuming we're fine with the naming and configuration I will update that too.

This pull request adds a new exec command that allows you to run configured scripts within the context of a container. You are able to set these at the environment-level and declare a cwd that the command should be ran in.

Why?

Right now to run scripts in your container, you need package.json scripts with a verbose syntax. This includes referencing container paths and having to deal with the current working directory. By moving these scripts out of package.json we can keep all of the paths and mapping to the environment that actually utilizes them. This means that we can then run wp-env exec tests-wordpress test:php and it can run a PHPUnit local to the package.

How?

We add a new exec command and configuration options for scripts. This uses the same underlying functionality as run but moves the script definition into the .wp-env.json file. For example, this would change the following package.json script:

"test:unit": "wp-env run tests-wordpress "/var/www/html/wp-content/plugins/woocommerce/vendor/bin/phpunit -c /var/www/html/wp-content/plugins/woocommerce/phpunit.xml"

Into the much less verbose:

"test:unit": "wp-env exec tests test:unit"

There is no longer any ambiguity about what these paths are referencing or why it's configured this way. Since the container is the place where the script is actually being executed, having it there feels like a convenience.

Testing Instructions

1. Add the following to your .wp-env.override.json.

{
    "scripts": {
        "test": {
            "script": "ls -la",
            "cwd": "wp-content/plugins/gutenberg"
        }
    }
}

2. Run npm run wp-env -- exec tests test and note that it prints the directory.
3. Play around with the "cwd" option, it supports absolute paths as well as being removed altogether.
4. Play around with env.tests and env.development overrides for "scripts".
5. Play around with run command to make sure it still works after the refactor.

[github-actions]

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @ObliviousHarmony! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

[noahtallen]

Neat, overall this makes sense to me! I think that this could be a useful feature. I think it might be helpful to see how exec can simplify things vs npm run. Could you add an example to the PR description maybe?

[ObliviousHarmony]

Thanks for the review @noahtallen. I've addressed all of your feedback and updated the PR description with an additional testing instruction as well as an example of what the benefit of this feature looks like.

[noahtallen]

So basically, something like this:

wp-env run tests-wordpress "/var/www/html/wp-content/plugins/woocommerce/vendor/bin/phpunit -c /var/www/html/wp-content/plugins/woocommerce/phpunit.xml"

Turns into:

{
    "scripts": {
        "test-php": {
            "script": "vendor/bin/phpunit -c phpunit.xml",
            "cwd": "wp-content/plugins/woocommerce"
        }
    }
}

plus wp-env exec tests-wordpress test-php.

One of the reasons I like this feature is that folks with new feature requests have an avenue to do some automation themselves. For example, there was a feature request in the past about importing some data. While that could be pretty verbose to write out all the time in the shell, it's a lot easier as a script. (And perhaps in the future, one could add hooks into existing scripts. Like a way to "run this script after running wp-env start")

The only thing I don't like about this feature is that it is very similar to npm run scripts or even composer run scripts. Will this feature make it more confusing to choose which place to define a script? The verbosity of the npm run script definitely isn't ideal, but at the same time, one rarely has to interact with it. It almost seems more verbose to have an extra layer of indirection where you go from npm run -> wp-env exec -> docker-compose run. That's not an issue when you're using wp-env exec by itself, but still worth thinking about IMO

[ObliviousHarmony]

perhaps in the future, one could add hooks into existing scripts. Like a way to "run this script after running wp-env start"

I think this is one of the larger benefits to be had. I could see us implementing pre and post support for commands. Since all this is doing is passing commands to the shell, you could use a poststart script to perform some configuration or customization within the container. Maybe you prepare it for E2E tests for instance, whatever the heart desires.

Will this feature make it more confusing to choose which place to define a script?

What helped me rationalize this feature was focusing more on the execution context than the specific tool. While it's true that there's some indirection going on here, as far as the configuration is concerned, you are writing the command that is being executed in the Docker container. You don't really have to be worried about how it gets there, just the context where it's being used. wp-env scripts are for those that run in the environment, package.json scripts are for those that are written with JavaScript dependencies, and composer.json scripts are for those that are written with PHP dependencies.

[ObliviousHarmony]

While updating the documentation in this PR @noahtallen, I realized something potentially a little annoying. The way I've set it up, you configure the script based on the environment rather than the Docker container. This doesn't feel wrong, but, it means that WP-CLI isn't accessible to scripts. I could see that being something people find problematic, so, I'm thinking about how to address that. One possibility could be to use a cli option that decides whether or not it uses the wordpress container or the cli container. Realistically speaking, is there any reason to use the web container?

Coming back to the overall usefulness of this feature, perhaps adding support for plugins would make more sense? A big part of why I went down this rabbit hole in the first place was to fix the argument passing, but, I found a way to do that without a separate command. With that in mind, the main benefit of this is the path stuff (which is questionable) and the potential for hooking. Hooking, however, could be far more robust as a plugin feature:

• Support custom configuration option parsing and validation using hooks.
• Support pre & post hooks for built-in commands.
• Support hooks for adding commands.

Since .wp-env.json is specifically for configuring the environments, and reading is deferred until after yargs is initialized, I might suggest a package.json configuration for plugins to be read from. This feels like it's probably a better route, what do you think?

[noahtallen]

I mean, it's definitely a pretty cool idea! My main concern is whether the effort is worth it right now. Do we have a lot of folks interested in this use case?

I think we'd want to get more buy-in and do more planning for this sort of feature, which would mean spending more time before implementation. Plus, big new features always have long-term maintenance issues. So if it's not super valuable to have right now, it'd be hard for me to recommend jumping to implement it

[noahtallen]

One possibility could be to use a cli option that decides whether or not it uses the wordpress container or the cli container. Realistically speaking, is there any reason to use the web container?

This is actually a great question in general, and I wonder if we really need two separate containers. As far as I understand, the CLI containers are just adding the CLI binary (and maybe some extra config?) on top of everything already in the wordpress containers.

[ObliviousHarmony]

So if it's not super valuable to have right now, it'd be hard for me to recommend jumping to implement it

I wouldn't, I'm just playing devil's advocate to whether this PR makes sense or not in the first place 😄 Realistically it feels easier to implement it here, but, I'm wondering if there's anyone else interested in this feature in the first place.

As far as I understand, the CLI containers are just adding the CLI binary (and maybe some extra config?) on top of everything already in the WordPress containers.

From what I can tell looking at the images, it seems like it just sets up the container with appropriate dependencies and PHP extensions to use WP-CLI. I don't really see a reason wp-env couldn't do that when it sets up the containers.

[noahtallen]

We definitely could. I think we've just been using upstream images to avoid having to do any extra maintenence on Dockerfiles

[noahtallen]

cc @youknowriad @gziolo @noisysocks For some extra feedback on this feature. Looking to get more ideas about:

• is this a valuable feature for wp-env?
• any API changes that might be useful?

[noisysocks]

Couldn't the same thing be achieved by putting wp-env run commands into package.json and using npm run?

[ObliviousHarmony]

@noisysocks Yes, but, the command ends up like this: "test:unit": "wp-env run tests-cli /var/www/html/wp-content/plugins/woocommerce/vendor/bin/phpunit -c /var/www/html/wp-content/plugins/woocommerce/phpunit.xml". It requires all of the package.json scripts to use absolute paths to files that only exist within the Docker container. From an organizational perspective, that script means absolutely nothing to the host operating system.

It feels natural for the scripts that execute in the environment to be defined in the environment. In this way, the paths being referenced in the script are in the same file as the mappings and other information about the environment.

As an added bonus, we can later look at adding support for hooking with "pre" and "post" scripts in the same manner as NPM. A "poststart" script can let folks run arbitrary scripts after the environment starts without the need for changing the DockerFile.

It's true you could wrap that in a package.json script too, but, at what point do wp-env commands just stop being used entirely?