Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conflicting options: 'override' and 'cascade' #89

Closed
llovett2 opened this issue Apr 12, 2023 · 5 comments · Fixed by #93
Closed

Conflicting options: 'override' and 'cascade' #89

llovett2 opened this issue Apr 12, 2023 · 5 comments · Fixed by #93

Comments

@llovett2
Copy link

The override option introduced in #87 by @jfairley conflicts with the cascade option when both are used together.

Example

.env

MY_NAME=Bob

.env.local

MY_NAME=Joe

Command

> dotenv -c -- bash -c 'echo $MY_NAME'
Joe
> dotenv -co -- bash -c 'echo $MY_NAME'
Bob

I would expect Joe from .env.local to override Bob from .env based on how cascade works, but when the override option is present Bob prevails because .env is processed last (see here and here).

@jfairley
Copy link
Contributor

That's interesting. You raise a valid point, but I'm not sure how to easily make both work together. The problem is dotenv doesn't support "cascade" natively. There's probably no way to avoid edge cases without dotenv supporting cascade natively.

@llovett2
Copy link
Author

What if we reversed the paths array if override is set before iterating through it? I'm trying to think if that will cause any negative side effects.

@entropitor
Copy link
Owner

Yeah, there was an earlier discussion: #63 (comment)

I think it's just incompatible. Feel free to make a PR to update the README to document this but this is not really solvable.

I think the best solution is just erroring out if using override together with multiple files, ...

@jfairley
Copy link
Contributor

jfairley commented Jun 27, 2023

Ooh man, this one bit me in the butt the other day.

Like @llovett2 found, when you run override with cascade, the order of files is effectively inverted such that values in .env will override .env.test.

We need to do something about this, because of the potential impact. In my case, a dev on my team ran unit tests that blew away our QA database.

I'm available to submit a new pull-request, but @entropitor, I'm looking to discover what you're likely to accept, considering this issue was closed before.

  1. Invert the files array when argv.c && argv.o.
  2. Prevent "cascade with override".
    • Literally throw a runtime error if argv.c && argv.o saying it's not supported.

@entropitor
Copy link
Owner

I think it's best to throw. I knew this was a problem when merging that's why I pushed back so hard on cascade. The other solution has a problem with expanding variables so I don't think that's valid either. I guess throwing is the most elegant solution! Happy to merge such a pr

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants