Allow existing environment variables to be overridden

[khalwat]

Description

It's pretty common to have a setup like Docker which will inject environment variables into the containers... with Craft's default craftcms/craft boilerplate, the phpdotenv package is called thusly:

// Load dotenv?
if (class_exists('Dotenv\Dotenv')) {
    Dotenv\Dotenv::createUnsafeImmutable(CRAFT_BASE_PATH)->safeLoad();
}

Unfortunately, this means if I want to edit an .env variable at runtime to test something, I need to spin the containers down, and then restart them, because createUnsafeImmutable() causes it to be unable to overwrite existing environment variables

I may be missing a use-case, but I'd think it'd be pretty common/desirable that what is in your .env can override environment variables that are already injected. The proposed change would be:

// Load dotenv?
if (class_exists('Dotenv\Dotenv')) {
    Dotenv\Dotenv::createUnsafeMutable(CRAFT_BASE_PATH)->safeLoad();
}

The default, btw, if you use ::create() is to use the mutable method... which makes me think there might be a specific reason why immutable was used?

I didn't create a PR, because it's relatively trivial, and also I'm guessing there are strong opinions backing up the current way of doing it?

Steps to reproduce

1. Have an environment variable already set via Docker or Nginx or whatever
2. Try to override that environment variable by changing the same variable in your .env file
3. Cry

Additional info

• Craft version: n/a
• PHP version: n/a
• Database driver & version: n/a
• Plugins & versions: n/a

[timkelty]

🤔 hmm…it's a bit tricky because generally we'd want our boilerplate to be "production-ready" by default, but a .env is by nature, a non-production concept (or should be).

Given that, it seems like createUnsafeMutable could be a reasonable default.

I'm guessing @angrybrad's reasoning for defaulting to immutable was to give lower-level processes (docker, nginx) to dictate settings?

But in the end, .env files should solely be a dev-environment thing, so it seems like the most flexible option is best?

[angrybrad]

The version we were coming from (3.6.x) is immutable by default (https://github.com/vlucas/phpdotenv/tree/4669484ccbc38fe7c4e0c50456778f2010566aad#immutability), which is the main reason I stuck with it in 5.x.

[khalwat]

Yeah it's all about what you think should take precedence, lower-level things like Docker, Nginx, etc. or the .env file itself.

The reason I prefer the idea of .env overriding environment variables that are already set by some lower-level process is that the way it is currently, you have to do something expensive like re-create a Docker container or restart Nginx if you want to change environment variables.

[angrybrad]

Will have to think on it this weekend... I agree it'd be convenient for local dev, but probably not something we'd want to encourage for production. Unfortunately, loading phpdotenv happens way too early in the request for us to be able to check things like devMode/YII_DEBUG.

[khalwat]

Yeah sounds like for production, it could be best combined with something like this, which would make the bootstrapping quicker, and also "seal" the config:

craftcms/cms#8418

Although I will say that in many environments, in production, people are likely using .env files as the sole source of environment variables anyway (ie, they aren't injected in any other manner).

It's not great for obvious reasons, but it's convenient, so many people do it.

[angrybrad]

Resolved in e84b53e and tagged 1.1.7