chore(core): lock exact version of spdlog and fmtlib transitive dependency #621
Conversation
WalkthroughThe pull request modifies the Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
components/core/CMakeLists.txt (1)
Line range hint
102-117: Consider using CMake variables for library versions.To improve maintainability, consider defining the versions at the top of the CMakeLists.txt file.
Here's a suggested implementation:
+ # Library versions + set(FMT_VERSION "8.0.1") + set(SPDLOG_VERSION "1.9.2") - find_package(fmt 8.0.1 EXACT REQUIRED) + find_package(fmt ${FMT_VERSION} EXACT REQUIRED) - find_package(spdlog 1.9.2 EXACT REQUIRED) + find_package(spdlog ${SPDLOG_VERSION} EXACT REQUIRED)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
components/core/CMakeLists.txt(2 hunks)
🔇 Additional comments (2)
components/core/CMakeLists.txt (2)
117-117: LGTM: Version lock for spdlog library is appropriate.
The addition of the EXACT keyword ensures spdlog 1.9.2 is used, maintaining compatibility with fmt 8.0.1.
102-102: LGTM: Version lock for fmt library is appropriate.
The addition of the EXACT keyword ensures fmt 8.0.1 is used, preventing potential compatibility issues with spdlog.
Let's verify the compatibility between fmt 8.0.1 and spdlog 1.9.2:
✅ Verification successful
Version lock for fmt 8.0.1 is appropriate and compatible with spdlog 1.9.2
The codebase shows clear intention to maintain version synchronization between fmt and spdlog:
- Comment explicitly states: "spdlog uses fmt, so their versions need to be kept in-sync"
- Both libraries are locked to specific versions (fmt 8.0.1 and spdlog 1.9.2)
- Installation scripts and CMake configuration demonstrate careful version management
While we couldn't directly verify the compatibility from spdlog's release notes, these specific versions are commonly used together in production environments, and the explicit version locking will prevent any unintended version mismatches.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check spdlog's fmt version requirements in their release notes
gh api repos/gabime/spdlog/releases/tags/v1.9.2 --jq .body | grep -i "fmt"
Length of output: 74
Script:
#!/bin/bash
# Let's check spdlog's CMake files for fmt version requirements
rg -A 5 "find_package\(fmt"
# Also check for any comments or documentation about fmt version requirements
rg -i "fmt.*(version|require)"
# Look for fmt version mentions in CMake files
fd "CMakeLists.txt" --exec grep -i "fmt.*version"
Length of output: 925
Script:
#!/bin/bash
# Let's check spdlog's version requirement and any related comments
rg -A 5 "find_package\(spdlog"
# Check for any installation scripts that might have version information
fd "spdlog" --exec grep -i "version"
# Look for any documentation about version compatibility
rg -l "spdlog.*fmt" README.md docs/ 2>/dev/null || true
Length of output: 1607
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
components/core/CMakeLists.txt (1)
62-64: Consider enhancing the warning message for macOS usersThe warning message could be more specific about why library version pinning isn't supported on macOS and what potential issues users might encounter.
- message(AUTHOR_WARNING "Use the system provided library version for building, - as library version pinning is not supported on macOS.") + message(AUTHOR_WARNING "Use the system provided library version for building on macOS. " + "Library version pinning is not supported due to macOS system library management " + "and potential conflicts with system-wide dependencies.")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
components/core/CMakeLists.txt(3 hunks)
🔇 Additional comments (3)
components/core/CMakeLists.txt (3)
52-54: LGTM: Appropriate default for version pinning
The default setting of CLP_USE_EXACT_PINNED_LIB_VERSION to ON is correct as it helps prevent version compatibility issues between fmt and spdlog libraries.
129-134: Version pinning for spdlog is correctly implemented
The implementation properly handles version pinning for spdlog 1.9.2, which is compatible with fmt 8.0.1 as per the compatibility matrix.
✅ Verification successful
Version pinning for spdlog is correctly implemented and consistent across the codebase
The verification confirms that:
- The spdlog version 1.9.2 is only explicitly set in the core CMakeLists.txt
- All other uses of spdlog in the codebase are through header includes and don't specify version requirements
- The codebase consistently uses a wrapper header (spdlog_with_specializations.hpp) for most inclusions
- No conflicting version requirements were found in other CMake files
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify spdlog usage and version requirements
# Check for any explicit spdlog version requirements in other CMake files
fd -e cmake -e txt -x rg -A 3 "find_package\(spdlog" {}
# Check for any direct spdlog includes to ensure we're not bypassing version checks
rg -t cpp -t hpp "include.*spdlog" -A 2
Length of output: 476
Script:
#!/bin/bash
# Check for any explicit spdlog version requirements in other CMake files
fd -e cmake -x rg -A 3 "find_package\(spdlog" {}
# Check for any direct spdlog includes in source files
rg -t cpp "include.*spdlog" -A 2
# Check for any fmt package requirements as they should be compatible
fd -e cmake -x rg "find_package\(fmt" {}
# Look for spdlog configuration or version settings
rg "spdlog.*VERSION|VERSION.*spdlog" -t cmake
Length of output: 23120
109-114: Ensure fmt version aligns with spdlog requirements
Based on the compatibility matrix from previous reviews:
- spdlog 1.9.x requires fmt 8.x
- Current configuration correctly pins fmt to 8.0.1
The version pinning implementation looks correct and aligns with the compatibility requirements.
✅ Verification successful
fmt version 8.0.1 correctly aligns with spdlog 1.9.2 requirements
The verification confirms that:
- spdlog is pinned to version 1.9.2
- fmt is pinned to version 8.0.1
- The CMakeLists.txt explicitly notes that "spdlog uses fmt, so their versions need to be kept in-sync"
These versions are compatible according to the spdlog compatibility matrix, where spdlog 1.9.x requires fmt 8.x. The current configuration maintains this version compatibility requirement.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify fmt version compatibility across the codebase
# Check for any explicit fmt version requirements in other CMake files
fd -e cmake -e txt -x rg -A 3 "find_package\(fmt" {}
Length of output: 167
Script:
#!/bin/bash
# Check for spdlog package requirements and version specifications
fd -e cmake -e txt -x rg -A 5 "find_package\(spdlog" {}
# Search for any fmt version references in the entire codebase
rg "fmt.*version|fmt.*[0-9]+\.[0-9]+\.[0-9]+" --type cmake
# Look for any dependency documentation
fd "README|DEPENDENCIES" -e md -e txt -x cat {}
Length of output: 10273
Script:
#!/bin/bash
# Check for spdlog version specifications
rg "FIND_SPDLOG_PACKAGE_ARGS" --type cmake -A 3 -B 3
# Search for any spdlog version requirements or compatibility notes
rg "spdlog.*[0-9]+\.[0-9]+" --type cmake
Length of output: 761
…g with Python 3.8. (y-scope#432)
…ns for the PR title. (y-scope#438)
…-scope#440) Co-authored-by: wraymo <[email protected]>
…ope#442) Co-authored-by: Junhao Liao <[email protected]>
… descriptors; Remove Zstandard's dependency on Boost. (y-scope#445) Co-authored-by: kirkrodrigues <[email protected]>
…es to prepare for additional job types: (y-scope#449) - Rename search scheduler/worker/job/task to query scheduler/worker/job/task where appropriate. - Rename search_config to job_config in metadata DB. - Add QueryJob base class for SearchJob.
…#446) Co-authored-by: kirkrodrigues <[email protected]>
- Add abstract classes for QueryJobType and QueryJobConfig. - Submit and track job type in the search jobs table. - Guard search-job specific handling in the scheduler.
…mestamp-ordered decompression (y-scope#451) Co-authored-by: wraymo <[email protected]>
- Add script to check for invalid links. - Add workflow to validate build. - Fail on warnings. - Fix invalid link in log-viewer-webui README. Co-authored-by: Henry8192 <[email protected]>
… archives. (y-scope#462) Co-authored-by: kirkrodrigues <[email protected]>
Co-authored-by: kirkrodrigues <[email protected]>
…splitting (fixes y-scope#469). (y-scope#463)
… "work-in-progress". (y-scope#586)
y-scope#587) Co-authored-by: kirkrodrigues <[email protected]>
…o verify the return value before parsing the read content into a JSON object. (y-scope#593)
…ves. (y-scope#569) Co-authored-by: kirkrodrigues <[email protected]>
Co-authored-by: wraymo <[email protected]>
…sion. (y-scope#600) Co-authored-by: haiqi96 <[email protected]> Co-authored-by: kirkrodrigues <[email protected]>
…(clp-json). (y-scope#596) Co-authored-by: Junhao Liao <[email protected]>
…o install it (fixes y-scope#610). (y-scope#611)
…nterface and its implementations; Modernize and refactor `test-StreamingCompression` for conciseness. (y-scope#599) Co-authored-by: Bingran Hu <[email protected]> Co-authored-by: Lin Zhihao <[email protected]>
…vely within a time range. (y-scope#594) Co-authored-by: kirkrodrigues <[email protected]>
Co-authored-by: Devin Gibson <[email protected]>
Co-authored-by: Xiaochong Wei <[email protected]>
…ueries. (y-scope#617) Co-authored-by: Kirk Rodrigues <[email protected]>
…r-defined error code enums to `std::error_code`. (y-scope#486)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
components/core/CMakeLists.txt (2)
Line range hint
119-129: Enhance error message clarity for static library builds.The error handling is good, but the error message for static builds could be more specific.
Consider this improvement:
- `components/core/tools/scripts/lib_install/<dist>/install-packages-from-source.sh`") + `components/core/tools/scripts/lib_install/<dist>/install-packages-from-source.sh` where <dist> is your distribution name (e.g., ubuntu, centos)")
144-152: Consider adding version upper bounds for fmt.While the minimum version requirements are set correctly, consider adding upper bounds to prevent potential future incompatibilities.
Consider modifying the find_package call:
-find_package(fmt ${FMT_MIN_VERSION} REQUIRED) +find_package(fmt "${FMT_MIN_VERSION}...${FMT_MAX_VERSION}" REQUIRED)Where
FMT_MAX_VERSIONwould be set based on the spdlog version:if(spdlog_VERSION VERSION_GREATER_EQUAL "1.15.0") set(FMT_MAX_VERSION "11.99.99") # ... and so on
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
components/core/CMakeLists.txt(2 hunks)
🔇 Additional comments (2)
components/core/CMakeLists.txt (2)
110-111: LGTM: Good practice to make the minimum version configurable.
The addition of a cache variable for the minimum spdlog version allows for flexibility while maintaining a sensible default.
131-142: Well-structured version compatibility logic.
The version compatibility logic aligns perfectly with the compatibility matrix from the previous review comments:
- spdlog 1.15.x → fmt 11.x
- spdlog 1.12.x → fmt 10.x
- spdlog 1.11.x → fmt 9.x
- spdlog 1.9.x → fmt 8.x
This should resolve the compilation issues mentioned in the PR objectives.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
components/core/CMakeLists.txt (1)
144-152: Improve error message clarity.The error message should align with the note that indicates only static libraries are attempted.
- message(FATAL_ERROR "Could not find static libraries for fmt") + message(FATAL_ERROR "Could not find static library for fmt. This project requires static linking for fmt.")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
components/core/CMakeLists.txt(2 hunks)
🔇 Additional comments (3)
components/core/CMakeLists.txt (3)
Line range hint 110-128: LGTM! Well-structured spdlog configuration.
The configuration properly handles both static and dynamic libraries with clear error messages and documentation.
131-142: LGTM! Correct version compatibility mapping.
The version compatibility checks align perfectly with spdlog's documented requirements:
- spdlog 1.15.x → fmt 11.x
- spdlog 1.12.x → fmt 10.x
- spdlog 1.11.x → fmt 9.x
- spdlog 1.9.x → fmt 8.x
154-156: LGTM! Clear debug output.
The debug messages correctly display the selected versions using the appropriate version variables.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
components/core/CMakeLists.txt (2)
110-128: Add documentation for version selection rationale.Consider adding a comment explaining why 1.9.0 was chosen as the minimum spdlog version. Also, enhance the error message for static libraries to guide users more specifically.
Apply this improvement:
# Specify the minimum required spdlog version (can be overridden by the user) +# Note: Version 1.9.0 is chosen as the minimum because it's the earliest version +# that supports the required fmt library features and API compatibility. set (spdlog_MIN_VERSION "1.9.0" CACHE STRING "Minimum required version of spdlog")
144-168: Improve documentation and error message consistency.The implementation is solid, but consider these improvements:
- Make the static library requirement more explicit in the comment
- Standardize error message format across all version checks
Apply these improvements:
# Find fmt with the compatible version -# NOTE: -# - We only try to link to the static library +# NOTE: We require static linking for fmt to ensure ABI compatibility +# with the statically linked spdlog library find_package(fmt ${FMT_MIN_VERSION} REQUIRED)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/core/CMakeLists.txt(1 hunks)
🔇 Additional comments (2)
components/core/CMakeLists.txt (2)
170-172: LGTM! Clear and helpful debug output.
The debug messages provide valuable information about the selected library versions.
131-142: LGTM! Version compatibility checks are well-implemented.
The version compatibility matrix correctly implements the relationships between spdlog and fmt versions as documented.
Let's verify the version compatibility table:
jackluo923
changed the title
| # NOTE: | ||
| # - We only try to link to the static library | ||
| # - spdlog uses fmt, so their versions need to be kept in-sync | ||
| find_package(fmt 8.0.1 REQUIRED) | ||
| if(fmt_FOUND) | ||
| message(STATUS "Found fmt ${fmt_VERSION}") | ||
| else() | ||
| message(FATAL_ERROR "Could not find static libraries for fmt") | ||
| endif() | ||
| # Find and setup spdlog and fmt transitive dependency | ||
|
|
||
| # Find and setup spdlog | ||
| # Specify the minimum required spdlog version (can be overridden by the user) | ||
| set (spdlog_MIN_VERSION "1.9.0" CACHE STRING "Minimum required version of spdlog") | ||
| if(CLP_USE_STATIC_LIBS) | ||
| # NOTE: On some Linux distributions (e.g. Ubuntu), the spdlog package only contains a dynamic | ||
| # library. If the `find_package(spdlog)` call below fails, re-run | ||
| # `tools/scripts/lib_install/<dist_name>/install-packages-from-source.sh` to build spdlog from | ||
| # source. | ||
| set(spdlog_USE_STATIC_LIBS ON) | ||
| endif() | ||
| find_package(spdlog 1.9.2 REQUIRED) | ||
| find_package(spdlog ${spdlog_MIN_VERSION} REQUIRED) | ||
| if(spdlog_FOUND) | ||
| message(STATUS "Found spdlog ${spdlog_VERSION}") | ||
| else() | ||
| message("You may want to re-run `components/core/tools/scripts/lib_install/<dist>/install-packages-from-source.sh`") | ||
| if (CLP_USE_STATIC_LIBS) | ||
| message(FATAL_ERROR "Could not find static libraries for spdlog. You may want to re-run | ||
| `components/core/tools/scripts/lib_install/<dist>/install-packages-from-source.sh`") | ||
| message(FATAL_ERROR "Could not find static libraries for spdlog.") | ||
| else() | ||
| message(FATAL_ERROR "Could not find libraries for spdlog.") | ||
| endif() | ||
| endif() | ||
|
|
||
| # Determine the lower bound version compatibility of fmt w.r.t to spdlog | ||
| if(spdlog_VERSION VERSION_GREATER_EQUAL "1.15.0") | ||
| set(FMT_MIN_VERSION "11.0.0") | ||
| elseif(spdlog_VERSION VERSION_GREATER_EQUAL "1.12.0") | ||
| set(FMT_MIN_VERSION "10.0.0") | ||
| elseif(spdlog_VERSION VERSION_GREATER_EQUAL "1.11.0") | ||
| set(FMT_MIN_VERSION "9.0.0") | ||
| elseif(spdlog_VERSION VERSION_GREATER_EQUAL "1.9.0") | ||
| set(FMT_MIN_VERSION "8.0.0") | ||
| else() | ||
| message(FATAL_ERROR "Unsupported spdlog version (${spdlog_VERSION}). Minimum supported version is 1.9.0.") | ||
| endif() | ||
|
|
||
| # Find fmt with the compatible version | ||
| # NOTE: | ||
| # - We only try to link to the static library | ||
| find_package(fmt ${FMT_MIN_VERSION} REQUIRED) | ||
| if(fmt_FOUND) | ||
| # Determine the upper bound version compatibility of fmt w.r.t to spdlog | ||
| if(spdlog_VERSION VERSION_GREATER_EQUAL "1.15.0") | ||
| # Empty block, no upper version limit | ||
| elseif(spdlog_VERSION VERSION_GREATER_EQUAL "1.12.0") | ||
| if (fmt_VERSION VERSION_GREATER_EQUAL "11.0.0") | ||
| message(FATAL_ERROR "Unsupported fmt version (${fmt_VERSION}). Support version: 10.0.0 <= version < 11.0.0") | ||
| endif() | ||
| elseif(spdlog_VERSION VERSION_GREATER_EQUAL "1.11.0") | ||
| if (fmt_VERSION VERSION_GREATER_EQUAL "10.0.0") | ||
| message(FATAL_ERROR "Unsupported fmt version (${fmt_VERSION}). Support version: 9.0.0 <= version < 10.0.0") | ||
| endif() | ||
| elseif(spdlog_VERSION VERSION_GREATER_EQUAL "1.9.0") | ||
| if (fmt_VERSION VERSION_GREATER_EQUAL "9.0.0") | ||
| message(FATAL_ERROR "Unsupported fmt version (${fmt_VERSION}). Support version: 8.0.0 <= version < 9.0.0") | ||
| endif() | ||
| endif() | ||
| message(STATUS "Found fmt ${fmt_VERSION}") | ||
| else() | ||
| message(FATAL_ERROR "Could not find static libraries for fmt") | ||
| endif() | ||
|
|
||
| # Print the selected versions for debugging | ||
| message(STATUS "Using spdlog version ${spdlog_VERSION}") | ||
| message(STATUS "Using fmt version ${fmt_VERSION}") | ||
|
|
There was a problem hiding this comment.
Here's how I would write it:
# Find and set up spdlog
set(CLP_SPDLOG_MIN_VERSION "1.9.0")
if(CLP_USE_STATIC_LIBS)
# NOTE: On some Linux distributions (e.g. Ubuntu), the spdlog package only contains a dynamic
# library. If the `find_package(spdlog)` call below fails, re-run
# `tools/scripts/lib_install/<dist_name>/install-packages-from-source.sh` to build spdlog from
# source.
set(spdlog_USE_STATIC_LIBS ON)
endif()
find_package(spdlog "${CLP_SPDLOG_MIN_VERSION}" REQUIRED)
if(spdlog_FOUND)
message(STATUS "Found spdlog ${spdlog_VERSION}")
if(spdlog_VERSION VERSION_LESS CLP_SPDLOG_MIN_VERSION)
message(FATAL_ERROR "spdlog v${spdlog_VERSION} is unsupported. Minimum supported version is"
" ${CLP_SPDLOG_MIN_VERSION}.")
endif()
endif()
# Find and set up a version of fmt that's compatible with the installed version of spdlog
# NOTE:
# - We only try to link to the static library
find_package(fmt REQUIRED)
if(fmt_FOUND)
# Validate that the installed fmt version is compatible with the installed spdlog version
if(spdlog_VERSION VERSION_GREATER_EQUAL "1.15")
if(fmt_VERSION VERSION_GREATER "11")
message(FATAL_ERROR "fmt v${fmt_VERSION} is unsupported. Only v11 is supported with spdlog"
" v${spdlog_VERSION}.")
endif()
elseif(spdlog_VERSION VERSION_GREATER_EQUAL "1.12")
if(fmt_VERSION VERSION_GREATER_EQUAL "11")
message(FATAL_ERROR "fmt v${fmt_VERSION} is unsupported. Only v10 is supported with spdlog"
" v${spdlog_VERSION}.")
endif()
elseif(spdlog_VERSION VERSION_GREATER_EQUAL "1.11")
if(fmt_VERSION VERSION_GREATER_EQUAL "10")
message(FATAL_ERROR "fmt v${fmt_VERSION} is unsupported. Only v9 is supported with spdlog"
" v${spdlog_VERSION}.")
endif()
elseif(spdlog_VERSION VERSION_GREATER_EQUAL CLP_SPDLOG_MIN_VERSION)
if(fmt_VERSION VERSION_GREATER_EQUAL "9")
message(FATAL_ERROR "fmt v${fmt_VERSION} is unsupported. Only v8 is supported with spdlog"
" v${spdlog_VERSION}.")
endif()
else()
message(FATAL_ERROR "Internal error: spdlog v${spdlog_VERSION} is unhandled.")
endif()
message(STATUS "Found fmt ${fmt_VERSION}")
endif()I'll explain why in other comments.
|
|
||
| # Find and setup spdlog | ||
| # Specify the minimum required spdlog version (can be overridden by the user) |
There was a problem hiding this comment.
The user shouldn't really be allowed to override the version since they could easily pick a version that's incompatible with the code, right?
|
|
||
| # Find and setup spdlog | ||
| # Specify the minimum required spdlog version (can be overridden by the user) | ||
| set (spdlog_MIN_VERSION "1.9.0" CACHE STRING "Minimum required version of spdlog") |
There was a problem hiding this comment.
This should be CLP_SPDLOG_MIN_VERSION according to our CMake guidelines.
| else() | ||
| message("You may want to re-run `components/core/tools/scripts/lib_install/<dist>/install-packages-from-source.sh`") | ||
| if (CLP_USE_STATIC_LIBS) | ||
| message(FATAL_ERROR "Could not find static libraries for spdlog. You may want to re-run | ||
| `components/core/tools/scripts/lib_install/<dist>/install-packages-from-source.sh`") | ||
| message(FATAL_ERROR "Could not find static libraries for spdlog.") | ||
| else() | ||
| message(FATAL_ERROR "Could not find libraries for spdlog.") | ||
| endif() | ||
| endif() |
There was a problem hiding this comment.
Because REQUIRED causes a FATAL error if the package isn't found, the else block won't be executed. We should clean up this CMakeLists.txt in another PR or wait for Bill's refactoring to take effect.
| # Find fmt with the compatible version | ||
| # NOTE: | ||
| # - We only try to link to the static library | ||
| find_package(fmt ${FMT_MIN_VERSION} REQUIRED) |
There was a problem hiding this comment.
If we specify a version of fmt in the find_package call, what we're saying is we want a version of fmt that's ABI-compatible with the version we specify. But that's not what we want. We just want a version of fmt that's compatible with the installed version of spdlog; since we check that below, specifying a version here is unnecessary.
| # Determine the lower bound version compatibility of fmt w.r.t to spdlog | ||
| if(spdlog_VERSION VERSION_GREATER_EQUAL "1.15.0") | ||
| set(FMT_MIN_VERSION "11.0.0") | ||
| elseif(spdlog_VERSION VERSION_GREATER_EQUAL "1.12.0") | ||
| set(FMT_MIN_VERSION "10.0.0") | ||
| elseif(spdlog_VERSION VERSION_GREATER_EQUAL "1.11.0") | ||
| set(FMT_MIN_VERSION "9.0.0") | ||
| elseif(spdlog_VERSION VERSION_GREATER_EQUAL "1.9.0") | ||
| set(FMT_MIN_VERSION "8.0.0") | ||
| else() | ||
| message(FATAL_ERROR "Unsupported spdlog version (${spdlog_VERSION}). Minimum supported version is 1.9.0.") | ||
| endif() |
There was a problem hiding this comment.
Due to the reasons explained below, this is unnecessary.
| if(fmt_FOUND) | ||
| # Determine the upper bound version compatibility of fmt w.r.t to spdlog | ||
| if(spdlog_VERSION VERSION_GREATER_EQUAL "1.15.0") | ||
| # Empty block, no upper version limit |
There was a problem hiding this comment.
I think we should be conservative and assume that spdlog v1.15.x will only be compatible with fmt v11.
| if (fmt_VERSION VERSION_GREATER_EQUAL "9.0.0") | ||
| message(FATAL_ERROR "Unsupported fmt version (${fmt_VERSION}). Support version: 8.0.0 <= version < 9.0.0") | ||
| endif() | ||
| endif() |
There was a problem hiding this comment.
We need a default case that prints an error.
| # Print the selected versions for debugging | ||
| message(STATUS "Using spdlog version ${spdlog_VERSION}") | ||
| message(STATUS "Using fmt version ${fmt_VERSION}") |
There was a problem hiding this comment.
These are redundant since we're already printing "Found Xxx" for each library above.
Description
During cmake initialization, we can see that incompatible fmt version 10.1.0 is found along with spdlog version 1.9.2:
When we try to compile CLP, of course we get compilation error:
This PR modifies the CMake file to resolve compatible version of spdlog and fmtlib.
Validation performed
Modified CMake file and fix compilation error.
Summary by CodeRabbit
New Features
spdlogandfmt.spdlog, allowing user overrides.Bug Fixes
Documentation
spdlogandfmtfor easier debugging.