Add support for PHP 8.0

[xvilo]

This does upgrade the code to allow for PHP 8.0, which can be merged as-is if #579 is something we do not want to add. However, if we want to add #579, then I can update this PR with the accompanying Rector changes :)

[xvilo]

@akondas can you tell me how buddy works, it seems it's not pickup the runner config change. According to the logs, the used CI image is still 7.4

[akondas]

I need to approve these changes, but there is also a problem with the public instance, it needs to be prepared for php 8

[akondas]

I have to think about how to do it so that it would not be downtime, I would have to install php 8 there beforehand, which is actually to be done

[akondas]

I still have something to finish this week, but from next week I will sit down to the repman and I will try to catch up with all PR (not only this one). We will catch up with this - I promise :wink:

[xvilo]

I have to think about how to do it so that it would not be downtime

The best would then be to "PHP": "7.4 || 8.0", merge this, update your vhost with the right PHP(-fpm) instance, and do a reload. Then we can fully move the codebase to 8.0, do you want me to update this one?

For compatibility we could already move CI to 8, so it does the checking and stuffs

[akondas]

yes, good approach, that should allow the update to be done without downtime, thanks for help :+1:

[xvilo]

@akondas I've pushed the suggested update, just have to wait for the CI. If this is green, can you review and merge this PR?

[xvilo]

I'm unsure why the CI fails, it should be fine. It appears it might install the dependencies from some sort of cached vendor dir. I could use some help with this :)

[xvilo]

@akondas can you do another check for this PR?

[xvilo]

@marmichalski / @akondas everything has been addressed, how is this PR looking now?

[xvilo]

@akondas lets finish this?

[akondas]

So from my point of view, plan looks like this:

• allow to use php 8 in composer.json
• switch pipeline to php 8
• upgrade public instance to php 8
• upgrade code (composer.lock) to php 8
• drop support for php 7.4

We are on first point here, so please do not run composer update on php 8.

[Jeroeny]

You can do this in composer.json to keep composer.lock depdencies on 7.4 for now:

"config":
        "platform": {
            "php": "7.4"
        },

[xvilo]

I have applied @Jeroeny's suggestion and installed a 7.4 compatible set of dependencies.

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (423985c) 0.00% compared to head (ca923ae) 99.02%.

:exclamation: Current head ca923ae differs from pull request most recent head 8b24958. Consider uploading reports for the commit 8b24958 to get more accurate results

Additional details and impacted files

@@              Coverage Diff              @@
##             master     #580       +/-   ##
=============================================
+ Coverage          0   99.02%   +99.02%     
- Complexity        0     1907     +1907     
=============================================
  Files             0      301      +301     
  Lines             0     5763     +5763     
=============================================
+ Hits              0     5707     +5707     
- Misses            0       56       +56     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[xvilo]

@akondas everything's green, with support for 8.0

[xvilo]

@akondas do you have some time to take a look at it?

[akondas]

Ok, let's go with this. Please resolve conflicts and we can merge this.

[xvilo]

@akondas I've resolved the merge conflicts. Unfortunately the test pipeline is not passing due to some weird behaviour regarding PHPStan. I suspect it caches some files that has been corrupt, locally it does all work fine

[paulwehage]

Any news here? Would love to use repman with PHP 8.0.

[xvilo]

In the past 24 days there were new merge conflicts. From my side it's good to go, but @akondas needs to give final approval + merge. I've resolved the new conflicts, but I will not be actively watching the status anymore for now (it's taking long enough) and we're running these changes already in prod with our internal fork.

[xvilo]

@akondas I have, again, resolved the merge conflicts. Could we please merge this PR? We've been running this code on our on-premise install for the past 2-3 months and it runs fine :)

[94noni]

@xvilo what about upgrading to php8.1?

[xvilo]

Good sugesstion, however, the upgrade path to 8.1 is a bit harder. I'd like to get this merged first. And honestly I'm wondering why this takes so long 😟

Also, an upgrade to Symfony 6.2 might be warranted. As this is also a fresh and hot release!

[xvilo]

@akondas We're closing in on a year, and have passed the EOL date of PHP 7.4. We've been running this code for almost a year with our private instance and haven't noticed any issues.

Can we get this reviewed and merged?

[xvilo]

@akondas Whats your idea? Do you still want this or not?

[xvilo]

It seems the tests fails due to a small timing issue, I tried to quickly require symfony/clock, but I can't due to https://github.com/repman-io/repman/issues/653

[ymorocutti]

I was able to reproduce multiple times, but I'm not sure it's related to PHP 8. My guess, you're having 2 DateTimes created within an interval of several milliseconds but are not on the same seconds. ie : 10.980 vs 11.002.

@akondas Might be a good idea to merge this one asap, as PHP 7.4 has reach end of life and should no longer be used :)

[xvilo]

Yeah I expected so, but since repman was broken I kept it as is. Again, we’re using this as a fork on our kubernetes environment and all works fine as is!

I would also like to work on PHP 8.1/2 compatibility and an upgrade to Symfony 6.3, but I will wait for this until we’re good with this one

[xvilo]

I thought we had this running internally already, but apparently we didn't yet. I've just switched our internal hosted version to this branch correctly and found one bug which is fixed with https://github.com/repman-io/repman/pull/580/commits/3f6bc36dcd9753d945e402b69e7ad4d668220f2b

For now I'll keep an eye on how well it works through Sentry