Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: Add CMake config and libraries #13

Merged
merged 11 commits into from
Oct 30, 2024
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitmodules
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
[submodule "tools/yscope-dev-utils"]
path = tools/yscope-dev-utils
url = https://github.com/y-scope/yscope-dev-utils.git
[submodule "submodules/Catch2"]
path = submodules/Catch2
url = https://github.com/catchorg/Catch2.git
Comment on lines +4 to +6
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Submodule initialization needs attention

The Catch2 submodule is present but appears to be missing its integration:

  • The submodule is initialized but not referenced in CMake files
  • No test files currently use the Catch2 framework
  • The submodule commit hash differs from the initialized state

Please ensure:

  • Add proper CMake integration for Catch2
  • Create test files that utilize the framework
  • Run git submodule update --init to sync the submodule
🔗 Analysis chain

LGTM! The Catch2 submodule is properly configured.

The addition follows Git submodule best practices with proper path organization and secure URL protocol.

Let's verify the submodule initialization:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify Catch2 submodule initialization and version

# Test: Check if submodule is properly initialized
git submodule status submodules/Catch2

# Test: Verify Catch2 version matches the one mentioned in AI summary
cd submodules/Catch2 2>/dev/null && git rev-parse HEAD

Length of output: 185


Script:

#!/bin/bash
# Let's verify if the submodule is properly referenced in CMake files
# since the PR objectives mention adding CMake config

# Search for Catch2 references in CMake files
fd -e cmake -e txt --exec rg -l "Catch2" {}

# Check if there's a proper add_subdirectory for Catch2
rg "add_subdirectory.*Catch2" -A 2 -B 2

# Look for test files that might use Catch2
fd -e cpp -e h --exec rg -l "catch2|CATCH2" {}

Length of output: 138

96 changes: 91 additions & 5 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,96 @@ if(NOT CMAKE_BUILD_TYPE)
)
endif()

add_executable(spider)
target_compile_features(spider PRIVATE cxx_std_20)
# Add local CMake module directory to CMake's modules path
set(CMAKE_MODULE_PATH
${CMAKE_MODULE_PATH}
"${CMAKE_SOURCE_DIR}/cmake/Modules/"
)

# Macro providing the length of the absolute source directory path so we can
# create a relative (rather than absolute) __FILE__ macro
string(LENGTH "${CMAKE_SOURCE_DIR}/" SOURCE_PATH_SIZE)
add_definitions("-DSOURCE_PATH_SIZE=${SOURCE_PATH_SIZE}")

# Profiling options
add_definitions(-DPROF_ENABLED=0)

# Compile-in debug logging statements
#add_definitions(-DSPDLOG_ACTIVE_LEVEL=SPDLOG_LEVEL_DEBUG)

Comment on lines +43 to +48
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Convert hard-coded options to CMake variables

Consider making these debug and profiling options configurable through CMake variables instead of hard-coding them.

-# Profiling options
-add_definitions(-DPROF_ENABLED=0)
+# Profiling options (default: OFF)
+option(SPIDER_ENABLE_PROFILING "Enable profiling" OFF)
+add_definitions(-DPROF_ENABLED=$<BOOL:${SPIDER_ENABLE_PROFILING}>)

-# Compile-in debug logging statements
-#add_definitions(-DSPDLOG_ACTIVE_LEVEL=SPDLOG_LEVEL_DEBUG)
+# Debug logging (default: OFF)
+option(SPIDER_ENABLE_DEBUG_LOGGING "Enable debug logging" OFF)
+if(SPIDER_ENABLE_DEBUG_LOGGING)
+    add_definitions(-DSPDLOG_ACTIVE_LEVEL=SPDLOG_LEVEL_DEBUG)
+endif()
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Profiling options
add_definitions(-DPROF_ENABLED=0)
# Compile-in debug logging statements
#add_definitions(-DSPDLOG_ACTIVE_LEVEL=SPDLOG_LEVEL_DEBUG)
# Profiling options (default: OFF)
option(SPIDER_ENABLE_PROFILING "Enable profiling" OFF)
add_definitions(-DPROF_ENABLED=$<BOOL:${SPIDER_ENABLE_PROFILING}>)
# Debug logging (default: OFF)
option(SPIDER_ENABLE_DEBUG_LOGGING "Enable debug logging" OFF)
if(SPIDER_ENABLE_DEBUG_LOGGING)
add_definitions(-DSPDLOG_ACTIVE_LEVEL=SPDLOG_LEVEL_DEBUG)
endif()

# Flush to disk switch
add_definitions(-DFLUSH_TO_DISK_ENABLED=1)

# Make off_t 64-bit
add_definitions(-D_FILE_OFFSET_BITS=64)

# Detect linking mode (static or shared); Default to static.
set(SPIDER_USE_STATIC_LIBS ON CACHE BOOL "Whether to link against static libraries")
if(SPIDER_USE_STATIC_LIBS)
if(APPLE)
set(SPIDER_STATIC_LIBS_UNSUPPORTED_PLATFORM "macOS")
elseif(EXISTS "/etc/centos-release")
set(SPIDER_STATIC_LIBS_UNSUPPORTED_PLATFORM "CentOS")
endif()

if(DEFINED SPIDER_STATIC_LIBS_UNSUPPORTED_PLATFORM)
message(
AUTHOR_WARNING
"Building with static libraries is unsupported on"
" ${SPIDER_STATIC_LIBS_UNSUPPORTED_PLATFORM}. Switching to shared libraries."
)
set(SPIDER_USE_STATIC_LIBS OFF)
endif()
endif()
if(SPIDER_USE_STATIC_LIBS)
set(SPIDER_LIBS_STRING "static")
else()
set(SPIDER_LIBS_STRING "shared")
endif()
message(STATUS "Building using ${SPIDER_LIBS_STRING} libraries")

# Find and setup fmt
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
if(SPIDER_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()
set(SPDLOG_FMT_EXTERNAL ON)
find_package(spdlog 1.9.2 REQUIRED)
if(spdlog_FOUND)
message(STATUS "Found spdlog ${spdlog_VERSION}")
else()
if(SPIDER_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()

# Find and setup MariaDBClient library
if(CLP_USE_STATIC_LIBS)
# NOTE: We can't statically link to MariaDBClient since it's GPL
message(AUTHOR_WARNING "MariaDBClient cannot be statically linked due to its license.")
endif()
find_package(MariaDBClient 3.1.0 REQUIRED)
if(MariaDBClient_FOUND)
message(STATUS "Found MariaDBClient ${MariaDBClient_VERSION}")
else()
message(FATAL_ERROR "Could not find ${CLP_LIBS_STRING} libraries for MariaDBClient")
endif()
Comment on lines +109 to +118
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix incorrect variable name in MariaDBClient configuration

There's a variable name inconsistency that needs to be corrected.

-if(CLP_USE_STATIC_LIBS)
+if(SPIDER_USE_STATIC_LIBS)
     # NOTE: We can't statically link to MariaDBClient since it's GPL
     message(AUTHOR_WARNING "MariaDBClient cannot be statically linked due to its license.")
 endif()
 find_package(MariaDBClient 3.1.0 REQUIRED)
 if(MariaDBClient_FOUND)
     message(STATUS "Found MariaDBClient ${MariaDBClient_VERSION}")
 else()
-    message(FATAL_ERROR "Could not find ${CLP_LIBS_STRING} libraries for MariaDBClient")
+    message(FATAL_ERROR "Could not find ${SPIDER_LIBS_STRING} libraries for MariaDBClient")
 endif()
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if(CLP_USE_STATIC_LIBS)
# NOTE: We can't statically link to MariaDBClient since it's GPL
message(AUTHOR_WARNING "MariaDBClient cannot be statically linked due to its license.")
endif()
find_package(MariaDBClient 3.1.0 REQUIRED)
if(MariaDBClient_FOUND)
message(STATUS "Found MariaDBClient ${MariaDBClient_VERSION}")
else()
message(FATAL_ERROR "Could not find ${CLP_LIBS_STRING} libraries for MariaDBClient")
endif()
if(SPIDER_USE_STATIC_LIBS)
# NOTE: We can't statically link to MariaDBClient since it's GPL
message(AUTHOR_WARNING "MariaDBClient cannot be statically linked due to its license.")
endif()
find_package(MariaDBClient 3.1.0 REQUIRED)
if(MariaDBClient_FOUND)
message(STATUS "Found MariaDBClient ${MariaDBClient_VERSION}")
else()
message(FATAL_ERROR "Could not find ${SPIDER_LIBS_STRING} libraries for MariaDBClient")
endif()


find_package(Threads REQUIRED)

set(SPIDER_SOURCES src/spider/spider.cpp)
target_sources(spider PRIVATE ${SPIDER_SOURCES})
add_subdirectory(src/spider)

target_include_directories(spider PRIVATE src/)
add_subdirectory(tests)
58 changes: 58 additions & 0 deletions cmake/Modules/FindLibraryDependencies.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
# Find the given libraries ignoring libraries that should be dynamically linked
# Params:
# fld_LIBNAME - Name of the library (without the 'lib' prefix)
# fld_PREFIX - Prefix for created variables
# fld_STATIC_LIBS - List of libraries to find
# Returns:
# ${fld_LIBNAME}_DYNAMIC_LIBS - List of libraries that should be dynamically linked
# ${fld_PREFIX}_LIBRARY_DEPENDENCIES - Found libraries
macro(FindStaticLibraryDependencies fld_LIBNAME fld_PREFIX fld_STATIC_LIBS)
# NOTE: libc, libm, libpthread, and librt should be dynamically linked
set(fld_DYNAMIC_LIBS "c;m;pthread;rt")

# Get absolute path of dependent libraries
foreach(fld_DEP_LIB ${fld_STATIC_LIBS})
# Skip dynamic libs
set(fld_IGNORE_LIB FALSE)
foreach(fld_DYNAMIC_LIB ${fld_DYNAMIC_LIBS})
if(${fld_DEP_LIB} STREQUAL ${fld_DYNAMIC_LIB})
set(fld_IGNORE_LIB TRUE)
list(APPEND ${fld_PREFIX}_DYNAMIC_LIBS "${fld_DEP_LIB}")
endif()
endforeach()
if(fld_IGNORE_LIB OR ${fld_DEP_LIB} STREQUAL ${fld_LIBNAME})
continue()
endif()

find_library(${fld_PREFIX}_${fld_DEP_LIB}_LIBRARY
NAMES ${fld_DEP_LIB}
PATH_SUFFIXES lib
)
if(${fld_PREFIX}_${fld_DEP_LIB}_LIBRARY)
list(APPEND ${fld_PREFIX}_LIBRARY_DEPENDENCIES
"${${fld_PREFIX}_${fld_DEP_LIB}_LIBRARY}")
else()
message(SEND_ERROR "Static ${fld_DEP_LIB} library not found")
endif()
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance library discovery robustness

The current implementation has several potential improvements:

  1. Results aren't cached between CMake runs
  2. No handling of system-specific library naming conventions
  3. No verification that found libraries are actually static

Consider these improvements:

         find_library(${fld_PREFIX}_${fld_DEP_LIB}_LIBRARY
             NAMES ${fld_DEP_LIB}
             PATH_SUFFIXES lib
+            # Cache results between runs
+            CACHE PATH "Path to ${fld_DEP_LIB} library"
         )
         if(${fld_PREFIX}_${fld_DEP_LIB}_LIBRARY)
+            # Verify library is static (platform-specific)
+            if(UNIX AND NOT APPLE)
+                execute_process(
+                    COMMAND file ${${fld_PREFIX}_${fld_DEP_LIB}_LIBRARY}
+                    OUTPUT_VARIABLE library_type
+                    OUTPUT_STRIP_TRAILING_WHITESPACE
+                )
+                if(library_type MATCHES "shared")
+                    message(WARNING "${fld_DEP_LIB} found at ${${fld_PREFIX}_${fld_DEP_LIB}_LIBRARY} is not static")
+                endif()
+            endif()
             list(APPEND ${fld_PREFIX}_LIBRARY_DEPENDENCIES
                 "${${fld_PREFIX}_${fld_DEP_LIB}_LIBRARY}")
         else()
-            message(SEND_ERROR "Static ${fld_DEP_LIB} library not found")
+            message(SEND_ERROR "Static ${fld_DEP_LIB} library not found. Searched for: ${fld_DEP_LIB}")
         endif()
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
find_library(${fld_PREFIX}_${fld_DEP_LIB}_LIBRARY
NAMES ${fld_DEP_LIB}
PATH_SUFFIXES lib
)
if(${fld_PREFIX}_${fld_DEP_LIB}_LIBRARY)
list(APPEND ${fld_PREFIX}_LIBRARY_DEPENDENCIES
"${${fld_PREFIX}_${fld_DEP_LIB}_LIBRARY}")
else()
message(SEND_ERROR "Static ${fld_DEP_LIB} library not found")
endif()
find_library(${fld_PREFIX}_${fld_DEP_LIB}_LIBRARY
NAMES ${fld_DEP_LIB}
PATH_SUFFIXES lib
# Cache results between runs
CACHE PATH "Path to ${fld_DEP_LIB} library"
)
if(${fld_PREFIX}_${fld_DEP_LIB}_LIBRARY)
# Verify library is static (platform-specific)
if(UNIX AND NOT APPLE)
execute_process(
COMMAND file ${${fld_PREFIX}_${fld_DEP_LIB}_LIBRARY}
OUTPUT_VARIABLE library_type
OUTPUT_STRIP_TRAILING_WHITESPACE
)
if(library_type MATCHES "shared")
message(WARNING "${fld_DEP_LIB} found at ${${fld_PREFIX}_${fld_DEP_LIB}_LIBRARY} is not static")
endif()
endif()
list(APPEND ${fld_PREFIX}_LIBRARY_DEPENDENCIES
"${${fld_PREFIX}_${fld_DEP_LIB}_LIBRARY}")
else()
message(SEND_ERROR "Static ${fld_DEP_LIB} library not found. Searched for: ${fld_DEP_LIB}")
endif()

endforeach()
endmacro()

# Find the given libraries
# Params:
# fld_PREFIX - Prefix for created variables
# fld_DYNAMIC_LIBS - List of libraries to find
# Returns:
# ${fld_PREFIX}_LIBRARY_DEPENDENCIES - Found libraries
macro(FindDynamicLibraryDependencies fld_PREFIX fld_DYNAMIC_LIBS)
foreach(fld_DEP_LIB ${fld_DYNAMIC_LIBS})
find_library(${fld_PREFIX}_${fld_DEP_LIB}_LIBRARY
NAMES ${fld_DEP_LIB}
PATH_SUFFIXES lib
)
if(${fld_PREFIX}_${fld_DEP_LIB}_LIBRARY)
list(APPEND ${fld_PREFIX}_LIBRARY_DEPENDENCIES "${${fld_PREFIX}_${fld_DEP_LIB}_LIBRARY}")
else()
message(SEND_ERROR "${fld_DEP_LIB} library not found")
endif()
endforeach()
endmacro()
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Apply similar robustness improvements to dynamic library finder

For consistency with the static library finder, consider implementing similar improvements:

  1. Cache find_library results
  2. Add optional verification of library type
  3. Enhance error messages
     foreach(fld_DEP_LIB ${fld_DYNAMIC_LIBS})
         find_library(${fld_PREFIX}_${fld_DEP_LIB}_LIBRARY
             NAMES ${fld_DEP_LIB}
             PATH_SUFFIXES lib
+            CACHE PATH "Path to ${fld_DEP_LIB} library"
         )
         if(${fld_PREFIX}_${fld_DEP_LIB}_LIBRARY)
+            # Optionally verify library is dynamic (platform-specific)
+            if(UNIX AND NOT APPLE AND SPIDER_VERIFY_LIBRARY_TYPE)
+                execute_process(
+                    COMMAND file ${${fld_PREFIX}_${fld_DEP_LIB}_LIBRARY}
+                    OUTPUT_VARIABLE library_type
+                    OUTPUT_STRIP_TRAILING_WHITESPACE
+                )
+                if(NOT library_type MATCHES "shared")
+                    message(WARNING "${fld_DEP_LIB} found at ${${fld_PREFIX}_${fld_DEP_LIB}_LIBRARY} might not be dynamic")
+                endif()
+            endif()
             list(APPEND ${fld_PREFIX}_LIBRARY_DEPENDENCIES "${${fld_PREFIX}_${fld_DEP_LIB}_LIBRARY}")
         else()
-            message(SEND_ERROR "${fld_DEP_LIB} library not found")
+            message(SEND_ERROR "${fld_DEP_LIB} library not found. Searched for: ${fld_DEP_LIB}")
         endif()
     endforeach()

Committable suggestion was skipped due to low confidence.

105 changes: 105 additions & 0 deletions cmake/Modules/FindMariaDBClient.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
# Try to find MariaDBClient
# NOTE: The FindMariaDBClient.cmake included with CMake has no support for static libraries, so we use our own.
#
# Set MariaDBClient_USE_STATIC_LIBS=ON to look for static libraries.
#
# Once done this will define:
# MariaDBClient_FOUND - Whether MariaDBClient was found on the system
# MariaDBClient_INCLUDE_DIR - The MariaDBClient include directories
# MariaDBClient_VERSION - The version of MariaDBClient installed on the system
#
# Conventions:
# - Variables only for use within the script are prefixed with "mariadbclient_"
# - Variables that should be externally visible are prefixed with "MariaDBClient_"

set(mariadbclient_LIBNAME "mariadb")

include(cmake/Modules/FindLibraryDependencies.cmake)

# Run pkg-config
find_package(PkgConfig)
pkg_check_modules(mariadbclient_PKGCONF QUIET "lib${mariadbclient_LIBNAME}")

# Set include directory
find_path(MariaDBClient_INCLUDE_DIR mysql.h
HINTS ${mariadbclient_PKGCONF_INCLUDEDIR}
PATH_SUFFIXES mariadb
)

# Handle static libraries
if(MariaDBClient_USE_STATIC_LIBS)
# Save current value of CMAKE_FIND_LIBRARY_SUFFIXES
set(mariadbclient_ORIG_CMAKE_FIND_LIBRARY_SUFFIXES ${CMAKE_FIND_LIBRARY_SUFFIXES})

# Temporarily change CMAKE_FIND_LIBRARY_SUFFIXES to static library suffix
set(CMAKE_FIND_LIBRARY_SUFFIXES .a)
endif()
Comment on lines +38 to +40
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Consider platform-specific static library suffixes

The current implementation only handles .a files, which might not work on all platforms. Consider using CMAKE_STATIC_LIBRARY_SUFFIX instead.

-    set(CMAKE_FIND_LIBRARY_SUFFIXES .a)
+    set(CMAKE_FIND_LIBRARY_SUFFIXES ${CMAKE_STATIC_LIBRARY_SUFFIX})
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Temporarily change CMAKE_FIND_LIBRARY_SUFFIXES to static library suffix
set(CMAKE_FIND_LIBRARY_SUFFIXES .a)
endif()
# Temporarily change CMAKE_FIND_LIBRARY_SUFFIXES to static library suffix
set(CMAKE_FIND_LIBRARY_SUFFIXES ${CMAKE_STATIC_LIBRARY_SUFFIX})
endif()


# Find library
find_library(MariaDBClient_LIBRARY
NAMES ${mariadbclient_LIBNAME}
HINTS ${mariadbclient_PKGCONF_LIBDIR}
PATH_SUFFIXES lib
)
if (MariaDBClient_LIBRARY)
# NOTE: This must be set for find_package_handle_standard_args to work
set(MariaDBClient_FOUND ON)
endif()

if(MariaDBClient_USE_STATIC_LIBS)
FindStaticLibraryDependencies(${mariadbclient_LIBNAME} mariadbclient
"${mariadbclient_PKGCONF_STATIC_LIBRARIES}")

# Restore original value of CMAKE_FIND_LIBRARY_SUFFIXES
set(CMAKE_FIND_LIBRARY_SUFFIXES ${mariadbclient_ORIG_CMAKE_FIND_LIBRARY_SUFFIXES})
unset(mariadbclient_ORIG_CMAKE_FIND_LIBRARY_SUFFIXES)
endif()

FindDynamicLibraryDependencies(mariadbclient "${mariadbclient_DYNAMIC_LIBS}")

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Ensure 'mariadbclient_DYNAMIC_LIBS' is defined before use

The variable mariadbclient_DYNAMIC_LIBS may not be set before being used in FindDynamicLibraryDependencies. Please verify that this variable is correctly defined prior to this call to avoid potential errors.

If mariadbclient_DYNAMIC_LIBS is intended to be populated from pkg-config, ensure it is assigned appropriately. For example:

+set(mariadbclient_DYNAMIC_LIBS ${mariadbclient_PKGCONF_LIBRARIES})
 FindDynamicLibraryDependencies(mariadbclient "${mariadbclient_DYNAMIC_LIBS}")

Committable suggestion was skipped due to low confidence.

# Set version
set(MariaDBClient_VERSION ${mariadbclient_PKGCONF_VERSION})

include(FindPackageHandleStandardArgs)
find_package_handle_standard_args(MariaDBClient
REQUIRED_VARS MariaDBClient_INCLUDE_DIR
VERSION_VAR MariaDBClient_VERSION
)
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add 'MariaDBClient_LIBRARY' to 'REQUIRED_VARS' in 'find_package_handle_standard_args'

Including MariaDBClient_LIBRARY in the REQUIRED_VARS ensures that both the include directory and library are validated during the configuration process. This prevents potential issues if the library is not found.

Apply this diff to include MariaDBClient_LIBRARY as a required variable:

 find_package_handle_standard_args(MariaDBClient
-    REQUIRED_VARS MariaDBClient_INCLUDE_DIR
+    REQUIRED_VARS MariaDBClient_INCLUDE_DIR MariaDBClient_LIBRARY
     VERSION_VAR MariaDBClient_VERSION
 )
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
include(FindPackageHandleStandardArgs)
find_package_handle_standard_args(MariaDBClient
REQUIRED_VARS MariaDBClient_INCLUDE_DIR
VERSION_VAR MariaDBClient_VERSION
)
include(FindPackageHandleStandardArgs)
find_package_handle_standard_args(MariaDBClient
REQUIRED_VARS MariaDBClient_INCLUDE_DIR MariaDBClient_LIBRARY
VERSION_VAR MariaDBClient_VERSION
)


if(NOT TARGET MariaDBClient::MariaDBClient)
# Add library to build
if (MariaDBClient_FOUND)
if (MariaDBClient_USE_STATIC_LIBS)
add_library(MariaDBClient::MariaDBClient STATIC IMPORTED)
else()
# NOTE: We use UNKNOWN so that if the user doesn't have the SHARED
# libraries installed, we can still use the STATIC libraries
add_library(MariaDBClient::MariaDBClient UNKNOWN IMPORTED)
endif()
endif()

# Set include directories for library
if(MariaDBClient_INCLUDE_DIR)
set_target_properties(MariaDBClient::MariaDBClient
PROPERTIES
INTERFACE_INCLUDE_DIRECTORIES "${MariaDBClient_INCLUDE_DIR}"
)
endif()

# Set location of library
if(EXISTS "${MariaDBClient_LIBRARY}")
set_target_properties(MariaDBClient::MariaDBClient
PROPERTIES
IMPORTED_LINK_INTERFACE_LANGUAGES "C"
IMPORTED_LOCATION "${MariaDBClient_LIBRARY}"
)

# Add component's dependencies for linking
if(mariadbclient_LIBRARY_DEPENDENCIES)
set_target_properties(MariaDBClient::MariaDBClient
PROPERTIES
INTERFACE_LINK_LIBRARIES "${mariadbclient_LIBRARY_DEPENDENCIES}"
)
endif()
endif()
endif()
Loading
Loading