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

build in a docker environment #25681

Closed
wants to merge 3 commits into from
Closed

Conversation

gregwebs
Copy link
Contributor

@gregwebs gregwebs commented Jul 23, 2022

This PR add a docker build. You can read more about it in the documentation file in the PR.

I was having trouble building this project so I switched to a dockerized build and thought I would share it with everyone.
This could greatly improve things for someone looking to make a first contribution or those looking to build from source to improve providence.

@gregwebs gregwebs force-pushed the build-docker branch 4 times, most recently from 4655135 to db99d9d Compare July 23, 2022 11:25
@chinggg
Copy link
Contributor

chinggg commented Jul 23, 2022

Thanks! It sounds like a good idea to have a Docker setup. I just realize that our official repo does not contain any Dockerfile or CI/CD file for building bitcoin. Though Cirrus CI is always running, it seems to run tests instead of checking the build process. FYI, I can find a Dockerfile in https://github.com/google/oss-fuzz/blob/master/projects/bitcoin-core/Dockerfile.

@sipa
Copy link
Member

sipa commented Jul 23, 2022

There has been earlier discussion here: bitcoin-core/packaging#55

@gregwebs
Copy link
Contributor Author

There is an image from that issue that points to a good Dockerfile. Although I saw that Dockerfile and it has lots of things and I didn't understand how they all worked.

But that is for packaging a reproducible build. This Dockerfile is designed for a development workflow. Those are two different use cases.

@maflcko
Copy link
Member

maflcko commented Jul 25, 2022

But that is for packaging a reproducible build. This Dockerfile is designed for a development workflow. Those are two different use cases.

What developers are you referring to? People working on Bitcoin Core that use docker/podman/whatever already have their scripts for their use cases fledged out. I don't think the dockerfile in this pull is going to supersede any of that. It seems more like a dev environment for your use case.

Some more references about docker use:

So I am wondering for whom you are writing the dockerfile in this pull.

@chinggg
Copy link
Contributor

chinggg commented Jul 25, 2022

So I am wondering for whom you are writing the dockerfile in this pull.

@MarcoFalke I think he is writing Dockerfile for someone who is new to Bitcoin-Core and has problem building it? As he said at first:

This could greatly improve things for someone looking to make a first contribution or those looking to build from source to improve providence.

However, I am also new to bitcoin but I think build-unix.md already teach me how to build, which is basically installing dependencies then configure and make. Personally I won't use Docker if I am new to a project.

@maflcko
Copy link
Member

maflcko commented Jul 25, 2022

See also #10903 (Add configuration files for a Docker-based development environment)

Copy link
Member

@jarolrod jarolrod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

concept nack

I don't see a strong motivation for introducing this into the repo. Ideally, someone completely new to the project could quickly get up to speed with the build docs and be able to successfully compile on their own. With the docs across build-* and depends/, I'd assume that trouble compiling is an issue of "not reading the docs" or us not placing information in the best way/correct places.

I was having trouble building this project ...

Any feedback on how we can improve the build documentation would be great.

FROM alpine

# build depndencies
RUN apk update && apk add \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This dependency list is making a biased decision on how to build bitcoin core, primarily on not including qt related dependencies to build bitcoin-qt

@maflcko
Copy link
Member

maflcko commented Jul 25, 2022

Concept ACK, if this helps on-board new devs or improves UX for existing devs, but it would be good to hear from at least one dev that will be using this first to avoid adding an unused script.

@laanwj
Copy link
Member

laanwj commented Jul 25, 2022

I have nothing against including a docker config, but it should not be required. I am sometimes annoyed by projects that have docker builds as the only (realistic) option for doing development. And it's not like we don't already have the depends system to facilitate easy install of dependencies 😄 .

@fanquake
Copy link
Member

Concept NACK. This isn't something we need to maintain in this repo. If anything, maybe the discussion can be restarted in https://github.com/bitcoin-core/packaging.

I was having trouble building this project

What issue were you having? Why was introducing Docker the solution?

db-c++ \
db-dev \
boost-system \
boost-program_options \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This list includes a number of dependencies that are not required to compile Bitcoin Core. i.e boost-filesystem, boost-system, boost-program-options, libressl.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed boost-program-options. Other docker images that the community has created contain the rest of these libraries so it seems like they are needed. It is now easy to test this out with the docker build, but I am not familiar with all the configure options.

@maflcko
Copy link
Member

maflcko commented Jul 25, 2022

Concept NACK. This isn't something we need to maintain in this repo. If anything, maybe the discussion can be restarted in https://github.com/bitcoin-core/packaging.

I think this pull was intended for developers, not for packaging, in which case this would be the right repo, but this should be clarified.

@gregwebs
Copy link
Contributor Author

Personally I don’t chase dependencies for snowflake installs anymore. I use Docker, nix, or a shell script and a single install command.

It seems there is a desire for a docker build given all the things that are being pointed to from this pull request.

With that being said I would rather not convince maintainers to maintain something they don’t want to. I should be able to put this into a separate repo. Then I think what is missing is some documentation suggesting these community solutions for those wanting to use Docker.

@gregwebs
Copy link
Contributor Author

I created a new repo here: https://github.com/gregwebs/bitcoin-docker-dev

@luke-jr
Copy link
Member

luke-jr commented Jul 26, 2022

This seems fine if there are developers (like @gregwebs) willing to maintain it. At least 2 others should probably agree to be available for reviewing it.

Mergers are supposed to be janitorial, not dictators over what is supported. If there are developers willing to do the work, that should be sufficient.

(Note that I do not personally use Docker, and will not be reviewing Docker stuff)

@gregwebs
Copy link
Contributor Author

@luke-jr thank you for the support. However, after making a separate repo I like it that way. I think it is just as convenient as in repo, and in some cases actually better because it is easy to build an older version. The reason to put it in repo is for it to be more official and to try to ensure a better review process. However, a Dockerfile is easy to audit, and the demand for this approach is unknown right now.

I would still like to create a document linking to different community efforts at Docker builds.

@gregwebs gregwebs closed this Jul 26, 2022
@bitcoin bitcoin locked and limited conversation to collaborators Jul 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants