Change all default settings and config to fast/safe production values

We've needed this for quite some time. Thanks for starting it!

One small suggestion I'd make, is to use environment variables for this. It's a very common convention elsewhere. Maybe that just means using this as an exampe:

$settings['environment'] = getenv('DRUPAL_ENV');

+++ b/core/includes/bootstrap.inc
@@ -61,6 +61,16 @@
+const ENVIRONMENT_PROD = 'production';
...
+const ENVIRONMENT_DEV = 'development';

Would it make sense to move these into the Settings class?

Crossposted with #2, +1 for it :)

Also consider the ideas in #1933638: Document how to disable all caching in settings.php for the dev environment.

+++ b/core/includes/errors.inc
@@ -94,6 +95,9 @@ function _drupal_error_handler_real($error_level, $message, $filename, $line, $c
+  if (Settings::get('environment', ENVIRONMENT_PROD) == ENVIRONMENT_DEV) {
+    return TRUE;
+  }

Nice!

I guess we need a small change notice on this one, but yeah this is straightforward.
In general I wonder what we do about changes which will be needed once the enviroment is changed. Would that has
to happen automatically or does the people have to trigger a rebuild of the container?

We could potentially base the container filename on the environment?

Another option would be using a null PhpStorage implementation when dev mode is enabled so that it never gets written/read out at all.

We could potentially base the container filename on the environment?

This is briliant! This allows you to switch immediately and provide dedicated services, which do stuff like logging etc.
without having our production code need to change.

Don't mind me, just keeping an eye on it.

1. +++ b/core/includes/bootstrap.inc
   @@ -61,6 +61,16 @@
   +const ENVIRONMENT_PROD = 'production';
   ...
   +const ENVIRONMENT_DEV = 'development';
   

   Why are we not using the existing Symfony Kernel identifiers "prod" and "dev"?

   Also, why do we need these constants?

   If we keep the constants, then we need to namespace them some more; i.e., either using a DRUPAL_ constant name prefix, or moving them into e.g. DrupalKernel.

2. +++ b/core/includes/errors.inc
   @@ -94,6 +95,9 @@ function _drupal_error_handler_real($error_level, $message, $filename, $line, $c
   +  if (Settings::get('environment', ENVIRONMENT_PROD) == ENVIRONMENT_DEV) {
   

   Passing a default value is unnecessary here. Instead, use a strict comparison (===).

   FWIW, with the proposals in

   #2207585: Find a new OO home for drupal_get_hash_salt()
   #2208475: Move Settings into Drupal\Core\Site\Settings

   ...we can actually add a dedicated Settings::getEnv() method to the Settings class.

Another alternative: The way Symfony fullstack handles this is to simply have a different front-controller file: app.php vs. app_dev.php. If you go to app_dev.php/whatever, that file passes "dev" to the kernel. If you go to app.php/whatever, that file passes "prod" to the kernel. (Clean URLs would then only make sense for prod, unless you tweak your .htaccess file. That may be a terminal problem for that approach; not sure, but I mention it for completeness.)

That starts to get tricky with protecting index_dev.php on live sites. If we completely disable caching there for example it would be an easy way to DDOS.

I can't really think of a way to lock it down automatically without something along the lines of update_free_access, which goes back to $settings anyway.

Should probably handled in a follow-up anyway, but since it's mentioned in the issue summary can someone elaborate on

Environment-specific configuration overrides. Downsides: make the config system more complex, and we don't know what to set it back to.

To me an EnvironmentConfigOverrideSubscriber would have seemed like a natural fit for this toggle.

Why are we not using the existing Symfony Kernel identifiers "prod" and "dev"?

I much prefer the full versions, which are more common outside of symfony (at least in ruby and clojure). The typical environments are production, development, and test.

Another thing we could disable is system_run_automated_cron() in dev mode. Someone I know was getting random OOM errors on a dev site, since the usually very expensive cron runs that only work via drush were running in the main request.

Since the entity render cache was enabled by default for nodes I've already seen two people asking about this in #drupal-contribute, this can really get in the way when working on frontend stuff, and should probably also be disabled for DEV?

Or there should be a separate setting for it...

Or maybe the page cache setting should be a combined render cache setting for page, blocks, entities, ...?

The same is true for twig stuff, where also some DEV friendly settings should be there.

Most FE Devs already enable twig_debug ...

Regarding render cache:

That is a mixed sword:

- You need to turn it off to be efficient in development (or need to increase a serial number).
- You need to turn it on to ensure your site still works correctly with it. (e.g. noticing that adding a path like condition in a field #pre_render for example is a bad idea ...)

msonnabaum: I don't know what the standards are in Clojure, but honestly I don't really care. We're using the Symfony core pipeline, as are a dozen other projects. Unless lots of them are using something other than dev/prod, that is the "principle of least surprise" here. There's no reason I see to not follow the conventions used by the code base we're leveraging, certainly not for something as trivial and bikeshed prone as this.

I won't fight it for the sake of bike shedding but I couldnt disagree more with #19.

I don't think we understand our scope which is making the rest of this discussion sort of awkward. By that i mean, fast by default is not the scope, this is creating an environment system. The patch then makes use of that to provide a default performance boost in production there are other uses which we should clarify.

To that end, I actually like what I'm seeing in the Symfony setup. Its just a string passed into the kernel at construction and returned by getEnvironment(). To that end, it actually does support test or for that matter other strings as we see here:

https://github.com/symfony/symfony-standard/blob/master/app/AppKernel.php

"dev" and "test" get extra slower code loaded and by default things are fast, however you can pass any string in and it will load up a different configuration. If I wanted to do some performance testings with "prod2" that swaps out some CMI settings, that should be legal IMHO even if it isn't a best practice.

I like #23 but i think we also need

/**
* Load local development override configuration, if available.
*
* Use settings.local.php to override variables on secondary (staging,
* development, etc) installations of this site. Typically used to disable
* caching, JavaScript/CSS compression, re-routing of outgoing e-mails, and
* other things that should not happen on development and testing sites.
*
* Keep this code block at the end of this file to take full effect.
*/
# if (file_exists(DRUPAL_ROOT . '/' . $conf_path . '/settings.local.php')) {
#   include DRUPAL_ROOT . '/' . $conf_path . '/settings.local.php';
# }

and @file missing in new settings.development.php ;)

I'd like to keep the settings.local.php convention for local development in settings.php, it's something many devs use in D7. Could we populate settings.local.php with these development settings instead? alternatively like @jibran says, leave settings.development.php the way it is in #23, but reference settings.local.php in settings.php.

+++ b/sites/default/default.settings.php
@@ -276,59 +276,6 @@
-# $settings['twig_debug'] = TRUE;
...
-# $settings['twig_auto_reload'] = TRUE;
...
-# $settings['twig_cache'] = FALSE;

@@ -511,16 +458,6 @@
-# $settings['rebuild_access'] = TRUE;

+++ b/sites/default/settings.development.php
@@ -0,0 +1,74 @@
+ * This setting can be enabled to allow Drupal's php and database cached

I also think this should just stay in here, commented out, and the dev file can be more barebones

But, +1 to the approach, it's elegant.

I'm good with the movement of the 'twig_ variables being moved to the settings.development.php. But also don't have strong feelings which one they live in, just a gut call. I'm just happy for the environment settings split.

I really like the environment separation here as I usually end up making some half assed version of this in D7 anyways.

Awesome that everyone likes the approach so far. Let us never speak of THE TOGGLE again!

moshe: There's plenty of value to a proper dev/prod toggle even with the file split. The infrastructure for it is already there, we're just not using it.

I'll be honest, I don't see why this issue even made it to 30 comments. We're biting off more than we should, again. Just add the damed toggle and let us figure out what to do in each case separately.

A strong –1 to the revised proposed solution:

Manually performed settings.php overrides have nothing to do with proper application environments. But we have proper environments now.

To be clear: The idea of optionally including environment-specific + site-specific service definitions is not bad, but much rather maps to this:

#2241633: Simplify site-specific service overrides

However, the fundamental difference is: application environment != site

In addition, a custom settings.php for development "overrides" requires way too much plumbing for all the stuff that just simply should be the defaults for a "dev" environment. A default dev environment is not about overrides, it's about sensible defaults.

Details aside, we've been on the absolutely right track with the earlier patches. This new approach makes no sense at all to me.

Hmmm, maybe so the render_cache issue doesn't get caught up in this discussion leave it on that other issue?
#2239909: Disable render cache through settings

I'm changing my mind, I'm going with sensible defaults and not separate file as was started in #1 and mentioned in by @sun in #32.

Throwing this out there to see if it sticks to the wall.

# core/lib/Drupal.php
// Setup some helpful constants in class Drupal.
// Common environment type constants for consistency and convenience
const PRODUCTION = 10;
const STAGING = 20;
const TESTING = 30;
const DEVELOPMENT = 40;

// Setup a global default to fall back on. Production for speed.
public static $environment = Drupal::PRODUCTION;

# sites/default/settings.php
// Choosing the Environment in settings.php from the server.
if (isset($_SERVER['DRUPAL_ENVIRONMENT'])) {
	Drupal::$environment = constant('Drupal::' . strtoupper($_SERVER['DRUPAL_ENVIRONMENT']));
}

// Load senible default environment settings for development.
if (Drupal::$environment == Drupal::DEVELOPMENT) {
  include DRUPAL_ROOT . '/' . $conf_path . '/settings.development.php';
}

// Load site specific override configuration, if available.
# if (file_exists(DRUPAL_ROOT . '/' . $conf_path . '/settings.local.php')) {
#   include DRUPAL_ROOT . '/' . $conf_path . '/settings.local.php';
# }

A couple references where #34 is coming from:
Kohana
https://github.com/kohana/core/blob/3.4/develop/classes/Kohana/Core.php#L22
https://github.com/kohana/kohana/blob/3.3/master/application/bootstrap.p...

Code Igniter
https://github.com/EllisLab/CodeIgniter/blob/519f87a07bd1fe3a9ec037f7276...

To continue + complete my major objection:

A site-specific override involves a dependency graph that is based 2D thinking: You vs. Core.

The reality is 3D though, it's: You vs. Modules/Extensions vs. Core

The modular extensibility is the exact point where sensible defaults per environment becomes even more important. An internal environment indicator (as natively built-in/copied from Symfony) allows extensions to use sensible defaults (!= overrides), too.

Contrary to defaults, further site-specific manipulations should consist of actual overrides only.

Drupal is a complex beast though. I'm skeptical that a single "dev" environment will be able to cut any sensible defaults for any particular party; i.e.:

I assume @joelpittet was super-happy, because

Yay! No more freakin' cache clears to get a new theme registry + stuff on each request!

I assume others were happy, because

Yay! My changed services.yml + routing.yml files will be picked up on each request!

But neither of both parties expected the whole damn system to not use any caches at all anymore + rebuild each and everything from scratch on every request!

That inherently gets us into the topic of user roles (developer roles), because "dev" != "dev".

In addition, it further underlines that a site-specific environment-settings-override approach doesn't resolve anything; the overall problem space is just moved from settings.php to settings.$env.php, which contains the same swath of painful one-off settings and variables that are incredibly hard to figure out and which you may or may not adjust. DX--

Instead, let's think about this:

/**
 * Choose your environment.
 *
 * Controls the default behavior of this site:
 *
 * - prod: Optimized for production sites. The site operates with the regular
 *   runtime configuration and standard settings.
 * - dev: Optimized for module authors. Disables discovery cache, so that new
 *   plugins and stuff is immediately found.
 * - theme: Optimized for theme authors. Disables the theme registry cache (to
 *   immediately find new theme hook/preprocess implementations), enables Twig
 *   debugging, automatic template reloading, and refreshes theme .info.yml
 *   files on every request.
 *
 * More specific overrides can be applied through other variables (below).
 */
$settings['env'] = 'prod';

Yes I agree with this. If we just commit the new defaults, then we have an issue to introduce a 'dev mode'. That's something that devel module could set up even if core can't (you'd have to do drush en devel after syncing from production, but that's not the worst).

Drupal has shipped with 'dev mode' by default for several releases, and I can think of several high profile sites off the top of my head that launched still in it. So for now let's fix that problem then introduce the toggle or whatever later.

That's precisely the thing: every developer has his own habits and preferences. Therefore, we should ship with a single high-level example (settings.development.php), and anything beyond that, is up to the developer to customize. That includes a settings.local.php file, settings.staging.php, settings.testing.php, and so on.
Why prefer settings.development.php over settings.local.php? Because it has much more meaning. The "local" version of a Drupal site may be used by a themer, a QA tester, a developer, a usability tester, and so on. The "development" version of a Drupal site, that is much clearer. Hence settings.development.php.

The advantage of settings.local.php is it means the settings local to the specific Drupal installation. i.e. I work on projects where production, staging, my local all have environment-specific settings, and settings.local.php is used for each. These either are not in version control, or more commonly they're in a directory outside the docroot, then symlinked (so settings.local.php is never actually in version control at all).

In all cases, once it's set up, you don't need to then go and hack settings.php to include differently named files.

In general I really like this approach! I use the settings.local.php pattern heavily on D7 sites (and like @catch I use settings.local.php on every environment and don't version control the files).

It's worth noting that some very popular Drupal hosting providers force you to include settings.php in version control - I think both Acquia and Pantheon do. For me this brings up a couple points:

1. If we just included a dev/prod toggle in settings.php it wouldn't take long for $env = prod to get accidentally committed and pushed to a production environment, this is why I prefer the settings.local.php pattern.
2. If settings.php is version controlled and we use settings.development.php, settings.staging.php, etc., then we need to check on every environment whether those files exist (and not version control them).

I like where this is going but I think we should consider leaving settings.local.php in settings.php and include settings.development.php as a "template" of sorts. Basically default.settings.local.php :)

I assume it's for performance reasons but I also wonder why we have the settings.local.php include commented out by default. IMO this whole set up would be more intuitive/discoverable if you just had to copy and rename some files rather than finding something to uncomment at the end of a (currently) huge settings file.

Edited last two paragraphs.

Yes the commenting out is for performance. file_exists() on a file that doesn't exist doesn't get cached.

Thanks @catch!

In #44.1 I meant to say $env = dev.

+++ b/core/core.services.yml
@@ -27,6 +27,10 @@ services:
+  cache.backend.memory:
+    class: Drupal\Core\Cache\MemoryBackendFactory
+  cache.backend.null:
+    class: Drupal\Core\Cache\NullBackendFactory

We already had issues to remove stuff like this from the container. Like #2113169: Memory backend defined as a service in core.services.yml. Devel module could easily just add this stuff back in. No need for this to have a permanent home in the prod container.

+1 on #47. I have no strong preference on #50 but I don't mind having them there; there's no runtime cost to doing so.

Fixing issue title; the settings.local.php concept already exists and is not introduced here.

1. +++ b/core/modules/system/lib/Drupal/system/Tests/System/ErrorHandlerTest.php
   +    // Ensure errors are displayed.
   +    \Drupal::config('system.logging')
   +      ->set('error_level', 'all')
   +      ->save();
   

   This really needs to be default for all tests.

   Otherwise, the error reporting in web tests will become close to useless.

2. +++ b/sites/default/default.settings.php
   -# $settings['twig_debug'] = TRUE;
   -# $settings['twig_auto_reload'] = TRUE;
   -# $settings['twig_cache'] = FALSE;
   
   +++ b/sites/default/settings.local.php
   +# $settings['twig_debug'] = TRUE;
   +# $settings['twig_auto_reload'] = TRUE;
   +# $settings['twig_cache'] = FALSE;
   

   An alternative and IMHO better + more appropriate approach is:

   #2251113: Use container parameters instead of settings

3. +++ b/sites/default/settings.local.php
   

   This additional file should be named default.settings.local.php and needs to be copied into the (effective) site directory by the installer.

   FWIW, #1757536: Move settings.php to /settings directory, fold sites.php into settings.php moves the default settings file into /core/default.settings.php.

1. +++ b/core/core.services.yml
   +  cache.backend.null:
   +    class: Drupal\Core\Cache\NullBackendFactory
   
   +++ b/sites/default/default.settings.local.php
   +// Disable the render cache, by using the Null cache back-end.
   +$settings['cache']['bins']['render'] = 'cache.backend.null';
   

   Do we really need to register a Null cache backend? — For the development case, a memory backend should be sufficient and the desired implementation, no?

2. +++ b/core/modules/system/lib/Drupal/system/Controller/BatchController.php
   @@ -88,6 +88,14 @@ public function batchPage(Request $request) {
   +      // @todo For some reason, _drupal_bootstrap_code()'s call to
   +      //   file_get_stream_wrappers() to register all stream wrappers is
   +      //   insufficient in the case of the no-JS version of the batch page:
   +      //   checking if a needed CSS aggregate already exists in the file system
   +      //   will fail unless we re-register the stream wrappers here! This may be
   +      //   a PHP bug.
   +      file_get_stream_wrappers();
   

   Did you verify whether _drupal_bootstrap_code() is invoked at all prior to writing this comment?

   I assume it isn't. — We have a range of situations in HEAD in which controllers are executed before the legacy bootstrap phases have been reached.

   It's also not clear from the @todo whether this only occurs in tests?

   In any case, it's not a PHP bug. If the idea is to commit this including this @todo, then we need a (critical) follow-up issue to resolve it.

3. +++ b/sites/default/default.settings.local.php
   

   Thanks for renaming it — however, the installer doesn't copy it into the actual site directory yet?

   What's the plan here? A major follow-up issue to fix the installer, or...?

4. +++ b/sites/default/default.settings.php
   -# $settings['rebuild_access'] = TRUE;
   

   I wonder whether it is legit to remove this setting entirely from settings.php — unless I'm mistaken, various documentation is pointing to it "in settings.php"... (?)

   Technically, that affects the other settings, too — How do we reference these settings if they are no longer located in settings.php, and given that the local settings file is optional and may not exist or be "enabled" ?

I reviewed this patch and like it so far. Two things I didn't fully grok:

1) The rename of default.settings.php to default.settings.local.php. Where does that come from and why is that better? The term 'local' isn't very meaningful as a lot of development environments aren't local these days. Better to call it 'debug' or something?

2) The "API changes" section of the issue summary appears to be out-of-date? I don't see a new $settings['environment'] setting. Would love to see the issue summary updated.

Why is MemoryBackend providing static caching not sufficient? Is code realistically going to render an entity, change something, then expect to see different results from rendering it again in the same request?

Thanks for the update Wim. I really like the approach taken in this patch. I think we can improve the developer experience a bit though:

1) Do we feel that 'settings.local.php' is the right name? I propose we rename it to 'settings.debug.php' or 'settings.development.php' or something more intuitive. I don't like the name 'local' as it incorrectly assumes that I do my development on my local machine. A lot of people use non-local development environments.

2) I'd document at the top of 'settings.local.php' how it is supposed to be used/activated; e.g. we should mention that it can be included from the bottom of settings.php. That way, when someone opens the file 'settings.local.php', he/she knows what it is, how it is used, etc. A little bit of extra documentation would go a long way.

settings.local.php was introduced in #1118520: Add inclusion of a settings.local.php file in settings.php

The name was chosen, because it's a common practice and documented in various blog posts and handbook pages. The concept of local settings wasn't invented by Drupal; it is used for years in many other applications already. Local settings may be used for development purposes, but you can also use it to host e.g. server-specific settings in a multi-webhead setup.

The end of [default.]settings.php already contains instructions for how to include settings.local.php.

We need follow-up issues for #56.2 + #56.3.

Agreed, settings.local.php is the de-facto standard for a file that holds local settings.php overrides.

That said, the file that we're including is *not* a default settings.local.php, it's a settings.local.php *for development*. So maybe that should be named development.settings.local.php or something?

We already have a precedent for using an "example" prefix for this sort of thing in example.sites.php. So why don't we name the file "example.settings.local.php" and leave comments at the top that note that the values below represent those of a typical developer environment.

Anything to stop further bike shedding from killing this poor patch (and the poor souls trying to work on it). :)

However, I agree with others that if we're going to pursue whether or not "local" as a filename makes sense (people are seeming to argue that it does based on the arguments in the issue that introduced it), we should do that in another issue because it's a "pre-existing condition" in HEAD.

I'm ok to commit this patch as is. We have to get the release out so we can bikeshed the details later. This patch is a great improvement as-is.

