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
On current version of default.settings.php there are some dangerous config overrides examples that - if enabled (aka "uncommented") before installing Drupal - they could break the website.
On example is installing Drupal 8 with the default child theme declared in settings.php with the $config['system.theme']['default'] variable results in fatal PHP error caused by the defined default theme and it's parent theme(s) not being installed during the Drupal install process. This also effect custom base themes by not installing the Stable base theme, as is updating Drupal core from a pre D8 RC1 version (any version that did not have the Stable base theme). This irregular and unexpected behaviours can also occur with other values that can be setting in this file.
In general there are few scenarios where you want config overrides in your settings.php before installation, and in general they should be used only by advanced users, only fully understanding the consequences.
Proposed resolution
The addition of information to the comments (and included examples) to warning of these scenarios and the removal of setting examples that can cause these issues.
Remaining tasks
All documentation tasks are being handled in the child issue Documentation of Configuration Overrides
| Comment | File | Size | Author |
|---|---|---|---|
| #59 | 2586887-59.patch | 874 bytes | LittleCoding |
Comments
Comment #2
LittleCodingComment #3
star-szrHi Rob!
This likely means you just need to run database updates (
drush updb/update.php). This will install the new, default Stable base theme. #2575421: Add a Stable base theme to core and make it the default if a base theme is not specifiedThis is under the assumption that you had already installed your theme before updating to RC1 and it's not a new theme you're creating. Keeping this issue as a bug report until we're sure what the circumstances were. If you can provide steps to reproduce the bug from a clean install that would be really helpful!
Thanks for the pointer to the docs by the way, we may want to update those.
Comment #4
LittleCodingHappy Thanksgiving weekend to you Scott,
Yes, this error has occurred for me updating from 8.0.0-beta16 to 8.0.0-rc1. fresh install of BETA16, manual download and replacement of all core files (excluding: /modules /sites /themes) of RC1.
This would be when the error starts to occurred and as we should expect it does not effect the admin theme. The error continues even after manually running the update via the admin panel or directly surfing to /update.php and completing database updates (without error messages displayed or logged).
Core themes are not effected by this due to the fact they are ether child themes of Classy (Bartik, Seven) or use a "base theme" key of "false" (Classy, Stark) in thier *.info.yml. Omission of other required keys in the *.info.yml does not trigger a PHP error (for example leaving out Description).
What would DRUSH be doing that is not covered in the manual update of core steps?
(I will also try this with a clean install of rc1 later today between working on the turkey)
Comment #5
star-szrHappy Thanksgiving weekend to you too :)
Thanks for the additional info! We added an upgrade path to install Stable for folks who have a custom theme with no base theme set, that should have covered you. I tested on a clean install of RC1 and creating a new theme didn't exhibit this problem for me.
Shouldn't make a difference whether you run the updates through drush or update.php but I'm wondering how you got past the Stable install update hook in your case. The update hook should have picked up that you had a theme installed that did not have a base theme declared and installed Stable for you via the database updates mechanism.
Comment #6
LittleCodingI went into the admin panel to check if the Stable theme was enabled (admin/appearance) and I got an error with the page generation for this admin page "The website encountered an unexpected error. Please try again later."
I gave a go at using the Stable theme for the base theme key value (clear cache and run the db update), it has fixed the error on the Appearance admin page. Then I went back to see if this had fix the first error (clear cache and run the db update), but no luck.
Comment #7
LittleCodingI think we pin pointed it there. When updating from BETA16 to RC1 the Stable theme did not install itself. When I looked at a clean install of RC1 the listed themes:
Installed themes
Uninstalled themes
but with the BETA16 to RC1 update the listed themes:
Installed themes
Uninstalled themes
There is almost 30 themes from what looks to be development/testing of the core modules. Odd that between rebuild and update these were not clean up.
Comment #8
star-szrThe test themes are visible because of a setting in settings.local.php, extension discovery something or other. Sorry, on my phone now :)
Comment #9
LittleCodingYes, you are correct that it would be the "$settings['extension_discovery_scan_tests']" value. And I have to do some more testing but, it looks like my use of the settings.local.php is tripping a few php errors (even with the default values). When I have some more time later this week. I will test with running the upgrade with and without the localized settings file in use.
Comment #10
LittleCodingThere is a problem with the install script. Doing a clean install of 8.0.0 RC2 with $config['system.theme']['default'] = 'MyCustomTheme'; defined in settings.php the theme is set to default BUT is not installed. I am thinking this is where it all is coming from. The install script being able to set the default theme but not able to install it or it's parent themes.
The manual selection and setting of a theme via Appearance admin page correctly installs any and all required parent themes (including Stable in the case of a custom base theme).
Comment #11
star-szr@LittleCoding I think that would make sense, thanks! Where are you getting that settings.php line from? Haven't seen that before.
Comment #12
LittleCodingIt is the D8 version of $conf['theme_default']. It is in the default.settings.php around line 615 of D8 RC2.
Comment #13
star-szrThanks, I think we've found our bug then. Let me know if you think this title makes sense. The fact that the line is included in default.settings.php IMO makes this major and perhaps we could do something similar here to what's going on in #2581443: Make Classy extend from the new Stable base theme.
Comment #14
LittleCodingNew title sounds good to me. I have taken a crack at a new summary too.
Comment #15
LittleCodingIf Classy is going to be a child theme of Stable then that would avoid part of the issue here in that the Stable theme would have to be installed as part of the default Drupal script.
Comment #16
star-szrAgreed with #15 but this is still a bug in general that should be solved IMO or at the very least a workaround provided. Thanks @LittleCoding!
Comment #17
LittleCodingA work around for the original trigger of this bug would be to update Defining a theme with an .info.yml file page, Making the "base theme" key of required, and give information on the use of the "false" value when creating a base theme (as is done with the Stark and Stable base themes provided with D8 core).
As for the install script part of the bug, I missed the Twig hangout yesterday and will try to chat with the Twig team this weekend on IRC to try and get may hands dirty with some coding.
Comment #18
star-szr@LittleCoding I don't think this bug is limited to the use of the Stable theme. base theme is not required, that was done very intentionally so that the default behaviour is to get the Stable theme :)
I think the real bug here is that if you declare a subtheme as your default theme, the dependencies are not auto-installed on install for you. Could be wrong but that's my interpretation.
Comment #19
LittleCoding@Cottser You are correct sir. I did an update from D8 RC1 to RC4 with a basic custom base theme set as default theme (once via settings.php and once via database settings), with the info.yml key value "base theme" not declared. And both before and after the update the base theme was working great, no PHP errors or warnings. So the bug is truly just with the Stable theme not being installed.
And it should be noted that as of RC4 core themes Stable and Classy have been defined as Hidden and no longer appear in the list of available themes in the Appearance admin page.
Comment #20
joelpittet@LittleCoding and with the the latest release there is an update.php thing that enables stable.
drush updbor run them in the browser. Does that help?Comment #21
LittleCoding@joelpittet It has improved things yes. although the custom theme (base or child) is not installed (activated) during the Drupal install, it does avoid the fatal PHP errors.
Comment #22
joelpittet@LittleCoding I'm working on fixing that here #2614014: Progress bar, fieldsets, messages broken in the installer due to theme ordering bug I think;)
Comment #24
alexpottDiscussed with @xjm, @Cottser, @joelpittet and @laurii. We decided that this issue is not a major issue. In order for a theme to be a default theme it needs to be installed in Drupal 8 because themes need to be installed (this is different from Drupal 7). This issue should be rescoped to remove it from default.settings. examples as generally this is an unsafe setting to override unless you know what you are doing.
Comment #25
jhodgdonComment #26
LittleCodingUpdated summary (Proposed resolution and Remaining Tasks)
Comment #32
LittleCodingComment #33
LittleCodingHave come back to this with fresh eyes...
There are a number of settings that can be added to the settings.php (or settings.*.php) before install that will not function as expected during the install process. For example, $databases will still be writing at the end of the settings.php file during install. And given the YAML file implementation for Install Profiles works great and is the consistent way of handling these types of operations for all projects (theme, module, etc), I feel adding the code to support what I so believe to be an unintended "feature" would be added unneeded complication and bloat to core.
The function/use of settings.php has change in D8+, providing install settings is no longer within it use. Providing a way to hard-set setting values and handle setting values that should/can not be stored in a database is now its function.
This actually requires simple solutions: coding comments and updates to online documentation.
Comment #34
LittleCodingComment #35
webchickJust noting there's a patch here, so marking "needs review."
Comment #37
alexpottThis is not always true. It's perfectly fine to set
$config['system.site']['name'] = 'My Drupal site';,$config['user.settings']['anonymous'] = 'Visitor';before installing Drupal. And the$config['system.performance']stuff too.For me the simplest thing here is to remove the
example because in D8 themes are not magically enabled by being the default theme.
Comment #38
LittleCodingalexpott yes there are values that can be placed in the settings.php / settings.*.php that will not cause errors and warnings. A site name is a good example: The site name being stored in the DB from the installation process not being the same as the hard-set override value in settings is not going to case a WSoD. But a user may expect that these values will be used by the installation process to ether per-populate installation form field values or save configuration build steps right after the install. The real settings do need to be in place before these hard-set override values should be used.
That being said, the wording here can be softened to something like:
or
Comment #39
alexpottThe thing is even after installing uncommenting
# $config['system.theme']['default'] = 'stark';can result in a broken site. This is easily testable.=> you certainly don't see the site themed in stark. It's not quite WSOD but not much better.
Comment #40
LittleCodingYes, you know this as an advanced user. but I am thinking of the novice/intermediate users when adding this kind of warning/information to the comments/documentation. Providing the information that these are settings that can not be in the stored in a DB or will supersede values in the DB can help move along a user with less experience to troubleshooting steps like you said.
and unrelated note: to fix the fail test in #34 all changes made to
sites/default/default.settings.phpmust be match incore/assets/scaffold/files/default.settings.phpComment #41
LittleCodingI think this strikes the right balance.
Comment #42
LittleCodingFix for the same test the failed patch #34
Comment #44
joachim CreditAttribution: joachim commented> example because in D8 themes are not magically enabled by being the default theme.
We could leave the example in, but add a comment to say specifically that it won't work if the theme is not enabled first.
The latest patch's changed don't match the issue summary at all -- looks like that needs updating to explain the fix that has been agreed.
Comment #45
alexpottWell I'm not sure we have agreement here. I think we should remove the default theme example from the file. It's prone to breaking your site and therefore shouldn't be there. This will only get even tricky once themes can depend on modules.
Comment #46
joachim CreditAttribution: joachim commentedFair enough. Setting status to needs work, in that case.
> I think we should remove the default theme example from the file. It's prone to breaking your site and therefore shouldn't be there. This will only get even tricky once themes can depend on modules.
Ok. Sounds like a good argument to remove it.
Comment #47
LittleCodingComment #48
LittleCodingComment #49
LittleCodingdid new pull to redo the patch as it failed to apply.
Comment #50
gambryRTBCing patch on #49. It does address concerns from #39/#45, by removing the config line to set the default theme, and the re-wording after comment #37 alerts the user to set config before installation with a only-if-you-know-what-you-are-doing meaning, which makes sense.
Also updating issue title and issue summary to reflect what this issue and latest patch are trying to do.
Thanks.
Comment #51
gambrytypo on the title.
Comment #52
webchickMaybe I'm just a little strange, but this message somewhat frustrates me, because it doesn't link me off to anything where I can learn more about the installation process, or learn about pitfalls and how to get myself out of them, etc. We don't put this kind of vague "babysitting" anywhere else, so I'm not sure we should do it here.
Better might be something like:
...or whatever. :) "A good error message has three parts: problem identification, cause details if helpful, and a solution if possible." from https://uxplanet.org/how-to-write-good-error-messages-858e4551cd4
Only bumping down to "needs review" vs. "needs work" in case I'm totally off-base here.
Comment #53
gambryYou are right @webchick , the message opens more questions than the answers is trying to give.
The problem is I don't think we can easily nail all - or most common -
[kind of errors]and theirwhys in a simply and still exhaustive narrative.To be honest, why don't we just remove that paragraph from the patch?
That paragraph is mainly referring to configuration overrides, where we already have some nice "scary" messages like:
You usually don't need to use this featureThere are particular configuration values that are risky to override.We are already removing from the examples one override which could cause problems -
system.theme.default-, this will avoid the problem itself and maybe the requirement of having the message?Thoughts?
Comment #54
LittleCodingI'm thinking:
Altering these configuration settings prior to installation could result in unexpected behaviors or fatal errors due to the provided override setting(s)s acting as an incomplete configuration. This settings do not replace Drupal site configuration via the admin UI or YML files. Ensure before adjusting you've checked to make sure that [common pitfall thing didn't happen], or [adjust them only post-installation? what's the alternative?].This should be cleaned up some more and the blanks can be filled in with some more testing.
Known:
Unknown:
Comment #55
gambryI see three problems in having this message:
Configuration overridessection. And specifically:Note that any values you provide in these variable overrides will not be viewable from the Drupal administration interface [...]What's the additional benefit of having the message?
Comment #56
webchickSince it sounds like there are a lot of variables here which might change over time, one approach that might make sense in that case is creating a documentation page on Drupal.org (which is much easier to edit/keep up to date) which can be linked to from here, and keeping the message pretty short and sweet, like:
We could also go with removing the paragraph for now, and figuring this all out in a second follow-up issue, since it sounds like the bit about simply removing the default theme line from settings.php is a good one that goes a long way towards solving the problem this issue was initially created for.
Comment #57
LittleCodingI do like the idea of a "living breathing" document page on Drupal.org, this type of page is a great help with the .info file for projects for example.
Note that not all settings can be overridden in this way, and overriding settings could have unintended consequences. See [url] for pitfalls to avoid.How does that read for you?
Comment #58
LittleCodingCreated child issue (https://www.drupal.org/project/drupal/issues/3091202) for drupal.org documentation
Comment #59
LittleCodingUpdate patch to only remove example from settings.php
Documentation to be handled in issue linked in comment #58
Comment #60
webchickAwesome, that looks good. I really like the idea of a dedicated issue to sort out the documentation, because it sounds like there's a lot here to unpack.
Comment #63
webchickRe-titling and adding credits.
Comment #64
alexpottCommitted and pushed 26d7911b42 to 9.0.x and e0352aae4c to 8.9.x and bbc328768f to 8.8.x. Thanks!
Backported to 8.8.x as this is documentation.
Comment #68
LittleCodingComment #71
tim.plunkett