-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[RFC] How to deprecate ContainerBuilder::get before compilation, again? #20643
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
Comments
This would be my choice. In 3 years time with Symfony 2 and up, I've done some spooky things, but I've never needed services at compile-time, there's always another way around it. This is the same advice I always give people on IRC. |
Added an extra option (5), coming from an ongoing discussion in #20533 I tend to go with option 2 as well, however it could be cool to allow configuring those services somehow using a DI container. For example it allows tagging compiler passes.. 😱 how cool is that? |
I'm 👎 for this deprecation. I don't now if we can find some bootstrapping procedure that could allow to safely get some early services to help building the next ones. But for now, I'm really fine with this sentence: "with great power, comes great responsibility". |
To be honest, for the DI component i find it a no-brainer adding it.
The DI component does exactly that (well.. it's not really "safe" at the moment ;-)..., ie. i think doing it in 2 steps makes sense. In theory we could initialize the actual container from the booting container (having a container as a service). Endless possibilities.. |
One idea I have in mind is making it possible to extract a subset from the DI graph. That is ask the container builder for a definition and all its dependencies. This subset (sub-container) could be compiled independently from the passed container builder and the resulting container could then be used to call |
I guess one way could be instead of extending the container, simply decorate it (once done). A few benefits I see here:
|
Imo. we should not rely on a container that is being / going to be build. Resolving dependencies, means it must be compiled, means we're done. Compiling a sub graph may still be an incomplete sub graph from the user pov. We should not mix&match imo. |
Why not using a new constant of |
Added to the list. Could actually work 👍 |
Those type of passes should be limited in the way they can modify the current state or another process of optimization should run (virtually two containers would be |
A while ago I had this problem in a bundle that used compiler passes (since then I replaced the compiler pass with a cache warmer and the problem disappeared). In any case, at that time, the proposal by @HeahDude is exactly what I needed: give me the chance to do things in the container after Symfony has finished all its work and just before the container is frozen. |
I think that pass might actually work out |
@HeahDude shouldn't using |
@GuilhemN the way I see it, it's not enough to determine internally if the pass is able to call |
@HeahDude i get it, looks legit :)
To make it even safer, my proposal would be: when a service is fetched in an If this satisfies us, we could even prevent calling |
@xabbuh this is in another wording what i was trying to discuss in #20533 (comment) . I think i might have not expressed that very well because Before making a final decision on the given options it would be good to have an overview of what can go wrong in the various scenario's. @stof gave some good info on this already. |
@GuilhemN freezing definitions would not solve the issue that you can get weird failures when getting a service which is not ready yet. Extracting a subset of the DI graph would probably not help, as compiling this subset fully would involve running all compiler passes again, which may trigger an infinite loop when reaching the compiler pass calling the service, and defining the subset itself is still as hard considering that the graph changes during each compiler pass (or can change at least, even if some compiler passes don't change it). |
@stof that's why freezing services should only be done after symfony's work. Do you think it's necessary to support fetching services before symfony's work? It looks very unsafe to me. |
@GuilhemN but most compiler passes run before Symfony's work, as adding new dependencies in the graph must be done before optimizing the graph (otherwise, optimizations would break things). |
Not sure, how i see @HeahDude's approach; // before calling the "last" passes
$container = new ContainerBuilder();
$container->merge($actualContainer);
$container->compile();
// further modify $actualContainer, while allowed to get() from $container
// compile $actualContainer as usual Meaning we probably need an additional pass interface, right? Ie. |
@javiereguiluz many compiler passes I saw getting services are using them to configure other services, and so they cannot run after Symfony's work. If you want to run after everything was done in the container compilation, this indeed looks like a use case for a cache warmer rather than a compiler pass. |
@stof that's a fair point. |
@GuilhemN we simply cannot optimize the container earlier, as optimization relies on knowing the dependency graph to identify things which can be inlined or removed. |
Option 1 it is i guess :) |
So, back at square one i guess :)
#20533 reverted the integrity check for accessing services, before the container builder is actually compiled. Making it real tricky, yet convenient. However, it is done for the right reasons, for now.
Where does that leave us? Will it be put back? And what is the alternative/right approach?
Imo. we all agree the check is needed to keep the container trusty, but we lose major flexibility on simple services (ones that can actually be resolved fully at some point before compilation).
Some ideas;
bootstrap
, making the full service available on booting, ie. one config and no duplicate service definitions.The text was updated successfully, but these errors were encountered: