Contrib.social

Constant definitions in .module break advanced container usage

Open on Drupal.org →
Created on 18 January 2024, 11 months ago
Updated 22 January 2024, 11 months ago

Problem/Motivation

In another module, we defined another container compiler pass which derives additional services based on those gathered in the first pass; however, as part of the process, alters are fired, which attempts to search enabled modules for hooks.

When it loads `search_api_fast.module` to search for hooks, it encounters the `\Drupal::config()` calls which attempt to make use of the service container from `\Drupal`; however, the service container is not yet set on the `\Drupal` class, and so it fails to entirely build the container, whitescreening/dumping a stacktrace.

Steps to reproduce

- Implement a compiler pass which attempts to make use of previous pass results and fire hooks
- an example in https://github.com/adam-vessey/alter_in_container_compilation
- attempt to rebuild Drupal's cache

Presently, encounter a stacktrace such as:

In Drupal.php line 169:

  [Drupal\Core\DependencyInjection\ContainerNotInitializedException]
  \Drupal::$container is not initialized yet. \Drupal::setContainer() must be called with a real container.


Exception trace:
  at /var/www/html/web/core/lib/Drupal.php:169
 Drupal::getContainer() at /var/www/html/web/core/lib/Drupal.php:411
 Drupal::config() at /var/www/html/web/modules/contrib/search_api_fast/search_api_fast.module:26
 include_once() at /var/www/html/web/core/lib/Drupal/Core/Extension/Extension.php:153
 Drupal\Core\Extension\Extension->load() at /var/www/html/web/core/lib/Drupal/Core/Extension/ModuleHandler.php:128
 Drupal\Core\Extension\ModuleHandler->load() at /var/www/html/web/core/lib/Drupal/Core/Extension/ModuleHandler.php:141
 Drupal\Core\Extension\ModuleHandler->loadAll() at /var/www/html/web/core/lib/Drupal/Core/Extension/ModuleHandler.php:152
 Drupal\Core\Extension\ModuleHandler->reload() at /var/www/html/web/core/lib/Drupal/Core/Extension/ModuleHandler.php:311
 Drupal\Core\Extension\ModuleHandler->buildHookInfo() at /var/www/html/web/core/lib/Drupal/Core/Extension/ModuleHandler.php:296
 Drupal\Core\Extension\ModuleHandler->getHookInfo() at /var/www/html/web/core/lib/Drupal/Core/Extension/ModuleHandler.php:631
 Drupal\Core\Extension\ModuleHandler->buildImplementationInfo() at /var/www/html/web/core/lib/Drupal/Core/Extension/ModuleHandler.php:596
 Drupal\Core\Extension\ModuleHandler->getImplementationInfo() at /var/www/html/web/core/lib/Drupal/Core/Extension/ModuleHandler.php:386
 Drupal\Core\Extension\ModuleHandler->invokeAllWith() at /var/www/html/web/core/lib/Drupal/Core/Extension/ModuleHandler.php:408
 Drupal\Core\Extension\ModuleHandler->invokeAll() at /var/www/html/web/modules/contrib/alter_in_container_compilation/src/DemoCompilerPass.php:19
[...]

Proposed resolution

There's been motion to move "global" constants from .module files over to interfaces: https://www.drupal.org/project/drupal/issues/2807785 →

One of:
- Delete the definition of these dynamically-defined constants
- Define these constants using a mechanism that's aware of the state of the container, possibly some event handler (is there an appropriate kernel event that could be targeted?)
- Something else?

Remaining tasks

User interface changes

API changes

Data model changes

🐛 Bug report
Status

Closed: works as designed

Version

2.0

Component

Code

Created by

🇨🇦Canada adam-vessey PE, Canada

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @adam-vessey
  • Comment 11 months ago →
    🇨🇦Canada adam-vessey PE, Canada

    Presently, we have elected to patch around this by jamming a test in the top of `search_api_fast.module`; however, this may result in these constants in question being left unset due to the use of `include_once` being used to load the `.module` files.

    The "test" in question, attached, essentially just a:

    if (!\Drupal::hasContainer()) {
  return;
}

    at the top of the file, as the .module does nothing else than define these constants, presently.

  • Comment 11 months ago →
    🇨🇦Canada adam-vessey PE, Canada

    On further investigation, it looks like it was breaking things in other weird and wonderful ways, such that `updb` and the like would break due to route rebuilding, so maybe the "advanced usage" we were attempting is just not supported? Moving away from the additional extension pass, so this would no longer be an issue.

    That said, removing/deprecating these constants may still be a good idea?

  • Status changed to Closed: works as designed 11 months ago
  • Comment 11 months ago →
    🇦🇺Australia dpi Perth, Australia

    Overall alter in container compilation sounds like a bad idea.

    Subscribers of events or hooks should expect the application/Drupal is fully booted. Im sure ModuleHandler even has these expectations.

    Sadly I dont think your module is viable, and so this issue should be works-as-designed.

    As for the altering-things-in-service-containers concept, I'd suggest prioritized (weighted) compiler passes: e.g https://git.drupalcode.org/project/sm/-/blob/1.x/src/SmServiceProvider.p...

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024