Skip to content

Conversation

@burgholzer
Copy link
Member

Description

Picked from #1403.
Builds spdlog as a shared library on library installs.
This reduces the wheel size from 14.3 MiB to 11.0 MiB on x86 linux.

Checklist:

  • The pull request only contains commits that are focused and relevant to this change.
  • I have added appropriate tests that cover the new/changed functionality.
  • I have updated the documentation to reflect these changes.
  • I have added entries to the changelog for any noteworthy additions, changes, fixes, or removals.
  • I have added migration instructions to the upgrade guide (if needed).
  • The changes follow the project's style guidelines and introduce no new warnings.
  • The changes are fully tested and pass the CI checks.
  • I have reviewed my own code changes.

@burgholzer burgholzer self-assigned this Dec 30, 2025
@burgholzer burgholzer added dependencies Pull requests that update a dependency file c++ Anything related to C++ code packaging Anything related to Python packaging labels Dec 30, 2025
@codecov
Copy link

codecov bot commented Dec 30, 2025

Codecov Report

βœ… All modified and coverable lines are covered by tests.

πŸ“’ Thoughts on this report? Let us know!

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 30, 2025

πŸ“ Walkthrough

Summary by CodeRabbit

  • Build
    • Introduced a new configuration option to control whether spdlog is built as a shared library or static library. Output directories for build artifacts are now unified for consistency across different build configurations.
  • Documentation
    • Updated changelog to reflect new build configuration capability.

✏️ Tip: You can customize this high-level summary in your review settings.

Walkthrough

Adds a CMake option to build spdlog as a shared library when project shared libs are enabled, sets spdlog target output directories to install-layout paths, and updates the changelog and a couple of CMake comments.

Changes

