Symfony6

[Berdir]

WIP, testing symfony 6 for #1588.

This should be compatible with Symfony 5.4 and Symfony 6. Return types are forward-compatible, you can add a return type in a child class if the parent doesn't have one.

RouteCollectionBuilder is deprecated in Symfony 5.1 and RoutingConfigurator exists from that version on.

[Berdir]

Ah, obviously failed on PHP 8.0, I updated composer.lock just for testing purposes, although it will be a bit tricky to stay on 5.x then for the default composer.lock with this I guess.

Have not worked with github actions before, maybe a the existing PHP 8.1 or specific new jobs could do a composer upgrade symfony/* to test Symfony 6 optionally.

[tvdijen]

I think you generated the composer.lock using php 8.1, but it should be run with php 8.0

[Berdir]

Found the remaining issue, the prefix feature still exists but in a different way.

I reverted the composer.lock changes, so this is back on Symfony 5 but I could successfully bootstrap, log in and click around in the admin backend using Symfony 6 locally. I did not do any testing of actual functionality.

[andypost]

Btw how about PHP 8.2 compatibility?

[tvdijen]

Nobody uses PHP 8.2.. It's not released yet.. The linux distributions are at 8.0 max, most are at 7.4

[codecov]

Codecov Report

Merging #1675 (6291fdb) into master (3bb3916) will not change coverage.
The diff coverage is 0.00%.

@@            Coverage Diff            @@
##             master    #1675   +/-   ##
=========================================
  Coverage     42.03%   42.03%           
  Complexity     3673     3673           
=========================================
  Files           156      156           
  Lines         10871    10871           
=========================================
  Hits           4570     4570           
  Misses         6301     6301           

[Berdir]

Oh wow, didn't expect it to happen this quickly. Yeah, Symfony 6 requires 8.0, so you get that if you run it on 8.0.

You could either run with that and require 8.0 (PHP 7.4 EOL is in 3 month) or set a plaform php version: https://andy-carter.com/blog/composer-php-platform, then it should only update packages that are PHP 7.4 compatible but others that require simplesamlphp into their projects can still use it with Symfony 6.

[tvdijen]

It's not EOL if the larger distributions keep patching it.. We try to follow those, so that people don't have to resort to trickery to get newer versions of PHP running on their distro. PHP 7.4 is what we're comfortable with for now.

[Berdir]

Sure, that's your call, but shouldn't the test matrix be expanded to cover PHP 7.4 and ensure that works then? And either run composer update on PHP 7.4 or set the platform php version to ensure that you can build a package that is compatible with that version. And maybe extend the matrix with symfony 5 and 6, so the lock file would ship with 5 and the symfony 6 matrix would explicitly run composer update. If you're interested, I can have a look at that when I find some time.

Thanks for merging the other MR. I managed to require simplesamlphp/simplesamlphp:dev-master into a Drupal 10 project now, only had a soft conflict on symfony/translation-contracts, I had to downgrade that, but that's still within the required constraints of all dependencies.

[tvdijen]

The test-matrix for the simplesamlphp-2.0 branch has PHP 7.4 in it... For master PHP 8.0 is the minimum

[Berdir]

Ok, sorry, not familiar with how you manage branches in this project. When will the current master branch become a release branch? will that be 2.1? or only 3.0?

I'll create another small MR for symfony/translation-contracts, our standard test infrastructure for Drupal doesn't like downgrading packages, so it's not quite working yet, but it's getting close I think:

https://www.drupal.org/project/simplesamlphp_auth/issues/3289683#comment-14650416
https://www.drupal.org/pift-ci-job/2450266

[tvdijen]

When will the current master branch become a release branch? will that be 2.1? or only 3.0?

Could be either one, depending on backward compatibility of the changes done in master.. We've only recently branched of a release-branch for 2.0 and because of the demand for Symfony 6 support, I've backported your PRs there so it will end up in the 2.0 final release.

[tvdijen]

@Berdir How was this supposed to work with PHP 7.4, because it breaks our 2.0 release...
You said "you can add a return type in a child class if the parent doesn't have one", but PHP 7.4 doesn't recognize the static return type.. I'm worried I have to revert this for the 2.0 release and keep it this change in master till the next major.

[Berdir]

Yeah, I didn't realize that you still support 7.4 in 2.0.

That comment was about supporting Symfony 5 and 6, not about PHP 7.4.

I don't really see a solution for that except some nasty tricks with class aliases or dynamic service registration if you want to keep PHP 7.4 support.

One note would be that technically, according to SemVer, you can update your own dependencies including the PHP version: https://semver.org/#what-should-i-do-if-i-update-my-own-dependencies-without-changing-the-public-api in a minor version as long as it doesn't cause API changes in your own API. The return types are a bit of an edge case if people subclass your classes, depends on whether or not you consider that your API. So you could release this as 2.1 or 2.2 if you're ready to drop PHP 7.4 support.

[tvdijen]

Yeah, that's what I was worried about.. I'm gonna revert the change for 2.0, sorry!

The recent release of RHEL 9 (with PHP 8.0) is probably a good reason to bump our minimum to 8.0 within the next couple of months, probably meaning a 3.0 major release, since I consider Template and RunnableResponse part of our public API.

[github-actions]

\n This pull request has been automatically locked since there has \n not been any recent activity after it was closed.\n Please open a new issue for related bugs.