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

PHP 8.3 + 8.4 to master branch #151

Merged
merged 23 commits into from
Mar 27, 2024
Merged

PHP 8.3 + 8.4 to master branch #151

merged 23 commits into from
Mar 27, 2024

Conversation

grogy
Copy link
Member

@grogy grogy commented Dec 29, 2023

PHP 8.3 + 8.4 for master branches. It is based from #150

@grogy grogy changed the title Master php 84 PHP 8.3 + 8.4 to master branch Dec 29, 2023
@jrfnl
Copy link
Collaborator

jrfnl commented Dec 29, 2023

This will need the commit from #152 to get a passing build 🤞🏻

jrfnl and others added 23 commits March 27, 2024 12:22
PHP 8.2 has been released today 🎉 and the `setup-php` action has announced support for PHP 8.3, so adding PHP 8.3 to the matrix and no longer allowing PHP 8.2 to fail the build.

Builds against PHP 8.3 are still allowed to fail for now.
... which is expected later today.
By putting the class name, which contains PHP 5.3 namespace separator tokens, as a variable and then dynamically calling the class, the parse error on PHP < 5.3 is prevented and the version check at the top of the file can work as expected.

Fixes 62
Previously, the PHP version check run at the start of the script would display `PHP Parallel Lint requires PHP 5.3.0 or newer.` if the PHP version was too low.

The message has been updated to include the currently detected PHP version and the path to the PHP binary.

Note: both constants used are available in PHP cross-version.
A number of predefined actions have had major release, which warrant an update to the workflow(s).

These updates don't actually contain any changed functionality, they are mostly just a change of the Node version used by the action itself (from Node 14 to Node 16).

Refs:
* https://github.com/actions/checkout/releases
* https://github.com/actions/download-artifact/releases
* https://github.com/actions/upload-artifact/releases
Generally speaking, the `$translate` parameter for the `SyntaxError::getNormalizedMessage()` method should only be passed as `true` when on a PHP version which doesn't do the PHP native token translation yet.

However, in edge cases, it could be possible that tokens could be double "translated", both by PHP itself as well as by the `SyntaxError::translateTokens()` method.

This minor fix prevents this by not matching token names when surrounded by parentheses.
…egex delimiter

If a file name would contain the delimiter used in the replacement regex, it would cause a PHP error like `preg_replace(): Unknown modifier 'f'`.

Fixed by telling `preg_quote()` explicitly which delimiter is being used.

Includes unit test.

Ref: https://www.php.net/manual/en/function.preg-quote.php
…n't always get stripped

In certain cases, PHP puts the full file name in the error message. In those cases the "in filename.php on line .." trailing part of the error message did not get stripped off.

Also, in some cases, when Windows slashes are used in the file path, the "in filename.php on line .." trailing part of the error message may not get stripped off.
This last case will be exceedingly rare as on Windows, those file paths are handled correctly and the chances of a non-Windows user passing a path using Windows slashes will be minuscule.

Even so, I've fixed both cases by making the path handling in the function more robust.
* When the initial stripping of the trailing part of the error message fails, it will be retried up to two times.
* The first time, if Windows slashes would be found in the file path _after_ the `basename()` function has been applied, a "manual" basename extraction is done and the stripping of the trailing part is retried.
* The second time, the full file path is used in a last attempt to strip the trailing part of the error message.

Fixes 94
Looks like the box project has moved organisations, so let's update the URL used to download the PHAR.

As GitHub will automatically redirect moved URLs, I'm only pulling this to the `develop` branch.
If there is a ruleset error, the `cs2pr` action doesn't receive an `xml` report and exits with a `0` error code, even though the PHPCS run failed (though not on CS errors, but on a ruleset error).

This changes the GH Actions workflow to allow for that situation and still fail the build in that case.
PHP 8.2 has been released today 🎉 and the `setup-php` action has announced support for PHP 8.3, so adding PHP 8.3 to the matrix and no longer allowing PHP 8.2 to fail the build.

Builds against PHP 8.3 are still allowed to fail for now.

Includes minor tweak setting PHP to `latest` for tasks where the PHP version isn't that relevant.
Caches used in GH Actions do not get updated, they can only be replaced by a different cache with a different cache key.

Now the predefined Composer install action this repo is using already creates a pretty comprehensive cache key:

> `ramsey/composer-install` will auto-generate a cache key which is composed of
the following elements:
> * The OS image name, like `ubuntu-latest`.
> * The exact PHP version, like `8.1.11`.
> * The options passed via `composer-options`.
> * The dependency version setting as per `dependency-versions`.
> * The working directory as per `working-directory`.
> * A hash of the `composer.json` and/or `composer.lock` files.

