-
Notifications
You must be signed in to change notification settings - Fork 249
[env] Add env flags to direnv commands #1354
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
Conversation
Just tested this. One issue with how this works is that the command will fail if the file passed with
A better solution might be to implement a "load if file exists" functionality, similar to direnv's https://direnv.net/man/direnv-stdlib.1.html#codedotenvifexists-ltdotenvpathgtcode |
Discussion clarified offline that the behavior won't break if the env-file is missing. Well, it won't break for direnv ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mikeland73 where is the logic that makes it forgiving if --env-file
file is missing?
Also - I don't understand why the shellenv hash change is related, and how it works...
internal/wrapnix/wrapper.go
Outdated
// If hash is already set, the env is up to date, no need for wrappers. | ||
if args.ShellEnvHash == os.Getenv(args.ShellEnvHashKey()) { | ||
return nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wait, this change is quite unrelated to this PR. Can we move it out?
I don't think this makes sense. We need the wrappers because the shellenv environment may change after the wrappers have been generated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a bad change that snuck in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
acceping to unblock, assuming:
- the change to make
--env-file
forgiving for direnv is added - the wrapper change is reverted
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice!
Summary
This allows users to generate .envrc files that use custom env variables like so:
Users can also manually modify
.envrc
files to add additional flags:devbox generate direnv --print-envrc -e foo=bar
How was it tested?
Generated .envrc file with custom env flags. Then used direnv and verified environment was setup as expected.
Backward compatibility
Newly generated
.envrc
files that contain--env
or--env-file
flags will not be compatible with previous versions of devbox.