Sleeping over this, I'd recommend to remove all the settings[.local].php changes from this patch and only keep the default config/setting value changes, because those appear to be the only part of this change proposal that is actually mature and which has a clear concept/plan.

I.e., essentially just the change that the issue title encompasses.

We can hash out how exactly we can implement environment-specific overrides in separate issues, what approach makes most sense, how to document that, etc.pp.

I think it is best to proceed with the patch in #66. It is a clear improvement over the status quo. We can always come back with a better approach in a follow-up issue.

Done! Committed to 8.x. Thanks. :-)

After studying the changes to tests and tinkering about that topic some more, I've created:

#2259167: Disable CSS/JS aggregation and caching in tests by default

If the MemoryBackend is not sufficient, why do we need both MemoryBackend and NullBackend in the container?

Sorry, but I am not concerned about the runtime cost, having this in the container is obviously going to be next to no overhead in that sense. It is just a house keeping thing - don't leave stuff in the container that is not needed for production sites. Same as the simpletest http client subscriber; overhead is minimal it just doesn't belong in the container all the time. Lead by example and all that.

I just opened a new issue #2260969: 404 error on css/js aggregated files in site located in a VirtualDocumentRoot, that reflected that something had changed in D8 in the last week or so that generated an incorrect front page immediately after install. I started looking at recent commits to see what may have caused this. I tested a couple with no impact then tried yours, mostly because it modified install. I found that reverting your patch corrected the problem I was seeing. I have poked at the patch to see what the actual cause might be and can't figure out why your patch would cause this behavior. But I can repeat the outcomes; with your patch I get a screwy front page, without it I don't. Maybe you have some insight.

The instructions in example.settings.local.php are wrong. The settings.local.php file needs to be in ../sites/default, not ../sites.

So I should have recognized that 'yoursite.com' translate to 'default' by default?

Agreed with #76. There's a runtime cost to a bloated container. A good way to avoid the container being bloated is by core setting a good example by only adding services that are needed - it might end up we only end up with 20 less services in core than if we added them in, but that's twenty less temptations to start using the container as a dumping ground.

Devel module can add any number of developer-friendly services it likes.

I agree with #82 that yoursite.com is not the best placeholder. Let's use example.com or a wildcard.

Can we please open specific follow-ups for whatever the remaining issues(s) are? This conversation has already gone 15 comments past the point of commit, so it's getting hard to track.

#2263059: Don't use yoursite.com as a multisite site name placeholder in example.settings.local.php

This is really, really annoying. Can we change it to per profile please? Make it fast / safe production for standard and developer friendly for minimal? Pretty please?

Updated https://drupal.org/node/1903374 to mention the new file & also disabling the render cache.

Follow-up #2280383: FIx settings.local.php after new bootstrap

This is really, really annoying. Can we change it to per profile please? Make it fast / safe production for standard and developer friendly for minimal? Pretty please?

See related issue I'm adding just now.