build: support containerized development via Docker; revert dash#4621 #4336
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kwvg
force-pushed
the
docker
branch
6 times, most recently
from
c0433e8 to
efa1f66
Compare
kwvg
changed the title
kwvg
force-pushed
the
docker
branch
2 times, most recently
from
8481dc3 to
c82b553
Compare
kwvg
changed the title
kwvg
requested review from
PastaPastaPasta and
UdjinM6
and removed request for
PastaPastaPasta
kwvg
force-pushed
the
docker
branch
2 times, most recently
from
e96aa64 to
87779b7
Compare
|
This pull request has conflicts, please rebase. |
UdjinM6
requested changes
Dec 18, 2021
UdjinM6
left a comment
UdjinM6
left a comment
There was a problem hiding this comment.
needs rebase and a tiny fix, LGTM otherwise
PastaPastaPasta
changed the title
PastaPastaPasta
approved these changes
Dec 28, 2021
Member
PastaPastaPasta
left a comment
PastaPastaPasta
left a comment
There was a problem hiding this comment.
ACK for merging via merge commit
5 tasks
PastaPastaPasta
added a commit
that referenced
this pull request
Mar 24, 2025
… add `multiprocess` and `tsan`, drop unneeded packages 76f99cd doc: update container documentation (Kittywhiskers Van Gogh) b8b9bb4 ci: split out `multiprocess` linting to a separate step on Actions (Kittywhiskers Van Gogh) 94dedba revert: tentatively drop multiprocess and tsan functional tests (Kittywhiskers Van Gogh) b379708 trivial: `lldbUpdAltArgs` -> `llvmUpdAltArgs` (Kittywhiskers Van Gogh) 90710b3 ci: avoid making copies when collecting logs, misc. script fixes (Kittywhiskers Van Gogh) 89d977d ci: use slim container for functional tests (Kittywhiskers Van Gogh) 22e9acd ci: remove old dependencies, packages not used in building or testing (Kittywhiskers Van Gogh) 630bb0c ci: consolidate package installation in `ci` container (Kittywhiskers Van Gogh) 837d10a ci: extract minimum environment needed to run functional tests (Kittywhiskers Van Gogh) 4bcd093 ci: stop using LLVM setup script to setup repo (Kittywhiskers Van Gogh) a4ce15b ci: rename `ci/Dockerfile`, switch to `devthefuture/dockerfile-x` (Kittywhiskers Van Gogh) 5413955 ci: move packages needed to run `dash-qt` into `develop` container (Kittywhiskers Van Gogh) Pull request description: ## Motivation [dash#6583](#6583) introduced functional tests to GitHub Actions and due to a combination of limited disk space available on runners and restrictions on ways to free up said disk space (as described in [dash#6583](#6583)), `multiprocess` and `tsan` builds had to be excluded ([commit](3461c14)). Despite measures to reign in disk usage ([source](https://github.com/dashpay/dash/blob/26ea618b81084c74c0a9bb1fed5321012c1ac8ac/ci/dash/slim-workspace.sh)), we are experiencing sporadic failures due to space exhaustion ([build](https://github.com/dashpay/dash/actions/runs/13572578591/job/37941577942), [build](https://github.com/dashpay/dash/actions/runs/13572201698/job/37940403595)). To take care of this, we are going to extract portions of our `ci` container into `ci-slim`, containing only the parts needed to run (sanitizer-enabled) binaries, functional tests and (some) linters. ## Additional Information * We have used [`edrevo/dockerfile-plus`](https://github.com/edrevo/dockerfile-plus) to allow container definitions inherit other container definitions, which has allowed for a ready-to-use interactive container ([`develop`](https://github.com/dashpay/dash/blob/26ea618b81084c74c0a9bb1fed5321012c1ac8ac/contrib/containers/develop/Dockerfile)) to be built on top of our CI container ([`ci`](https://github.com/dashpay/dash/blob/26ea618b81084c74c0a9bb1fed5321012c1ac8ac/contrib/containers/ci/Dockerfile)) with ease (introduced in [dash#4336](#4336)). Unfortunately, since then, there has been little activity at [`edrevo/dockerfile-plus`](https://github.com/edrevo/dockerfile-plus), last code contribution was ~4 years ago ([commit](edrevo/dockerfile-plus@90dc08d)) and as this PR splits off `ci` to `ci-slim`, it appears `dockerfile-plus` has difficulty in working with `Dockerfiles` that inherit other `Dockerfiles` (see below). <details> <summary>Error message:</summary> ``` $ docker-compose build #0 building with "default" instance using docker driver [...] #1 [container internal] load build definition from Dockerfile #1 transferring dockerfile: 1.57kB done #1 DONE 0.0s [...] #7 [container internal] load build definition from Dockerfile #7 DONE 0.0s failed to solve: failed to create LLB definition: dockerfile parse error line 9: unknown instruction: INCLUDE+ ``` </details> To mitigate this, we are switching over to [`devthefuture/dockerfile-x`](https://codeberg.org/devthefuture/dockerfile-x), which come with minor changes in syntax. * As [`devthefuture/dockerfile-x`](https://codeberg.org/devthefuture/dockerfile-x) (like [`edrevo/dockerfile-plus`](https://github.com/edrevo/dockerfile-plus)) relies on BuildKit, it has been explicitly enabled for GitLab, without which, it fails ([build](https://gitlab.com/dashpay/dash/-/jobs/9276738177#L194)). * As scoping in `Dockerfile`s are relative to the `Dockerfile` itself ([source](https://codeberg.org/devthefuture/dockerfile-x#scoping)), `develop/Dockerfile` can include `ci/Dockerfile` by setting the context one directory behind (which is possible for the root `Dockerfile`) but when `ci/Dockerfile` is parsed, it will search within the `ci` folder and not from the context folder set earlier and so, including `ci-slim/Dockerfile` would come up empty as it would resolve to `ci/ci-slim/Dockerfile`, which doesn't exist. To work around this, both `ci` and `ci-slim` definitions need to share the same directory and to allow for that, `ci/Dockerfile` is now `ci/ci.Dockerfile`, so that `develop/Dockerfile` can set the context a directory behind, find `ci/ci.Dockerfile`, which will search within `ci` for `./ci-slim.Dockerfile`, which will be found successfully. * We have switched away from using the official LLVM installation script to importing the repository manually as we need only a select few packages to run sanitizer-enabled builds and functional tests and the script only gives you a choice between default packages and all packages. * While avoiding LLVM packages entirely in the `ci-slim` container was most desirable, doing so breaks sanitizer builds ([build](https://github.com/kwvg/dash/actions/runs/13585208794/job/37981496193#step:6:2571)) * Switching over to manual import also let us drop a few packages as the script has to probe and work around bugs while our base environment remains relatively similar. * Space occupied by containers. Calculated by modifying `develop/docker-compose.yml` to point at different `Dockerfile`s and building them with `docker compose`, then passing them to [dive](https://github.com/wagoodman/dive) v0.12.0. | Name | Commit | Size | Efficiency Score | Potential Waste | | --------- | -------------------- | ------ | ---------------- | --------------- | | `ci` | `develop` (26ea618) | 4.4 GB | 99% | 74 MB | | `develop` | `develop` (26ea618) | 4.5 GB | 99% | 78 MB | | `ci-slim` | This PR (db6a3fd0) | 1.6 GB | 97% | 64 MB | | `ci` | This PR (db6a3fd0) | 4.3 GB | 99% | 70 MB | | `develop` | This PR (db6a3fd0) | 4.3 GB | 99% | 74 MB | ## Checklist - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] I have added or updated relevant unit/integration/functional/e2e tests **(note: N/A)** - [x] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: UdjinM6: utACK 76f99cd PastaPastaPasta: utACK 76f99cd Tree-SHA512: c0afc570dfc9f6db3dbacbf40b4a2416e993b5e2e503694b9b564a4e00600aa4c290b7c7395f0abc1a7b269c488ed86f4c4c2e64436f38b7ea1653057db83259
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why?
There is a possibility that a contributor's host system does not match the distribution used by the CI system that Dash uses, which makes debugging CI failures an increasing pain. Travis CI offered a feature where you could SSH into the CI instance and debug the cause of failure. GitLab CI, to my knowledge does not offer that