Provide a single command to install & run Drupal

Maybe similar to #2876001: Installerless Drupal?

Btw. One command to install and run Drupal:

drush qd

See: What is the easiest way to install clean Drupal from scratch using drush?

Perhaps also related is #2242947: Integrate Symfony Console component to natively support command line operations as that will make running commands simpler.

Another idea would be to add a command to the root composer.json that can do this. See https://getcomposer.org/doc/articles/scripts.md#writing-custom-commands

In #1543858: Add a startup configuration for the built-in PHP server that supports clean URLs we discussed the need for documentation in core files. This issue should add it.

Just tested the command - so sweet. Nice work @dawehner.

I think we should add some resilience - for example what happens if sqlite is not available? Also the output could be more instructive as to the url to visit and the admin user password. Ah... see code review @dawehner++

1. +++ b/core/scripts/dev-start.php
   @@ -0,0 +1,104 @@
   +  // Installs drupal using sqlite.
   +  if (!extension_loaded('pdo_sqlite')) {
   +    $output->writeln('Sqlite required ...');
   +    return;
   +  }
   

   Lol I should have looked at code. So we need a better message but awesome to see this!

2. +++ b/core/scripts/dev-start.php
   @@ -0,0 +1,104 @@
   +      // @todo Do we want to make this configurable?
   +      'profile' => 'standard',
   +      // @todo A different langcode would be nice as well?
   +      'langcode' => 'en',
   

   Yep this would be super nice. Of course such things need validation :(

3. +++ b/core/scripts/dev-start.php
   @@ -0,0 +1,104 @@
   +        // form_type_checkboxes_value() requires NULL instead of FALSE values
   +        // for programmatic form submissions to disable a checkbox.
   +        'enable_update_status_module' => NULL,
   +        'enable_update_status_emails' => NULL,
   

   These look like sensible defaults for a dev environment.

4. +++ b/core/scripts/dev-start.php
   @@ -0,0 +1,104 @@
   +  if (file_exists('sites/default/settings.php')) {
   +    $result = unlink('sites/default/settings.php');
   +    if ($result === FALSE) {
   +      $output->writeln('Removing settings.php failed, please do it manually ...');
   +      return;
   +    }
   +  }
   +  if (file_exists('sites/default/files/.sqlite')) {
   +    $result = unlink('sites/default/files/.sqlite');
   +    if ($result === FALSE) {
   +      $output->writeln('Removing .sqlite failed, please do it manually ...');
   +      return;
   +    }
   +  }
   

   I think before doing something destructive we should involve the user.

5. +++ b/core/scripts/dev-start.php
   @@ -0,0 +1,104 @@
   +  ->register('dev-start')
   

   It might be worth having two command dev-start and dev-install. dev-start should only do an install if there's no settings.php otherwise it could just start the webserver.

On my localhost when I access to core/scripts/dev-start.php through web it tries to remove settings.php file.
I know this should be handled by proper server configuration but anyway it would be more secure if we take care about it.
For instance in the core/scripts/db-tools.php there is the following condition.

if (PHP_SAPI !== 'cli') {
  return;
}

+ $builder = new ProcessBuilder([$binary, '-S', 'localhost:8888' , '.ht.router.php']);
There is a nice implementation for this in Symfony but unfortunately it is tied to the framework.
https://github.com/symfony/symfony/issues/24534

❤️❤️❤️

Slightly adjusting the tag, since with this version it says this on the project page. 😂

What is the difference between #2926633: Provide a script to install a Drupal testsite and this?

1. +++ b/core/lib/Drupal/Core/Command/DevStartCommand.php
   @@ -0,0 +1,168 @@
   +    ;
   +    ¶
   +    parent::configure();
   +  }
   

   Some unintended whitespace.

2. +++ b/core/lib/Drupal/Core/Command/DevStartCommand.php
   @@ -0,0 +1,168 @@
   +    $root = dirname(dirname(__DIR__));
   

   Needs to be $root = dirname(dirname(dirname(dirname(dirname(__DIR__)))));

3. +++ b/core/lib/Drupal/Core/Command/DevStartCommand.php
   @@ -0,0 +1,168 @@
   +
   +    install_drupal($classLoader, $parameters);
   +    $output->writeln('Drupal successfully installed.');
   

   This process can take a long time. Maybe we can use the process command to fork and run this in a separate process and use Symfony's stuff to have a progress bar? At the very least we should have some output prior to this saying we're installing and this might take some time.

4. +++ b/core/lib/Drupal/Core/Command/DevStartCommand.php
   @@ -0,0 +1,168 @@
   +    $output->writeln('Starting webserver ...');
   +    $process->run();
   

   The output here should list how to connect to the site and credentials etc..

5. +++ b/core/lib/Drupal/Core/Command/DevStartCommand.php
   @@ -0,0 +1,168 @@
   +  /**
   +   * {@inheritdoc}
   +   */
   +  protected function execute(InputInterface $input, OutputInterface $output) {
   +    $root = $this->boot($output);
   +    $this->install($this->classLoader, $output, $input->getOption('install_profile'), $input->getOption('langcode'));
   +    $this->start($root, $output);
   +  }
   

   Can we move this near the top of the file - after configure I was surprised where this was in the file.

6. Is it worth separating the install and starting webserver commands so that one can terminate the webserver command and restart it without having to re-install?

Also, should have said typing in a single command to get umami up and running for a demo is really cool! Nice work @dawehner

1. +++ b/core/lib/Drupal/Core/Command/DevInstallCommand.php
   @@ -0,0 +1,145 @@
   +use Symfony\Component\Process\PhpExecutableFinder;
   +use Symfony\Component\Process\ProcessBuilder;
   

   Unused use statements.

2. +++ b/core/lib/Drupal/Core/Command/DevInstallCommand.php
   @@ -0,0 +1,145 @@
   +   * @var object
   

   Can we use more specific type Composer\Autoload\ClassLoader?

3. +++ b/core/lib/Drupal/Core/Command/DevInstallCommand.php
   @@ -0,0 +1,145 @@
   +  public function __construct($classLoader) {
   

   The method parameter needs to be in snake case.

4. +++ b/core/lib/Drupal/Core/Command/DevInstallCommand.php
   @@ -0,0 +1,145 @@
   +   * Constructs a new DevStartCommand command.
   

   - DevStartCommand
   + DevInstallCommand

5. +++ b/core/lib/Drupal/Core/Command/DevInstallCommand.php
   @@ -0,0 +1,145 @@
   +    $root = $this->boot($output);
   

   Unused variable $root.

6. +++ b/core/lib/Drupal/Core/Command/DevStartCommand.php
   @@ -0,0 +1,83 @@
   +  /**
   +   * Boots up a Drupal environment.
   +   *
   +   * @return string
   +   *   Returns the path to the Drupal root.
   +   */
   +  protected function boot() {
   +    // Installs drupal using sqlite.
   +    if (!extension_loaded('pdo_sqlite')) {
   +      $output->writeln('Sqlite required ...');
   +      return;
   +    }
   +    $root = dirname(dirname(dirname(dirname(dirname(__DIR__)))));
   +    chdir($root);
   +    return $root;
   +  }
   

   The function name and description do not correspond to what this function really does. Also return type should be string|null.

7. +    // Installs drupal using sqlite.
   +    if (!extension_loaded('pdo_sqlite')) {
   +      $output->writeln('Sqlite required ...');
   +      return;
   +    }
   

   Same thing here. This code does not install Drupal. Also 'drupal' needs to be capitalized.

8. +    $root = dirname(dirname(dirname(dirname(dirname(__DIR__)))));
   

   Whi not $root = __DIR__ . '/../../../../..';?

9. +++ b/core/lib/Drupal/Core/Command/DevStartCommand.php
   @@ -36,104 +35,29 @@ public function __construct($classLoader) {
   -      ->addOption('install_profile', NULL, InputOption::VALUE_OPTIONAL, 'Install profile to install the site in. Defaults to testing', 'standard')
   

   Per the option description default value should be 'testing'.

10. +++ b/core/lib/Drupal/Core/Command/DevInstallCommand.php
    @@ -0,0 +1,145 @@
    +   * Installs the standard install profile.
    

    Not quite correct, since this can install any installation profile.

11. +++ b/core/lib/Drupal/Core/Command/DevInstallCommand.php
    @@ -0,0 +1,145 @@
    +        $output->writeln('Removing settings.php failed, please do it manually ...');
    

    This needs to be printed to std error.
    $output->getErrorOutput()->writeln('PDO SQLite extension is required for this opertaion.');
    Also I would make the message more verbose and remove ellipsis.

12. +++ b/core/lib/Drupal/Core/Command/DevStartCommand.php
    @@ -0,0 +1,83 @@
    +  ¶
    

    Trailing whitespace

13. +++ b/core/lib/Drupal/Core/Command/DevStartCommand.php
    @@ -4,13 +4,12 @@
     use Symfony\Component\Process\ProcessBuilder;
    

    ProcessBuilder is deprecated.

14. +++ b/core/scripts/dev-site.php
    @@ -0,0 +1,13 @@
    +$application = new Application('dev-start', '0.1.0');
    

    What does '0.1.0' stand for? Can we use \Drupal::VERSION isntead?

15. +++ b/core/lib/Drupal/Core/Command/DevInstallCommand.php
    @@ -0,0 +1,145 @@
    +        $output->writeln('Removing settings.php failed, please do it manually ...');
    +        return;
    

    This return statement does not stop the application from beeing exectured. Since both boot() and install() methods are rather small I propose placing the entire command code to execute() method. In any case we need to return correct exit code from the application.

16. +++ b/core/scripts/dev-site.php
    @@ -0,0 +1,13 @@
    +<?php
    +
    +use Drupal\Core\Command\DevInstallCommand;
    +use Drupal\Core\Command\DevStartCommand;
    +use Symfony\Component\Console\Application;
    

    Missing file doc comment.

17. +++ b/core/lib/Drupal/Core/Command/DevStartCommand.php
    @@ -0,0 +1,83 @@
    +    $builder = new ProcessBuilder([$binary, '-S', 'localhost:8888' , '.ht.router.php']);
    

    Per the coding standard each element of this array should be broken into its own line. Also space after localhost:8888 is redundand.

18. +++ b/core/lib/Drupal/Core/Command/DevInstallCommand.php
    @@ -0,0 +1,145 @@
    +      ->addUsage('--install_profile demo_umami --langcode fr')
    +    ;
    

    Another CS issue. The ending semicolon should not be on its own line.

Adding in something from Slack chat. We can support the builtin webserver on a random port. Here's some code I used to pick random ports for PhantomJS and Chrome web drivers

 protected static function getAvailablePort() {
    $address = '0.0.0.0';
    $port = 0;
    $socket = socket_create(AF_INET, SOCK_STREAM, SOL_TCP);
    socket_bind($socket, $address, $port);
    socket_listen($socket, 5);
    socket_getsockname($socket, $address, $port);
    return $port;
  }

Source https://github.com/mglaman/webdrivers/blob/master/src/DriverFactory.php#L63

From the issue summary

Run $ php core/scripts/dev-start.php from the command line

But there isn't a default command (still `list`). So I see

Available commands:
  help     Displays help for a command
  install  Installs a Drupal dev site. This is not meant for production or any custom development. It is a quick and easy way to get Drupal running.
  list     Lists commands
  start    Starts up a webserver for the dev site.

Do we want to default to a command which runs `install` and then `start`? so it can be an all in one command? I guess DevInstallCommand is not re-entrant safe and assumes you want to reinstall if you run it again. And DevStartCommand should only worry about running the built-in server.

Wherever this is documented should mention it is a two-step process for the first run, single command thereafter.

EDIT: Command outputs

Install

[03:43 PM]-[mglaman@Matts-MBP]-[~/Drupal/sites/drupal8/web]-[git 2911319] 
$ php core/scripts/dev-site.php install
Drupal installation started.
Drupal successfully installed.

Run builtin

[03:47 PM]-[mglaman@Matts-MBP]-[~/Drupal/sites/drupal8/web]-[git 2911319] 
$ php core/scripts/dev-site.php start
Starting webserver on http://localhost:65264
[03:48 PM]-[mglaman@Matts-MBP]-[~/Drupal/sites/drupal8/web]-[git 2911319] 
$ php core/scripts/dev-site.php start
Starting webserver on http://localhost:65293

Random ports ftw! Able to access installed site.

+++ b/composer.json
@@ -54,6 +54,7 @@
+        "dev-site": "core/scripts/dev-site.php install && core/scripts/dev-site.php start",

+++ b/core/lib/Drupal/Core/Command/DevInstallCommand.php
@@ -0,0 +1,138 @@
+class DevInstallCommand extends Command {

I totally missed this. This is great but has one problem.

As stated in my last comment, DevInstallCommand is not reentrant. So if an unexpected user tried to execute `dev-site` thinking it'd execute like `npm start` they would blow away their existing site.

I think when `install` checks for an existing install it should exit successfully if there is a site. Unless a `--force` option is passed. Kind of like how Drush asks you "Are you sure?" for `site-install`. This way we can provide a `npm start` like command for people to get up and go.

After reviewing this issue after #2242947: Integrate Symfony Console component to natively support command line operations
I see 2 different ways to boostrap core & checks for ability to do it.
Mostly polished class Application extends GenericApplication (generic is SF one)
So if install command will fail we just get NULL and call to start dev server without checking "bootstrapability" of core
Making both host:port to listen configurable
Maybe parts of .htrouter could be reused
Probably #2289405: [META] Port all shell scripts to console commands is better place to discus bootstrap & error handling

1. +++ b/core/lib/Drupal/Core/Command/DevInstallCommand.php
   @@ -0,0 +1,165 @@
   +      $output->getErrorOutput()->writeln('You need to have sqlite installed.');
   +      return;
   ...
   +        $output->getErrorOutput()->writeln('Removing settings.php failed, please do it manually ...');
   +        return;
   ...
   +        $output->getErrorOutput()->writeln('Removing .sqlite failed, please do it manually ...');
   +        return;
   
   +++ b/core/lib/Drupal/Core/Command/DevStartCommand.php
   @@ -0,0 +1,101 @@
   +      throw new \RuntimeException('Unable to find the PHP binary.');
   

   all this places reports to err and return non-zero (or exception?) that's what raised in 2242947#98

   let's came to common ground for all commands to be POSIX in #2289405-21: [META] Port all shell scripts to console commands

2. +++ b/core/lib/Drupal/Core/Command/DevStartCommand.php
   @@ -0,0 +1,101 @@
   +  }
   +  ¶
   

   nit, trailing whitespace

3. +++ b/core/lib/Drupal/Core/Command/DevStartCommand.php
   @@ -0,0 +1,101 @@
   +   * Boots up a Drupal environment.
   ...
   +  protected function boot() {
   +    $root = dirname(dirname(dirname(dirname(dirname(__DIR__)))));
   +    chdir($root);
   +    return $root;
   

   chdir() does not boot core, maybe better call it chroot5Dirs()

   Actually this is what #2242947-96: Integrate Symfony Console component to natively support command line operations means that it can catch by tests - so could be a problem if this method will be called twice

4. +++ b/core/lib/Drupal/Core/Command/DevStartCommand.php
   @@ -0,0 +1,101 @@
   +      'localhost:' . $port,
   ...
   +    $output->writeln('Starting webserver on http://localhost:' . $port);
   

   Why only port configurable? host should be as well

5. +++ b/core/scripts/dev-site.php
   @@ -0,0 +1,18 @@
   +$application = new Application('dev-start', '0.1.0');
   +$application->add(new DevInstallCommand($classloader));
   +$application->add(new DevStartCommand($classloader));
   +$application->run();
   

   Please, make separate commands) It will be more useful cos I don't wanna reinstall site each time I wanna run dev server

#33.5 Maybe option --listen with host (default to localhost) and port (8080 default) may pipe dev server command

PS: That could a good path to https://github.com/php-pm/php-pm-drupal

btw SF using https://github.com/symfony/symfony-installer/blob/master/symfony to check requirements and start server installer(demo)

+++ b/core/lib/Drupal/Core/Command/DevStartCommand.php
@@ -0,0 +1,101 @@
+    $address = '0.0.0.0';
+    $port = 0;
+    $socket = socket_create(AF_INET, SOCK_STREAM, SOL_TCP);
+    socket_bind($socket, $address, $port);
+    socket_listen($socket, 5);
+    socket_getsockname($socket, $address, $port);

I bet it fails because you bind/listen the same port (on 0.0.0.0) so webserver can't start cos port already used by other process

@dawehner, tt's pretty common case using something different form localhost on local machine, especially with Docker based environment.

I bet it fails because you bind/listen the same port (on 0.0.0.0) so webserver can't start cos port already used by other process

What fails? This is how you find an available port in PHP to bind to. See #30 where I run the command twice and receive ports that do not collide.

@dawehner, tt's pretty common case using something different form localhost on local machine, especially with Docker based environment.

This boots the builtin server. I think we can punt that to avoid scope bloat. The fac remains this gives new users a one time command to run that mimcs `npm start`

+++ b/core/lib/Drupal/Core/Command/DevInstallCommand.php
@@ -49,9 +56,29 @@ protected function configure() {
+    $request = Request::createFromGlobals();
+    DrupalKernel::createFromRequest($request, $this->classLoader, 'prod');
+
+    return ($connection = Database::getConnection()) && $connection->schema()->tableExists('sessions');

What if we just followed install.php

    // Do not install over a configured settings.php.
    if (Database::getConnectionInfo()) {
      throw new AlreadyInstalledException($container->get('string_translation'));
    }

If there is a settings.php and a default connection defined, require forced install.

@kenorb DrupalConsole provides a similar command named quick:start

Actually, this command is something we called a chain command
A chain command is a custom command that helps you automate multiple command execution, allowing you to define and read an external YAML file containing the definition name, options, and arguments of multiple commands and execute that list based on the sequence defined in the file.

DrupalConsole documentation about chain commands => https://docs.drupalconsole.com/en/chains/what-is-a-chain-command.html

YAML code is here:
https://github.com/hechoendrupal/drupal-console-core/blob/master/config/...

The previous definition will execute several commands, in this case, commands that will download and install Drupal using SQLite, and finally start the PHP's built-in server.

I have an incoming patch to change the default behavior to N otherwise the dev script runs kind of weird. Also to highlight the default option and simplify install check.

$ php core/scripts/dev-site.php  install
Drupal installation started.
Drupal successfully installed.

$ php core/scripts/dev-site.php  install
There is already an existing installation. Confirm whether you want to override it. (Y/n) 
Drupal installation started.
Drupal successfully installed.

$ php core/scripts/dev-site.php  install
There is already an existing installation. Confirm whether you want to override it. (Y/n) n

EDIT: This is why to change default

$ composer run-script dev-site
> core/scripts/dev-site.php install && core/scripts/dev-site.php start
Removing settings.php failed, please do it manually ...
Starting webserver on http://localhost:51928

Here is the patch which changes the default confirmation answer so there isn't a reinstall when executing dev-start.

Executing composer run-script dev-start is like running npm start on a JavaScript project. You get an installed and running environment with minimal work. There should not be any need to configure the host or specify the port. The goal is to one and done get an evaluation up and running.

@mglaman can you check #26, some of this notes are not addressed yet. Also #12.

Sorry, missed those Chi.

+++ b/core/scripts/dev-site.php
@@ -0,0 +1,18 @@
+$application = new Application('dev-start', '0.1.0');
+$application->add(new DevInstallCommand($classloader));
+$application->add(new DevStartCommand($classloader));
+$application->run();

Needs check to ensure CLI.

This addresses items in #12 and #26 (latter to best I could find remaining items.)

What are the chances the database would be empty if there data connection string was available? That's the check that core does on its install (at least from what I could see in install_begin_request.) The session table is a fair signal, but this can be a bit safer in case the database is that bad (has sessions table, but not others.)

EDIT: You can still install on an empty database, you just need to force.

+++ b/core/lib/Drupal/Core/Command/DevInstallCommand.php
@@ -22,19 +21,19 @@ class DevInstallCommand extends Command {
+   * @var \Composer\Autoload\ClassLoader
...
+   * @param object $class_loader

Note that per #28 the object type hint was chosen intentionally. Personally, I cannot think of any use case with different autoloader in this command. So I would prefer using full class name for type hinting. Same type is used in upcoming patch in #2242947: Integrate Symfony Console component to natively support command line operations.

However, in either case we should use only one type hint across this file for the purpose of consistency. Currently both of them are used.

I also found something else while using this for a Drupal Commerce demo. When my example profile had Bootstrap it exploded.

Adding define('MAINTENANCE_MODE', 'install'); fixed it. We're currently not defining the maintenance mode, as expected on install, to install

Just cleaning up the whitespace errors in patch so it applies clean.

Hmmm I need a version of this patch with no root composer changes. Ignore this patch.

+++ b/composer.json
@@ -54,6 +54,7 @@
+        "dev-site": "core/scripts/dev-site.php install && core/scripts/dev-site.php start",

this should be "dev-site": "@php docroot/core/scripts/dev-site.php install && @php docroot/core/scripts/dev-site.php start",

Hmmm... actually this works:

        "dev-site": [
            "@php docroot/core/scripts/dev-site.php install",
            "@php docroot/core/scripts/dev-site.php start"
        ],

Two quick questions (and I apologise if I've missed the answer for them in the comments)

1: core/scripts/dev-site.php seems to need to be executable before composer run-script will execute it. Is making it executable something that happens during a build of core prior to distribution or is it something that should be done as a part of this patch?

2: I can't see that the comment regarding setting a username and password in #11 was actually address, is it work that needs doing? (I'd be happy to pitch in)

Ok, I should obviously have read the #60 with regards to my first question :)

As penance I've verified that the approach does indeed work - dev-site.php does not need to be executable if we run it using @php

Patch and interdiff attached.

+  protected function getAvailablePort() {
+    $address = '0.0.0.0';

Should this be 127.0.0.1 to match that we bind to localhost below? Otherwise, could we be grabbing an available port on the wrong interface?

Comment #11 is visible for me - if not, it notes that there's no way to actually log in to the installed site. Rather than showing a user name and password, we could print a one-time login with the destination set to <front>?

I also got the following exception after leaving the server open for a few minutes:

[Symfony\Component\Process\Exception\ProcessTimedOutException]
The process "'/usr/local/Cellar/php/7.2.3/bin/php' -d allow_url_fopen='1' -d disable_functions=''
-d memory_limit='1536M' core/scripts/dev-site.php start" exceeded the timeout of 300 seconds.

Should this be 127.0.0.1 to match that we bind to localhost below? Otherwise, could we be grabbing an available port on the wrong interface?

Everything I've read, and when using this, says to bind to 0.0.0.0 which is your host.

if not, it notes that there's no way to actually log in to the installed site.

I tried looking at #11 but don't see where it says there is no way to log in. It spits out the localpost:PORT. However, I do notice see we do not mention the username and password. I feel like generating a one-time login link just adds more scope bloat. I'd rather just print the credentials.

I also got the following exception after leaving the server open for a few minutes:

Ditto. I don't know how we can work around this. You'd have to call Composer with its timeout flag. See the following

$ composer run --help
Usage:
  run-script [options] [--] [<script>] [<args>]...

Arguments:
  script                         Script name to run.
  args                           

Options:
      --timeout=TIMEOUT          Sets script timeout in seconds, or 0 for never.

Nice! I eat my words about login link. That was much simpler than I thought.

+++ b/composer.json
@@ -14,7 +14,8 @@
+        "process-timeout": 0

This looks a bit dodgy :)

Hm. Not sure why this is in the ideas queue. :)

Also bumping to "major" since it's a blocker for JS testing.

+++ b/core/lib/Drupal/Core/Command/DevInstallCommand.php
@@ -50,44 +48,35 @@ protected function configure() {
-   * Boots up a Drupal environment.
-   *
-   * @param \Symfony\Component\Console\Output\OutputInterface $output
-   *   The console output.
+   * Changes the directory to the Drupal root.
    *
    * @return string
    *   Returns the path to the Drupal root.
    */
-  protected function boot(OutputInterface $output) {
-    // Installs drupal using sqlite.
-    if (!extension_loaded('pdo_sqlite')) {
-      $output->writeln('Sqlite required ...');
-      return;
-    }
-
+  protected function changeRoot() {

This change needs to be applied to the DevStartCommand as well.

Should this be 127.0.0.1 to match that we bind to localhost below? Otherwise, could we be grabbing an available port on the wrong interface?

Everything I've read, and when using this, says to bind to 0.0.0.0 which is your host.

I think trying to bind to a port on 0.0.0.0 will fail if any interface on the server is using that port. So, this shouldn't end up in a situation where getAvailablePort() gives a port that's not actually available on 127.0.0.1. If anything, it'll skip ports that are available on 127.0.0.1 if they are used on other interfaces, which shouldn't cause any problems.

Hey why IPv4 only? There's IPv6 so this indeed should be compatible with it.
PHP did fix that in https://bugs.php.net/bug.php?id=68420

+++ b/core/lib/Drupal/Core/Command/DevStartCommand.php
@@ -0,0 +1,101 @@
+    $address = '0.0.0.0';
...
+      'localhost:' . $port,

+++ b/core/tests/Drupal/Tests/Core/Command/DevSite.php
@@ -0,0 +1,43 @@
+        $response = $guzzle->get('http://localhost:' . $match[1]);

There's total mess in interfaces & hostnames

Hey why IPv4 only? There's IPv6 so this indeed should be compatible with it.

I mean, we're talking about something that's ultimately listening on localhost. Do you really need to connect to localhost over IPv6? Sure, I guess this means that the available port checking is not including the IPv6 interfaces, but we really only care about is the available ports on localhost.

This is meant to be a quick start command. We do not need to worry about IPv6 or IPv4.

I think trying to bind to a port on 0.0.0.0 will fail if any interface on the server is using that port.

On the docs: http://php.net/manual/en/function.socket-getsockname.php. Port is handled by reference to be updated to the actual port it ended up binding to. As mentioned in https://www.drupal.org/project/drupal/issues/2911319#comment-12490747 I have used this to run 30-40 concurrent user load tests using ChromeDriver on random ports.

Per #73 what is the problem? localhost maps to 127.0.0.1 which is where the port binds.

This seems to be a concern by many, but not reproduced in error or steps to reproduce. I have been using that line for quite a while, which is why I suggested it.

The scope of the command is to just provide a quick development server and SQLite install for a demo purpose.

RE: @mglaman in #75, I think you're misunderstanding what I was trying to say, and I guess, I probably said it poorly. :-)

I was trying to say that the code in getAvailablePort() would fail to bind on any already in use port on any interface, ie. it would find a port that was free across any interface (well, any IPv4 one ;-)) on the machine. So, just that that code is being extra paranoid, and so wouldn't cause problems with the fact that in the end the server is being run on localhost.

I think we could change getAvailablePort() to just check '127.0.0.1' rather than '0.0.0.0' which would only look for a port that was free specifically on localhost, but what's the point? The current code is more paranoid than necessary, not less, so that discrepancy shouldn't cause any problems. That's the point I was trying to make.

Can someone of you make a suggest what we do otherwise?

As stated above in #72 and #74, I think what the code is currently doing is fine.

EDIT: Actually, to try and clarify even more: when I'm writing above (and in #72) that it will "fail to bind" I'm referring specifically to the socket_bind() call in this code in the patch:

+  protected function getAvailablePort() {
+    $address = '0.0.0.0';
+    $port = 0;
+    $socket = socket_create(AF_INET, SOCK_STREAM, SOL_TCP);
+    socket_bind($socket, $address, $port);
+    socket_listen($socket, 5);
+    socket_getsockname($socket, $address, $port);
+    return $port;
+  }

So, I'm not referring to ultimately running the server failing or anything like that. Just that the socket_bind() above would not succeed if it were trying a port that was in use on any IPv4 interface.

Whew. Writing words is hard. :-) Sorry for not being clear the first time around!

Just wanted to add notes on how to test the current patch quickly in a fresh environment (with no Drupal or anything else), for my (and others?) reference:

1. Ensure you have PHP 7+, Git, and Composer installed.
2. Run git clone --branch 8.6.x https://git.drupal.org/project/drupal.git && cd drupal
3. Apply the latest patch: wget -q -O - https://www.drupal.org/files/issues/2911319-67.patch | git apply -
4. Run composer install
5. Run composer dev-site
6. Grab the URL (e.g. localhost:XXXXX) and append the 'Login using' string to the end.

One minor nit from a UX perspective: What if we make the entire login URL copy-pasteable? So instead of a new user having to realize they copy out the localhost:XXXXX part, then copy the user login path, then merge the two together to get logged in, they can just copy out the full URL and get logged in immediately?

Or provide both, but the login URL prefixed with the http://localhost:XXXXX part, just to make it easier to do the copy/paste. OR, final suggestion would be to do what Drush does and just provide a random admin password, e.g.:

[notice] Starting Drupal installation. This takes a while.
[success] Installation complete.  User name: admin  User password: nFoGTR6xEb

At beginners sprint at midcamp, working on issue.

We found a typo in the wget patch URL in #79. It is missing "/issues" after "files". Resummarizing, the instructions should read:

1. Ensure you have PHP 7+, Git, and Composer installed.
2. Run git clone --branch 8.6.x https://git.drupal.org/project/drupal.git && cd drupal
3. Apply the latest patch: wget -q -O - https://www.drupal.org/files/issues/2911319-67.patch | git apply -
4. Run composer install
5. Run composer dev-site
6. Grab the URL (e.g. localhost:XXXXX) and append the 'Login using' string to the end.

@scotty: It looks like "/issues" after "files" wasn't added in the correction :-)

Thanks, just edited my comment :)

Notes: From Mid-camp.

OSX doesn't come with wget out of the box. Had to install Homebrew and use it to install wget.

Upon running composer install we got a number of OpenSSL failures. For instance:

Installing instaclick/php-webdriver (1.4.5): Downloading (failed)       
Downloading (failed)       
Downloading (failed)    Failed to download instaclick/php-webdriver from dist: The "https://api.github.com/repos/instaclick/php-webdriver/zipball/6fa959452e774dcaed543faad3a9d1a37d803327" file could not be downloaded: SSL operation failed with code 1. OpenSSL Error messages:
error:1407742E:SSL routines:SSL23_GET_SERVER_HELLO:tlsv1 alert protocol version
Failed to enable crypto
failed to open stream: operation failed
    Now trying to download from source
  - Installing instaclick/php-webdriver (1.4.5): Cloning 6fa959452e from cache
> Drupal\Core\Composer\Composer::vendorTestCodeCleanup

So the files do download from cache, but it looks like it's failing but it does actually download and install... It would be good to re-run the test from a clean install of OSX.

Notes: From Mid-camp.

The site does install and run.

I just updated to PHP 7.1 on my Mac running OSX 10.12.6 and ran through the instructions for testing.

I had some problem with the wget, but once I downloaded and applied the patch, it worked as expected.

It created and installed a version of Drupal 8 and I was able to access the site with the provided URL, login with the supplied temporary login, and edit a node.

I am working with @scotty, @droffats, and @ex DJ at MidCamp2018!

@droffats, @scotty, @ex DJ

I think the next steps are

1. Update the issue summary (How to test) to include the steps that @scotty posted in #81 with any additional gotchas in #85.
2. The issue summary has a remaining task of ensuring that the dev-site composer script doesn't "kill" an existing database. So what that means is that you should install Drupal 8 through the usual install.php process on your local environments MySQL and SQLite. Then run composer dev-site and ensure that you are still able to access the original site you installed.
3. Finally the patch in #67 needs instructions for users in INSTALL.txt. So the patch needs to be modified and re-uploaded. Ensure that you create an interdiff of your patch with #67 if you are doing this.

Setting to needs work for Documentation changes needed in INSTALL.txt.

I edited the installed drupal site, Then re-ran

composer dev-site

and confirmed that it did not over-write the existing .sqlite db.

Notes from MidCamp.

It might be helpful to add a comment to the instructions, noting what kind of Drupal user should be using this command; i.e., intended as a quick way to get Drupal running for a basic introduction, not for serious developers.

With @droffats, @scotty, and a BIG assist from @stpaultim and @mradcliffe

Thanks to everybody, great work! I went through these steps for Ubuntu 16.04 to be able to install Drupal 8 with the steps outlined in #81:

Install Git and required PHP packages, and check it went well

$ sudo apt-get install git php7.0 php7.0-curl php7.0-dom php7.0-gd php7.0-mbstring php7.0-sqlite3
$ php --version
$ git --version

Download, move and check if Composer is installed correctly

$ curl -sS https://getcomposer.org/installer | php;
$ sudo mv composer.phar /usr/local/bin/composer;
$ composer -V

Setting to needs work for Documentation changes needed in INSTALL.txt.

Why would we need to change INSTALL.txt? This command is for evaluation and not a legitimate installation setup.

The issue summary says that the plan for this issue has the following, @mglaman:

> Communicate that as the simplest way to try out Drupal on your system by adding documentation to INSTALL.txt

Original issue as reported by @dawehner had "Communicate that as the simplest way to try out Drupal on your system" in the Detailed plan section. @alexpott added that requirement in #9 saying that we should add this to core documentation and changed the issue summary to add INSTALL.txt.

Is that still relevant?

Edit: for grammar.

Know it may be a tall order, but something along the lines of Dev Desktop is a good onboarding method for non-developers to get acquainted with Drupal. It's easy for experienced people to forget how intimidating the command line and associated processes can be for some.

It's part of the reason Drupal has had problems expanding into creative fields, where people there tend to gravitate to WordPress for apparent ease-of-use.

Yes, a packaged launch/install method may not be the best for launching industrial-strength projects, but is a quick way to people to spin up an install to try things out.

It's part of the reason Drupal has had problems expanding into creative fields, where people there tend to gravitate to WordPress for apparent ease-of-use.

Look at the node.js community; everyone there uses npm on the command line to at least install/start/stop a project. I don't think it's too much to ask someone to run a few CLI commands in 2018, especially if this way they don't need an extra bunch of software (Apache and MySQL) installed on their computer.

@geerlinguy, true, but that's making some assumptions about users--1) their system doesn't have older/conflicting installs, and 2) they're comfortable with the CLI. Having helped out onboard users onto Drupal over the years, I've seen some pretty head-scratching issues, which can require some in-depth problem solving and fixing to get a stack stood up.

Matthew Grasmick had recently posted on this. Some of the casual/non-technical evaluators may be the ones making the business decision of whether to go Drupal or not. It can look pretty simple for someone with technical chops, even if snags are hit--again, this is where past experience helps ease the troubleshooting.

Even for non-code projects, like a data-analytics app I'm working on--ease of use is a critical consideration in a competitive market.

It's easy for experienced people to forget how intimidating the command line and associated processes can be for some.

@karolus: I couldn't agree more, and that's why I posted the commands required to get Ubuntu 16.04 ready for Drupal 8 single command installation in #93.

Perhaps someone could check if the commands work for Mac as well, or they need tweaking?

I am now so comfortable with the CLI that I can wipe my machine for everything PHP- and Composer-related, and reinstall the necessary programs, just to see what the bare minimum is. The newbie-me would have taken forever to just get to the point where I could actually run the single commands which installs Drupal, or might never have gotten that far.

Let's help non-developers get acquainted with Drupal, as much as we possibly can.

Really nice to see the progress here and thank you to everyone.

However, I don't see an un-install option. Am I missing it or is that not a consideration?

@dawehner
Ubuntu should be ready for single command install with three lines:

$ sudo apt-get install git php7.0 php7.0-curl php7.0-dom php7.0-gd php7.0-mbstring php7.0-sqlite3
$ curl -sS https://getcomposer.org/installer | php;
$ sudo mv composer.phar /usr/local/bin/composer;

That is a fairly short process, but if your suggested lamp-server solution is faster and easier to set up, we should of course use that, if it reduces the barrier of introduction to Drupal.

Would you care to share the steps involved?

I don't want to pollute this issue with irrelevant discussions, but I feel that getting a new Drupal user ready to actually send off the single command which installs Drupal is as important as the single command solution being built here.

Having helped out onboard users onto Drupal over the years, I've seen some pretty head-scratching issues, which can require some in-depth problem solving and fixing to get a stack stood up.

I'm just saying that no matter what option we choose, there will be head-scratching issues. At MidCamp last week, I was trying to help someone on a Windows laptop figure out why a fresh install of ADD2 on a new Windows 10 Pro machine had an error message while adding a Drupal site that a zip file couldn't be expanded. After 10 minutes neither of us could figure it out (and I had to run to a session :( ).

At least if we use PHP (and maybe Composer) we limit the amount of dependencies and permissions required to almost bare minimum :)

My ultimate end goal (not saying this is "Drupal.org's end goal", just mine) is that we have something akin to a "quick start guide" for Drupal, and it's literally:

1. Download Drupal
2. Run Drupal with some command
3. Now you're running Drupal!

Which this issue achieves pretty handily. Again, comparing more to Node.js, Go, Python, Symfony, Laravel, etc. communities, this is super standard. I don't know if we want to compare Drupal to Wordpress anymore—we jumped the shark to using Composer, giant dependency trees, modern PHP etc., and we live with that (or own it).

That makes sense. I noticed that the biggest difficulty for getting this setup wasn't the patch or the composer command, but 1) applying the patch (getting a wget/curl with OpenSSL support) and 2) getting the prerequisites installed (PHP and Composer).

But I also just noticed the test fail in #67 too so it's NW anyway. :*(

re #109: I personally prefer to use wget myself, but curl is much more universally available. Curl is a requirement for PHP and Drupal. So perhaps we should write any instructions with that in mind and use curl in future examples.

@dawehner wondered if this needed product manager feedback. Not sure, but this is an outstanding idea of course :)
I see this pattern of "3 commands in the terminal to running X" everywhere as well and it would be great to provide the same for Drupal. Great in the sense of catching up with the rest of the world but more importantly as something that would enable us to clean up and focus our onboarding materials (guides, documentation etc).

I personally prefer to use wget myself, but curl is much more universally available. Curl is a requirement for PHP and Drupal. So perhaps we should write any instructions with that in mind and use curl in future examples.

This is great. Would be good to make any necessary decisions with this perspecitive in mind.

I think it's okay that PHP + composer is a requirement before the 3 commands as that is the same exact situation as Node.js. You need node.js to run npm projects. It's sad that PHP doesn't have a native packaging utility bundled like node.js, but that used to be the case with node.js a few years ago. I don't think that getting PHP + composer should be considered one of the 3 commands in that case.

Also composer install is only necessary when testing this issue because the intended use case is someone downloads a Drupal 8.6.0 release, then they can run composer dev-site directly, which saves one step. :-)

