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
dockerTools: add environment variables inheritance #37760
Conversation
@GrahamcOfBorg test docker-tools |
Thank you for your pull request. I appreciate your effort to make
|
IMHO think that we should have two types of tools. Unfortunately, they would be incompatible with the current
|
I don't really have a strong opinion on this, but I think this makes sense since the structure resembles a dockerfile very closely. @roberth While this is indeed a breaking change, I'm not sure if it's a problem in practice. |
Well it does say |
@roberth awesome, I vote to make this default behavior too
good point, maybe I could add support for other config variables (reference), like Volumes, Labels. |
What is the status of this pull request? |
@mmahut , the inheritance of envs is working (was), the tests are implemented I didn't find tool that is specialized for merging two configs and could merge also volumes, labels, etc. (now I think its not that hard to implement it) but I've lost interest |
@roberth how can we move this forward? Is it possible to merge just this functionality (or do it in a new PR if this is too out-of-date), without the additional merging of volumes and labels? |
@goosetherumfoodle It has two conflicting files now so those have to be fixed at least. If it's easy someone can add them, but env only is also ok. |
@roberth I have fixed conflicts and tests have passed, I think it can be merged |
Oh, I see #87253 was merged and have the same functionality. There is no one to blame except me that this pr wasn't merged in due time. |
Motivation for this change
Fixes #37172
Things done
build-use-sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)Tested with