[Cmake] Split c++ and c find modules for Iso9660 and Cdio #26821
Conversation
|
@fuzzard this needs a rebase |
There was a problem hiding this comment.
Pull Request Overview
This PR splits the find modules for C++ and C libraries into separate files for iso9660 and cdio, allowing each module to explicitly check its required dependency. Key changes include:
- Introducing new find modules: FindIso9660.cmake, FindIso9660pp.cmake, and FindCdiopp.cmake.
- Updating FindCdio.cmake to alias proper targets and changing the required dependency version for Cdio in CMakeLists.txt.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| cmake/modules/FindIso9660pp.cmake | Updated to use find_package and module helper macros correctly |
| cmake/modules/FindIso9660.cmake | New module added for iso9660 with explicit dependency checks |
| cmake/modules/FindCdiopp.cmake | New module added for cdio++ with explicit dependency checks |
| cmake/modules/FindCdio.cmake | Modified to update target aliasing and dependency options |
| CMakeLists.txt | Updated dependency version requirement for Cdio |
| endif() | ||
| else() | ||
| include(FindPackageMessage) | ||
| find_package_message(Iso9660pp "Iso9660: Can not find libcdio (REQUIRED)" "") |
There was a problem hiding this comment.
The error message in this file incorrectly references 'Iso9660pp' for a missing 'libcdio' dependency; it should consistently refer to 'Iso9660' to match the module's purpose.
| find_package_message(Iso9660pp "Iso9660: Can not find libcdio (REQUIRED)" "") | |
| find_package_message(Iso9660 "Iso9660: Can not find libcdio (REQUIRED)" "") |
Copilot uses AI. Check for mistakes.
| endif() | ||
| else() | ||
| include(FindPackageMessage) | ||
| find_package_message(Cdiopp "Cdiopp: Can not find libcdio (REQUIRED)" "") |
There was a problem hiding this comment.
The error message states that 'libcdio' is missing, but since this module is for cdio++, it should indicate that 'libcdio++' is required.
| find_package_message(Cdiopp "Cdiopp: Can not find libcdio (REQUIRED)" "") | |
| find_package_message(Cdiopp "Cdiopp: Can not find libcdio++ (REQUIRED)" "") |
Copilot uses AI. Check for mistakes.
Description
Introduce explicit find modules for cdio/cdio++ and iso9660/iso9660++
Motivation and context
Ideally each find module should find 1 library only. This will allow macro standardisation, but still explicitly check for required libs, as each is independent.
How has this been tested?
Configure with/without the pkgconfig files (iso9660.pc, iso9660++.pc, libcdio.pc, libcdio++.pc). iso9660pp is correctly disabled when any of the four are missing, but cdio is still found with only libcdio.pc. version requirements for find checks are also explicitly followed
What is the effect on users?
N/A
Screenshots (if appropriate):
Types of change
Checklist: