Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid run of Request handle() and use of container at compiler pass to fix issues 3091 and 3553 #3558

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 32 additions & 2 deletions src/Bootstrap/AddServicesCompilerPass.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
use Drupal\Console\Utils\TranslatorManager;
use Drupal\Console\Extension\Extension;
use Drupal\Console\Extension\Manager;
use Drupal\Core\Cache\ListCacheBinsPass;
use GuzzleHttp\Client;

/**
* FindCommandsCompilerPass
Expand Down Expand Up @@ -77,15 +79,35 @@ public function process(ContainerBuilder $container)
* @var Site $site
*/
$site = $container->get('console.site');
\Drupal::getContainer()->set(
$container->set(
'console.root',
$this->root
);

// The AddServicesCompilerPass cache pass is executed before the
// ListCacheBinsPass causing exception: ParameterNotFoundException: You
// have requested a non-existent parameter "cache_default_bin_backends"
$cache_pass = new ListCacheBinsPass();
$cache_pass->process($container);

// Fix ContainerNotInitializedException: \Drupal::$container is not initialized yet.
// \Drupal::setContainer() must be called with a real container. The use of stream_wrapper.temporary
// service at cachedServicesFileExists method uses the container that in compiler pass
// is not ready to use: https://github.com/symfony/symfony/issues/22125#issuecomment-288734689
// This is workaroud works but seems that we cannot depend on services at this stage.
\Drupal::setContainer($container);

// Avoid Error: Call to undefined function
// Drupal\Core\StreamWrapper\file_directory_temp() in TemporaryStream
// that seems to be needed to the cache services and when no other
// stream wrapper is available defaults ont TemporaryStream.
$site->loadLegacyFile('/core/includes/file.inc');

if (!$this->rebuild && $site->cachedServicesFileExists()) {
$loader->load($site->getCachedServicesFile());
} else {
$site->removeCachedServicesFile();

$finder = new Finder();
$finder->files()
->name('*.yml')
Expand All @@ -105,10 +127,18 @@ public function process(ContainerBuilder $container)
);
}

// Avoid to use the container to get the console.extension_manager due
// container is not ready and cause exceptions.

/**
* @var GuzzleHttp\Client $httpClient
*/
$httpClient = new Client;
/**
* @var Manager $extensionManager
*/
$extensionManager = $container->get('console.extension_manager');
$extensionManager = new Manager($site, $httpClient, $this->appRoot);

/**
* @var Extension[] $modules
*/
Expand Down
21 changes: 20 additions & 1 deletion src/Bootstrap/Drupal.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
use Drupal\Console\Core\Utils\ArgvInputReader;
use Drupal\Console\Core\Bootstrap\DrupalConsoleCore;
use Drupal\Console\Core\Utils\DrupalFinder;
use Drupal\Component\FileCache\FileCacheFactory;
use Drupal\Core\Site\Settings;

class Drupal
{
Expand Down Expand Up @@ -143,9 +145,26 @@ public function boot()
$io->writeln("\r\033[K\033[1A\r<info>✔</info>");
$io->writeln('➤ Rebuilding container');
}

// Fix an exception of FileCacheFactory not prefix not set when
// container is build and looks that as we depend on cache for
// AddServicesCompilerPass but container is not ready this prefix
// needs to be set manually to allow use of the cache files.
FileCacheFactory::setPrefix($this->drupalFinder->getDrupalRoot());

// Invalidate container to ensure rebuild of any cached state
// when boot is processed.
$drupalKernel->invalidateContainer();
$drupalKernel->rebuildContainer();

// Looks that the boot process is handling an initializeContainer
// so looks that rebuildContainer repeats what we finally do in boot().
//$drupalKernel->rebuildContainer();

// Load legacy libraries, modules, register stream wrapper, and push
// request to request stack but without trigger processing of '/'
// request that invokes hooks like hook_page_attachments().
$drupalKernel->boot();
$drupalKernel->preHandle($request);

if ($debug) {
$io->writeln("\r\033[K\033[1A\r<info>✔</info>");
Expand Down
12 changes: 11 additions & 1 deletion src/Bootstrap/DrupalKernel.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,17 @@ public static function createFromRequest(Request $request, $class_loader, $envir
$kernel = new static($environment, $class_loader, $allow_dumping, $app_root);
static::bootEnvironment($app_root);
$kernel->initializeSettings($request);
$kernel->handle($request);
// Calling the request handle causes that a page request "/" is
// processed for any console execution even: help or --version and
// with sites that have globally displayed blocks contexts are not
// ready for blocks plugins so this causes lot of problems like:
// https://github.com/hechoendrupal/drupal-console/issues/3091 and
// https://github.com/hechoendrupal/drupal-console/issues/3553 Also
// handle does a initializeContainer which originally was invalidated
// and rebuild at Console Drupal Bootstrap. By disabling handle
// and processing the boot() at Bootstrap commands that do not
// depend on requests works well.
//$kernel->handle($request);
return $kernel;
}

Expand Down
29 changes: 22 additions & 7 deletions src/Extension/Manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,6 @@ private function getExtensions(
$extensions[$name] = $extension;
}


return $nameOnly?array_keys($extensions):$extensions;
}

Expand All @@ -221,11 +220,6 @@ private function getExtensions(
*/
private function discoverExtensions($type)
{
if ($type === 'module') {
$this->site->loadLegacyFile('/core/modules/system/system.module');
system_rebuild_module_data();
}

if ($type === 'theme') {
$themeHandler = \Drupal::service('theme_handler');
$themeHandler->rebuildThemeData();
Expand All @@ -237,8 +231,29 @@ private function discoverExtensions($type)
*/
$discovery = new Discovery($this->appRoot);
$discovery->reset();
$extensions = $discovery->scan($type);

if ($type === 'module') {
// Using system_rebuild_module_data causes an error:
// Constructing service "logger.factory" from a parent definition is not supported at build time.
//$this->site->loadLegacyFile('/core/modules/system/system.module');
//system_rebuild_module_data();

// Looks that dependency on rebuild module data is just to determine
// the installed status so alternatively we can just look on installed
// modules config and apply to discovered extensions.
$installed_modules = \Drupal::config('core.extension')->get('module') ?: [];

/**
* @var \Drupal\Core\Extension\Extension $extension
*/
foreach ($extensions as $name => $extension) {
$extensions[$name]->weight = isset($installed_modules[$name]) ? $installed_modules[$name] : 0;
$extensions[$name]->status = (int) isset($installed_modules[$name]);
}
}

return $discovery->scan($type);
return $extensions;
}

/**
Expand Down