Use path relative to DRUPAL_ROOT in configuration directories

Discussed with @berdir on IRC and agreed that this was important due to the fact that:

because right now, it's weird to have it still within drupal but outside of the files path

So for example if you want to store configuration in sites/all/configM and keep the staging under git you'd have to use an absolute path. With this patch you got use a path relative to DRUPAL_ROOT. This makes transporting settings.php easier.

Hm. The original idea of #1741804: Implement a config import/staging directory was to allow to move the configuration directories outside of the public document root. For example, for security reasons.

Looking at the configured paths in settings.php in this patch, it appears we'd be removing support for that option?

That said, I was never really happy with the extra 'absolute' key. Normally, you'd expect file paths to work exactly as in any other PHP code; i.e.:

sites/default/files/config_active → DRUPAL_ROOT/sites/default/files/config_active
/var/drupal/config → /var/drupal/config
public://config_active → DRUPAL_ROOT/sites/default/files/config_active

The focus is on natural absolute vs. relative paths. The last stream wrapper example can be ignored, if it's not possible due to bootstrap dependencies.

re #14 as far as I understand that is exactly what Berdir is proposing :)

I assume that this change is the error:

+++ b/core/includes/install.inc
@@ -452,17 +452,10 @@ function drupal_install_config_directories($mode = NULL) {
     $settings['config_directories'] = array(
-      CONFIG_ACTIVE_DIRECTORY => array(
-        'path' => (object) array(
-          'value' => 'config_' . $config_directories_hash . '/active',
-          'required' => TRUE,
-        ),
-      ),
-      CONFIG_STAGING_DIRECTORY => array(
-        'path' => (object) array(
-          'value' => 'config_' . $config_directories_hash . '/staging',
-          'required' => TRUE,
-        ),
+      'value' => array(
+        CONFIG_ACTIVE_DIRECTORY => conf_path() . '/files/config_' . $config_directories_hash . '/active',
+        CONFIG_STAGING_DIRECTORY => conf_path() . '/files/config_' . $config_directories_hash . '/staging',
+        'required' => TRUE,
       ),
     );

Unfortunately, I'm not familiar with the structure/features of install_rewrite_settings().

But it looks like 'required' is placed in the wrong spot and the 'value' + 'required' actually need to be properties of an anonymous object?

The patch attached should fix it.

After applying this patch the values in my settings.php are:

$config_directories['active'] = 'sites/default/files/config_BZwtrpONxujo5cY2DwnJzOvHuTpNg5NBRzNssQBYDo0/active';
$config_directories['staging'] = 'sites/default/files/config_BZwtrpONxujo5cY2DwnJzOvHuTpNg5NBRzNssQBYDo0/staging';

Go testbot go!

I think we need to fix the installer tests too

22: 1887750.22.patch queued for re-testing.

22: 1887750.22.patch queued for re-testing.

Looks good to me, let's move forward here.

So config was not using absolute paths before but because file_get_contents used in FileStorage::read defaults to $use_include_path = false this does not pose a performance problem. Then, I guess we need to file a separate issue for listAll not supporting outside of webroot paths, right?

22: 1887750.22.patch queued for re-testing.

This patch breaks drush si:

is_dir() expects parameter 1 to be string, array given install.inc:522                  [warning]
strpos() expects parameter 1 to be string, array given file.inc:267                     [warning]
rtrim() expects parameter 1 to be string, array given file.inc:532                      [warning]
strpos() expects parameter 1 to be string, array given file.inc:267                     [warning]
rtrim() expects parameter 1 to be string, array given file.inc:532                      [warning]
WD php: Exception: File system:                                                         [error]

The directory  does not exist. The directory  does not exist. An automated attempt to
create this directory failed, possibly due to a permissions problem. To proceed with the
installation, either create the directory and modify its permissions manually or ensure
that the installer has the permissions to create it automatically. For more information,
see INSTALL.txt or the online handbook. in install_display_requirements() (line 2569 of
/Users/webchick/Sites/8.x/core/includes/install.core.inc).
Exception: File system: 

The directory <em class="placeholder"></em> does not exist. The directory <em class="placeholder"></em> does not exist. An automated attempt to create this directory failed, possibly due to a permissions problem. To proceed with the installation, either create the directory and modify its permissions manually or ensure that the installer has the permissions to create it automatically. For more information, see INSTALL.txt or the <a href="http://drupal.org/server-permissions">online handbook</a>. in install_display_requirements() (line 2569 of /Users/webchick/Sites/8.x/core/includes/install.core.inc).
Drush command terminated abnormally due to an unrecoverable error.                      [error]

This might be unavoidable, or it might be pointing to a bug. Setting back to needs review to find out.

@webchick drush si works fine for me. I suspect that you did not re-create your settings.php and therefore had the $config_directories global declared with the old structure as that would cause the error you are seeing.

What this does mean (and it is unavoidable) is that this patch will break every existing install of Drupal 8.

Also, unless a patch stops the testbot from installing with drush (we have no choice then) we should never hold up patches on that. It should be up to drush to keep up with core.

Committed/pushed to 8.x, thanks!

Needs change notice (or update to one) for the changed $settings.