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

Fix for PHP <7.4 compatibility #1183

Merged
merged 1 commit into from
Dec 8, 2023
Merged

Fix for PHP <7.4 compatibility #1183

merged 1 commit into from
Dec 8, 2023

Conversation

fracsi
Copy link
Contributor

@fracsi fracsi commented Dec 6, 2023

Follow up for #1174
Fixes #1182
Arrow functions are introduced in php 7.4 as well.

@chalasr
Copy link
Collaborator

chalasr commented Dec 6, 2023

Would you mind double-checking there is no more similar issues please?

@fracsi
Copy link
Contributor Author

fracsi commented Dec 6, 2023

@chalasr Found an another.
I ran throught the code with PHP 7.2 (min version for Symfony 5.4).
#1041 and #1167 introduced the incompatibilities.

I have no idea why the checks were green for PHP 7.2 and Symfony 5.4

@chalasr
Copy link
Collaborator

chalasr commented Dec 6, 2023

@fracsi Good question, it seems that build is totally broken despite it doesn't show up as such!
https://github.com/lexik/LexikJWTAuthenticationBundle/actions/runs/7107317672/job/19348521987
Gonna look into this asap.

@chalasr
Copy link
Collaborator

chalasr commented Dec 8, 2023

Good catch, thanks @fracsi.

@chalasr chalasr merged commit a4df695 into lexik:2.x Dec 8, 2023
9 checks passed
chalasr added a commit that referenced this pull request Dec 27, 2023
This PR was squashed before being merged into the 2.x branch.

Discussion
----------

[CI] Fixed CI for PHP 7.2

This PR is my attempt at fixing the "always green" build for 7.2 (mentioned in #1183 (comment))

Things done:
1) I've bumped the PHPUnit Bridge version to the latest one.

According to [PHPUnit Bridge doc](https://symfony.com/doc/current/components/phpunit_bridge.html#installation):
```
The PHPUnit bridge is designed to work with all maintained versions of Symfony components, even across different major versions of them. You should always use its very latest stable major version to get the most accurate deprecation report.
```

Allowing phpunit-bridge range of `"^4.4|^5.4|^6.0|^7.0"` together with `--prefer-lowest` means that the 4.4.0 version of it is installed - which doesn't have all the required bugfixes.

I've chosen ^7.0 as the latest one, for the 7.x series 7.0.1 is needed because it contains the bugfix for this issue: symfony/demo#1457 (comment)

2) Set PHPUnit 9.5 as the max version

The 7.2 build cannot use PHPUnit 9.5, [it requires PHP 7.3](https://github.com/sebastianbergmann/phpunit/blob/9.5.0/composer.json#L24)

I've set this as max version instead - it will be used if possible, otherwise lower versions will be used.

3) Bumped lowest supported version of lcobucci/jwt to 3.4.6

One of the tests was failing:
```
1) Lexik\Bundle\JWTAuthenticationBundle\Tests\Functional\GetTokenTest::testGetTokenWithCustomClaim
Failed asserting that two arrays are identical.
--- Expected
+++ Actual
@@ @@
 Array &0 (
     0 => 'foo'
-    1 => 'bar'
 )
```

This was fixed by bumping the version of lcobucci/jwt to 3.4.6 - I suspect it contains bugfix not present in 3.4.0

4) Run PHP 7.2 tests on ubuntu-20

I found this issue when googling: shivammathur/setup-php#720
```
If you are running this on the ubuntu-latest or ubuntu-22.04 runner, that has OpenSSL 3 and support for it was added to PHP in 8.1. Please use ubuntu-20.04 runner with the older PHP versions and this should work.
```

And it was right, all of a sudden the segmentation faults have disappeared.

Overall I feel everything I did was a bit hacky, but the build is now green 🎉

Commits
-------

29c88f2 [CI] Fixed CI for PHP 7.2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Version 2.20+ requiring php 7.1 >= but using arrow functions
3 participants