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

Allow env variables to reference previously defined env vars in same configuration file #1262

Closed
pdecat opened this issue Dec 24, 2023 · 4 comments · Fixed by #1519
Closed
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@pdecat
Copy link
Contributor

pdecat commented Dec 24, 2023

I often find myself having to reference environment variables from others, e.g.:

rtx env-vars TEST_RTX1="1"
rtx env-vars TEST_RTX2="{{env.TEST_RTX1}}/bin"
rtx error parsing config file: ~/.rtx.toml
rtx failed to parse template: env.TEST_RTX2='{{env.TEST_RTX1}}/bin'
rtx Failed to render '__tera_one_off'
rtx Variable `env.TEST_RTX1` not found in context while rendering '__tera_one_off'
rtx Run with --verbose or RTX_VERBOSE=1 for more information
[env]
TEST_RTX1 = "1"
TEST_RTX2 = "{{env.TEST_RTX1}}"

Would it possible to add support for this?

@pdecat pdecat added the enhancement New feature or request label Dec 24, 2023
@pdecat
Copy link
Contributor Author

pdecat commented Dec 24, 2023

Bonus points to allow referencing such variables in env_path too. :)

@jdx jdx added the help wanted Extra attention is needed label Dec 24, 2023
@jdx
Copy link
Owner

jdx commented Dec 24, 2023

I think this could be done. We read each env var one at a time so we could modify the tera context while we read in the file.

Here is where we parse the env vars in the toml parser:

https://github.com/jdx/rtx/blob/2b4388833081d9cb7433c614d529b7c62716bc22/src/config/config_file/rtx_toml.rs#L116-L132

What it does is call parse_template() which uses self.context which has the env vars inside of it. The context is generated when the parsing starts:

https://github.com/jdx/rtx/blob/2b4388833081d9cb7433c614d529b7c62716bc22/src/config/config_file/rtx_toml.rs#L46-L47

Then it looks like it becomes immutable and never changes. I think what we would want to do is probably wrap it in a Mutex so we can modify it and each time we read an env var we will write to the context and update values.

However this is only within a single file. If you wanted to have ~/src/myproj/.rtx.toml read env vars that ~/.config/rtx/config.toml created that would be far, far more difficult because config parsing happens in parallel. I think what we would need to do is make just the env var populating happen in series but the rest of the parsing still happen in parallel.

@jdx jdx pinned this issue Jan 14, 2024
jdx added a commit that referenced this issue Jan 24, 2024
This lazy-loads the env vars from config files.
This is a prereq for #1262 and other env var changes. It will make it easier to perform the template logic in order across the env vars rather than when the file is parsed
jdx added a commit that referenced this issue Jan 24, 2024
This lazy-loads the env vars from config files.
This is a prereq for #1262 and other env var changes. It will make it easier to perform the template logic in order across the env vars rather than when the file is parsed
@jdx jdx closed this as completed in #1519 Jan 26, 2024
@jdx
Copy link
Owner

jdx commented Jan 26, 2024

you have no idea how much work this was, but it's out now. I had to completely rewrite basically everything involving env vars in the project. I felt this was an essential feature though.

@pdecat
Copy link
Contributor Author

pdecat commented Jan 27, 2024

Awesome, thanks @jdx!

@jdx jdx unpinned this issue May 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants