Problem/Motivation

When installing with a Drupal site which has it's database connection predefined for SQLite (and maybe without), the Drupal site crashes due to the file_system service not being available

<br />
<b>Fatal error</b>:  Uncaught Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException: You have requested a non-existent service &quot;file_system&quot;. in /persist/drupal/vendor/symfony/dependency-injection/ContainerBuilder.php:992
Stack trace:
#0 /persist/drupal/vendor/symfony/dependency-injection/ContainerBuilder.php(568): Symfony\Component\DependencyInjection\ContainerBuilder-&gt;getDefinition('file_system')
#1 /persist/drupal/vendor/symfony/dependency-injection/ContainerBuilder.php(531): Symfony\Component\DependencyInjection\ContainerBuilder-&gt;doGet('file_system', 1)
#2 /persist/drupal/web/core/lib/Drupal.php(197): Symfony\Component\DependencyInjection\ContainerBuilder-&gt;get('file_system')
#3 /persist/drupal/web/core/modules/sqlite/src/Driver/Database/sqlite/Install/Tasks.php(79): Drupal::service('file_system')
#4 /persist/drupal/web/core/lib/Drupal/Core/Database/Install/Tasks.php(150): Drupal\sqlite\Driver\Database\sqlite\Install\Tasks-&gt;connect()
#5 /persist/drupal/web/core/includes/install.core.inc(1228): Drupal\Core\Database\Install\Tasks-&gt;runTasks()
#6 /persist/drupal/web/core/includes/install.core.inc(1177): install_database_errors(Array, './sites/default...')
#7 /persist/drupal/web/core/includes/install.core.inc(391): install_verify_database_settings('sites/default')
#8 /persist/drupal/web/core/includes/install.core.inc(119): install_begin_request(Object(Composer\Autoload\ClassLoader), Array)
#9 /persist/drupal/web/core/install.php(48): install_drupal(Object(Composer\Autoload\ClassLoader))
#10 {main}
  thrown in <b>/persist/drupal/vendor/symfony/dependency-injection/ContainerBuilder.php</b> on line <b>992</b><br />

The minimal container created in install_begin_request doesn't setup the service before invoking install_verify_database_settings

Steps to reproduce

Try to install Drupal with an existing sites/default/settings.php that contains

$databases['default']['default'] = array (
  'database' => 'sites/default/files/.sqlite',
  'prefix' => '',
  'driver' => 'sqlite',
  'namespace' => 'Drupal\\sqlite\\Driver\\Database\\sqlite',
  'autoload' => 'core/modules/sqlite/src/Driver/Database/sqlite/',
);

Proposed resolution

Register file_system

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3463572

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

mglaman created an issue. See original summary.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented
Issue summary: View changes
mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented

The comment here in \Drupal\sqlite\Driver\Database\sqlite\Install\Tasks::connect makes no sense:

        // We cannot use \Drupal::service('file_system')->getTempDirectory()
        // here because we haven't yet successfully connected to the database.
        $connection_info['default']['database'] = \Drupal::service('file_system')->tempnam(sys_get_temp_dir(), 'sqlite');

\Drupal\Core\File\FileSystem::getTempDirectory does not use the database. It reads from settings and other static methods.

However, \Drupal\Core\File\FileSystem::tempnam uses stream wrappers which requires a full container.

EDIT: Maybe it's due to this line

    if (empty($temporary_directory)) {
      // If no directory has been found default to 'files/tmp'.
      $temporary_directory = PublicStream::basePath() . '/tmp';
mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented

Okay, given this code:

$connection_info['default']['database'] = \Drupal::service('file_system')->tempnam(sys_get_temp_dir(), 'sqlite');

The result of sys_get_temp_dir() will not use a stream wrapper. \Drupal\Core\File\FileSystem::tempnam is mostly for handling a tempnam the starts with a stream wrapper scheme. We'll never have that. We should just use tempnam directly.

mglaman opened merge request !8913

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented
Status: Active » Needs review
mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented
Status: Needs review » Needs work

This won't work because the connection class uses file_system in createDatabase: https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/sqlit...

  public function createDatabase($database) {
    // Verify the database is writable.
    $db_directory = new \SplFileInfo(dirname($database));
    if (!$db_directory->isDir() && !\Drupal::service('file_system')->mkdir($db_directory->getPathName(), 0755, TRUE)) {
      throw new DatabaseNotFoundException('Unable to create database directory ' . $db_directory->getPathName());
    }
  }
andypost’s picture

andypost
he/him
Primary language Russian
commented
Related issues: +#3039026: Deprecate file_directory_temp() and move to FileSystem service, +#3225473: Use \Drupal\Core\File\FileSystemInterface::getTempDirectory() instead of sys_get_temp_dir() where possible, +#3063704: Deprecate \Drupal\Component\FileSystem\FileSystem::getOsTemporaryDirectory() and replace with sys_get_temp_dir()

I think that's related to #3225473: Use \Drupal\Core\File\FileSystemInterface::getTempDirectory() instead of sys_get_temp_dir() where possible

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented

It's related, and adds more reliable on the file_system (which I think is not avoidable.)

andypost’s picture

andypost
he/him
Primary language Russian
commented
Related issues: +#2724963: Make the public file system an optional configuration

That fix also has own issue) #2724963: Make the public file system an optional configuration

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented
Issue tags: +Starshot blocker
moshe weitzman’s picture

moshe weitzman commented

I've come across this too when testing the Drush site:install command on sqlite.

pameeela’s picture

pameeela commented
Issue tags: -Starshot blocker

No longer a Starshot blocker now that WASM is out.

mxr576’s picture

mxr576
Primary language Hungarian
Location Hungary
commented

No longer a Starshot blocker now that WASM is out.

For other curious ppl like /me who would like to what this means and why: #3486415: Remove the WebAssembly trial

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.