Fix fmt 9 compatibility, and use system libs for (most) CI jobs - #163
Merged
Merged
Conversation
fmt's ADL `format_as` hook only reaches non-enum types from fmt 10, so the three types using it here -- human_size, the XWingKeys-derived device and account keys, and devices.cpp's Processing -- are unformattable against a system fmt 9 (Debian bookworm), which fails as a wall of template errors deep inside fmt rather than as anything that names them. Specializations work from fmt 9 through 12, and are also the only form std::format offers, so nothing here has to be rewritten if we ever move to it. The output is unchanged. human_size's formatter goes in session/format.hpp with the others, keeping session/util.hpp free of fmt headers as before; what was its format_as is now human_size::str(). `fmt::format_context` is spelled out in the devices.cpp formatters: an out-of-line partial specialization does not get namespace fmt into its unqualified lookup on gcc 12 with fmt 9, where the unqualified name is not found and the parameter silently becomes `int&`.
Every debian_build now installs the libraries the distro has and configures with -DBUILD_STATIC_DEPS=OFF, rather than compiling its own copy of each dependency. That is considerably faster, and it is what puts distro library versions in front of our code: Debian 12's fmt 9 is a version we have to support and was, until now, never built against in CI. A too-old system library still falls back to building that one dependency (libsodium on Debian 12 and Ubuntu 22.04), so this does not narrow what we can build on. One full static-deps build is kept, on Ubuntu 22.04 as the oldest distribution we support, so that the vendored dependency builds -- what the release artifacts use -- keep being exercised.
jagerman
force-pushed
the
system-deps-ci
branch
3 times, most recently
from
September 21, 2026 17:18
890d8b0 to
e84f8d2
Compare
Test images were missing various shared libs that they need.
jagerman
force-pushed
the
system-deps-ci
branch
4 times, most recently
from
September 21, 2026 20:04
bf37659 to
c0f9f85
Compare
jagerman
force-pushed
the
system-deps-ci
branch
from
September 21, 2026 20:05
c0f9f85 to
b6b7b94
Compare
jagerman
force-pushed
the
system-deps-ci
branch
from
September 21, 2026 20:28
3a0e06f to
7e83902
Compare
It defaulted to BUILD_STATIC_DEPS, which the new Ubuntu 22.04 job turns on alongside the debian_build default of BUILD_SHARED_LIBS=ON. Our own libraries are then shared, libsession_static_bundle() skips every non-static target, and libsession-util.a is assembled from the dependencies alone -- which surfaces only at the end of the build as undefined libsession symbols when static-bundle-test links against it. Nothing relied on the default: every static build goes through utils/static-bundle.sh, which passes -DSTATIC_BUNDLE=ON itself. Also reject the combination outright rather than quietly building a useless archive.
7494a1b dropped the duplicate external/nlohmann-json submodule and repointed oxenc, oxen-logging and oxen-libquic at their nested copies, but left this add_subdirectory pointing at the deleted directory. It went unnoticed because session-router's own external/CMakeLists.txt defines the target first, so with the default ENABLE_NETWORKING_SROUTER=ON the guard above skips the dead path entirely. The Windows cross build turns srouter off, and so is the only build that reaches it.
The nested arguments to check_submodule are looked up as gitlinks of the first argument's repo, but oxen-logging is recorded by oxen-libquic and fmt/spdlog by oxen-logging, so naming them by their path from session-router asked session-router's tree for gitlinks it does not have. It went unnoticed because the recursion is broken upstream and every nested check silently passed; they fail for real once the deps submodule carries that fix. Root each level at the repo that records it, and check oxen-encoding too, since we build against it.
The macro is sessiondep_or_submodule, and the submodule list still named three that were dropped in 7494a1b while omitting the four that are actually here.
Picks up 15dc00d, which fixes check_submodule's recursion: it passed the parent's relative path as the nested call's working directory, which execute_process resolves against the build tree, so both git calls failed silently and two empty strings compared equal -- every nested submodule reported up-to-date no matter its state, under a mangled path. Also brings everything else that accumulated since 30b2009: dependency updates (nettle 4.0, gnutls 3.8.13, libsodium 1.0.22, ngtcp2 1.25.0, simdutf 9.2.0, sqlite3mc 2.5.1 among others), the android and mingw cross-build fixes, and the deps repo's own CI.
nettle 4.0 dropped the length argument from the *_digest functions, so the five calls we make stopped compiling as soon as the deps bump moved the vendored nettle to 4.0. The distro nettle is still 3.x everywhere we build against system libraries, so both spellings have to keep working. Dispatch on the signature the headers actually declare rather than on NETTLE_VERSION_MAJOR: the version macro would be one more thing to get wrong for a nettle that backports or diverges, and the arity is what we actually care about. Every call already asked for the full digest length, which is what nettle 4 assumes, so the two are equivalent.
gcc 11 does not consider the constraints on an out-of-line constrained partial specialization, and so rejected our formatter for XWingKeys-derived types as a redefinition of libquic's formatter<T, char> for to_string-ables. Reopening namespace fmt instead of spelling the specialization out-of-line makes gcc 11 compare the constraints properly.
service_node.hpp is a public header that includes nlohmann/json.hpp, and core.hpp includes it in turn, so every consumer of core needs the json include path. Linux builds only worked by accident: ENABLE_NETWORKING_SROUTER links session-router::core publicly, which leaked the path in. The Windows build has no session-router, so client failed to find the header.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
format_asonly started working with fmt starting in version 10, and so builds on bookworm using system libfmt were failing; this fixes it.This also changes CI jobs to use system libs as much as possible, for faster builds since in many cases we don't have to rebuild dependencies. It keeps a jammy-based static build just to ensure that that path still works properly.