chore(deps): upgrade conan dependencies#784
Conversation
Bump: - catch2 3.14.0 -> 3.15.1 - libpcap 1.10.5 -> 1.10.6 - npcap 1.70 -> 1.86 (Windows) - opentelemetry-cpp 1.24.0 -> 1.26.0 - spdlog 1.15.0 -> 1.17.0 - libcurl 8.19.0 -> 8.20.0 Drop the explicit fmt/10.2.1 pin; fmt is now pulled transitively via spdlog (fmt/12.x). protobuf is intentionally left at 6.33.5: opentelemetry-cpp/1.26.0 requires protobuf<7, so a bump to 7.x is not currently viable. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b08b180854
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Mirror the libpcap force=True override for the Windows npcap requirement so the pinned version wins over any transitive constraint. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
opentelemetry-cpp 1.26.0 generates its protobuf headers with `protoc --cpp_out=dllexport_decl=OPENTELEMETRY_PROTO_API`. For a static build the macro must expand to nothing. Upstream defines it on the proto CMake target, but the Conan recipe does not propagate that definition to consumers, so the generated *.pb.h fail to compile in pktvisor (the macro is parsed as a stray type name, cascading into bogus protobuf errors). The generated headers are included from several targets (visor-core, handler unit tests, visor_test helpers), so define the macro globally. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
fmt 10 exposed fmt::join transitively, but fmt 12 no longer does, so every translation unit using fmt::join must include <fmt/ranges.h> explicitly. Add the include to the files that use it (GeoDB, StreamHandler, InputStream, DNS v1/v2 stream handlers, NetProbe input). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Pull request overview
This PR upgrades several Conan-managed dependencies (notably spdlog → 1.17.0 and opentelemetry-cpp → 1.26.0) and adjusts the build/source to stay compatible with the resulting transitive dependency changes (especially fmt moving to 12.x).
Changes:
- Bump Conan dependency versions (catch2, libpcap/npcap, opentelemetry-cpp, spdlog, libcurl) and drop the explicit fmt pin.
- Add
#include <fmt/ranges.h>wherefmt::joinis used, to match newer fmt header requirements. - Add a global
OPENTELEMETRY_PROTO_APIcompile definition to work around opentelemetry-cpp protobuf header generation/consumption behavior.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/StreamHandler.h |
Adds fmt/ranges.h include for fmt::join usage in header code. |
src/InputStream.h |
Adds fmt/ranges.h include for fmt::join usage in header code. |
src/inputs/netprobe/NetProbeInputStream.cpp |
Adds fmt/ranges.h include (fmt range/join support). |
src/handlers/dns/v2/DnsStreamHandler.cpp |
Adds fmt/ranges.h include (fmt range/join support). |
src/handlers/dns/v1/DnsStreamHandler.cpp |
Adds fmt/ranges.h include (fmt range/join support). |
src/GeoDB.cpp |
Adds fmt/ranges.h include for joining sin6_addr bytes. |
conanfile.py |
Upgrades dependency versions; removes fmt pin (left as commented line). |
CMakeLists.txt |
Adds global OPENTELEMETRY_PROTO_API= compile definition for otel protobuf headers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
LCOV of commit
|
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ment openssl 3.6.2 -> 3.6.3 and yaml-cpp 0.8.0 -> 0.9.0. openssl stays on the 3.x line: a transitive dependency caps it at <4, so 4.0.1 conflicts in the graph. Also removes the leftover commented-out fmt/10.2.1 pin (fmt is now resolved transitively via spdlog). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…a DLL) On Windows the Conan opentelemetry-cpp/1.26.0 package builds opentelemetry_proto as a shared library (DLL + import lib), so its proto symbols are exported and consumers must reference them with __declspec(dllimport). Defining the macro empty (correct for the static libs on macOS/Linux) left the symbols unresolved on MSVC (LNK2019 on _Gauge_default_instance_, ScopeMetrics/ExportMetricsServiceRequest vftables). Use dllimport on Windows, empty elsewhere. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
8140aae to
a414f1c
Compare
nlohmann_json 3.11.3 -> 3.12.0 with force=True: opentelemetry-cpp and json-schema-validator pin an exact 3.11.3, so the override is required to resolve the graph (nlohmann_json is header-only and API-compatible). cpp-httplib 0.18.3 -> 0.27.0. This is the highest musl-safe version: starting at 0.28.0 the ConanCenter recipe unconditionally links libanl (non-blocking getaddrinfo) with no opt-out, which musl lacks and breaks the static cross-builds. 0.27.0 predates that change (needs only pthread). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
a414f1c to
e9454d0
Compare
What
Upgrade Conan dependencies to current versions and fix the build issues the upgrades surfaced.
Upgraded
Also drops the explicit
fmt/10.2.1pin —fmtis now pulled transitively viaspdlog(resolves tofmt/12.x).Intentionally held back (dependency-graph / platform constraints)
opentelemetry-cpp/1.26.0constrainsprotobuf/[>=4.25.3 <7], so 7.x conflicts in the graph.<4, so 4.0.1 conflicts.libanl(non-blockinggetaddrinfo) with no opt-out option;libanldoes not exist on musl, which breaks the static musl cross-builds. 0.27.0 is the newest version that predates that recipe change.force=True— opentelemetry-cpp and json-schema-validator pin an exact3.11.3; the override is required to resolve the graph (header-only, API-compatible).Build fixes required by the upgrades
OPENTELEMETRY_PROTO_API— opentelemetry-cpp 1.26.0 generates its protobuf headers with--cpp_out=dllexport_decl=OPENTELEMETRY_PROTO_API, but the Conan recipe does not propagate the macro to consumers. Defined inCMakeLists.txt: empty on macOS/Linux (static proto lib),__declspec(dllimport)on Windows (the Conan package builds proto as a DLL there).#include <fmt/ranges.h>— fmt 12 no longer exposesfmt::jointransitively, so the files using it now include it explicitly.Testing
CI is green on all platforms:
unit-tests-mac,unit-tests-linux,build-win64, bothpktvisormusl cross-builds (x86_64 + aarch64),code-coverage, allpktvisor-clivariants, and CodeQL (Go + Python).🤖 Generated with Claude Code