-
-
Notifications
You must be signed in to change notification settings - Fork 15.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
Preserve environment variables from base image in Docker buildImage #50690
Conversation
Friendly ping, does anyone know who would be best to review this? If this is received positively I'd also like to open another PR to preserve the "history" fields from the previous image (in a pure way of course). |
Are there any updates on this pull request, please? |
Thank you for your contributions.
|
@thomasjm, if you have time to fix the merge conflicts, I'd be glad to properly review this and see if we can't get it merged in time for 21.05. Seems like a really good idea to me. |
Hi @nrdxp -- actually, after making this PR I ended up doing a more extensive refactor of the Docker stuff. I've been using my fork ever since but I always meant to rebase it on Nixpkgs and see if the community was interested. You can see what it looks like here. Highlights include moving large shell scripts into their own files, adding a test suite, fixing various bugs, adding a couple new options, and improving support for multi-layered images (including what this PR does for all of the config fields). EDIT: oh and it also stores images as unzipped by default, similar to how the I know there's been some divergence from the main Nixpkgs, but would you be interested in some of this stuff? |
I believe their are many users who could benefit from your fork, if you don't mind doing the heavy work to factor it into a larger PR, I'd be glad to review and attempt to get some eyes on it. |
FYI, I believe this particular feature has already been implemented in #87253 since then. It would be nice to merge any other feature you've been adding to your fork though ! |
Oh nice -- I'll close this then, and put it on my todo list to investigate what parts of my fork would make sense to merge. |
Motivation for this change
Normal Dockerfiles preserve the
config.Env
setting from the image they're based upon. This change makes thedockerTools
buildImage
command work the same way: if building on afromImage
, extract the environment variables setting and prepend it to the new image's setting.For reference: the Docker image spec
Things done
I've tested building images with this on my local machine. Most of the steps below don't seem applicable and I'm not sure how
dockerTools
is tested.sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)nix path-info -S
before and after)