Run build-cs composer install/update via post-install/update-cmd scripts

[westonruter]

Summary

Fixes #961

This fixes the composer scripts for lint, phpstan, and format if composer install has not been run in the build-cs directory before. It also speeds up running those scripts by avoiding running composer update with each call.

Relevant technical choices

Adds a post-install-cmd in the root composer.json which runs composer install in the build-cs directory if the PHP version is at least 8.1. Likewise adds a post-update-cmd to call run composer update in that directory. Additionally, when running PHPUnit tests on the various versions of PHP in the GHA workflow, instead of calling composer update directly it instead calls composer update via wp-env.

This fixes an issue which was introduced in #544.

Checklist

• PR has either [Focus] or Infrastructure label.
• PR has a [Type] label.
• PR has a milestone or the no milestone label.

[westonruter]

Naturally running composer install fails on the older PHP versions, for example PHP 7.0: https://github.com/WordPress/performance/actions/runs/7716621796/job/21033857878

So in 3df9c5d I made the install/update conditional if the system is on PHP 8+. That made the jobs pass, but for some reason it still seems like composer update is running:

https://github.com/WordPress/performance/actions/runs/7716757155/job/21034333302#step:5:78

I was expecting to see:

Skipping composer update for build-cs since not on PHP 8.

[mukeshpanchal27]

@westonruter @swissspidy not sure if it's related issue but npm run lint-php is not working.

Error:

> prelint-php
> wp-env run composer 'install --no-interaction'

wp-env run <container> [command...]

Runs an arbitrary command in one of the underlying Docker containers. A double
dash can be used to pass arguments to the container without parsing them. This
is necessary if you are using an option that is defined below. You can use
`bash` to open a shell session and both `composer` and `phpunit` are available
in all WordPress and CLI containers. WP-CLI is also available in the CLI
containers.

Positionals:
  container  The underlying Docker service to run the command on.
              [string] [required] [choices: "mysql", "tests-mysql", "wordpress",
                                          "tests-wordpress", "cli", "tests-cli"]
  command    The command to run.                           [array] [default: []]

Options:
  --help     Show help                                                 [boolean]
  --debug    Enable debug output.                     [boolean] [default: false]
  --version  Show version number                                       [boolean]
  --env-cwd  The command's working directory inside of the container. Paths
             without a leading slash are relative to the WordPress root.
                                                         [string] [default: "."]

The 'composer' container has been removed. Please use 'wp-env run cli --env-cwd=wp-content/path/to/plugin composer' instead.

[westonruter]

@mukeshpanchal27 ah, well it seems that is obsolete now anyway. I think we should delete this:

performance/package.json

Line 34 in 3df9c5d

"prelint-php": "wp-env run composer 'install --no-interaction'",

That was slowing down linting anyway. No need to re-install with every call. Does it work for you with that removed?

[westonruter]

@mukeshpanchal27 I've removed those pre scripts in 2591052. The Prerequisites section of the handbook page indicates that Composer is optional but the Set up the development environment section does not. I think contributors should have to always do npm install and composer install to contribute.

[westonruter]

Oh, I guess this isn't needed because it's running unit tests in the container.

[westonruter]

But it would allow composer install to work properly without having to run it in the container?

[westonruter]

I don't get it. The --env-cwd arg is supposed to allow changing the working directory for running a command with wp-env run. However, it doesn't seem to be working.

On my local machine, I'm supposed to be able to do:

npm run wp-env run tests-cli --env-cwd='wp-content/plugins/$(basename $(pwd))' pwd

And it should output:

/var/www/html/wp-content/plugins/performance

But it isn't. It's outputting instead:

/var/www/html

As if the --env-cwd wasn't even supplied.

@mukeshpanchal27 As for the error you're seeing in #962 (comment), that makes sense because the composer container was removed in v7.0.0:

Remove the composer and phpunit Docker containers. If you are currently using the run composer or run phpunit command you can migrate to run cli composer or run tests-cli phpunit respectively. Note that with composer, you will need to use the --env-cwd option to navigate to your plugin's directory as it is no longer the default working directory.

So it makes sense that this wouldn't have worked anymore and should be removed (or at least updated):

performance/package.json

Line 34 in 4396a84

"prelint-php": "wp-env run composer 'install --no-interaction'",

But again I don't get why --env-cwd isn't working.

Update: An additional -- needed to be inserted before the argument to ensure it was passed properly. Props @joemcgill in 9e926dd.

[westonruter]

Converting to draft because this needs some more work, namely to fix running composer install in the tests-cli container.

[westonruter]

OK, I see why composer update was needed instead of composer install. Composer would fail to install on PHP versions older than 8.1 which is the version the composer.lock was created for:

> wp-env
> wp-env run tests-cli --env-cwd=wp-content/plugins/performance composer install --no-interaction

ℹ Starting 'composer install --no-interaction' on the tests-cli container. 

Installing dependencies from lock file (including require-dev)
Verifying lock file contents can be installed on current platform.
Your lock file does not contain a compatible set of packages. Please run composer update.

  Problem 1
    - doctrine/instantiator is locked to version 2.0.0 and an update of this package was not requested.
    - doctrine/instantiator 2.0.0 requires php ^8.1 -> your php version (7.4.33) does not satisfy that requirement.
  Problem 2
    - doctrine/instantiator 2.0.0 requires php ^8.1 -> your php version (7.4.33) does not satisfy that requirement.
    - phpunit/phpunit [9](https://github.com/WordPress/performance/actions/runs/7733447799/job/21085541195?pr=962#step:7:10).6.[16](https://github.com/WordPress/performance/actions/runs/7733447799/job/21085541195?pr=962#step:7:17) requires doctrine/instantiator ^1.3.1 || ^2 -> satisfiable by doctrine/instantiator[2.0.0].
    - phpunit/phpunit is locked to version 9.6.16 and an update of this package was not requested.

✖ Command failed with exit code 2
Command failed with exit code 2
Error: Process completed with exit code 1.

This being said, I wonder if we should even be committing the composer.lock file? I guess it is still beneficial for local development, but it seems unfortunate to have to run it in GHA for older versions of PHP since (1) it takes longer and (2) the package versions won't always be consistent.

[westonruter]

I was expecting to see:

Skipping composer update for build-cs since not on PHP 8.

OK, I'm seeing it now: https://github.com/WordPress/performance/actions/runs/7733545501/job/21085843025#step:7:99

[westonruter]

Alright, I think it's in a good state now. As can be seen in the PHP 7.0 job for running PHPUnit, if you run composer update you now get the following in the terminal:

Skipping composer update for build-cs since not on PHP 8.1+. You are running: 
PHP 7.0.33 (cli) (built: Dec 21 2018 03:17:53) ( NTS )
Copyright (c) 1997-2017 The PHP Group
Zend Engine v3.0.0, Copyright (c) 1998-2017 Zend Technologies
    with Zend OPcache v7.0.33, Copyright (c) 1999-2017, by Zend Technologies