Cohort / File(s) Summary
Build configuration
cmake/ExternalDependencies.cmake
Adds cmake_dependent_option(SPDLOG_BUILD_SHARED ...) gated by BUILD_MQT_CORE_SHARED_LIBS; after fetching, if target spdlog exists sets LIBRARY_OUTPUT_DIRECTORY/ARCHIVE_OUTPUT_DIRECTORY to ${CMAKE_BINARY_DIR}/${CMAKE_INSTALL_LIBDIR} and RUNTIME_OUTPUT_DIRECTORY to ${CMAKE_BINARY_DIR}/${CMAKE_INSTALL_BINDIR}.
Changelog
CHANGELOG.md
Adds entry: "Build spdlog as a shared library on project installs" and adds PR reference [#1411].
Minor comment edits
src/qdmi/na/CMakeLists.txt, src/qdmi/sc/CMakeLists.txt
Small comment wording change from "nlohmann_json, spdlog" to "nlohmann_json and spdlog" (no functional changes).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I sniffed the CMake trail with care,
A shared spdlog now hops in there,
Options set and folders neat,
Builds more flexible, clean, and sweet. πŸ₯•

Pre-merge checks

βœ… Passed checks (3 passed)
Check name Status Explanation
Title check βœ… Passed The title clearly summarizes the main change: building spdlog as a shared library on project installs, which directly corresponds to the changeset modifications in cmake configuration and changelog.
Description check βœ… Passed The description follows the template structure, includes context (picked from #1403), motivation (wheel size reduction), and a complete checklist with all items marked as completed.
Docstring Coverage βœ… Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

πŸ“œ Recent review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

πŸ“₯ Commits

Reviewing files that changed from the base of the PR and between 867826b and f0e908d.

πŸ“’ Files selected for processing (4)
  • CHANGELOG.md
  • cmake/ExternalDependencies.cmake
  • src/qdmi/na/CMakeLists.txt
  • src/qdmi/sc/CMakeLists.txt
🧰 Additional context used
🧠 Learnings (8)
πŸ“š Learning: 2025-10-14T14:37:38.047Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/yaqs PR: 212
File: CHANGELOG.md:12-15
Timestamp: 2025-10-14T14:37:38.047Z
Learning: In the munich-quantum-toolkit/yaqs project, changelog entries follow the template: "- $TITLE ([#$NUMBER]($URL)) ([**AUTHOR**](https://github.com/$AUTHOR))". Issue references should not be included in changelog entries; the PR number is sufficient for traceability.

Applied to files:

  • CHANGELOG.md
πŸ“š Learning: 2025-12-05T17:45:37.602Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1360
File: .github/workflows/reusable-mlir-tests.yml:40-43
Timestamp: 2025-12-05T17:45:37.602Z
Learning: In the munich-quantum-toolkit/core repository, patch releases of LLVM dependencies don't require documentation updates, changelog entries, or additional tests beyond what's validated by passing CI checks.

Applied to files:

  • CHANGELOG.md
  • src/qdmi/na/CMakeLists.txt
πŸ“š Learning: 2025-12-01T11:00:40.342Z
Learnt from: flowerthrower
Repo: munich-quantum-toolkit/core-plugins-catalyst PR: 1
File: CHANGELOG.md:18-18
Timestamp: 2025-12-01T11:00:40.342Z
Learning: In the munich-quantum-toolkit/core-plugins-catalyst repository, the CHANGELOG.md intentionally references the parent MQT Core repository's release notes (https://github.com/munich-quantum-toolkit/core/releases) because the plugin repository is based on work performed in the parent repository.

Applied to files:

  • CHANGELOG.md
πŸ“š Learning: 2025-11-24T10:19:41.147Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1326
File: python/mqt/core/__init__.py:22-22
Timestamp: 2025-11-24T10:19:41.147Z
Learning: In the munich-quantum-toolkit/core repository, Ruff is configured with 'ALL' rules enabled by default, and only specific rules are selectively disabled. When reviewing changes that enable previously-disabled rules (like PLC0415), noqa directives for those rules become necessary and should be retained.

Applied to files:

  • CHANGELOG.md
πŸ“š Learning: 2025-12-28T17:14:53.890Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1403
File: src/qdmi/na/CMakeLists.txt:31-38
Timestamp: 2025-12-28T17:14:53.890Z
Learning: In the munich-quantum-toolkit/core repository, the NA device generator target (mqt-core-qdmi-na-device-gen) is intentionally propagated to MQT_CORE_TARGETS via list(APPEND) because it's publicly linked to the NA device library (the NA device uses a public function from the generator). The SC device generator is not propagated because it has no such dependency with the SC device library.

Applied to files:

  • src/qdmi/na/CMakeLists.txt
  • src/qdmi/sc/CMakeLists.txt
πŸ“š Learning: 2025-11-03T23:09:26.881Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1287
File: test/qdmi/dd/CMakeLists.txt:9-21
Timestamp: 2025-11-03T23:09:26.881Z
Learning: The CMake functions `generate_device_defs_executable` and `generate_prefixed_qdmi_headers` used in QDMI device test CMakeLists.txt files are provided by the external QDMI library (fetched via FetchContent from https://github.com/Munich-Quantum-Software-Stack/qdmi.git), specifically in the cmake/PrefixHandling.cmake module of the QDMI repository.

Applied to files:

  • cmake/ExternalDependencies.cmake
πŸ“š Learning: 2025-12-05T15:57:39.701Z
Learnt from: flowerthrower
Repo: munich-quantum-toolkit/core-plugins-catalyst PR: 3
File: lib/Conversion/CatalystQuantumToMQTOpt/CMakeLists.txt:22-25
Timestamp: 2025-12-05T15:57:39.701Z
Learning: The munich-quantum-toolkit projects (core and core-plugins-catalyst) use `file(GLOB_RECURSE ...)` patterns in CMakeLists.txt files to collect header files, following an established convention in the parent repository for consistency across the codebase.

Applied to files:

  • cmake/ExternalDependencies.cmake
πŸ“š Learning: 2025-10-10T08:09:54.528Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1246
File: bindings/CMakeLists.txt:0-0
Timestamp: 2025-10-10T08:09:54.528Z
Learning: In the Munich Quantum Toolkit (MQT) Core project, scikit-build-core is configured with `wheel.install-dir = "mqt/core"` in pyproject.toml, which automatically prefixes all CMake `DESTINATION` paths with `mqt/core/` during wheel installation. Therefore, CMake install destinations are relative to the `mqt/core` package namespace, not `site-packages`.

Applied to files:

  • cmake/ExternalDependencies.cmake
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (17)
  • GitHub Check: 🐍 Test (macos-15-intel) / 🐍 macos-15-intel
  • GitHub Check: 🐍 Test (windows-2022) / 🐍 windows-2022
  • GitHub Check: 🐍 Test (ubuntu-24.04) / 🐍 ubuntu-24.04
  • GitHub Check: πŸ‡¨β€Œ Test 🐧 (ubuntu-24.04, gcc, Release) / 🐧 ubuntu-24.04 gcc Release
  • GitHub Check: 🐍 Test (macos-14) / 🐍 macos-14
  • GitHub Check: 🐍 Test (ubuntu-24.04-arm) / 🐍 ubuntu-24.04-arm
  • GitHub Check: πŸ‡¨β€Œ Test 🍎 (macos-14, clang, Release) / 🍎 macos-14 clang Release
  • GitHub Check: πŸ‡¨β€Œ Test 🍎 (macos-15-intel, clang, Release) / 🍎 macos-15-intel clang Release
  • GitHub Check: πŸ‡¨β€Œ Test 🐧 (ubuntu-24.04-arm, gcc, Release) / 🐧 ubuntu-24.04-arm gcc Release
  • GitHub Check: πŸ‡¨β€Œ Test 🍎 (macos-14, clang, Debug) / 🍎 macos-14 clang Debug
  • GitHub Check: πŸ‡¨β€Œ Test 🐧 (ubuntu-24.04, gcc, Debug) / 🐧 ubuntu-24.04 gcc Debug
  • GitHub Check: πŸ‡¨β€Œ Test 🏁 (windows-11-arm, msvc, Release) / 🏁 windows-11-arm msvc Release
  • GitHub Check: πŸ‡¨β€Œ Test 🏁 (windows-2022, msvc, Debug) / 🏁 windows-2022 msvc Debug
  • GitHub Check: πŸ‡¨β€Œ Test 🏁 (windows-2022, msvc, Release) / 🏁 windows-2022 msvc Release
  • GitHub Check: 🐍 Lint / 🚨 Lint
  • GitHub Check: πŸ‡¨β€Œ Lint / 🚨 Lint
  • GitHub Check: πŸ‡¨β€Œ Coverage / πŸ“ˆ Coverage
πŸ”‡ Additional comments (7)
CHANGELOG.md (2)

28-28: LGTM! Changelog entry follows the correct format.

The entry is properly placed in the "Changed" section with the correct template format including the emoji prefix, descriptive title, PR reference, and author attribution.


291-291: LGTM! PR reference link correctly added.

The PR link is properly added in the correct alphabetical/numerical position in the PR links section.

src/qdmi/sc/CMakeLists.txt (1)

29-29: LGTM! Minor comment clarity improvement.

The updated comment improves readability by using "and" instead of a comma to connect the two libraries being linked.

src/qdmi/na/CMakeLists.txt (1)

29-29: LGTM! Comment improvement for consistency.

The comment change improves clarity and maintains consistency with the corresponding change in src/qdmi/sc/CMakeLists.txt.

cmake/ExternalDependencies.cmake (3)

13-13: LGTM! GNUInstallDirs properly included early.

The early inclusion of GNUInstallDirs ensures that CMAKE_INSTALL_LIBDIR and CMAKE_INSTALL_BINDIR are defined before being used in lines 110-112, addressing the previous review concern.


98-99: LGTM! SPDLOG_BUILD_SHARED option correctly configured.

The cmake_dependent_option creates a cache variable that is visible to the spdlog subproject fetched via FetchContent. When BUILD_MQT_CORE_SHARED_LIBS is ON, spdlog will be built as a shared library; otherwise it will be static. The empirical evidence (wheel size reduction from 14.3 MiB to 11.0 MiB) confirms this is working as intended.


106-113: LGTM! Output directories correctly configured for shared library layout.

This block ensures that spdlog libraries (shared or static) are placed in a common layout that aligns with the install structure. The defensive if(TARGET spdlog) check prevents errors if the target doesn't exist. Setting these output directories is essential for:

  • Proper RPATH resolution at runtime
  • Consistent wheel packaging structure
  • Finding shared libraries during Python builds

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❀️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

πŸ“œ Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

πŸ“₯ Commits

Reviewing files that changed from the base of the PR and between f748baa and 867826b.

πŸ“’ Files selected for processing (4)
  • CHANGELOG.md
  • cmake/ExternalDependencies.cmake
  • src/qdmi/na/CMakeLists.txt
  • src/qdmi/sc/CMakeLists.txt
🧰 Additional context used
🧠 Learnings (7)
πŸ“š Learning: 2025-12-05T17:45:37.602Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1360
File: .github/workflows/reusable-mlir-tests.yml:40-43
Timestamp: 2025-12-05T17:45:37.602Z
Learning: In the munich-quantum-toolkit/core repository, patch releases of LLVM dependencies don't require documentation updates, changelog entries, or additional tests beyond what's validated by passing CI checks.

Applied to files:

  • src/qdmi/na/CMakeLists.txt
  • CHANGELOG.md
πŸ“š Learning: 2025-12-28T17:14:53.890Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1403
File: src/qdmi/na/CMakeLists.txt:31-38
Timestamp: 2025-12-28T17:14:53.890Z
Learning: In the munich-quantum-toolkit/core repository, the NA device generator target (mqt-core-qdmi-na-device-gen) is intentionally propagated to MQT_CORE_TARGETS via list(APPEND) because it's publicly linked to the NA device library (the NA device uses a public function from the generator). The SC device generator is not propagated because it has no such dependency with the SC device library.

Applied to files:

  • src/qdmi/na/CMakeLists.txt
  • src/qdmi/sc/CMakeLists.txt
πŸ“š Learning: 2025-10-14T14:37:38.047Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/yaqs PR: 212
File: CHANGELOG.md:12-15
Timestamp: 2025-10-14T14:37:38.047Z
Learning: In the munich-quantum-toolkit/yaqs project, changelog entries follow the template: "- $TITLE ([#$NUMBER]($URL)) ([**AUTHOR**](https://github.com/$AUTHOR))". Issue references should not be included in changelog entries; the PR number is sufficient for traceability.

Applied to files:

  • CHANGELOG.md
πŸ“š Learning: 2025-12-01T11:00:40.342Z
Learnt from: flowerthrower
Repo: munich-quantum-toolkit/core-plugins-catalyst PR: 1
File: CHANGELOG.md:18-18
Timestamp: 2025-12-01T11:00:40.342Z
Learning: In the munich-quantum-toolkit/core-plugins-catalyst repository, the CHANGELOG.md intentionally references the parent MQT Core repository's release notes (https://github.com/munich-quantum-toolkit/core/releases) because the plugin repository is based on work performed in the parent repository.

Applied to files:

  • CHANGELOG.md
πŸ“š Learning: 2025-11-24T10:19:41.147Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1326
File: python/mqt/core/__init__.py:22-22
Timestamp: 2025-11-24T10:19:41.147Z
Learning: In the munich-quantum-toolkit/core repository, Ruff is configured with 'ALL' rules enabled by default, and only specific rules are selectively disabled. When reviewing changes that enable previously-disabled rules (like PLC0415), noqa directives for those rules become necessary and should be retained.

Applied to files:

  • CHANGELOG.md
πŸ“š Learning: 2025-11-03T23:09:26.881Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1287
File: test/qdmi/dd/CMakeLists.txt:9-21
Timestamp: 2025-11-03T23:09:26.881Z
Learning: The CMake functions `generate_device_defs_executable` and `generate_prefixed_qdmi_headers` used in QDMI device test CMakeLists.txt files are provided by the external QDMI library (fetched via FetchContent from https://github.com/Munich-Quantum-Software-Stack/qdmi.git), specifically in the cmake/PrefixHandling.cmake module of the QDMI repository.

Applied to files:

  • cmake/ExternalDependencies.cmake
πŸ“š Learning: 2025-10-10T08:09:54.528Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1246
File: bindings/CMakeLists.txt:0-0
Timestamp: 2025-10-10T08:09:54.528Z
Learning: In the Munich Quantum Toolkit (MQT) Core project, scikit-build-core is configured with `wheel.install-dir = "mqt/core"` in pyproject.toml, which automatically prefixes all CMake `DESTINATION` paths with `mqt/core/` during wheel installation. Therefore, CMake install destinations are relative to the `mqt/core` package namespace, not `site-packages`.

Applied to files:

  • cmake/ExternalDependencies.cmake
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (17)
  • GitHub Check: 🐍 Test (macos-14) / 🐍 macos-14
  • GitHub Check: 🐍 Test (ubuntu-24.04) / 🐍 ubuntu-24.04
  • GitHub Check: 🐍 Test (ubuntu-24.04-arm) / 🐍 ubuntu-24.04-arm
  • GitHub Check: 🐍 Test (macos-15-intel) / 🐍 macos-15-intel
  • GitHub Check: πŸ‡¨β€Œ Test 🐧 (ubuntu-24.04, gcc, Release) / 🐧 ubuntu-24.04 gcc Release
  • GitHub Check: πŸ‡¨β€Œ Test 🍎 (macos-14, clang, Release) / 🍎 macos-14 clang Release
  • GitHub Check: πŸ‡¨β€Œ Test 🍎 (macos-14, clang, Debug) / 🍎 macos-14 clang Debug
  • GitHub Check: 🐍 Test (windows-2022) / 🐍 windows-2022
  • GitHub Check: πŸ‡¨β€Œ Test 🏁 (windows-2022, msvc, Debug) / 🏁 windows-2022 msvc Debug
  • GitHub Check: πŸ‡¨β€Œ Test 🍎 (macos-15-intel, clang, Release) / 🍎 macos-15-intel clang Release
  • GitHub Check: πŸ‡¨β€Œ Test 🐧 (ubuntu-24.04-arm, gcc, Release) / 🐧 ubuntu-24.04-arm gcc Release
  • GitHub Check: πŸ‡¨β€Œ Test 🏁 (windows-11-arm, msvc, Release) / 🏁 windows-11-arm msvc Release
  • GitHub Check: πŸ‡¨β€Œ Test 🐧 (ubuntu-24.04, gcc, Debug) / 🐧 ubuntu-24.04 gcc Debug
  • GitHub Check: πŸ‡¨β€Œ Test 🏁 (windows-2022, msvc, Release) / 🏁 windows-2022 msvc Release
  • GitHub Check: 🐍 Lint / 🚨 Lint
  • GitHub Check: πŸ‡¨β€Œ Lint / 🚨 Lint
  • GitHub Check: πŸ‡¨β€Œ Coverage / πŸ“ˆ Coverage
πŸ”‡ Additional comments (3)
src/qdmi/na/CMakeLists.txt (1)

29-29: LGTM! Comment style improvement.

The grammar improvement (comma β†’ "and") enhances readability.

src/qdmi/sc/CMakeLists.txt (1)

29-29: LGTM! Consistent comment style improvement.

Matches the style improvement in src/qdmi/na/CMakeLists.txt.

CHANGELOG.md (1)

28-28: LGTM! Changelog entry follows established format.

The entry is correctly placed in the "Changed" section and follows the project's changelog template with appropriate emoji, description, PR reference, and author attribution.

Also applies to: 291-291

@burgholzer burgholzer merged commit b9f6cf8 into main Dec 30, 2025
34 checks passed
@burgholzer burgholzer deleted the spdlog-shared branch December 30, 2025 22:48
burgholzer added a commit that referenced this pull request Jan 8, 2026
## Description

Picked from #1403.
Builds `spdlog` as a shared library on library installs.
This reduces the wheel size from 14.3 MiB to 11.0 MiB on x86 linux.

## Checklist:

<!---
This checklist serves as a reminder of a couple of things that ensure
your pull request will be merged swiftly.
-->

- [x] The pull request only contains commits that are focused and
relevant to this change.
- [x] I have added appropriate tests that cover the new/changed
functionality.
- [x] I have updated the documentation to reflect these changes.
- [x] I have added entries to the changelog for any noteworthy
additions, changes, fixes, or removals.
- [x] I have added migration instructions to the upgrade guide (if
needed).
- [x] The changes follow the project's style guidelines and introduce no
new warnings.
- [x] The changes are fully tested and pass the CI checks.
- [x] I have reviewed my own code changes.

Signed-off-by: burgholzer <[email protected]>
Signed-off-by: Lukas Burgholzer <[email protected]>
(cherry picked from commit b9f6cf8)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Anything related to C++ code dependencies Pull requests that update a dependency file packaging Anything related to Python packaging

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants