Skip to content

build: make pkg-config optional for the system libunwind/breakpad options - #2007

Open
mertefesensoy wants to merge 3 commits into
getsentry:masterfrom
mertefesensoy:pkg-config-optional/libunwind/breakpad
Open

build: make pkg-config optional for the system libunwind/breakpad options#2007
mertefesensoy wants to merge 3 commits into
getsentry:masterfrom
mertefesensoy:pkg-config-optional/libunwind/breakpad

Conversation

@mertefesensoy

@mertefesensoy mertefesensoy commented Aug 20, 2026

Copy link
Copy Markdown

Problem

SENTRY_LIBUNWIND_SYSTEM and SENTRY_BREAKPAD_SYSTEM locate their dependencies with find_package(PkgConfig REQUIRED) + pkg_check_modules(... REQUIRED IMPORTED_TARGET ...), and sentry-config.cmake.in repeats that for consumers of a static build:

  • CMakeLists.txt:708SENTRY_WITH_LIBUNWIND (Linux only) + SENTRY_LIBUNWIND_SYSTEM
  • CMakeLists.txt:806SENTRY_BACKEND_BREAKPAD + SENTRY_BREAKPAD_SYSTEM
  • CMakeLists.txt:937 — the crash daemon's libunwind-ptrace lookup
  • sentry-config.cmake.in:19,24 — the same two lookups, at consumer time

Those are the only places in this project that touch pkg-config, they are all Linux-only, and both options default to OFF (the only other user is vendored crashpad, on Linux with CRASHPAD_ENABLE_STACKTRACE=ON). But because the requirement is unconditional and hard, packagers end up declaring the tool for every platform they build sentry-native for. vcpkg's port is the concrete example — it lists pkgconf as a host dependency of the default backend feature for !android & !ios, so vcpkg install sentry-native:x64-windows resolves pkgconf@3.0.3 into the install plan even though nothing in the sentry-native build will ever run it.

That is not free. Since pkgconf 2.9.90 the release tarball contains tests/lib1/të😋st/lib/pkgconfig/utf8.pc (added in pkgconf commit 943a4497a95939c8b9c7aea442b417a193c51220, still present in 3.0.5; vcpkg is on 3.0.3). GitHub's git archive stores that name as raw UTF-8 in the 100-byte ustar name field — the full path is 64 bytes, so no pax path= record is emitted, and git archive emits no hdrcharset either. Windows' tar.exe (libarchive) therefore falls back to archive_string_default_conversion_for_read() and decodes header names with the machine's legacy code page. Where that code page rejects the bytes, archive_mstring_copy_mbs_len_l() clears every string form of the entry name, archive_entry_pathname_w() returns NULL, and extraction aborts with Invalid empty pathname; tar.exe exits non-zero and vcpkg fails the port. The same failure mode has been reported for other ports whose sources contain non-ASCII paths, e.g. microsoft/vcpkg#43984

None of that is sentry-native's bug — but sentry-native is what pulled the tool onto platforms that never needed it. Making the dependency honest is the part that belongs in this repo.

Change

Add cmake/sentry-find-system-library.cmake with a sentry_find_system_library() helper that

  • prefers the pkg-config metadata when both the tool and the requested .pc module are present (unchanged behaviour, including transitive Requires: / Libs.private:), and
  • otherwise resolves the library and its headers with find_library() / find_path().

Both branches define the same imported target — sentry::libunwind, sentry::libunwind-ptrace, sentry::breakpad-client — so the call sites and the installed config no longer have to know which lookup succeeded. The module is installed next to sentry-config.cmake and reused by it, so consumers of a static build recreate the targets the same way and no longer need pkg-config themselves.

Two incidental improvements fall out of this:

  • A missing dependency now reports what was not found and how to fix it, instead of failing inside FindPkgConfig with Could NOT find PkgConfig (missing: PKG_CONFIG_EXECUTABLE).
  • The crash daemon's libunwind-ptrace lookup no longer depends on an earlier find_package(PkgConfig) call having run ~230 lines above it in the same file.

The vendored-libunwind default path, non-Linux builds, and builds that do have pkg-config are untouched.

Follow-up (not in this PR)

