-
Notifications
You must be signed in to change notification settings - Fork 179
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
Minimal docker support #669
Conversation
Concept ACK But does not work as is on a Gentoo Linux, as it requires
So it's hard for me to test it properly. |
And you have typo in title - miminal vs minimal. |
@kristapsk You could test within a Docker image. I tested with Debian, buster-slim image. Just run This PR isn't intended to install on Gentoo because it might mess with how Gentoo wants to install Python packages. In which case, it makes sense to use the default installation method, without the Thank you for taking a look at the PR. |
The proper way to install on Gentoo is probably to create an ebuild which installs all the dependencies as expected for Gentoo. One would have to maintain it along side the |
e8e37c2
to
45296c0
Compare
45296c0
to
4a7a480
Compare
This PR might be useful for containerization of JoinMarket for Umbrel. |
I couldn't say. I use this patch to build my own Dockerfile and use it in a Compose configuration I have. I've never used Umbrel. |
@kristapsk as per above, apparently this is already in active use, is this mergeable, in some version? |
@AdamISZ Will look at this. Likely needs to be tested on different platforms as it has changes in |
Agreed. |
If it's easy to logically distinguish that using the |
Changes does not seem to break anything for non-Docker users (at least from my tests and code review). But when running
|
4a7a480
to
542b826
Compare
I rebased to master and I didn't get any errors from running I think I got your documentation changes in my latest commit. I couldn't figure out how to git a .patch file from the Github comments so I eyeballed it. |
Have the same error about ffi.h missing. Host OS should not matter as building happens inside Docker container. I'm currently running Docker version 20.10.9, build c2ea9bc90b under Gentoo Linux. |
Did you get the latest upstream image? I used |
Actual issue is some lines above:
For some reason on my machine it thinks libffi and libsodium are already installed, so it ends up missing libffi on JM install (last) phase... |
Maybe in your build cache. That could be an issue with the Dockerfile setup actually. I should ignore files already built outside of Docker. Adding the appropriate |
@kristapsk Can you try the build again with the I reproduced the error on my machine and fixed it with the |
I noticed the Darwin install path was missing the checks I added for Debian install. I'm not sure it is even possible to create a Darwin image but the For that matter, the |
Now it works also for me. ACK, I'm ready to merge this, but you should squash commits into one. |
…e docs on how to use it.
e620084
to
c28bfd5
Compare
This PR allows
install.sh
to run with a--docker-install
option which installs JoinMarket without virtualenv or sudo. By not installing with virtualenv, it makes running scripts through Docker containers more straight forward. Sudo is also unnecessary because Docker containers run as root by default.Also included is a minimal Dockerfile which can be used as a starting point for more Docker based JoinMarket services. Documentation is also provided explaining how to use the Dockerfile.