Installation fails if a valid config sync directory is defined

Not sure if it's related but I had to prepend CONFIG_SYNC_DIRECTORY with a dot to get it working.

$config_directories = array(
CONFIG_SYNC_DIRECTORY => './some/valid/path',
);

(I only enabled Config Export after a minimal install)

I just encountered this too.
I put this in my default.settings.php

$config_directories = array(
  CONFIG_ACTIVE_DIRECTORY => '../config/active',
  CONFIG_SYNC_DIRECTORY => '../config/sync',
);

and when I ran drush site-install, I got this error:

exception 'Drupal\Core\Database\ConnectionNotDefinedException' with message 'The specified database connection is not defined: default' in /home/myname/workspace/mywebsite/web/core/lib/Drupal/Core/Database/Database.php:366[error]
Stack trace:
#0 /home/myname/workspace/mywebsite/web/core/lib/Drupal/Core/Database/Database.php(171): Drupal\Core\Database\Database::openConnection('default', 'default')
#1 /home/myname/workspace/mywebsite/web/core/lib/Drupal/Core/Config/BootstrapConfigStorageFactory.php(45): Drupal\Core\Database\Database::getConnection()
#2 /home/myname/workspace/mywebsite/web/core/lib/Drupal/Core/Config/BootstrapConfigStorageFactory.php(36): Drupal\Core\Config\BootstrapConfigStorageFactory::getDatabaseStorage()
#3 /home/myname/workspace/mywebsite/web/core/includes/install.core.inc(486): Drupal\Core\Config\BootstrapConfigStorageFactory::get()
#4 /home/myname/workspace/mywebsite/web/core/includes/install.core.inc(113): install_begin_request(Object(Composer\Autoload\ClassLoader), Array)
#5 /home/myname/workspace/mywebsite/vendor/drush/drush/includes/drush.inc(721): install_drupal(Object(Composer\Autoload\ClassLoader), Array)
#6 /home/myname/workspace/mywebsite/vendor/drush/drush/includes/drush.inc(706): drush_call_user_func_array('install_drupal', Array)
#7 /home/myname/workspace/mywebsite/vendor/drush/drush/commands/core/drupal/site_install.inc(78): drush_op('install_drupal', Object(Composer\Autoload\ClassLoader), Array)
#8 /home/myname/workspace/mywebsite/vendor/drush/drush/commands/core/site_install.drush.inc(289): drush_core_site_install_version(NULL, Array)
#9 [internal function]: drush_core_site_install()
#10 /home/myname/workspace/mywebsite/vendor/drush/drush/includes/command.inc(364): call_user_func_array('drush_core_site...', Array)
#11 /home/myname/workspace/mywebsite/vendor/drush/drush/includes/command.inc(215): _drush_invoke_hooks(Array, Array)
#12 [internal function]: drush_command()
#13 /home/myname/workspace/mywebsite/vendor/drush/drush/includes/command.inc(183): call_user_func_array('drush_command', Array)
#14 /home/myname/workspace/mywebsite/vendor/drush/drush/lib/Drush/Boot/BaseBoot.php(65): drush_dispatch(Array)
#15 /home/myname/workspace/mywebsite/vendor/drush/drush/includes/preflight.inc(64): Drush\Boot\BaseBoot->bootstrap_and_dispatch()
#16 /home/myname/workspace/mywebsite/vendor/drush/drush/drush.php(12): drush_main()
#17 {main}

And have you tried this?

https://www.drupal.org/node/2431247#comment-10426127

It worked for me!

Here's a fix that allows the install to continue.

Here's a test.

+++ b/core/includes/install.core.inc
@@ -489,7 +489,12 @@ function install_begin_request($class_loader, &$install_state) {
+    catch (\Exception $e) {
+      // If this fails assume there is no active configuration.
+    }

Can we actually throw a more specific exception which we could catch? Not sure whether it matters though

@dawehner I considered catching the database exception wrapper but then I thought better of it and copied the implementation in DrupalKernel that exists to deal with the early installer...

  protected function getConfigStorage() {
    if (!isset($this->configStorage)) {
      // The active configuration storage may not exist yet; e.g., in the early
      // installer. Catch the exception thrown by config_get_config_directory().
      try {
        $this->configStorage = BootstrapConfigStorageFactory::get($this->classLoader);
      }
      catch (\Exception $e) {
        $this->configStorage = new NullStorage();
      }
    }
    return $this->configStorage;
  }

Ideally the above code would not be in DrupalKernel - but it could be in InstallerKernel and we could make it public and then this could just use that.

I think I'll explore that as a solution.

Yep this is nicer - @dawehner++

Nice!!

1. +++ b/core/includes/install.core.inc
   @@ -489,8 +488,7 @@ function install_begin_request($class_loader, &$install_state) {
   +    if (count($kernel->getConfigStorage()->listAll())) {
   

   Why specifically count() and not !empty()?

2. +++ b/core/lib/Drupal/Core/Installer/InstallerKernel.php
   @@ -33,4 +33,14 @@ public function resetConfigStorage() {
   +  /**
   +   * Returns the active configuration storage used during early install.
   +   *
   +   * @return \Drupal\Core\Config\StorageInterface
   +   *   The config storage.
   +   */
   +  public function getConfigStorage() {
   +    return parent::getConfigStorage();
   +  }
   +
   

   Could we add a note that this overrides the parent method to change the visibility? I had to look that up to see why we were doing it.

3. +++ b/core/modules/system/src/Tests/Installer/InstallerExistingConfigDirectoryTest.php
   @@ -0,0 +1,34 @@
   +/**
   + * Tests the installer with an existing settings file with config_directory set
   + * up.
   + *
   

   This should either be shorter or be allowed to go over 80 chars - or is that only function summaries?

4. +++ b/core/modules/system/src/Tests/Installer/InstallerExistingConfigDirectoryTest.php
   @@ -0,0 +1,34 @@
   +    $this->assertResponse(200);
   

   Let's assert something other than just 200 here since that can be a fatal error.

Thanks for the review @catch

1. Because we're expecting an array and count is for arrays. empty() is far less specific in PHP.
2. Fixed
3. Fixed
4. I'm not sure this is necessary - what we're testing here is that we get past the first two steps of the installer. There's lots of other tests that ensure things work after an install.

Since these are only docs fixes moving back to rtbc.

Committed/pushed to 8.2.x and cherry-picked to 8.1.x. Thanks!

Why closed? Issue is persistent on 8.1.10, actual stable version.