Since the addition of a the core drush command to run a site installation, drush no longer works correctly to install. Drush doesn't read existing database connection information. In fact it never reads the settings.php at all.

Ref: #2911319: Provide a single command to install & run Drupal - https://cgit.drupalcode.org/drupal/commit/?id=573702f

Using Drush 9.2.3

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

pookmish created an issue. See original summary.

pookmish’s picture

Issue summary: View changes
cilefen’s picture

Status: Active » Closed (won't fix)

The core command is not Drush, correct? If Drush must adapt, that is important, however Drush’s issue queue is on Github. Please open an issue there https://github.com/drush-ops/drush/issues

If you did not already, post a comment on #2911319: Provide a single command to install & run Drupal about this.

pookmish’s picture

Moved to https://github.com/drush-ops/drush/issues/3552

I thought that since the bug was introduced from a change core, the bug report would belong here. thanks @cilefen

alexpott’s picture

Status: Closed (won't fix) » Needs work

This is being caused by \Drupal\Core\Installer\InstallerKernel::createFromRequest() which we added in #2911319: Provide a single command to install & run Drupal

alexpott’s picture

Priority: Normal » Major
Status: Needs work » Needs review
FileSize
1.18 KB

This fixes Drush 9 install with an existing settings.php and doesn't break the quick-start command or tests.

Not sure how to write a test yet. Making this major because we've regressed important functionality for drush users and probably drupal console user.

alexpott’s picture

Title: Drush site-install command doesn't work » Drush site-install command doesn't work when settings.php is present
alexpott’s picture

Fix wrong comma.

pookmish’s picture

Patch in #8 worked well for me. seem to be a pretty simple as well.

alexpott’s picture

Here's an automated test. It's a bit close to the code but fails as expected if this fix is not there.

The test only patch is the interdiff.

The last submitted patch, 10: 2969715-10.test-only.patch, failed testing. View results

dawehner’s picture

  1. +++ b/core/lib/Drupal/Core/Installer/InstallerKernel.php
    @@ -71,10 +72,16 @@ public function getInstallProfile() {
       public static function createFromRequest(Request $request, $class_loader, $environment, $allow_dumping = TRUE, $app_root = NULL) {
    -    // This override exists because we don't need to initialize the settings
    -    // again as they already are in install_begin_request().
         $kernel = new static($environment, $class_loader, $allow_dumping, $app_root);
         static::bootEnvironment($app_root);
    +    // Don't initialize settings again if they already have been. This can occur
    +    // in install_begin_request().
    +    try {
    +      Settings::getInstance();
    +    }
    +    catch (\BadMethodCallException $e) {
    +      $kernel->initializeSettings($request);
    +    }
         return $kernel;
       }
    

    I'm curious, should we not override initializeSettings and check there instead whether settings are initialized already?

  2. +++ b/core/tests/Drupal/Tests/Core/Installer/InstallerKernelTest.php
    @@ -0,0 +1,54 @@
    + * Tests the image class.
    + *
    

    Nitpick: This comment is wrong

alexpott’s picture

@dawehner I don't get point 1. Do you mean move this into DrupalKernel? Or do you mean only call createFromRequest if we need to initialise settings and do the other parts in ... ah I see (I think). Great idea. Much simpler.

And solves point 2 because there is no test anymore because the InstallerKernel is no longer overriding createFromRequest() because we just don't need to do that.

Tested with Drush 9 and the installer works again with this smaller patch.

pookmish’s picture

Also confirming #13 works with Drush 9.2.3.

thursday_bw’s picture

I just applied #13 using composer patches, it applied successfully but didn't resolve my issue.

```
[error] Failed to create database: sh: 1: psql: not found
```

Maybe it's a postgres issue?.. I DO NOT have psql command installed, and I shouldn't need it, as I should be needing to create a database, in this case the database already exists albeit it's empty.

alexpott’s picture

@thursday_bw your issue is not related to this issue. Drupal core does not issue any commands to "psql" via the shell. Drush does however.

dawehner’s picture

Thank you @alexpott for this great simplification.

singularo’s picture

This solved the drush site-install issue with 8.6.0 referenced here - https://github.com/drush-ops/drush/issues/3552

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

I already tried to RTBC it last week.

catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed f70f4f4 and pushed to 8.6.x. Thanks!

  • catch committed f70f4f4 on 8.6.x
    Issue #2969715 by alexpott, pookmish, dawehner: Drush site-install...

Status: Fixed » Closed (fixed)

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