So

A. Get and install PHP.
B. Get and install Composer.

1. Download Drupal 8.6.0
2. Extract drupal-8.6.0
3. Open terminal
4. Change directory to drupal-8.6.0
5. composer dev-site
6. Copy URL.
6. Open web browser and paste URL into address bar.

That's 2 commands (cd and composer) in the terminal.

Edit: the following is probably too much bike shedding. Sorry.

+    $process = new Process([
+      $binary,
+      '-S',
+      'localhost:' . $port,
+      '.ht.router.php',
+    ], $root, NULL, NULL, NULL);
+    $output->writeln('Starting webserver on http://localhost:' . $port);
+    $process->run();

Could we write output after this to mention that the command succeeded? That was also confusing when we were testing. We didn't think it was "done" or ready and that the command had stalled out so we killed the process.

💧 Your site is ready for you at URL

On my linux desktop that droplet isn't going to render (and maybe not on Windows either), but emojis are friendly, right?.

+  protected function install($class_loader, ConsoleOutputInterface $output, $profile = 'standard', $langcode = 'en', $site_path = NULL) {

+    $output->writeln('Drupal successfully installed.');
+
+    $user = User::load(1);
+    $one_time_login = user_pass_reset_url($user);
+    $output->writeln("Login using $one_time_login");
+  }

