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

[bug] Funky .env Interpolation #760

Closed
shellscape opened this issue Mar 31, 2023 · 8 comments
Closed

[bug] Funky .env Interpolation #760

shellscape opened this issue Mar 31, 2023 · 8 comments
Assignees
Labels
bug Something isn't working

Comments

@shellscape
Copy link

Describe the bug

We're seeing some interpolation from our .env file with envFile that's unexpected.

Steps to reproduce

  1. Create an .env file at the repo root, add this to it:
export NODE_ENV=dev
export DEPLOY_ENV=$NODE_ENV
  1. Create a moon.yml in any project, add this to it:
tasks:
  foo:
    command: echo "$NODE_ENV"
    options:
      envFile: /.env
  1. Run moon run [project]:foo
  2. Change $NODE_ENV to $DEPLOY_ENV in the task and run again

Actual behavior:

▪▪▪▪ proj:foo
dev
▪▪▪▪ proj:foo (3ms, 597b81cb)

▪▪▪▪ proj:foo
_ENV
▪▪▪▪ proj:foo (3ms, 597b81cb)

That _ENV output looks like bad interpolation/variable expansion.

Expected behavior

▪▪▪▪ proj:foo
dev
▪▪▪▪ proj:foo (3ms, 597b81cb)

▪▪▪▪ proj:foo
dev
▪▪▪▪ proj:foo (3ms, 597b81cb)

Screenshots

Environment

MacOS Ventura M2

Additional context

This is using the new support for /.env to point to the workspace root (thanks again!)

@shellscape shellscape added the bug Something isn't working label Mar 31, 2023
@milesj
Copy link
Collaborator

milesj commented Mar 31, 2023

@shellscape Does DEPLOY_ENV=${NODE_ENV} work? How interpolation works in an .env file is entirely abstracted away from us, so we have no control over it.

You also probably shouldn't have export in the file.

@shellscape
Copy link
Author

I'm gonna push back on not having export in there; that's a very long time shell standard for sourcing shell and environment variables from file. export is important as is designates the variable following it as available to child processes of the shell as an environment variable. When not preceded by export it becomes only a shell variable. (forgive the explanation if you already knew, observers may not)

It also looks like dotenvy has direct support for export https://github.com/allan2/dotenvy/blob/e330110a7cd9064ec913e554c9fc325c9d7cdb88/dotenv/src/parse.rs#L51-L52

DEPLOY_ENV=${NODE_ENV} does indeed work, so we'll move forward with that. I'm not sure what implications that will have with our cross-tool-use of the same .env file. If I find anything, I'll pass that along. I'll open an issue upstream with dotenvy as well.

I stumbled across this issue allan2/dotenvy#11 which would be a nice thing to add to the documentation here as well. Most (if not all) Node-based users coming to moon are going to expect parity with https://github.com/motdotla/dotenv which the gold-standard for .env files in Node land. Having it documented that the parsing is more strict would be a net positive. The need for curlies for expansion should also probably go into the docs around envFile somewhere, citing the difference in behavior between other env processors.

@milesj
Copy link
Collaborator

milesj commented Mar 31, 2023

The docs do mention the curly braces: https://moonrepo.dev/docs/config/project#envfile But I'll also update them with a mention about strictness.

@shellscape
Copy link
Author

shellscape commented Mar 31, 2023

Looks like dotenvy has a few issues that don't follow spec. Reported to them and hopefully they pick that up.

Variables defined in the file support value substitution using ${VAR_NAME} syntax.

I'd recommend changing that to:

Upstream dependencies require that variables defined in the file support using value substitution/expansion wrap the variable name in curly brackets as such: ${VAR_NAME} syntax.

Or some such, to make it more explicit. Folks familiar with bash expansion rules may think that's a helpful tip for those who don't know any better, knowing that regular parameter expansion doesn't require it. (we know I thought it was optional 😄 )

@milesj
Copy link
Collaborator

milesj commented Mar 31, 2023

Yeah fair enough. I'll watch those dotenvy tickets, and if there's no movement for a while, maybe I'll just write an implementation myself.

@shellscape
Copy link
Author

Dope, thanks man.

@milesj
Copy link
Collaborator

milesj commented Apr 3, 2023

Gonna close this since there's no action I can take right now. I did update the docs though.

@milesj milesj closed this as completed Apr 3, 2023
@shellscape
Copy link
Author

No worries, appreciate it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

No branches or pull requests

2 participants