ci: Refactor build-related nix / docker / workflows#7408
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #7408 +/- ##
=========================================
- Coverage 82.4% 82.3% -0.0%
=========================================
Files 1011 1011
Lines 76911 76911
Branches 8965 8965
=========================================
- Hits 63343 63332 -11
- Misses 13559 13570 +11
Partials 9 9 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR refactors CI-related Docker/Nix assets and GitHub Actions workflows to standardize on Nix-based CI images, add multi-arch image building/merging, and introduce dedicated workflows for building/publishing the Nix and packaging images.
Changes:
- Switch multiple workflows to new
ghcr.io/xrplf/xrpld/nix-*container images and bump the Linux CI image tag. - Add reusable workflows and top-level workflows to build per-arch Docker images and merge them into multi-arch manifests (Nix images + packaging images).
- Add/relocate Docker test harness scripts and C++ sources used to validate toolchains/sanitizer runtime behavior inside the Nix Docker images.
Reviewed changes
Copilot reviewed 12 out of 21 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| package/README.md | Updates packaging documentation for new matrix/image approach (but currently contains inconsistencies vs linux.json). |
| package/install-packaging-tools.sh | New script to install deb/rpm tooling in packaging base images. |
| package/Dockerfile | New packaging image Dockerfile that installs packaging tools into a vanilla distro base. |
| nix/docker/test_files/run-test-binaries.sh | New runtime test runner to validate sanitizer binaries emit expected diagnostics. |
| nix/docker/test_files/cpp_sources/ubsan.cpp | New UBSan-triggering test program source. |
| nix/docker/test_files/cpp_sources/tsan.cpp | New TSan-triggering test program source. |
| nix/docker/test_files/cpp_sources/regular.cpp | New non-sanitized “regular” test program source. |
| nix/docker/test_files/cpp_sources/asan.cpp | New ASan-triggering test program source (also checks sanitizer headers). |
| nix/docker/test_files/compile-cpp-sources.sh | New build-time compiler script to compile and patchelf the test binaries. |
| nix/docker/loader-path.sh | New helper script to compute target PT_INTERP loader path by arch. |
| nix/docker/install-sanitizer-libs.sh | New script to install sanitizer runtime libs into non-Nix base images for runtime validation. |
| nix/docker/Dockerfile | Updates Nix CI Dockerfile to use relocated scripts and add build/run sanity checks. |
| nix/docker/check-tools.sh | New script to verify expected tools are present and that git HTTPS cloning works. |
| .github/workflows/reusable-upload-recipe.yml | Switches recipe upload workflow container to the new Nix-based image. |
| .github/workflows/reusable-build-merge-docker-images.yml | New reusable workflow that builds per-arch images and merges them into multi-arch tags. |
| .github/workflows/reusable-build-docker-image.yml | Small naming tweak; builds single-platform image with arch-suffixed tags for later merging. |
| .github/workflows/publish-docs.yml | Switches docs build workflow container to the new Nix-based image and simplifies env printing. |
| .github/workflows/build-packaging-images.yml | New workflow to build/push packaging images for Debian + RHEL base images. |
| .github/workflows/build-nix-images.yml | New workflow to build/push Nix CI images for multiple base distros, multi-arch. |
| .github/workflows/build-nix-image.yml | Removes the prior single-workflow implementation in favor of the new reusable/multi-arch flow. |
| .github/scripts/strategy-matrix/linux.json | Bumps image_tag used for Nix-based Linux CI images. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
bthomee
left a comment
There was a problem hiding this comment.
Not for this PR, but should the publish-docs workflow be renamed to build-publish-docs? Or, should other workflows be renamed to not include build in their names? Building is usually a prerequisite for the action we're really interested in, whether pushing, publishing, or merging.
Separately, the reusable-build-docker-image workflow file also pushes, which is not reflected in the name. Similarly, the reusable-build-merge-docker-images does what it advertised, but the workflow name inside the file doesn't contain "merge".
There's some inconsistency in the naming overall.
| # Packaging runs in a vanilla distro image, so the tooling has to come | ||
| # from the distro's archive: debhelper for deb, rpm-build (and the | ||
| # systemd / find-debuginfo macros it depends on) for rpm. | ||
| # The container also uses git (real history) for | ||
| # build_pkg.sh's SOURCE_DATE_EPOCH; otherwise it falls back to a tarball | ||
| # download and the timestamp comes from wall-clock time. |
There was a problem hiding this comment.
I see variable line lengths here. While this is just cosmetic and not in a need of fixing right now, it would be nice if we could enforce filling up the full 100 line length to the extent possible in the near future.
Do any such tools exist? I recall you mentioned markdownlint a while back, but even that can fix it for .md files, it would presumably not work for source code, Dockerfiles, etc. We'd need something generic.
There was a problem hiding this comment.
I don't think tools are changing comments that much, because sometimes there is custom formatting, like:
# To achieve this:
# - do this
# - and that
``
It looks nice, even though it's short.
I have no idea how to write a static tool which would make all these cases work nicely
I am not sure we'll be able to achieve 100% consistency. I fixed |
High Level Overview of Change
This:
What images are left, will be done separately:
Context of Change
API Impact
libxrplchange (any change that may affectlibxrplor dependents oflibxrpl)