Problem/Motivation

\Drupal\Core\Command\DrupalApplication::bootstrap() contains the following code:

      // We need to not load a cached copy of the container from disk. For
      // example, inside Kernel tests, we need to fully build the container so
      // we discover and register commands, instead of reusing the container
      // from the Kernel test itself. Therefore, we pass `FALSE` for the
      // `$allow_dumping` parameter here.
      $kernel = new DrupalKernel('prod', $this->classloader, FALSE);

Why should the command line use a ContainerBuilder container? This does not feel correct.

Proposed resolution

The reason why this was necessary for kernel testing is because of the changes made by \Drupal\KernelTests\KernelTestBase::register to the container.

Use the context to be able to set things about the kernel to make testing commands in KernelTestBase simple but still use a regular dumping kernel when not testing.

Remaining tasks

User interface changes

None

Introduced terminology

N/a

API changes

N/a

Data model changes

N/a

Release notes snippet

N/a

Issue fork drupal-3611084

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:

Comments

alexpott created an issue. See original summary.

alexpott’s picture

Issue summary: View changes
alexpott’s picture

Status: Active » Needs review
catch’s picture

Pretty sure drush uses container builder, at least this is what it looked like when I profiled a couple of drush commands last week, so this should make cli operations feel a lot snappier across the board.

MR looks very tidy to me.

alexpott’s picture

Issue summary: View changes
moshe weitzman’s picture

Status: Needs review » Reviewed & tested by the community

I independently came up with the same fix at #3594334: Add a module and theme install command 🙌. So now that issue is blocked by this one

  • catch committed af82c328 on 11.4.x
    fix: #3611084 DrupalApplication should use the compiled container
    
    By:...

  • catch committed 1392f07b on 11.x
    fix: #3611084 DrupalApplication should use the compiled container
    
    By:...

  • catch committed 854ccc02 on main
    fix: #3611084 DrupalApplication should use the compiled container
    
    By:...

catch’s picture

Version: main » 11.4.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to main and cherry-picked to 11.x and 11.4.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.

Status: Fixed » Closed (fixed)

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