-
Notifications
You must be signed in to change notification settings - Fork 10.1k
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
added docker-multi-stage builds #10832
Conversation
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.
Please also fix coding style as reported by editorconfig workflow
bfdf494
to
72d0847
Compare
@ngxson I am really sorry about the spam, starting to use act. |
72d0847
to
eee1ea4
Compare
@ngxson ok figured out the cache, it will be using github cache, still a bit experimental but it works best at this point in time. In the dockerfiles there is a
can this be changed to a mv? It would same some space. Or I can just copy this from the cpu dockerfile
instead of copying the full /app in the full versions. |
Yes I think we can, I don't see any problem with this as we never run I'll test on cpu + CUDA in the next few days when I come back from vacation |
Yes, I don't see why the |
@slaren agree, but I would be happy to know what do I need to copy for the full image to work instead of the complete /app folder just to try and keep these images has small has possible for now. From what I can tell it's the python scripts, the .so and the build/bin. |
I already tried to copy as little as possible for the full image in the |
735b9b0
to
bf1caab
Compare
I normalized all docker images, improved the code a bit, I think everything is ok. |
bf1caab
to
9de4023
Compare
It's pushing multiple untaged images, checking. |
9de4023
to
435bce0
Compare
Fixed, added provenance to build-and-push. |
The tag name for How I tested it: https://github.com/ngxson/llama.cpp-test-ci/pkgs/container/llama.cpp-test-ci/versions |
Other than that, I can confirm that the built image is working as expected. Tested with CPU and CUDA images. |
435bce0
to
70ccb0c
Compare
70ccb0c
to
e7623e5
Compare
@ngxson Put up a fix, I think the versioning is ok now. |
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.
Working as expected now, thanks.
Side note, this change may affect users who rely on the old Dockerfile to specifically build server or cli (cmake with --target
). In case someone ask, we will find a solution later on.
More than happy to help, if any issues arise call me up. |
Added multi-stage dockerfile builds, improved total build time to under 2 hours, added Vulkan and Full-intel.
Updated rocm dockerfile.
Hopefully it will all work without any problems.