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

Remove Symfony6 support #1772

Closed

Conversation

theofidry
Copy link
Member

I'm having too much of a hard time in #1765. The root of the issue is:

  • Symfony6 was introduced in Allow Symfony 6 #1606 but without tests
  • Symfony6 was actually never executed ever in the Infection CI, so any issue raised was missed
  • There is signature incompatibilities between Symfony5 and Symfony6, which is a problem for code such as DummyFileSystem (fatal error kind of problem – so any autoloading of the file is breaking things hard)

I think we'll have to either review a few usages, assuming we can make it work, or drop Symfony5 support in the next version.

Meanwhile however I think it's best to (temporarily) disallow Symfony6 to allow to bump the minimum required PHP version to 8.1.

@theofidry theofidry changed the title Disable Symfony6 Remove Symfony6 support Dec 11, 2022
@sanmai
Copy link
Member

sanmai commented Dec 12, 2022

What's the cost of dropping Symfony 5?

@theofidry
Copy link
Member Author

5.4 is an LTS so it will be around for a while, hence I'm not too happy about dropping it. But I also don't really see a way at the moment.

That said, there is this PR and the 8.1 upgrade to do first, maybe someone can find a way to make it work once we are done with that.

@sidz
Copy link
Member

sidz commented Dec 12, 2022

I don't see a reason to drop symfony 6 support. Instead we need to run testsuites against symfony 6 and fix all possible cases.
I suppose we won't release new Infection version without symfony 6 and than reintroduce it.

@theofidry
Copy link
Member Author

I don't see a reason to drop symfony 6 support. Instead we need to run testsuites against symfony 6 and fix all possible cases.

Because I can't make it work 😅

You can check #1765 which attempted to do that. There's been several back & forth. So either someone else give it a stab and succeeds, or we accept this PR and move incrementally in another direction, or this gets stuck for a while. What is clear is that bumping to 8.1 as the min-requirement cannot be done in one go due to a lot of broken jobs in the CI.

@theofidry
Copy link
Member Author

Just to clarify in case it is misunderstood: when I talk of removing and adding back for Symfony6 I mean for the next release, not in a distant future.

@sidz
Copy link
Member

sidz commented Dec 12, 2022

Could you please check my comment in #1765 (comment) ?

@theofidry
Copy link
Member Author

@sidz I did fix it; might need to double check though

@theofidry
Copy link
Member Author

Closing this as we have both #1765 and #1775 attempting to fix it directly, and #1786 drafting another way that would make it easier to bump PHP without bumping Symfony, i.e. allowing #1765 to carry one without the Symfony6 issue being a blocker.

@theofidry theofidry closed this Dec 21, 2022
@theofidry theofidry deleted the feature/disable-symfony6 branch December 21, 2022 22:07
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.

None yet

3 participants