Problem/Motivation

Let's try removing this odd code...

    // Prime the module list and corresponding Extension objects.
    // @todo Remove System module. Needed because
    //   \Drupal\Core\Datetime\DateFormatter has a (needless) dependency on the
    //   'date_format' entity, so calls to DateFormatter::format() and
    //   DateFormatter::formatInterval() cause a plugin not found exception.
    $this->moduleList = [
      'system' => 0,
    ];
    $this->moduleData = [
      'system' => new Extension($this->root, 'module', 'core/modules/system/system.info.yml', 'system.module'),
    ];

Proposed resolution

Remove the code - it is no longer necessary. The plugin not found exception does not happen.

Remaining tasks

User interface changes

None

Introduced terminology

N/a

API changes

None

Data model changes

None

Release notes snippet

N/a

Issue fork drupal-3554308

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

alexpott created an issue. See original summary.

alexpott opened merge request !13569

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Active » Needs review
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Issue summary: View changes
berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Reviewed & tested by the community

This looks good I think. Not sure I understand why the container dump wasn't necessary before, but it makes sense to add that.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

The reason we have to set $this->containerNeedsRebuild to TRUE is the following code in \Drupal\Core\DrupalKernel::initializeContainer

    // If the module list hasn't already been set in updateModules and we are
    // not forcing a rebuild, then try and load the container from the cache.
    if (empty($this->moduleList) && !$this->containerNeedsRebuild) {
      $container_definition = $this->getCachedContainerDefinition();
    }

So setting this flag to TRUE stops us from trying to get a container definition. In tests this prevents us from trying to access the database when there is potentially none set up. We could remove the empty($this->moduleList) here but probably that's best for a follow-up with a CR etc.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Good to get rid of this finally. Committed/pushed to 11.x, thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

  • catch committed bca5e96f on 11.x 
    Issue #3554308 by alexpott, berdir, nicxvan: Fix @todo in...

catch closed merge request !13569

  • catch committed 36b823e7 on 11.3.x 
    Issue #3554308 by alexpott, berdir, nicxvan: Fix @todo in...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.