Skip to content

Fix some CMake build issues - #363

Merged
godlygeek merged 2 commits into
bloomberg:mainfrom
godlygeek:cmake_improvements
Sep 22, 2026
Merged

godlygeek merged 2 commits into
bloomberg:mainfrom
godlygeek:cmake_improvements

Conversation

@godlygeek

Copy link
Copy Markdown
Contributor

Fix some issues that have cropped up when doing CMake builds in various environments. Note that these changes should have no effect on wheels built by cibuildwheel, so this PR only affects maintainers' ability to test on and build directly on different environments.

The CMake PkgConfig module doesn't handle statically linking as nicely
as you would expect. When it finds a `.pc` file, that file tells it what
flags to use when linking statically vs what flags to use when linking
dynamically, but it does not tell it whether the distribution providing
that `.pc` file actually provides a library that can be linked
dynamically or not. Because of that, we've been using the variables
holding the dynamic linking flags unconditionally, leading to broken
builds in environments that only provide static libraries.

Handle this by detecting whether or not the main library found for each
of these dependencies is `libelf.a` or `libdw.a`, and set up a CMake
imported target that passes the right flags depending on whether or not
the library was detected to be an archive rather than a shared library.

Signed-off-by: Matt Wozniski <mwozniski@bloomberg.net>
Because GCC requires a linker plugin to enable LTO, some environments
may have GCC installed but unable to successfully link an LTO build.
Detect that condition at CMake configure time and disable LTO.

Signed-off-by: Matt Wozniski <mwozniski@bloomberg.net>
@godlygeek
godlygeek requested review from a team and pablogsal September 22, 2026 00:09
@godlygeek godlygeek self-assigned this Sep 22, 2026
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.19%. Comparing base (a0ec380) to head (af0d7d7).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #363   +/-   ##
=======================================
  Coverage   78.19%   78.19%           
=======================================
  Files          58       58           
  Lines        6690     6690           
  Branches      630      630           
=======================================
  Hits         5231     5231           
  Misses       1459     1459           
Flag Coverage Δ
cpp 78.19% <ø> (ø)
python 78.19% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@godlygeek
godlygeek merged commit ce356bf into bloomberg:main Sep 22, 2026
72 of 74 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants