chore(core): lock exact version of spdlog and fmtlib transitive dependency #621
+49
−14
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -105,37 +105,72 @@ else() | |
| message(FATAL_ERROR "Could not find ${CLP_LIBS_STRING} libraries for Boost") | ||
| endif() | ||
|
|
||
| # Find and setup spdlog and fmt transitive dependency | ||
|
|
||
| # 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 |
||
| 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 ${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.") | ||
| else() | ||
| message(FATAL_ERROR "Could not find libraries for spdlog.") | ||
| endif() | ||
| endif() | ||
|
Comment on lines
122
to
129
There was a problem hiding this comment. Because |
||
|
|
||
| # 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() | ||
|
Comment on lines
+131
to
+142
There was a problem hiding this comment. Due to the reasons explained below, this is unnecessary. |
||
|
|
||
| # Find fmt with the compatible version | ||
| # NOTE: | ||
| # - We only try to link to the static library | ||
| find_package(fmt ${FMT_MIN_VERSION} REQUIRED) | ||
kirkrodrigues marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. If we specify a version of fmt in the |
||
| 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. |
||
| 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() | ||
|
There was a problem hiding this comment. We need a default case that prints an error. |
||
| 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}") | ||
|
Comment on lines
+170
to
+172
There was a problem hiding this comment. These are redundant since we're already printing "Found Xxx" for each library above. |
||
|
|
||
|
Comment on lines
-109
to
+173
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 libarchive | ||
| if(CLP_USE_STATIC_LIBS) | ||
| set(LibArchive_USE_STATIC_LIBS ON) | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?