With this merged, ports/sentry-native/vcpkg.json can drop its pkgconf host dependency outright. I verified with the real resolver that doing so removes pkgconf (and its own vcpkg-tool-meson host dependency) from the install plan on both x64-windows and x64-linux, and that the Linux build still configures, builds, installs and links downstream with no pkg-config on the machine at all. Happy to open that upstream. Worth noting the port's $comment on that dependency is inaccurate today: it attributes pkgconf to the breakpad backend, but the port never enables SENTRY_BREAKPAD_SYSTEM; the real user is the system-libunwind lookup on Linux.

Verification

On Linux, for each of three states — pkg-config available / pkg-config physically removed from the machine / pkg-config present but the .pc module missing:

  • SENTRY_BACKEND=native -DSENTRY_LIBUNWIND_SYSTEM=ON, static and shared: configure, build, cmake --install, then find_package(sentry) from a downstream project, build and run it. master fails to configure in the "removed" state with Could NOT find PkgConfig; this branch succeeds. Confirmed sentry-crash links libunwind, libunwind-ptrace and libunwind-generic in every case, and that sentry-targets.cmake carries sentry::libunwind in the static link interface.
  • SENTRY_BACKEND=breakpad -DSENTRY_BREAKPAD_SYSTEM=ON against a synthetic breakpad-client package: both branches resolve to the same include root (<prefix>/include/breakpad), matching the client/<os>/handler/exception_handler.h includes in src/backends/sentry_backend_breakpad.cpp.
  • The not-found path produces the intended diagnostic.
  • Default (vendored libunwind) build and make test-unit still pass.

Supersedes #2006, which closed automatically when its head branch was renamed.

…ions

`SENTRY_LIBUNWIND_SYSTEM` and `SENTRY_BREAKPAD_SYSTEM` located their
dependencies with `find_package(PkgConfig REQUIRED)` +
`pkg_check_modules(... REQUIRED ...)`, and the exported
`sentry-config.cmake` repeated that for consumers of a static build.
Those are the only places in this project that ever touch pkg-config,
and only on Linux, but because the requirement is unconditional and
hard, packagers have to provide the tool on every platform they build
sentry-native for -- including Windows and macOS, where it is never
invoked.

Add `cmake/sentry-find-system-library.cmake`, which prefers the
pkg-config metadata when both the tool and the `.pc` module are present
and otherwise resolves the library and its headers with `find_library()`
/ `find_path()`. Both branches define the same imported target
(`sentry::libunwind`, `sentry::libunwind-ptrace`,
`sentry::breakpad-client`), so the call sites and the installed config
no longer care which lookup succeeded, and a missing dependency now
reports what could not be found instead of failing inside FindPkgConfig.

The module is installed next to `sentry-config.cmake` so consumers of a
static build recreate the targets the same way. As a side effect the
crash daemon's `libunwind-ptrace` lookup no longer depends on an earlier
`find_package(PkgConfig)` call having run elsewhere in the file.

Verified on Linux for the system-libunwind path (static and shared,
`SENTRY_BACKEND=native`) and the system-breakpad path, each with
pkg-config available, with pkg-config unavailable, and with pkg-config
present but the `.pc` module missing; including install and a downstream
`find_package(sentry)` build in each case.
Brings in fb337fb (getsentry#2004). The only conflict was CHANGELOG.md, where
both getsentry#2004 and this branch appended an entry to the same Unreleased
"Fixes" list; both entries are kept, with getsentry#2004 first since it is
already on master.
Copilot AI lite review requested due to automatic review settings August 20, 2026 10:33

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@codecov

codecov Bot commented Aug 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.43%. Comparing base (69639a0) to head (2a6bac0).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2007      +/-   ##
==========================================
- Coverage   74.69%   74.43%   -0.27%     
==========================================
  Files         104      104              
  Lines       26182    26182              
  Branches     4740     4740              
==========================================
- Hits        19557    19488      -69     
- Misses       5295     5370      +75     
+ Partials     1330     1324       -6     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mertefesensoy

Copy link
Copy Markdown
Author

@jpnurmi @JoshuaMoelans
Could one of you take a look, or point me at the right reviewer? Build-system only (CMake + the exported config), CI green on all platforms, branch up to date with master.

@jpnurmi

jpnurmi commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator

The changes look promising, though I've only briefly glanced through them.

It makes me wonder whether this is really best solved independently in each individual project, though. sentry-native is certainly not alone here, and the necessary discovery logic gets pretty noisy and error-prone.

Just an idea: could this pattern be proposed upstream to CMake as a reusable abstraction?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants