WIP: ci: Tidy CI pipeline for readability and performance #924
Closed
tcpipuk
wants to merge 1 commit from
tom/fixes into main
pull from: tom/fixes
merge into: continuwuation:main
continuwuation:main
continuwuation:aranje/illegal-car-mods
continuwuation:renovate/crate-ci-typos-1.x
continuwuation:renovate/rust-patch-updates
continuwuation:renovate/cargo-bins-cargo-binstall-1.x
continuwuation:renovate/rspress-plugin-client-redirects-2.x-lockfile
continuwuation:nex/stateres-refactor
continuwuation:renovate/rspress-plugin-preview-2.x-lockfile
continuwuation:renovate/rspress-core-2.x-lockfile
continuwuation:nex/feat/complement
continuwuation:ginger/deps/update-rspress
continuwuation:jade/admin-announce-improvements
continuwuation:ginger/xtask-improvements
continuwuation:jade/improve-admin-config-display
continuwuation:nex/fix/better-stateres-error-logs
continuwuation:renovate/crate-ci-committed-1.x
continuwuation:jade/sender-timeouts
continuwuation:nex/feat/custom-v12-room-ids
continuwuation:ginger/update-metadata
continuwuation:renovate/rand-0.x
continuwuation:nex/feat/admin-force-logout
continuwuation:renovate/lddtree-0.x
continuwuation:tom/max-perf-docs
continuwuation:renovate/github-actions-non-major
continuwuation:nex/fix/invalid-appservice-reg
continuwuation:nex/feat/antispam
continuwuation:nex/feat/account-locking
continuwuation:jade/logging-cleanup
continuwuation:jade/remove-legacy-appservice-auth
continuwuation:nex/fix/key-query
continuwuation:jade/update-prek
continuwuation:nex/fix/room-summaries
continuwuation:ginger/restrict-admin-commands
continuwuation:ginger/enable-console-by-default
continuwuation:jade/tag-fixes
continuwuation:jade/otlp
continuwuation:nex/meta/pull-req-template
continuwuation:nex/fix/fed-invite-compliance
continuwuation:nex/feat/build-commit
continuwuation:nex/feat/join-logging
continuwuation:jade/mailmap-updates
continuwuation:jade/hack-ci-tmp
continuwuation:jade/v12-stable
continuwuation:jade/relations
continuwuation:renovate/axum-monorepo
continuwuation:ginger/database-refactor
continuwuation:jade/fix-ldap-uiaa
continuwuation:nex/fix/validation
continuwuation:ginger/nuke-invalid-msc4133-fields-in-migration
continuwuation:ginger/downgrade-artifact-actions
continuwuation:oddlid/reload-fix
continuwuation:jade/fix-assert
continuwuation:ginger/sync-v3-cleanup
continuwuation:renovate/ruma-digest
continuwuation:ginger/remove-absolute-action-urls
continuwuation:renovate/https-code.forgejo.org-actions-checkout-6.x
continuwuation:renovate/actions-checkout-6.x
continuwuation:jade/website
continuwuation:renovate/https-code.forgejo.org-actions-checkout-digest
continuwuation:nex/fix/backoff
continuwuation:renovate/bytes-1.x-lockfile
continuwuation:ginger/fix-mdbook-for-0.5
continuwuation:ginger/no-docker-on-prs
continuwuation:nex/mods
continuwuation:backport/v0.5.0-rc.8-1
continuwuation:renovate/hyper-1.x-lockfile
continuwuation:nex/fed-improvements
continuwuation:dahsa_uwu/axum-0.8
continuwuation:jade/rust-1.90
continuwuation:jade/mirror-dockerhub
continuwuation:jade/clippy-fixes
continuwuation:jade/fix-support
continuwuation:jade/clean-images
continuwuation:jade/wal-compression-type
continuwuation:jade/flake-clone
continuwuation:ginger/upload-rpms-on-schedule
continuwuation:nex/fix/incoming-fetch
continuwuation:nex/fix/upgrade
continuwuation:tom/ci-fedora-rpm
continuwuation:jade/ci-release-fix
continuwuation:jade/rocksdb-10-5
continuwuation:ginger/fix-msc4133-migration
continuwuation:ginger/migrate-busted-tz
continuwuation:hydra/public
continuwuation:nex/feat/manual-extremities
continuwuation:nex/feat/async-media
continuwuation:nex/feat/fast-joins-hack-do-not-use-DO-NOT-USE
continuwuation:nex/feat/better-logging
continuwuation:trigger-ci-so-latest-isnt-on-illegal-car-mods
continuwuation:nex/feat/pins-backfill
continuwuation:jade/tuwunel-2025-06-old
continuwuation:jade/ai-slop-db-docs
continuwuation:nex/fix-create-auth
continuwuation:jade/version-stats
continuwuation:jade/read-receipts
continuwuation:jade/rust-toolchain-no-targets
continuwuation:jade/logging-features
continuwuation:jade/syncv5-typing
continuwuation:jade/msc2815
continuwuation:jade/purge-sync-tokens
continuwuation:morguldir/see-eye
continuwuation:jade/css-small-screen
continuwuation:nex/wip-751
continuwuation:tuwunel-rebase
continuwuation:test
continuwuation:oddlid/rename-admin-room-bot
continuwuation:strawberry/nix-ci-stuff
continuwuation:strawberry/valgrind
continuwuation:phonemain
continuwuation:strawberry/morgs-snake-sync-jason-main
continuwuation:newer-media-endpoints
continuwuation:folly-coroutines-async-io
continuwuation:federation-retry-timer-port
continuwuation:bad-attempt-at-extracting-homeserver-signing-key
continuwuation:room-deletion-attempt-do-not-use
No reviewers
Labels
Clear labels
This pull request or issue is currently blocked from being merged/closed
Something isn't working as intended
Commits picked from other conduit projects
This requires or includes changes to the database
Something dependency related
Automatic dependency upgrades by Renovate
Low difficulty to implement - touches few parts of the codebase, low complexity
High difficulty to implement - touches many parts of the codebase, high complexity
Medium difficulty to implement - touches more parts of the codebase, higher complexity
Improvements or additions to documentation
New feature or request
Good for newcomers
Additional eyes and keyboards are required for this one
Issues that have been inhereted from the project pre-fork
Features pertaining to homeserver administration
Features pertaining to the appservice API
Features pertaining to authentication
Features pertaining to client-to-server interactions
Issues relating to core matrix functionality, such as state resolution and PDU formats
Features pertaining to server-to-server interactions
Issues related to room version 12 and related changes (temporary label)
Features pertaining to unstable matrix features
Features pertaining to media interactions
Related to housekeeping, maintenance, or other repo-meta.
Issues related to CI changes
Packaging
This issue is blocking the next release
This issue is very important
This issue is of a rather low priority
This item is related to general security
This issue has enough information and is confirmed
This issue or pull request already exists
This issue doesn't seem right
This issue needs further investigation
Questions or support requests
This will not be worked on
Ci/CD
Pull requests that update Rust code
Blocked
This pull request or issue is currently blocked from being merged/closed
Bug
Something isn't working as intended
Cherry-picking
Commits picked from other conduit projects
Database
This requires or includes changes to the database
Dependencies
Something dependency related
Dependencies/Renovate
Automatic dependency upgrades by Renovate
Difficulty
Easy
Low difficulty to implement - touches few parts of the codebase, low complexity
Difficulty
Hard
High difficulty to implement - touches many parts of the codebase, high complexity
Difficulty
Medium
Medium difficulty to implement - touches more parts of the codebase, higher complexity
Documentation
Improvements or additions to documentation
Enhancement
New feature or request
Good first issue
Good for newcomers
Help wanted
Additional eyes and keyboards are required for this one
Inherited
Issues that have been inhereted from the project pre-fork
Matrix/Administration
Features pertaining to homeserver administration
Matrix/Appservices
Features pertaining to the appservice API
Matrix/Auth
Features pertaining to authentication
Matrix/Client
Features pertaining to client-to-server interactions
Matrix/Core
Issues relating to core matrix functionality, such as state resolution and PDU formats
Matrix/Federation
Features pertaining to server-to-server interactions
Matrix/Hydra
Issues related to room version 12 and related changes (temporary label)
Matrix/MSC
Features pertaining to unstable matrix features
Matrix/Media
Features pertaining to media interactions
Meta
Related to housekeeping, maintenance, or other repo-meta.
Meta/CI
Issues related to CI changes
Meta/Packaging
Packaging
Priority
Blocking
This issue is blocking the next release
Priority
High
This issue is very important
Priority
Low
This issue is of a rather low priority
Security
This item is related to general security
Status
Confirmed
This issue has enough information and is confirmed
Status
Duplicate
This issue or pull request already exists
Status
Invalid
This issue doesn't seem right
Status
Needs Investigation
This issue needs further investigation
Support
Questions or support requests
To-Merge
Wont fix
This will not be worked on
old/ci/cd
Ci/CD
Archived
old/rust
Pull requests that update Rust code
Archived
No labels
Blocked
Bug
Cherry-picking
Database
Dependencies
Dependencies/Renovate
Difficulty
Easy
Difficulty
Hard
Difficulty
Medium
Documentation
Enhancement
Good first issue
Help wanted
Inherited
Matrix/Administration
Matrix/Appservices
Matrix/Auth
Matrix/Client
Matrix/Core
Matrix/Federation
Matrix/Hydra
Matrix/MSC
Matrix/Media
Meta
Meta/CI
Meta/Packaging
Priority
Blocking
Priority
High
Priority
Low
Security
Status
Confirmed
Status
Duplicate
Status
Invalid
Status
Needs Investigation
Support
To-Merge
Wont fix
old/ci/cd
old/rust
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".
No due date set.
Dependencies
No dependencies set.
Reference
continuwuation/continuwuity!924
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "tom/fixes"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
While looking at complement, I've prepared some tidy-ups that shave precious time off the CI checks, use caching more, and update prefligit to the new version of prek.
Pipeline Modernisation:
Workflow Restructuring:
Performance Improvements:
Dependency Management:
a71e584a2598632d511bFwiw sccache only needs the secrets to download files from GitHub without rate limiting, I'm not sure why that action forces them. If you can get the binary in a different way that might be better
Renaming things is redundant by the way, and breaks the branch protection rule (
Checks / *)@ -0,0 +43,4 @@// Extract hostname from server URLconst serverUrl = '${{ github.server_url }}'const registry = serverUrl.replace(/^https?:\/\//, '')This is wrong - would extract the registry as github.com rather than ghcr.io on github for example. Perhaps there's an env variable for this? Anyway
There isn't,
github.server_urlis the only one I could find in the docs... we could specify an env var for the preferred Docker repository though, which would make this easier - my problem was that it wasn't at all portable, so when I tried to run CI on my fork, it immediately collapsed when trying to auth with forgejo.ellis.linkPerhaps
env.BUILTIN_REGISTRYcould be removed from the top and we just declare it on the repo, then I'll adjust this to skip trying to upload an image if that's missing?Yeah that works
It would be vars.
I've set
BUILTIN_REGISTRYon my version and this repo, and testing a version that usesvars.BUILTIN_REGISTRYinstead, if that's ok?(Edit: Ah ha, you'd already replied!)
@ -0,0 +120,4 @@env:DOCKER_METADATA_ANNOTATIONS_LEVELS: manifest,index- name: Get commit SHAShort commit sha specified explicitly here for a reason
I can't actually review this with the renames because it breaks the diff.
@ -0,0 +194,4 @@labels: ${{ steps.meta.outputs.labels }}annotations: ${{ steps.meta.outputs.annotations }}cache-from: type=gha# cache-to: type=gha,mode=maxHave you tried enabling this again?
@ -0,0 +57,4 @@LOG_LEVEL: ${{ inputs.logLevel || 'info' }}# Forgejo/Gitea specificRENOVATE_GIT_AUTHOR: 'Renovate Bot <renovate@noreply.${{ github.server_url }}>'Should be configurable so I can get this to actually match up with the account
Sure! Would you prefer it configurable as one line, or individual vars for name and email?
Probably easier as just one line
Cool, I've added
RENOVATE_AUTHORwith an example value if you want to update it 🙂@ -0,0 +60,4 @@RENOVATE_GIT_AUTHOR: 'Renovate Bot <renovate@noreply.${{ github.server_url }}>'# PR settingsRENOVATE_PR_HOURLY_LIMIT: '2'Pretty sure you already set this in the config file
98632d511b667f5f3472667f5f347269d8c82a7f69d8c82a7f087beca7fc087beca7fca20fe8fc8ea20fe8fc8e741a26a7c5741a26a7c539e5e2b11939e5e2b1198c1b59c6848c1b59c68474b0760982ci: tidy CI pipeline for readability and performanceto ci: Tidy CI pipeline for readability and performance74b0760982a08c6aacc5a08c6aacc5b0ebdb59edb0ebdb59ed50250f51fc@ -0,0 +225,4 @@# Stage 2: Final scratch imageFROM scratchARG GIT_COMMIT_HASHthese are all for build time
They were being passed in as build args from the workflow, but I'll convert them so we're directly substituting into envs in the workflow.
@ -0,0 +204,4 @@# For native builds, extract libraries if dynamically linkedif [ "${{ matrix.platform }}" = "linux/amd64" ]; thenif ldd target/${{ matrix.rust_target }}/release/conduwuit 2>/dev/null | grep -q "=>" ; thenLDD doesn't work cor cross compile
(you will end with none of the needed libs on cross-compiles)
It was only being done for
linux/amd64(which we can do it for) but I'll remove it to make all of the builds identical - when we don't extract them, we just end with a larger static build.@ -0,0 +185,4 @@--release \--locked \--features "standard,jemalloc_prof,perf_measurements,tokio_console" \--target ${{ matrix.rust_target }}Missing SBOM, missing ability to build for a specific target CPU (eg haswell), not doing cross-language LTO afaict
I've added SBOM and updated the x86_64 arch from "base" to "haswell" and enabled LTO 🙂
We still need to build for base because not all CPUs are post haswell
Ok, I'd appreciate if you could be a bit clearer on how you'd prefer this to look then - I couldn't see any Docker tags for haswell, so how would you prefer it be delivered?
The ability to pass in a target CPU that gets set as the needed compiler flags -
ARG TARGET_CPU- and then to build that as a separate job in the matrix and publish the image with a suffix. This wasn't used previously because of the extra build time. Automatic selection of the correct binary from a single image could be achieved via https://forgejo.ellis.link/continuwuation/hwcaps-image but I never got around to it and I'm not sure about the thing being GPL3
@ -0,0 +168,4 @@gcc-package: ${{ matrix.gcc_package }}gxx-package: ${{ matrix.gxx_package }}liburing-package: ${{ matrix.liburing_package }}is-cross-compile: ${{ matrix.is_cross_compile }}Cross compiling should be effectively identical to normal builds
That would be the dream, but there are differences - in particular,
pkg-configneeds to be told it's a cross-compile, but we also need to download arch-specific liburing too.See how the Dockerfile handles it. For example, it has two sets of deps - host and target - and always installs both sets.
I believe Docker emulates the architecture outside the container, so inside the container we just install for emulated architecture?
In this case, to use tools like sccache, we're cross-compiling without using Docker (just adding the binary to Docker afterwards) so we need to configure cross-compilation and tell
pkg-configwe're cross-compiling.@ -0,0 +123,4 @@cxx: g++linker: gccdpkg_arch: ""gcc_package: gccWhat's going on here? this really shoudn't need to be specified for every platform in the matrix.
The inputs save defining any OS-specific config inside the action, so literally all the compile config is defined in the workflow, then the action just configures the Rust environment to use what it's been told.
.forgejo/actions/rust-with-cache/action.yml- where all the logic is - is already tied to bash and debian very specifically."OS" was intended to mean "architecture" but the runner commands are only written to use Debian-type because the runner is
ubuntu-latest- the actual cross-compile config provided in the matrix strategy can target Darwin, FreeBSD, or even Windows if we have the necessary libraries.@ -1,235 +0,0 @@ARG RUST_VERSION=1Shouldn't be deleting the dockerfile anyway.
It seemed a risk to leave the Dockerfile laying around if we never actually use it for builds, as it'll quickly fall out of maintenance... I can leave it in to be retired on a future date, whatever works for you 🤷♂️
I'll still be using (a slightly modified version) of it for my prod deploys.
But the main reason was I was still experimenting with getting it and #912 to work
I've restored the Dockerfile, I'll look at whether anything needs updating later.
@ -0,0 +1,409 @@name: Checks / Build / PublishChecks /means it blocks merges - should probably be separate (you can add depends across workflows IIRC)I'm not sure what you mean here - the current checks and builds run on every branch/PR, so I've organised them into one workflow to stop 6 jobs trying to run at once, to generate better cache, and complete faster. The same jobs will run, it'll just run more efficiently (and fast-fail the other jobs when linting/clippy fails).
Everything under
Checks /has to finish before you can hit merge - so you have to wait for the images to publish now, when you didn't before. Not as much of an issue because it's faster now though@ -0,0 +626,4 @@echo "::group::🎯 Cross-compilation target setup"# Wait for basic Rust setup to complete firstWAIT_START=$(date +%s)for al bash scripts, given there are so many, it would probably be better if they were in files where they're syntax highlighted and not indented
I'm not sure we can use GHA variables the same way inside Bash scripts in separate files... I could convert it to JavaScript if you'd rather, or just split into more actions so it's easier to audit in chunks?
GHA vars can still be passed in via
env:inputs - it's up to you tbh. It just gets kind of hard to read & edit code embedded into yaml lolIt is a big chonk, but it was partly why I tried to shift most of the common "make Rust work please" dependencies into a big action that extrapolates a lot of the mess out of the other workflows.
I'd be up for splitting it into more actions, just think it might be a rough experience to have all of the shell scripts separate in folders. I can see what that'd look like though?
Nah it's OK if you don't think it's worth it.
50250f51fcb9b33e3db7@ -0,0 +216,4 @@# Copy binarycp target/${{ matrix.rust_target }}/release/conduwuit docker-context/# Note: We rely on Rust producing mostly-static binaries with --releaseI'd prefer words as well as screenshots to communicate, but I think what you're saying here is that the current build method (that does everything inside an emulated architecture inside Docker) can use
lldto pull out dynamic libraries, but you've also pointed out that we can't just uselldto pull out dynamic libraries on cross-compiled binaries.I'm currently testing switching from
gcctoclangso thatlldcan support the cross-compilation environment and potentially replicate how this was working in Docker... if that doesn't work then 🤷♂️This is done in the docker image using lddtree/goblin, which does not need to execute the files and so can work on cross compiles.
Oh and you're gonna need to switch to clang anyway for the cross-language LTO to work
b9b33e3db7b481ff31c0ci: Tidy CI pipeline for readability and performanceto WIP: ci: Tidy CI pipeline for readability and performanceMarking this WIP till it produces working images
Pull request closed