-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[DependencyInjection][FrameworkBundle][SecurityBundle][TwigBundle] Require Composer's runtime API to be present #43788
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
[DependencyInjection][FrameworkBundle][SecurityBundle][TwigBundle] Require Composer's runtime API to be present #43788
Conversation
I'm personally fine doing that but we should throw a deprecation somewhere in 5.4. |
In |
Possibly yes. In a place that allows the deprecation to be collected for the profiler (with other compile-time deprecations). 👍 for Composer 2.1 but let's remove the change on ErrorHandler to allow using the component with less constraints if that doesn't matter much |
Oh btw, if Symfony 6.0 is Composer 2.1 only, we could release Flex 2, which would bump to the same, with a cleaned up code base (eg the http downloader which is now in C2) |
04472b4
to
5993128
Compare
I've bumped to 2.1 and reverted the ErrorHandler changes. |
6783d6a
to
12ae615
Compare
What about moving this to frameworkbundle actually? That would give better guarantees for what we consider "Symfony 6 apps", without imposing a restriction on standalone DI users. |
What do you mean by "this"? The |
I mean the composer constraint |
The usage of the composer runtime api is in the DI component, not in FrameworkBundle. So that's where the dependency is. |
If we keep the "class_exists" check that is in place, DI doesn't have a dependency on C2.1. My fear is that by adding the constraint to DI, ppl that try to upgrade to SF6 with C1 will get dependencies resolved just fine... with DI 5.4... |
But that
… for full-stack Symfony apps. The component standalone would still behave undpredictably if
We could solve that by either adding the Runtime API dependency to FWB as well or by letting FWB conflict with DI 5.4. |
if willBeAvailable() is used yes. this might not be enough a reason to force requiring C2.1 in DI, nor to make FWB 6 conflict with DI 5.4
I'm all for adding the bump to fwb. Not for conflicting. And then I wonder why we should add it to DI. We like optional deps everywhere else. |
Okay, what about my proposal of letting |
Throwing works for me, with a deprecation on 5.4 :) |
Here you go: #43804 |
…[TwigBundle] Deprecate Composer 1 (derrabus) This PR was merged into the 5.4 branch. Discussion ---------- [DependencyInjection][FrameworkBundle][SecurityBundle][TwigBundle] Deprecate Composer 1 | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | no | New feature? | yes | Deprecations? | yes | Tickets | Prepares #43788 | License | MIT | Doc PR | N/A This PR deprecates configuring one of those core bundles that make use of `ContainerBuilder::willBeAvailable()` if dependencies have been installed with Composer 1. `ContainerBuilder::willBeAvailable()` also triggers a deprecation now that will be suppressed in all calls coming from our core bundles' extensions. Commits ------- f058905 Deprecate Composer 1
12ae615
to
7df301a
Compare
Ready. |
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
Outdated
Show resolved
Hide resolved
7df301a
to
2cab2f5
Compare
…ecise about the required Composer version (derrabus) This PR was merged into the 5.4 branch. Discussion ---------- [FrameworkBundle][SecurityBundle][TwigBundle] Be more precise about the required Composer version | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | #43788 (comment) | License | MIT | Doc PR | N/A Commits ------- 295240a Be more precise about the required Composer version
…ecise about the required Composer version (derrabus) This PR was merged into the 5.4 branch. Discussion ---------- [FrameworkBundle][SecurityBundle][TwigBundle] Be more precise about the required Composer version | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | symfony/symfony#43788 (comment) | License | MIT | Doc PR | N/A Commits ------- 295240a4f5 Be more precise about the required Composer version
2cab2f5
to
e08b362
Compare
Thank you @derrabus. |
Recently, I was asked to help debugging a strange behavior of my client's app that surfaced only on some developer machines while others were fine. Turns out, a particular package was installed as a dev dependency (which was fine because we used it for dev tooling only at that time) and the difference between the environments was that the broken ones used Composer 2.
With
ContainerBuilder::willBeAvailable()
, we have introduced logic into the very heart of the framework that exposes significantly different behavior for Composer 1 and 2.With this PR, I'd like to propose to make Composer's runtime API a requirement, essentially making the use of Composer 2 a requirement. Composer 2 has been released over a year ago and by now every developer should have been able to upgrade to version 2. I don't think that this constraint would push the ecosystem too hard.
Let's make everyone's lives easier by moving on to Composer 2.