Follow-up to #2016629: Refactor bootstrap to better utilize the kernel

$conf_path is not found in settings.php

CommentFileSizeAuthor
#1 2280383-1.patch549 bytesandypost
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

andypost’s picture

FileSize
549 bytes
sun’s picture

Why is $conf_path no longer defined when settings.php is read-in?

That sounds like a major bug.


Nevertheless, changing this to use __DIR__ would be fine either way.

donquixote’s picture

Status: Needs review » Reviewed & tested by the community

I like this, __DIR__ is always preferable if we know that it has to be in the same directory.
Even if the $conf_path variable DID exist, I would still prefer __DIR__.

rtbc from me.
Unless you want more feedback from others?

sun’s picture

Status: Reviewed & tested by the community » Needs review

We definitely need to figure out what happened to the $conf_path variable. It should be set in the local scope and be available when settings.php is loaded.

donquixote’s picture

what happened to the $conf_path variable.

It was renamed to $site_path.

Old code in drupal_settings_initialize(), removed in #2016629:

-function drupal_settings_initialize() {
-  // Export these settings.php variables to the global namespace.
-  global $base_url, $cookie_domain, $config_directories, $config;
-  $databases = array();
-  $settings = array();
-  $config = array();
-
-  // Make conf_path() available as local variable in settings.php.
-  $conf_path = conf_path();
-  if (is_readable(DRUPAL_ROOT . '/' . $conf_path . '/settings.php')) {
-    require DRUPAL_ROOT . '/' . $conf_path . '/settings.php';
-  }

New code in DrupalKernel::initialize(), added in #2016629:

+  public static function initialize($site_path) {
+    // Export these settings.php variables to the global namespace.
+    global $base_url, $cookie_domain, $config_directories, $config;
+    $settings = array();
+    $config = array();
+    $databases = array();
+
+    // Make conf_path() available as local variable in settings.php.
+    if (is_readable(DRUPAL_ROOT . '/' . $site_path . '/settings.php')) {
+      require DRUPAL_ROOT . '/' . $site_path . '/settings.php';
+    }
donquixote’s picture

the $conf_path variable. It should be set in the local scope and be available when settings.php is loaded.

Do we need it there? What would be a use case?
And would it be ok if people have to type $site_path instead? Imo, $site_path is even more logical..

neclimdul’s picture

Status: Needs review » Reviewed & tested by the community

My bad, but that's definitely better any ways. Leaking the implementation of the method into the include makes re-factoring like this harder.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed eb3e786 and pushed to 8.x. Thanks!

  • Commit eb3e786 on 8.x by alexpott:
    Issue #2280383 by andypost: Fixed settings.local.php after new bootstrap...

Status: Fixed » Closed (fixed)

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