this is broken, as it will always be false here (DI extensions are isolated from each other). You may need to use a compiler pass instead

This looks overkill to me. Isn't there any other solution? In other extensions, for example inthese lines ofFrameworkExtension, they use the same code as here:

if ($container->getParameter('kernel.debug')) {$loader->load('templating_debug.xml');// ...}

The big difference is that parameters are copied to the local container passed toExtension#load(), whereas services aren't.

And btw, this can only be done withkernel.* parameters, as these are set by the Kernel. All other parameters will only be available based on the order of the registered bundles in AppKernel: You don't want to depend on the order.

@wouterj I don't understand your comment. In any case, I asked why this is wrong:

if ($container->getParameter('kernel.debug')) {$loader->load('security_debug.xml');}

When this is considered right:

if ($container->getParameter('kernel.debug')) {$loader->load('templating_debug.xml');// ...}

It's about this:

if ($container->has('profiler')) {// ...}

Versus this:

if ($container->getParameter('kernel.debug')) {// ...}

The$container parameter ofExtension::load() isnot the actual container, it's a copy. Seehttps://github.com/symfony/dependency-injection/blob/master/Compiler/MergeExtensionConfigurationPass.php#L47-L49 Only the parameters are copied from actual to tmp container, services aren't.

This is why$container->has('profiler') willalways return false.

Sure. I didn't use$container->has('profiler') at first. I used$container->getParameter('kernel.debug') but I was told that it was not completely right. But I still don't understand, so I can't fix it.

@javiereguiluz the issue is that kernel.debug is only the default value for enabling the profiler. It is possible to enable it in non-debug mode.

And this mismatch is precisely one of the issues you have to fix, as the profiler always assumes that the debug access decision manager is there.
The alternative would be to handle properly the absence of this service (storing less info in the profiler panel)

@javiereguiluz I tried this PR and the issue still exists :(

https://github.com/symfony/symfony/blob/master/src/Symfony/Bundle/SecurityBundle/Resources/config/collectors.xml#L13: Thedata_collector.security service ("which is always loaded" L66) requiredebug.security.access.decision_manager, so when$container->getParameter('kernel.debug') isfalse this service isn't loaded:

php bin/console --env=dev --no-debug  [Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException]  The service"profiler" has a dependency on a non-existent service"debug.security.access.decision_manager".

The road should be herehttps://github.com/symfony/symfony-standard/issues/968#issuecomment-223022665:

@stof: ... It should be based on whether the profiler is enabled instead of being based on the debug mode ...

Is it OK to load thecollectors.xml file unconditionally? Is it really used whendebug = false ? If this can be changed, the solution would be trivial: move$loader->load('collectors.xml'); under theif ($container->getParameter('kernel.debug')) condition.

well, if you do it, the security panel would not appear when enabling the profiler in no-debug mode, which might be confusing

Could we checkweb_profiler.controller.profiler service definition ? it's defined only if WebProfileBundle is enabled. BUT, this would imply that all panels profile should be loaded only if this "WebProfileBundle" is enabled ? Currenly this is not taken into consideration in others collectors.

@yceruto WebProfilerBundle is not related to enabling the profiler (the bundle does not provide the profiler. It is only about providing a visualization UI).
The proper service would be detectingprofiler (coming from FrameworkBundle), but this cannot be done in a DI extension, because DI extension cannot see services defined in other extensions (see the previous discussion)

@stof you're right, then

#18934 (comment)

... Only the parameters are copied from actual to tmp container ...

We can create a new parameter something likeprofiler.collect intoFrameworkExtension::registerProfilerConfiguration ?:

$container->setParameter('profiler.collect',$config['collect']);

and use it here ?

if ($container->hasParameter('profiler.collect') &&$container->getParameter('profiler.collect')) {$loader->load('collectors.xml');$loader->load('security_debug.xml');}

@javiereguiluz WDYT?

@yceruto my guess is that they won't accept to introduce a parameter for this. Let's try to do the compilar pass that@stof suggested in the first place.

@javiereguiluz the issue still occurs ifprofiler service is not defined, sodata_collector.security require ofdebug.security.access.decision_manager

Could we solve this via config?

<argumenttype="service"id="debug.security.access.decision_manager"on-invalid="ignore" />

Or maybe just load the collector in debug?

@HeahDude apparently we can't do that. See#18934 (comment)

Ok, I see another solution, now that thesecurity.access.decision_manager is properly decorated, just pass it to the collector instead of thedebug.security.access.decision_manager,this line should ensure everything is fine, right?

👍 Let's try that! Thanks.

Why did you remove the debug check?

#18934 (comment)

Yes but now the check is done in the collector, so no need to load the debug adm when not in debug mode.

You should keep the check withhasParameter() since some tests are still failing

if the compiler pass was removed then when this argument is changed bydebug.security.access.decision_manager ?
I missing something ?

@yceruto the service is decorated when debug is on by loadingsecurity_debug.xml.