I can't really review it from a code perspective, though I suspect the mkdir() calls are now unnecessary.

I think the install_profile setting should be documented in default.settings.php. Particularly since it is now required when settings.php is not writable (it wasn't required before). Can that be part of this issue?

So far, I've only noticed this problem when installing with drush, which *does* allow to install when your settings.php is not writable, as long as all the required information is already there.

The fix makes sense for the UI think, although it might be a bit confusing, not sure. Also not exactly sure how that will work in combination with a distribution, which skips that step too?

For drush, it's a bit confusing/unexpected too. Because you are required to provide the install profile upfront, but it will then actually not be used. To test, drush si -y standard, and then drush si -y minimal will install again in standard. Without ever telling you so. Maybe that's something that drush needs to take care of, not sure.

@Berdir all this patch does is say that if install_profile is set in settings.php then that is what is used which is consistent with what we for databases and config_directories. HEAD is currently very broken because if you had an exclusive distribution with a non writable settings.php which did not have the install_profile set and install using drush then modules from the profile are used during installation but once Drupal is installed they won't be.

Discussed this with @Berdir in IRC we decided to not force the profile according to what is in settings.php but to instead ensure that we rewrite if necessary. This means that drush si will crash if working with a write protected settings.php and a mismatched install profile.

+++ b/core/modules/system/src/Tests/Installer/InstallerExistingSettingsTest.php
@@ -33,10 +33,8 @@ protected function setUp() {
// Actually the install profile should be skipped to because it is written
// to settings.php.
- // @todo https://www.drupal.org/node/2451369 Fix install_profile so that it
- // is written to an existing settings.php if possible or if set used.
$this->settings['settings']['install_profile'] = (object) array(
- 'value' => 'testing',

Comment is a bit strange.. why Actually, should "skipped to" be "skipped too", but too doesn't make sense to me either...

+++ b/sites/default/default.settings.php
@@ -246,6 +246,16 @@
+ * If this is set prior to installation this value will used regardless of any
+ * parameters passed to install_drupal(). Changing this after installation is
+ * not recommended as it changes which directories are scanned during extension
+ * discovery.
+ */
+# $install_profile = '';