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 in these lines of FrameworkExtension, 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 to Extension#load(), whereas services aren't.

And btw, this can only be done with kernel.* 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 of Extension::load() is not the actual container, it's a copy. See https://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') will always 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: The data_collector.security service ("which is always loaded" L66) require debug.security.access.decision_manager, so when $container->getParameter('kernel.debug') is false 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 here https://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 the collectors.xml file unconditionally? Is it really used when debug = false ? If this can be changed, the solution would be trivial: move $loader->load('collectors.xml'); under the if ($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 check web_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 detecting profiler (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 like profiler.collect into FrameworkExtension::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 if profiler service is not defined, so data_collector.security require of debug.security.access.decision_manager

Could we solve this via config?

<argument type="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 the security.access.decision_manager is properly decorated, just pass it to the collector instead of the debug.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 with hasParameter() since some tests are still failing

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

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