DarthSim / overmind

Process manager for Procfile-based applications and tmux
MIT License
2.78k stars 79 forks source link

.env overriding overmind's environment #173

Open wjlroe opened 4 months ago

wjlroe commented 4 months ago

If you run overmind like so:

SOMETHING=123 overmind start

and .env contains:

SOMETHING=456

then you will experience this:

SOMETHING=123 overmind run bash
bash-3.2$ echo $SOMETHING
456
bash-3.2$ 

As you can see, because each .env file is loaded using dotenv.Overload(), they take precedence over existing environment variables.

This doesn't match how a lot of other tools work, where config files don't override the environment itself, and it precludes the possibility of launching your code with a setting in an environment variable on the command line, as above.

To change this behaviour by default would mean loading each .env file with dotenv.Read() into a map, then loading the environment (os.Environ) into that map (overriding anything from the .env files).

I don't know if this is something people would like changed, and if so, whether they'd want it configurable somehow (bit clunky but that'd probably have to be an environment variable like OVERMIND_KEEP_ENV or something to indicate that overmind's environment should not be overritten by .env files).

I could open a PR with the code for the above, but wanted to ask about this first.

Envek commented 4 months ago

Thanks for such a detailed issue!

As for me, I would also expect shell environment to take precedence over env files, so please make a pull request!


Overload usage traces back to https://github.com/DarthSim/overmind/pull/19 by @yewton

I can't see any traces that there was an intention to overwrite existing env vars, probably it was done just to overwrite env vars set in env files of more top level (e.g. in the home dir).

You can go with your proposal, also probably you will need to reverse the order in that env files are loaded to keep current precedence of env-files.


@DarthSim, any thoughts? Or does current behavior was made on purpose?

DarthSim commented 4 months ago

Hey everyone,

This was an intentional decision as I was seeing it as proper behavior. Honestly, I never thought of the case you've described. And it seems like nobody did since you're the first who mentioned it in 7 years.

This behavior can be changed by using godotenv.Load() instead of godotenv.Overload() and reverting the order in which the files are loaded.

I'm not sure if it is a good idea to change the default behavior as it allows, for example, changing PATH. But I'm OK with adding a flag for changing this behavior.