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

Allow PHP 8 in 2.x #980

Closed
wants to merge 6 commits into from
Closed

Allow PHP 8 in 2.x #980

wants to merge 6 commits into from

Conversation

greg0ire
Copy link
Member

Q A
Type improvement
BC Break no
Fixed issues n/a

Summary

We want to let downstream project experiment while we fix our issues if we have some. Failures are allowed for now, and should no longer be once feature freeze starts.

@greg0ire greg0ire marked this pull request as draft May 26, 2020 19:33
@greg0ire
Copy link
Member Author

There is currently no version of doctrine/migrations that supports DBAL 3, so maybe let's wait until we do have one and merge this in that branch instead of committing to supporting php 8 on 2.2.x. cc @alcaeus

@goetas
Copy link
Member

goetas commented May 27, 2020

does dbal 2 allow php?

@greg0ire
Copy link
Member Author

greg0ire commented May 27, 2020

All versions of DBAL require php :trollface:
but if you mean php 8, then no, it's only DBAL 3, since a BC-break is required.

@greg0ire greg0ire linked an issue Aug 23, 2020 that may be closed by this pull request
@ausi ausi mentioned this pull request Sep 10, 2020
42 tasks
@peterrehm
Copy link

@greg0ire As dbal 2 is compatible with PHP8, is there anything blocking a version of migrations and migrations-bundle for php8?

@greg0ire
Copy link
Member Author

In theory no, good point! In practice, the build is currently broken, so I will get on to it when #1069 is resolved.

@greg0ire
Copy link
Member Author

greg0ire commented Nov 8, 2020

@peterrehm actually, we depend on ocramius/proxy-manager, and I just noticed it does not seem to have a version compatible with PHP 8 😬

UPD: see Ocramius/ProxyManager#628

@greg0ire
Copy link
Member Author

That hurdle is passed now, the next one is doctrine/orm#8303

@nicwortel
Copy link

@greg0ire hi! I'm happy to see that work is already in progress to have doctrine/migrations support PHP 8.0.

doctrine/orm#8303 is merged now. Am I correct in assuming that a new version of doctrine/orm has to be released before doctrine/migrations can support PHP 8.0? Is there anything I can do to help? Thanks!

@greg0ire
Copy link
Member Author

greg0ire commented Dec 1, 2020

Hi @nicwortel! You are correct.

Is there anything I can do to help?

As a matter of fact yes!

In this list there are some packages that still use Travis CI. We are going to migrate them to Github Actions, which is a big undertaking, but in the meantime, if you want to add a build job for image 8.0snapshot that does not allow failure, that would be great! It's as easy as adding one item to this list: https://github.com/doctrine/event-manager/blob/348f72ec92c7e0b1f5462f6616af2b448ba3cbb1/.travis.yml#L13

If you do want to help with this, please try on one package first, get a review, and then if it gets merged, proceed with the other PRs.

@nicwortel
Copy link

In this list there are some packages that still use Travis CI. We are going to migrate them to Github Actions, which is a big undertaking, but in the meantime, if you want to add a build job for image 8.0snapshot that does not allow failure, that would be great! It's as easy as adding one item to this list: https://github.com/doctrine/event-manager/blob/348f72ec92c7e0b1f5462f6616af2b448ba3cbb1/.travis.yml#L13

If you do want to help with this, please try on one package first, get a review, and then if it gets merged, proceed with the other PRs.

Sure! So if I understand you correctly, you're asking me to add 8.0snapshot (since 8.0 isn't available yet) jobs to the Travis CI builds of the repositories listed in the "Allow PHP8 and test packages with it" card that don't have a checked checkbox yet, correct? So currently that would be event-manager, data-fixtures, lexer, migrations, DoctrineMigrationsBundle and DoctrineMongoDBBundle.

afbeelding

Would you like me to try to immediately migrate from Travis CI to GitHub Actions in the same PRs (assuming it's doable and the PRs don't become too big) or would you prefer that to happen in separate PRs?

@greg0ire
Copy link
Member Author

greg0ire commented Dec 1, 2020

You understood correctly what I was asking 🙂
I'm not even sure the builds currently pass because of Composer 2, so try small at first please, you might be surprised to see how it grows into "upgrade the coding standard library", and so on and so on.

@fpesch
Copy link

fpesch commented Dec 4, 2020

any eta on this?

@greg0ire
Copy link
Member Author

greg0ire commented Dec 4, 2020

@bpesch yes: it's the ETA of all the blockers + 1 day I suppose.

@maxhelias
Copy link

maxhelias commented Dec 10, 2020

It seems that only Ocramius/ProxyManager#628 left which blocks. If you need help, maybe I can try something ?

@greg0ire
Copy link
Member Author

@maxhelias thanks but as you pointed out, we are blocked, and we don't need help being blocked, we can do that ourselves 😄

@ghost
Copy link

ghost commented Dec 16, 2020

This merge request is cancel ?

@ghost ghost mentioned this pull request Dec 17, 2020
@SenseException
Copy link
Member

Blocked is not cancelled.

@alcaeus
Copy link
Member

alcaeus commented Dec 23, 2020

As was done in #1101, we can use the FriendsOfPHP/proxy-manager-lts package to unblock running on PHP 8. This allows generating proxies on PHP 8, as long as they only use 7.4 language features. Using PHP 8 features (e.g. union return types) will be possible once this is supported in the upstream package. I suggest using the LTS fork in 2.x as well.

@alcaeus alcaeus changed the title Allow PHP 8 Allow PHP 8 in 2.x Dec 23, 2020
@nicolas-grekas nicolas-grekas mentioned this pull request Dec 23, 2020
@alcaeus
Copy link
Member

alcaeus commented Dec 23, 2020

Replaced by #1102.

@alcaeus alcaeus closed this Dec 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for PHP8 in 2.x
8 participants