Compare matrix<->Composer versions, maybe exit #73
Conversation
|
Note the difference between the failed workflow in dist-archive-command: 
. and the succeeded-with-individual-failures as this behaved: 
|
| - name: Get composer.json PHP version | ||
| id: parse-composer | ||
| run: | | ||
| echo "COMPOSER_PHP_VERSION=$(jq '.require.php' composer.json | sed 's/[^0-9.|]//g' | jq -Rc 'split("|") | min | tonumber')" >> $GITHUB_OUTPUT; |
There was a problem hiding this comment.
One thought I had was that we should use composer show or composer check-platform-reqs to have composer reconcile the minimum PHP version from all of the dependencies.
However, we could end up in an (unlikely) scenario where the minimum PHP version is sufficiently high, causes all of the tests to be skipped, and the entire build passes without running any tests.
I kinda feel like we should read a MINIMUM_TEST_PHP_VERSION variable from a .env file so it's:
- Explicitly defined.
- Entirely outside of Composer.
There was a problem hiding this comment.
I don't feel strongly about this but since it was already defined in composer.json I used that.
I think the with: ... minimum-php: '7.1' (wp-cli/dist-archive-command@d2adeed) is a better approach than adding a .env file – the minimum-php value can be read before the actions/checkout step, and there is no existing .env file in the repo so I'd hesitate to add it just for this.
Aside, I have been thinking a bit about reading the minimum PHP version from composer.lock, since I often update dependencies while on PHP 8 but have 7.4 in my plugin headers – it'd be a nice automated check before running dist-archive-command.
There was a problem hiding this comment.
@schlessera @swissspidy Do either of you have a preference?
There was a problem hiding this comment.
I guess I'm open to using the require.php in composer.json. We can treat that as the explicit "minimum PHP version".
| - name: Get composer.json PHP version | ||
| id: parse-composer | ||
| run: | | ||
| echo "COMPOSER_PHP_VERSION=$(jq '.require.php' composer.json | sed 's/[^0-9.|]//g' | jq -Rc 'split("|") | min | tonumber')" >> $GITHUB_OUTPUT; |
There was a problem hiding this comment.
I guess I'm open to using the require.php in composer.json. We can treat that as the explicit "minimum PHP version".
| echo "COMPOSER_PHP_VERSION=$(jq '.require.php' composer.json | sed 's/[^0-9.|]//g' | jq -Rc 'split("|") | min | tonumber')" >> $GITHUB_OUTPUT; | ||
|
|
||
| - name: Exit early if matrix PHP version lower than composer.json required PHP version | ||
| run: if [[ $(bc -l <<< "$COMPOSER_PHP_VERSION > $MATRIX_PHP_VERSION") -eq 1 ]]; then exit 1; fi |
There was a problem hiding this comment.
| run: if [[ $(bc -l <<< "$COMPOSER_PHP_VERSION > $MATRIX_PHP_VERSION") -eq 1 ]]; then exit 1; fi | |
| run: if [[ $(bc -l <<< "$COMPOSER_PHP_VERSION > $MATRIX_PHP_VERSION") -eq 1 ]]; then exit 0; fi |
Is this a horrible idea?
There was a problem hiding this comment.
That change would not exit the job (matrix instance run) – return 0 (exit success) is the same as any command being run in that step returning 0, and GitHub Actions continues to the next step on success. We have to return failure to exit the job.
|
Closing in favour of a cleaner approach. |
This exits a workflow run (i.e. an instance of a matrix run) if the PHP version in
composer.jsonis higher than the matrix's PHP version.I'm not certain what the result will say on the PR runs – the overall workflow has a "success" green tick, and the individual skipped runs have the red failure x. The failure email does not get sent.
That might not work exactly as desired, but there doesn't seem to be an early-exit command – note the 750+ likes for that feature request! The REST API and
gh run cancelusegithub.run_idwhich is the workflow id rather thangithub.run_numberwhich is the matrix run instance, so cancelling the PHP 5.6 run cancels all of them.