Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
The core installer in D8 appends the database configuration at the end of the settings.php file, instead of after the $databases structure is documented, currently around line 220. For a person new to setting up Drupal this makes managing the settings file more confusing than it needs to be. For example, the documentation around the $databases variable is near the top of the file whereas the variable is added at the end.
Note: D7 and older used to work that way, so this is a regression.
Steps to reproduce
Run the site installer with a codebase that does not have an existing settings.php file present.
Proposed resolution
Change drupal_rewrite_settings() so that it replaces variables that already exist in the file rather than appending them.
Remaining tasks
Work out a fix to drupal_rewrite_settings() that replaces variables which already exist in the file.
Add test coverage.
User interface changes
n/a
API changes
n/a
Data model changes
n/a
Release notes snippet
TBD
| Comment | File | Size | Author |
|---|---|---|---|
| #13 | drupal_rewrite_settings_in_place-2466799-13.patch | 4.13 KB | xumepadismal |
Comments
Comment #1
dawehnerIn which sense is that a problem? Isn't the end the place where you actually have a easy time to look it up?
Comment #2
DamienMcKennaThe config file is broken up into several sections, each section devoted to specific things with oodles of comments. Why would the code not be inserted where the corresponding comments are? It seems like the legacy code was replaced with simple file concatenation rather than going through the effort of inserting the values where they should actually be.
Comment #3
DamienMcKennaIn short, in D7 and older the file made sense, you could scan through the file and see the appropriate configuration details. In D8 this is no longer possible because all of the customized details are dumped at the end. We don't need justification of why it is the way it is, or defense of why people should be able to deal with it, it's a bad DX we just need to fix it.
Comment #4
dawehnerI totally understand the point from where you are coming from.
Honestly though, I really enjoy having all actual configuration though in one place, as I hate to remember those line numbers to look things up :)
Just a pointer,
core/lib/Drupal/Core/Installer/Form/SiteSettingsForm.php:158is writing out this settings usingdrupal_rewrite_settings(),so we might want to fix that one.
Comment #5
DamienMcKennaThanks for the pointer, I'll try to dig into it soon.
Comment #6
justindodge CreditAttribution: justindodge commentedSince there's some question about the validity of this as an issue, here is a use-case where it causes a problem:
In our setup, one of the local overrides to the settings file is the database connection information itself, which on our development server is derived programmatically based on the location of the installation.
When Drupal rewrites the settings file and appends this information to the bottom of the file, it overrides any database configuration that you are already overriding in a local settings include because the new database variable is appended below the include (note - I'm not actually using the convention of settings.local.php, but a different file name in a common php include location - but I don't think drupal_rewrite_settings takes this into account anyway).
In D8 this type of local override is actually documented in the settings.php file itself rather than just being inferred by common convention like in D7 and prior, so it seems fair to call this a bug to me because the 'appendage' will potentially break the documented behavior.
Comment #7
frobThis issue should probably be around this function being flat out broken. There is lots of logic that is supposed to make this not happen. I do not know why it doesn't work, but this function is supposed to add the settings inline, replacing their previous placement.
I am updating the title to reflect this.
Comment #11
xumepadismal CreditAttribution: xumepadismal as a volunteer commentedOkay, that's a first try. $databases and $config_directories should be OK, but it lacks some logic/tests for those $settings and globals, commented out in default.settings.php. I guess that
$settings['install_profile']still in the end of the file...Comment #13
xumepadismal CreditAttribution: xumepadismal as a volunteer commentedComment #24
DamienMcKennaI updated the issue title and summary per suggestion from lendude.