And maybe this could be done so that the Login using is printed after the web server is started? I know that install/start are distinct, but is there ever a reason not to generate a one-time login URL during start if I'm evaluating or trying to get started quickly? I think I would want to be in the administration UI to start exploring or making content right away.

Also to consider, angular cli, drush and webpack dev servers can start a browser on some devices. I personally find this annoying, but others find it useful and it would eliminate the final step of navigating / copying and pasting the URL into the browser.

Also to consider, angular cli, drush and webpack dev servers can start a browser on some devices

Composer has this built into it too:
composer browse drupal/examples takes you to the cgit repo and
composer browse -H drupal/examples takes you to the project page in your system configured browser.

Seems relatively simple to do: https://github.com/composer/composer/blob/685ff8699ba378cc815849fb7daa8d...

Added followup to create an uninstall command as suggested in #101. #2953810: Provide a single command to un-install after installing with single command

Testing with #116 -

Browser does open with one time login link.

However, I get WSOD on quite a few pages, such as /, /admin/content, /admin/reports/dblog. This could be 8.6 issue, or an issue with this patch, not sure.

Debugging this isn't easy as no logs are printed to the screen after the server is started. Would it be possible to have logging output the same way `drush run-server` does?

I tested same patch applied code base and it seems to be a problem with 8.6, not this patch. So marking this back for review.