This means that aside from other factors, the cache will always be busted when changes are made to the (committed) `composer.json` or the `composer.lock` file (if the latter exists in the repo).

For packages running on recent versions of PHP, it also means that the cache will automatically be busted once a month when a new PHP version comes out.

### The problem

For runs on older PHP versions which don't receive updates anymore, the cache will not be busted via new PHP version releases, so effectively, the cache will only be busted when a change is made to the `composer.json`/`composer.lock` file - which may not happen that frequently on low-traffic repos.

But... packages _in use_ on those older PHP versions - especially dependencies of declared dependencies - may still release new versions and those new versions will not exist in the cache and will need to be downloaded each time the action is run and over time the cache gets less and less relevant as more and more packages will need to be downloaded for each run.

### The solution

To combat this issue, a new `custom-cache-suffix` option has been added to the Composer install action in version 2.2.0.
This new option allows for providing some extra information to add to the cache key, which allows for busting the cache based on your own additional criteria.

This commit implements the use of this `custom-cache-suffix` option for all relevant workflows in this repo.

Refs:
* https://github.com/ramsey/composer-install/#custom-cache-suffix
* https://github.com/ramsey/composer-install/releases/tag/2.2.0
The recent build failure was due to the `setup-php` action running into a rate limit and not downloading the required version of Composer (`2.2`), which meant that the PHAR ended up being build with Composer 2.4.

The `setup-php` action runner defaults to _showing_ these type errors in the logs, but not stopping the workflow run.

For the creation of the binary it is really important that the correct Composer version is used as otherwise the PHAR file won't be compatible with PHP 5.3 - 5.5.

So, specifically for those jobs, I'm adding the `fail-fast` option to `setup-php` to fail the build if the action runner ran into any errors.

Ref: https://github.com/shivammathur/setup-php#fail-fast-optional
Follow up on PR #131

Oops... realized I implemented it incorrectly. Fixed now.
Left pad with whitespace so that the 100% line doesn't pop out of alignment.
Since Composer 2.4.0, Composer will prompt users if they are sure they want to install something as `require` instead of `require-dev` if the package contains certain "dev" related keyword, like `static analysis`.

This adds the keywords to this package to help users add the package in the most appropriate place.

Refs:
* https://getcomposer.org/doc/04-schema.md#keywords
* composer/composer#10960

Co-authored-by: Markus Staab <maggus.staab@googlemail.com>
Fixes all issues that emit deprecation notices on PHP 8.4 for implicit nullable parameter type declarations.

See:
 - [RFC](https://wiki.php.net/rfc/deprecate-implicitly-nullable-types)
 - [PHP 8.4: Implicitly nullable parameter declarations deprecated](https://php.watch/versions/8.4/implicitly-marking-parameter-type-nullable-deprecated)
The `OutputTest::testGitLabOutput()` was being marked as risky for the first test case (no errors) as no assertions were being run.

This commit adds an extra assertion to prevent this, but also to make the test more thorough as the test will now also fail if the number of errors expected versus received does not match.
A number of predefined actions have had major releases, which warrant an update to the workflow(s).

These updates don't actually contain any changed functionality, they are mostly just a change of the Node version used by the action itself (from Node 16 to Node 20).

Refs:
* https://github.com/actions/checkout/releases
* https://github.com/actions/download-artifact/releases
* https://github.com/actions/upload-artifact/releases
* https://github.com/ramsey/composer-install/releases
* https://github.com/softprops/action-gh-release/releases
As things were, the workflow was creating two releases, one in draft and one published, but without a good name/changelog.
I noticed this last time there was a release, but never got round to fixing it (and it wasn't urgent as no new release was needed).

As a new release is needed soon, now seemed like a good time to fix this.

The `actions/create-release` action runner has been abandoned and mentions the `softprops/action-gh-release` action runner as a replacement.

This commit now updates the workflow to solely use the `softprops/action-gh-release` action runner and updates the version and configuration to make sure it only creates a draft release, so we can still edit it and add the changelog before publishing.
Copy link
Collaborator

@jrfnl jrfnl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed in #155, so all good.

@jrfnl
Copy link
Collaborator

jrfnl commented Mar 27, 2024

Note: PHP 8.4 build is failing due to incompatibilities in the Nette test framework, not due to incompatibilities in PHP Parallel Lint.

@jrfnl jrfnl merged commit 359379e into master Mar 27, 2024
16 of 17 checks passed
@jrfnl jrfnl deleted the master-php-84 branch March 27, 2024 12:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

5 participants