-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
feat(compile): support --env #24166
feat(compile): support --env #24166
Conversation
…em for generated executable file
Hello @dsherret, |
cli/mainrt.rs
Outdated
@@ -79,6 +80,8 @@ fn main() { | |||
match standalone { | |||
Ok(Some(future)) => { | |||
let (metadata, eszip) = future.await?; | |||
util::logger::init(metadata.log_level); | |||
load_env_variables_from_env_file(metadata.env_file.as_ref()); |
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.
Sorry, just looking at this PR in detail. I'm not so sure about just providing a reference to the file because the file won't necessarily exist at runtime. Probably we need to embed the env variables into the executable and warn that we're doing it.
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.
Makes sense.
I'll sort it out according to your vision.
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.
Hello @dsherret ,
I made changes to embed the env variables from the env file into the generated executable and to warn the user about this action.
When running the executable, it will load the embedded variables as env variables.
Signed-off-by: HasanAlrimawi <141642411+HasanAlrimawi@users.noreply.github.com>
Maybe it's worth using |
@birkskyum the flag already shipped in Deno a while ago as |
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.
LGTM. Thanks @HasanAlrimawi!
@birkskyum I would suggest opening a separate issue if you think the flag name should be changed. It's still possible for Deno 2. |
@bartlomieju sure, I made a ticket here: |
Supported the use of --env flag with the compile subcommand, so that the generated executable/binary file can access the passed env file.
The commit is made to address issue #22105 .
Use-case
deno compile --env=some_env_file file_using_env_vars.js
-> executable/binary file "file_using_env_vars" is created.file_using_env_vars
to run it.The problem that prevented the generated executable from finding the env file's environment variables
CliOptions::new
and so is the env file passed.std::env::set_var
.std::env::set_var
sets the env variables exclusively for the currently running process, and thus the env variables loaded within compile time are exclusive for the operation itself "The compile operation".Solution of the problem
Pass the environment file to the executable/binary file, then load the environment variables whenever the file is ran.
Code changes
A) Added an attribute to hold the environment file name to Metadata struct within deno/cli/standalone/binary.rs, since the metadata is passed to the binary file by
fn write_standalone_binary()
.B) Added a getter for the environment file name to the CliOptions methods, since the Flags attribute that holds env_file attribute is private.
C) Added
fn load_env_variables_from_env_file()
to deno/cli/mainrt.rs which will be responsible for loading the env variables from the environment file at execution time.@dsherret
@littledivy