However, just to show my point for consideration -

dev-site output while getting WSOD pages:

> @php core/scripts/dev-site.php install
> @php core/scripts/dev-site.php start
Starting webserver on http://localhost:50057
^C

drush run-server output while getting WSOD pages:

HTTP server listening on 127.0.0.1, port 8889 (see http://127.0.0.1:8889/), serving site default, logged in as admin...
PHP 7.1.14 Development Server started at Sun Mar 18 07:44:39 2018
Listening on http://127.0.0.1:8889
Document root is /Users/cash/Code/Drupal/scratch/d86
Press Ctrl-C to quit.
[Sun Mar 18 07:44:41 2018] Uncaught PHP Exception Drupal\Core\Field\FieldException: "Attempt to create a field user_picture that does not exist on entity type user." at /Users/cash/Code/Drupal/scratch/d86/core/modules/field/src/Entity/FieldConfig.php line 308
[Sun Mar 18 07:44:41 2018] 127.0.0.1:50256 [500]: /
[Sun Mar 18 07:44:43 2018] Uncaught PHP Exception Drupal\Core\Field\FieldException: "Attempt to create a field user_picture that does not exist on entity type user." at /Users/cash/Code/Drupal/scratch/d86/core/modules/field/src/Entity/FieldConfig.php line 308
[Sun Mar 18 07:44:43 2018] 127.0.0.1:50258 [500]: /

Patch attached fixes the dev start command so that the webserver is started before the browser opens (and also outputs all the output from php -s as per #118 because I was mucking around with the process object).

It also automatically logs the user in to save a button click.

It also removes the global composer timeout setting in favour of something specific for the dev-site command.

It also does a couple of coding standards cleanups.

Here's a hack at the INSTALL.txt docs. It was the minimum instructions that still used Curl. It defaulted to using unzip as that is more readily available on Windows.

I also changed the install message. It was a bit jarring to just see a "wait" message without any expectation what would happen next.

That should mean this can be reviewed for its final time as all the key points in the IS are now covered.

Here's a patch with a test that works. After implementing the test I'm wondering if we can't share a lot of core with #2926633: Provide a script to install a Drupal testsite since that install command seems to be a very special case of what we're implementing here.

Hmmm that test passes locally. Let's see if we can work out what is going on on the bot.

Fixing test coverage and addressing #126 - apart from #126.2 - I have no idea about Windows unzip documentation.

The last interdiff was wrong it should have been.

We don't need so much change to install_begin_request() - the additional parameter is unnecessary.

1. +++ b/core/includes/install.core.inc
   @@ -324,7 +326,7 @@ function install_begin_request($class_loader, &$install_state) {
   -  $site_path = DrupalKernel::findSitePath($request, FALSE);
   +  $site_path = empty($install_state['site_path']) ? DrupalKernel::findSitePath($request, FALSE) : $install_state['site_path'];
   
   +++ b/core/lib/Drupal/Core/Installer/InstallerKernel.php
   @@ -66,4 +67,15 @@ public function getInstallProfile() {
   +  /**
   +   * {@inheritdoc}
   +   */
   +  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);
   +    return $kernel;
   +  }
   

   For reviewers, these are the changes to the installer to support the new command. They really exist only for testing so that we can install to a path other than sites/default.

2. +++ b/core/lib/Drupal/Core/Command/DevInstallCommand.php
   @@ -0,0 +1,178 @@
   +      ->addOption('site_path', NULL, InputOption::VALUE_OPTIONAL, 'Forces to use a specific site directory.')
   
   +++ b/core/lib/Drupal/Core/Command/DevStartCommand.php
   @@ -0,0 +1,199 @@
   +      ->addOption('site_path', NULL, InputOption::VALUE_OPTIONAL, 'Forces to use a specific site directory.')
   

   In reality these command options are only really for testing the commands so I think we need to update the docs. And they wouldn't actually work. If we want a user to be able to do this then we need to change \Drupal\Core\DrupalKernel::findSitePath() to allow us to determine the site path from an environmental variable.

Patch attached removes the site_path option since it is not going to work for regular users without changes to DrupalKernel::findSitePath(). Instead we use an environment variable DRUPAL_DEV_SITE_PATH to make the commands testable. We can open a followup to use this same variable in DrupalKernel::findSitePath() and then we could add the options back. This would be neat as you could then demo two sites at the same time (perhaps a before and after).

The following line in core/INSTALL.txt is showing some weird characters.

Wait… installation can take a minute or two.

@kenorb that's a unicode ellipsis... what are you seeing in \Drupal\Core\Asset\CssCollectionOptimizer::optimize() there's a comment in there that uses exactly the same character. We use it all over the place.

Patch addresses #134. Also I tried to test the --force option and it just didn't work cleanly. In order to actually delete the sqlite db and to properly re-install you have to delete the entire files directory. So rather than do all of this here I think we should just tell people that the site is installed already and be done. For me that's all that is necessary for a viable patch. If people need a way to re-install a site then we can work on that in followups.

Added test coverage of trying install over the top of an existing site.

With #135 when running the composer run-script dev-site I see:

> Drupal\Core\Composer\Composer::removeTimeout
> @php core/scripts/dev-site.php install
Drupal is already installed.
> @php core/scripts/dev-site.php start
PHP 7.1.10 Development Server started at Wed Mar 21 11:39:59 2018
Listening on http://localhost:62293
Document root is /Volumes/devdisk/dev/sites/drupal8alt.dev
Press Ctrl-C to quit.
[Wed Mar 21 11:40:14 2018] ::1:62336 [200]: /core/assets/vendor/normalize-css/normalize.css?p5wkm7
[Wed Mar 21 11:40:14 2018] ::1:62337 [200]: /core/themes/stable/css/system/components/ajax-progress.module.css?p5wkm7
[Wed Mar 21 11:40:14 2018] ::1:62338 [200]: /core/themes/stable/css/system/components/align.module.css?p5wkm7
// ... and all the rest.

I think this looks good and is working as expected. The one thing that would be nice is if the composer command passed the arguments on the install command so you could select the profile. Or maybe if there was no profile argument it asked.

Composer doesn't allow us to interact with the user https://github.com/composer/composer/issues/5856 :(

I'm not sure we should provide a composer command.

@dawehner totally! One thing I was thinking about is are the two commands actually necessary and whether dev-site was the right name. I think a single quick-start command might be a good idea. This command would install (if necessary) and start up the PHP web server.

I really dig this patch, man. I sense much win within, and couldn't find any serious problems. Only random questions and nits.

1. +++ b/core/INSTALL.txt
   @@ -10,6 +11,31 @@ CONTENTS OF THIS FILE
   +Wait… installation can take a minute or two.
   

   "Wait" is followed by a smart ellipsis. Can it be three periods instead?

2. +++ b/core/INSTALL.txt
   @@ -10,6 +11,31 @@ CONTENTS OF THIS FILE
   +- A successful installation will result in logging in to your new site in your
   +  web browser.
   

   Why is this separated in a list item? It seems like it could come right after "...take a minute or two."

3. +++ b/core/includes/install.core.inc
   @@ -324,7 +326,7 @@ function install_begin_request($class_loader, &$install_state) {
   +  $site_path = empty($install_state['site_path']) ? DrupalKernel::findSitePath($request, FALSE) : $install_state['site_path'];
   

   Nit: I think we could shorten the line by using the ?: construction.

4. +++ b/core/lib/Drupal/Core/Command/DevInstallCommand.php
   @@ -0,0 +1,171 @@
   +      ->addOption('install_profile', NULL, InputOption::VALUE_OPTIONAL, 'Install profile to install the site in. Defaults to standard', 'standard')
   +      ->addOption('langcode', NULL, InputOption::VALUE_OPTIONAL, 'The language to install the site in. Defaults to en', 'en')
   

   The descriptions should probably end with periods.

5. +++ b/core/lib/Drupal/Core/Command/DevInstallCommand.php
   @@ -0,0 +1,171 @@
   +      ->addOption('site_name', NULL, InputOption::VALUE_OPTIONAL, 'Set the site name.', 'Drupal')
   

   This should say what the default value will be.

6. +++ b/core/lib/Drupal/Core/Command/DevInstallCommand.php
   @@ -0,0 +1,171 @@
   +    if (!extension_loaded('pdo_sqlite')) {
   +      $output->getErrorOutput()->writeln('You need to have sqlite installed.');
   +      return;
   +    }
   

   It seems to me that we should allow a user to specify a database URL, in case they don't have SQLite or don't want to use it. But we should continue to default to SQLite. Maybe that's more follow-up material, though.

   Also, the error message could be more helpful. Maybe it could include a URL on php.net explaining how to install the extension?

7. +++ b/core/lib/Drupal/Core/Command/DevInstallCommand.php
   @@ -0,0 +1,171 @@
   +    if (!file_exists($site_path)) {
   

   is_dir() might be preferable here.

8. +++ b/core/lib/Drupal/Core/Command/DevInstallCommand.php
   @@ -0,0 +1,171 @@
   +      mkdir($site_path, 0775);
   +    }
   +    if (!file_exists("{$site_path}/settings.php")) {
   +      copy("sites/default/default.settings.php", "{$site_path}/settings.php");
   +    }
   

   We should probably throw exceptions and fail if copy() or mkdir() fail.

9. +++ b/core/lib/Drupal/Core/Command/DevInstallCommand.php
   @@ -0,0 +1,171 @@
   +    $site_path = getenv('DRUPAL_DEV_SITE_PATH');
   +    return $site_path ?: 'sites/default';
   

   Nit: This could be collapsed to a single line.

10. +++ b/core/lib/Drupal/Core/Command/DevStartCommand.php
    @@ -0,0 +1,208 @@
    +      ->addOption('suppress_login', 's', InputOption::VALUE_NONE, 'Disables opening a login URL.')
    +      ->addOption('no_tty', NULL, InputOption::VALUE_NONE, 'Disables tty.');
    

    Shouldn't the option names have dashes, not underscores?

11. +++ b/core/lib/Drupal/Core/Command/DevStartCommand.php
    @@ -0,0 +1,208 @@
    +   *   The drupal kernel.
    

    Nit: 'drupal' should be 'Drupal'.

12. +++ b/core/lib/Drupal/Core/Command/DevStartCommand.php
    @@ -0,0 +1,208 @@
    +   * @throws \Exception
    +   *   Exception thrown if kernel does not boot.
    

    I"m not sure we need this, because the method itself does not throw any exceptions.

13. +++ b/core/lib/Drupal/Core/Command/DevStartCommand.php
    @@ -0,0 +1,208 @@
    +    $address = '0.0.0.0';
    +    $port = 0;
    +    $socket = socket_create(AF_INET, SOCK_STREAM, SOL_TCP);
    +    socket_bind($socket, $address, $port);
    +    socket_listen($socket, 5);
    +    socket_getsockname($socket, $address, $port);
    +    return $port;
    

    Is there anything here that might fail? If so, how should we handle those failures?

14. +++ b/core/lib/Drupal/Core/Command/DevStartCommand.php
    @@ -0,0 +1,208 @@
    +   * opens a url in your system default browser
    

    'opens' should be 'Opens', and 'url' should be 'URL'.

15. +++ b/core/lib/Drupal/Core/Command/DevStartCommand.php
    @@ -0,0 +1,208 @@
    +   *   The URL to brower to.
    

    s/brower/browse

16. +++ b/core/lib/Drupal/Core/Command/DevStartCommand.php
    @@ -0,0 +1,208 @@
    +      return $output = $process->run();
    

    Never seen this kind of thing before. What will it return?

17. +++ b/core/lib/Drupal/Core/Command/DevStartCommand.php
    @@ -0,0 +1,208 @@
    +    $linux = (new Process('which xdg-open'))->run();
    +    $osx = (new Process('which open'))->run();
    

    Nit: Can these variables be prefixed with is_?

18. +++ b/core/lib/Drupal/Core/Command/DevStartCommand.php
    @@ -0,0 +1,208 @@
    +    if ($this->openBrowser("http://localhost:$port$one_time_login/login", $io) === 1) {
    

    We're assuming the server name is localhost. 127.0.0.1 might be safer here. Even better, it'd be cool if the server name could be passed in as a command option.

19. +++ b/core/lib/Drupal/Core/Command/DevStartCommand.php
    @@ -0,0 +1,208 @@
    +      $io->error('Error while opening up a one time login URL');
    

    This error message is superfluous because openBrowser() will echo out a useful error message if the browser cannot be opened.

20. +++ b/core/lib/Drupal/Core/Command/DevStartCommand.php
    @@ -0,0 +1,208 @@
    +    if (($binary = $finder->find()) && $binary === FALSE) {
    

    This logic seems strange. What will find() return if it fails? If it's false, won't $binary automatically fail the check, and cause the exception?

21. +++ b/core/lib/Drupal/Core/Command/DevStartCommand.php
    @@ -0,0 +1,208 @@
    +  protected function getSitePath() {
    +    $site_path = getenv('DRUPAL_DEV_SITE_PATH');
    +    return $site_path ?: 'sites/default';
    +  }
    

    Nit: Could be collapsed into a single line.

22. +++ b/core/scripts/dev-site.php
    @@ -0,0 +1,22 @@
    +$application = new Application('dev-start', '8.0.x');
    

    Shouldn't the version be 8.6.x, or \Drupal::VERSION?

+In the instructions below, replace the version 8.x.x with the specific version
+you wish to download. Example: 8.6.0.zip. You can find the latest stable version
+at https://www.drupal.org/project/drupal.
+
+- curl -sS https://ftp.drupal.org/files/projects/drupal-8.x.x.zip --output drupal-8.x.x.zip
+- unzip drupal-8.x.x.zip
+- cd drupal-8.x.x
+- composer dev-site
+

I think the first two commands need additional review. Maybe in a follow-up?

Let's say I'm a new user. I go to drupal.org. I click "Download & Extend". I click "Download .zip". I find the downloaded file and double-click it to extract it (works on Windows and MacOS). I read INSTALL.txt. Do I need to run curl, unzip and cd again?

My next step is to open a command prompt (Terminal.app, PowerShell, Git Bash, Cmd or Terminal/xterm/aterm/iterm/whatever). I think some of the more OS-specific commands are best suited for online documentation guide at https://www.drupal.org/docs/8/install rather than INSTALL.txt or even drupal.com/getdrupal.com.

Would this work?

In the instructions below, replace the version 8.x.x with the specific version
you wish to download. Example: 8.6.0. You can find the latest stable version at
https://www.drupal.org/project/drupal. Run the following commands after you
have downloaded Drupal and extracted the .zip or .tar.gz file. Now open your
:"

1. cd drupal-8.x.x/
2. composer dev-site

I am not sure how to describe change directory since who knows where an user will have downloaded Drupal to. It's probably ~/Downloads or C:\Users\username\Downloads by default.

I am not sure how to describe change directory since who knows where an user will have downloaded Drupal to

The pattern I followed in my book was always cd /path/to/drupal-8.x.x. Helps convey that it isn't _right_ there all the time.

Discussed these changes with @dawehner.

• The patch moves the command to core/scripts/quick-start.php
• The command is named quick-start because this is not only for developers and in the future it could be expanded to be the CLI installer for Drupal so let's use quick-start and add a disclaimer
• It removes the composer dependency - commands run via composer can not ask the user questions
• It allows the user to select the profile the same way the interactive installer does
• The user is logged in and redirected to the front page - this way they get a more welcoming start than the user edit form

The security tag was added when the script didn't do

if (PHP_SAPI !== 'cli') {
 return;
}

it does now... and anyway on a well configured apache I get a 403 cause of .htaccess when I tried to access the script but the double protection is good. Also the script will not install over an existing site so that's good too.

Installing the standard profile via the quick-start command was producing an error because the CommentManager service is required to generate the 1 time login link and some where in it's dependencies it required a request... so adding one to the stack fixes that.

1. +++ b/core/lib/Drupal/Core/Command/QuickStartCommand.php
   @@ -0,0 +1,70 @@
   +      ->addOption('install_profile', NULL, InputOption::VALUE_OPTIONAL, 'Install profile to install the site in.')
   +      ->addOption('langcode', NULL, InputOption::VALUE_OPTIONAL, 'The language to install the site in. Defaults to en.', 'en')
   +      ->addOption('site_name', NULL, InputOption::VALUE_OPTIONAL, 'Set the site name. Defaults to Drupal', 'Drupal')
   +      ->addOption('suppress-login', 's', InputOption::VALUE_NONE, 'Disables opening a login URL.')
   +      ->addOption('show-server-output', NULL, InputOption::VALUE_NONE, 'Shows output from the PHP development server')
   

   Some of the options have underscores, and some have dashes. They should be consistent (I think dashes are preferred, since that's the standard for command-line options.) Also, some of the descriptions mention the default value, and others don't. Some end with periods, and others don't.

2. +++ b/core/lib/Drupal/Core/Command/QuickStartInstallCommand.php
   @@ -0,0 +1,242 @@
   +
   +  /**
   +   * Constructs a new QuickStartInstallCommand command.
   +   *
   +   * @param object $class_loader
   +   *   The class loader.
   +   */
   +  public function __construct($class_loader) {
   +    parent::__construct('install');
   +    $this->classLoader = $class_loader;
   +  }
   

   Seeing as how the $classLoader member is type hinted, should the $class_loader parameter be as well?

3. +++ b/core/lib/Drupal/Core/Command/QuickStartInstallCommand.php
   @@ -0,0 +1,242 @@
   +      ->addOption('install_profile', NULL, InputOption::VALUE_OPTIONAL, 'Install profile to install the site in.')
   +      ->addOption('langcode', NULL, InputOption::VALUE_OPTIONAL, 'The language to install the site in. Defaults to en.', 'en')
   +      ->addOption('site_name', NULL, InputOption::VALUE_OPTIONAL, 'Set the site name. Defaults to Drupal', 'Drupal')
   

   Underscores or dashes? All the commands in this patch should be consistent. Also, the descriptions should mention default values, if any, and end with periods.

4. +++ b/core/lib/Drupal/Core/Command/QuickStartInstallCommand.php
   @@ -0,0 +1,242 @@
   +    if (!extension_loaded('pdo_sqlite')) {
   +      $output->getErrorOutput()->writeln('You need to have sqlite installed.');
   +      return;
   +    }
   

   I think we should rephrase this error message to be more helpful. Maybe "You must have the SQLite PHP extension installed. See https://secure.php.net/manual/en/sqlite.installation.php for instructions."

5. +++ b/core/lib/Drupal/Core/Command/QuickStartInstallCommand.php
   @@ -0,0 +1,242 @@
   +   * @param string $profile
   +   *   (optional) The installation profile to use.
   +   * @param string $langcode
   +   *   (optional) The language to install the site in.
   +   * @param string $site_path
   +   *   (optional) The path to install the site to, like 'sites/default'.
   

   These should mention what the default values are.

6. +++ b/core/lib/Drupal/Core/Command/QuickStartInstallCommand.php
   @@ -0,0 +1,242 @@
   +    if (!is_dir($site_path)) {
   +      mkdir($site_path, 0775);
   +    }
   

   mkdir() may return FALSE, we should throw an exception if it does.

7. +++ b/core/lib/Drupal/Core/Command/QuickStartInstallCommand.php
   @@ -0,0 +1,242 @@
   +   * @see \_install_select_profile()
   

   Nit: Does this need a leading backslash?

8. +++ b/core/lib/Drupal/Core/Command/QuickStartServerCommand.php
   @@ -0,0 +1,222 @@
   +    if (($binary = $finder->find()) && $binary === FALSE) {
   

   This logic is a little convoluted and makes me uncomfortable. Can it be refactored into a more "traditional" form that doesn't feel as implicit/magical?

Thanks for the review.

1. Fixed - had just standardised on -
2. We can not know the classloader type. So object is the honest typehint
3. Fixed
4. Fixed
5. Fixed by removing - these defaults are unnecessary - we're getting them all from somewhere else.
6. Fixed
7. Fixed
8. Fixed

Also

• implemented a test for running the server command when there is no install and made the error that occurs when you do that more helpful.
• Always generate a one time login url - this helps to test standard and ensure we don't have #146 happen. Also if you suppress the automatic login the one time link is displayed to the user.

So how should the profile selector work?

machine name | proper name

 Select an installation profile [Standard]:
  [standard  ] Standard
  [minimal   ] Minimal
  [demo_umami] Demo: Umami Food Magazine (Experimental)

machine name | description

 Select an installation profile [Install with commonly used features pre-configured.]:
  [standard  ] Install with commonly used features pre-configured.
  [minimal   ] Build a custom site without pre-configured functionality. Suitable for advanced users.
  [demo_umami] Install an example site that shows off some of Drupal’s capabilities.

proper name | description

 Select an installation profile [Install with commonly used features pre-configured.]:
  [Standard                                ] Install with commonly used features pre-configured.
  [Minimal                                 ] Build a custom site without pre-configured functionality. Suitable for advanced users.
  [Demo: Umami Food Magazine (Experimental)] Install an example site that shows off some of Drupal’s capabilities.

Not in all cases Console helps you auto-complete so that's really good. Personally I really like having the description because it tells you why you are selecting an installation profile. I also really like having the machine name because it is what you'd pass in if you want to avoid the prompt. So I've coded up option 2.

The other thing I'm pondering is if we should make the install profile an argument rather than an option - to be more like drush site-install?