From 06a77de531eeda0f37ef7bf252b652ef5a91c0e8 Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Tue, 15 Sep 2026 14:40:33 -0300 Subject: [PATCH 1/2] Format our own types through formatter specializations 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&`. --- include/session/format.hpp | 14 ++++++++++++++ include/session/util.hpp | 9 ++++++--- src/core/devices.cpp | 39 ++++++++++++++++++++++++++++++-------- src/util.cpp | 8 ++++---- tests/quic-files.cpp | 1 + 5 files changed, 56 insertions(+), 15 deletions(-) diff --git a/include/session/format.hpp b/include/session/format.hpp index 24e73ccbb..24fc32dc9 100644 --- a/include/session/format.hpp +++ b/include/session/format.hpp @@ -10,7 +10,9 @@ #include #include #include +#include #include +#include #include namespace session { @@ -41,6 +43,18 @@ inline namespace literals { namespace fmt { +/// Formats a byte count with an SI prefix: "12.3 kB". See `session::human_size`. +/// +/// A formatter specialization rather than fmt's `format_as` hook, as everywhere else here: the +/// hook only reaches non-enum types from fmt 10, and a specialization is also what `std::format` +/// takes, should we ever want it. +template <> +struct formatter : formatter { + auto format(session::human_size s, format_context& ctx) const { + return formatter::format(s.str(), ctx); + } +}; + // Disable fmt's generic range formatter for byte spans so that our byte_spannable formatter takes // precedence (avoids ambiguity when fmt/ranges.h is also included). template diff --git a/include/session/util.hpp b/include/session/util.hpp index 7ebf5e386..44a363c6b 100644 --- a/include/session/util.hpp +++ b/include/session/util.hpp @@ -388,12 +388,15 @@ std::vector zstd_compress( std::optional> zstd_decompress( std::span data, size_t max_size = 0); -/// Wrapper for formatting byte sizes with SI prefixes via fmt/oxen-logging. Provides a -/// `format_as` friend function discoverable via ADL, so no fmt headers are needed here. +/// Wrapper for formatting byte sizes with SI prefixes via fmt/oxen-logging. The fmt formatter +/// lives in session/format.hpp, alongside the others, so that no fmt headers are needed here; +/// formatting one of these means including that header. /// Usage: `log::info(cat, "Size: {}", human_size{12345});` => "Size: 12.3 kB" struct human_size { int64_t bytes; - friend std::string format_as(human_size s); + + /// "12.3 kB", which is what the formatter prints. + std::string str() const; }; /// NTTP helper struct for the `_bytes` user-defined literal. diff --git a/src/core/devices.cpp b/src/core/devices.cpp index 493d80db2..3c54b398a 100644 --- a/src/core/devices.cpp +++ b/src/core/devices.cpp @@ -30,6 +30,22 @@ #include "../internal-util.hpp" +/// Logs a key pair as "X25519[abcd…wxyz], MLKEM768[abcd…wxyz]". +/// +/// Out here, rather than beside the type, because a formatter specialization belongs to namespace +/// fmt and `session::core` does not enclose it. A specialization rather than fmt's ADL `format_as` +/// hook because that hook only reaches non-enum types from fmt 10 onwards, and because this is also +/// the form `std::format` takes. +/// (`fmt::format_context` spelled out: an out-of-line partial specialization like this one does +/// not get namespace fmt into its unqualified lookup on gcc 12 / fmt 9.) +template Keys> +struct fmt::formatter : fmt::formatter { + auto format(const Keys& k, fmt::format_context& ctx) const { + return formatter::format( + fmt::format("X25519[{:9.4}], MLKEM768[{:9.4}]", k.x25519_pub, k.mlkem768_pub), ctx); + } +}; + namespace session::core { using namespace fmt::literals; @@ -149,13 +165,6 @@ namespace { } // namespace -// format_as for XWingKeys-derived types (DeviceKeys, AccountKeys), defined in session::core so -// that fmtlib's ADL-based lookup can find it when logging these types. -template Keys> -std::string format_as(const Keys& k) { - return "X25519[{:9.4}], MLKEM768[{:9.4}]"_format(k.x25519_pub, k.mlkem768_pub); -} - Devices::DeviceKeys Devices::rotate_device_keys() { // We store just one single seed value, then use SHAKE256 to expand it into separate X25519 // (32B) and MLKEM-768 (64B) seeds. @@ -771,7 +780,7 @@ namespace { Removed = 3, // device newly transitioned to Unregistered }; - constexpr std::string_view format_as(Processing p) { + constexpr std::string_view to_string(Processing p) { switch (p) { case Processing::LinkRequest: return "link-request"; case Processing::Registered: return "registered"; @@ -816,6 +825,20 @@ namespace { } // namespace +} // namespace session::core + +/// Logs a `Processing` as the word `to_string` gives for it. Out here for the reason given at the +/// top of this file; `session::core::Processing` names the type because the unnamed namespace it +/// lives in is reachable from its enclosing namespace. +template <> +struct fmt::formatter : fmt::formatter { + auto format(session::core::Processing p, fmt::format_context& ctx) const { + return formatter::format(to_string(p), ctx); + } +}; + +namespace session::core { + std::vector Devices::encrypt_device_data(const device::map& devices) { cleared_b32 a; random::fill(a); diff --git a/src/util.cpp b/src/util.cpp index 9cd33f2e3..81948c46c 100644 --- a/src/util.cpp +++ b/src/util.cpp @@ -41,11 +41,11 @@ std::vector split(std::string_view str, const std::string_view return results; } -std::string format_as(human_size s) { - if (s.bytes < 1000) - return fmt::format("{} B", s.bytes); +std::string human_size::str() const { + if (bytes < 1000) + return fmt::format("{} B", bytes); constexpr std::array prefixes = {'k', 'M', 'G', 'T'}; - double b = s.bytes; + double b = bytes; for (auto prefix : prefixes) { b /= 1000.; if (b < 1000.) diff --git a/tests/quic-files.cpp b/tests/quic-files.cpp index d48a4580e..991c3a4ab 100644 --- a/tests/quic-files.cpp +++ b/tests/quic-files.cpp @@ -15,6 +15,7 @@ #include #include #include +#include #include #include "dns_utils.hpp" From 922e102658881a3d10055b5181b4581e54ede3e6 Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Tue, 15 Sep 2026 14:42:09 -0300 Subject: [PATCH 2/2] CI: build against system libraries, with one static-deps build 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. --- .drone.jsonnet | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/.drone.jsonnet b/.drone.jsonnet index 0e878bbec..a97e05892 100644 --- a/.drone.jsonnet +++ b/.drone.jsonnet @@ -18,6 +18,30 @@ local default_deps_nocxx = [ local default_deps = ['g++'] + default_deps_nocxx; +// Everything we can link against rather than compiling our own copy of, for builds that are not +// deliberately static (see `static_deps` in debian_build). Two reasons: such a build is much +// faster, and it is the only thing that tests us against the library versions distros actually +// ship -- Debian 12's fmt 9, for instance, which our own code has to stay compatible with. +// +// liboxen-quic-dev pulls in liboxen-logging-dev, which is older than we accept +// (OXEN_LOGGING_MIN_VERSION in external/CMakeLists.txt); the submodule is used for that one and +// builds against the system fmt/spdlog, which is what puts fmt 9 in front of our code. +// +// A too-old system library is not an error: cmake falls back to building that one dependency. +// libsodium is that case on Debian 12 and Ubuntu 22.04, which ship less than the 1.0.21 we need. +local system_deps = [ + 'libevent-dev', + 'libfmt-dev', + 'liboxen-quic-dev', + 'liboxenc-dev', + 'libsodium-dev', + 'libspdlog-dev', + 'libsqlite3-dev', + 'libutf8proc-dev', + 'libzstd-dev', + 'nettle-dev', +]; + local default_test_deps = libngtcp2_deps; local docker_base = 'registry.oxen.rocks/'; @@ -102,6 +126,7 @@ local debian_build(name, image, arch='amd64', deps=default_deps, + static_deps=false/* build our own dependencies instead of using the distro's */, test_deps=default_test_deps, build_type='Release', lto=false, @@ -120,7 +145,7 @@ local debian_build(name, name, image, arch=arch, - deps=deps, + deps=deps + (if static_deps then [] else system_deps), stf_repo=stf_repo, kitware_repo=kitware_repo, allow_fail=allow_fail, @@ -130,6 +155,7 @@ local debian_build(name, 'cmake .. -DCMAKE_CXX_FLAGS=-fdiagnostics-color=always -DCMAKE_BUILD_TYPE=' + build_type + ' ' + (if werror then '-DWARNINGS_AS_ERRORS=ON ' else '') + (if shared_libs then '-DBUILD_SHARED_LIBS=ON ' else '') + + '-DBUILD_STATIC_DEPS=' + (if static_deps then 'ON ' else 'OFF ') + '-DUSE_LTO=' + (if lto then 'ON ' else 'OFF ') + '-DWITH_LTO=' + (if lto then 'ON ' else 'OFF ') + '-DWITH_TESTS=' + (if tests then 'ON ' else 'OFF ') + @@ -444,6 +470,10 @@ local static_build(name, debian_build('Debian 12', docker_base + 'debian-bookworm'), debian_build('Ubuntu latest', docker_base + 'ubuntu-rolling'), debian_build('Ubuntu LTS', docker_base + 'ubuntu-lts'), + // The one build that compiles every dependency itself rather than taking the distro's, on the + // oldest distro we support: what the release artifacts do, and the only thing that notices when + // a dependency we vendor stops building. + debian_build('Ubuntu 22.04 (static deps)', docker_base + 'ubuntu-jammy', static_deps=true), // ARM builds (ARM64 and armhf) debian_build('Debian sid (ARM64)', docker_base + 'debian-sid', arch='arm64', jobs=4),