-
Notifications
You must be signed in to change notification settings - Fork 7
build(cmake): Allow ystdlib-cpp to be imported into other CMake projects as a library #65
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
base: main
Are you sure you want to change the base?
Changes from all commits
dec0712
4aa941c
662e768
3fbc72e
93c4d50
98e1c54
0a88c49
e4dc708
a3bc9d5
d80cf86
601c462
2e54cdb
f5e2b4b
89f53c1
1d313c3
2f9f30b
a63a4f2
847162f
5e55b58
77fc7ea
521c1f9
1c44077
c8a3c93
ef48733
f62ee82
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
# gersemi: off | ||
# this file is preprocessed and should not be linted like a CMake file | ||
include(CMakeFindDependencyMacro) | ||
|
||
@PACKAGE_INIT@ | ||
|
||
find_dependency(Boost REQUIRED) | ||
|
||
set_and_check(ystdlib_INCLUDE_DIR "@PACKAGE_ystdlib_INSTALL_INCLUDE_DIR@") | ||
|
||
check_required_components(ystdlib) | ||
|
||
# Avoid repeatedly including the targets | ||
if(NOT TARGET ystdlib::ystdlib) | ||
include("${CMAKE_CURRENT_LIST_DIR}/ystdlib-targets.cmake") | ||
endif() | ||
# gersemi: on |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,33 +1,35 @@ | ||
cmake_minimum_required(VERSION 3.22.1) | ||
cmake_minimum_required(VERSION 3.23) | ||
|
||
list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_LIST_DIR}/CMake") | ||
include(ystdlib-cpp-helpers) | ||
set(ystdlib_VERSION "0.0.1" CACHE STRING "Project version.") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Ensure consistent variable naming prefix. 🤖 Prompt for AI Agents
|
||
|
||
project(YSTDLIB_CPP LANGUAGES CXX) | ||
project(ystdlib VERSION "${ystdlib_VERSION}" LANGUAGES CXX) | ||
|
||
set(YSTDLIB_CPP_VERSION "0.0.1" CACHE STRING "Project version.") | ||
list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_LIST_DIR}/CMake") | ||
include(ystdlib-cpp-helpers) | ||
include(CMakePackageConfigHelpers) | ||
include(GNUInstallDirs) | ||
|
||
option(BUILD_SHARED_LIBS "Build using shared libraries." OFF) | ||
option(YSTDLIB_CPP_BUILD_TESTING "Build the testing tree for ystdlib-cpp." ON) | ||
option(ystdlib_BUILD_TESTING "Build the testing tree for ystdlib-cpp." ON) | ||
|
||
# Require compiler versions that support the C++20 features necessary for compiling ystdlib-cpp | ||
if("AppleClang" STREQUAL "${CMAKE_CXX_COMPILER_ID}") | ||
set(YSTDLIB_CPP_CMAKE_CXX_COMPILER_MIN_VERSION "16") | ||
set(ystdlib_CMAKE_CXX_COMPILER_MIN_VERSION "16") | ||
elseif("Clang" STREQUAL "${CMAKE_CXX_COMPILER_ID}") | ||
set(YSTDLIB_CPP_CMAKE_CXX_COMPILER_MIN_VERSION "16") | ||
set(ystdlib_CMAKE_CXX_COMPILER_MIN_VERSION "16") | ||
elseif("GNU" STREQUAL "${CMAKE_CXX_COMPILER_ID}") | ||
set(YSTDLIB_CPP_CMAKE_CXX_COMPILER_MIN_VERSION "11") | ||
set(ystdlib_CMAKE_CXX_COMPILER_MIN_VERSION "11") | ||
Comment on lines
+17
to
+21
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) DRY up compiler version logic. 🤖 Prompt for AI Agents
|
||
else() | ||
message( | ||
FATAL_ERROR | ||
"Unsupported compiler: ${CMAKE_CXX_COMPILER_ID}. Please use AppleClang, Clang, or GNU." | ||
) | ||
endif() | ||
if("${CMAKE_CXX_COMPILER_VERSION}" VERSION_LESS "${YSTDLIB_CPP_CMAKE_CXX_COMPILER_MIN_VERSION}") | ||
if("${CMAKE_CXX_COMPILER_VERSION}" VERSION_LESS "${ystdlib_CMAKE_CXX_COMPILER_MIN_VERSION}") | ||
message( | ||
FATAL_ERROR | ||
"${CMAKE_CXX_COMPILER_ID} version ${CMAKE_CXX_COMPILER_VERSION} is too low. Must be at \ | ||
least ${YSTDLIB_CPP_CMAKE_CXX_COMPILER_MIN_VERSION}." | ||
least ${ystdlib_CMAKE_CXX_COMPILER_MIN_VERSION}." | ||
Comment on lines
+28
to
+32
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Refine multiline error message. message(FATAL_ERROR
"${CMAKE_CXX_COMPILER_ID} version ${CMAKE_CXX_COMPILER_VERSION} is too low; at least ${ystdlib_CMAKE_CXX_COMPILER_MIN_VERSION} is required."
) 🤖 Prompt for AI Agents
|
||
) | ||
endif() | ||
|
||
|
@@ -39,7 +41,7 @@ set(CMAKE_EXPORT_COMPILE_COMMANDS | |
FORCE | ||
) | ||
|
||
if(YSTDLIB_CPP_IS_TOP_LEVEL) | ||
if(ystdlib_IS_TOP_LEVEL) | ||
# Include dependency settings if the project isn't being included as a subproject. | ||
# NOTE: We mark the file optional because if the user happens to have the dependencies | ||
# installed, this file is not necessary. | ||
|
@@ -49,18 +51,16 @@ if(YSTDLIB_CPP_IS_TOP_LEVEL) | |
include(CTest) | ||
endif() | ||
|
||
if(BUILD_TESTING AND YSTDLIB_CPP_BUILD_TESTING) | ||
set(YSTDLIB_CPP_ENABLE_TESTS ON) | ||
if(BUILD_TESTING AND ystdlib_BUILD_TESTING) | ||
set(ystdlib_ENABLE_TESTS ON) | ||
endif() | ||
|
||
find_package(outcome REQUIRED) | ||
if(outcome_FOUND) | ||
message(STATUS "Found outcome.") | ||
else() | ||
message(FATAL_ERROR "Could not find libraries for outcome.") | ||
find_package(Boost REQUIRED) | ||
if(Boost_FOUND) | ||
message(STATUS "Found Boost ${Boost_VERSION}.") | ||
Comment on lines
+58
to
+60
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Specify Boost components explicitly. find_package(Boost REQUIRED COMPONENTS headers) or list specific components to ensure consistency. 🤖 Prompt for AI Agents
|
||
endif() | ||
Comment on lines
+58
to
61
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainVerify Boost dependency scope and completeness. The switch from outcome to Boost appears incomplete. Based on past review comments, there should be proper linkage to Boost targets for downstream consumers. Let me verify the current dependency setup and ensure proper target linkage: 🏁 Script executed: #!/bin/bash
# Check for Boost target linkage in library targets
echo "=== Checking for Boost target linkage in ystdlib libraries ==="
find src -name "CMakeLists.txt" -exec grep -l "Boost::" {} \;
echo -e "\n=== Checking specific Boost targets used ==="
rg "Boost::" src/ -A 2 -B 2
echo -e "\n=== Checking if main ystdlib target links to Boost ==="
rg "target_link_libraries.*ystdlib" CMakeLists.txt -A 5 -B 2 Length of output: 882 Ensure main We found that only • In CMakeLists.txt (lines 58–61), after target_link_libraries(ystdlib PUBLIC Boost::headers) (or include any other Boost components in use) • Confirm your Suggested diff: find_package(Boost REQUIRED)
if(Boost_FOUND)
message(STATUS "Found Boost ${Boost_VERSION}.")
endif()
+target_link_libraries(ystdlib PUBLIC Boost::headers) 🤖 Prompt for AI Agents
|
||
|
||
if(YSTDLIB_CPP_ENABLE_TESTS) | ||
if(ystdlib_ENABLE_TESTS) | ||
find_package(Catch2 3.8.0 REQUIRED) | ||
if(Catch2_FOUND) | ||
message(STATUS "Found Catch2 ${Catch2_VERSION}.") | ||
|
@@ -84,4 +84,50 @@ if(YSTDLIB_CPP_ENABLE_TESTS) | |
catch_discover_tests(${UNIFIED_UNIT_TEST_TARGET} WORKING_DIRECTORY ${CMAKE_BINARY_DIR}/testbin) | ||
endif() | ||
|
||
set(ystdlib_INSTALL_CONFIG_DIR ${CMAKE_INSTALL_LIBDIR}/cmake/ystdlib) | ||
set(ystdlib_INSTALL_INCLUDE_DIR ${CMAKE_INSTALL_INCLUDEDIR}) | ||
Comment on lines
+87
to
+88
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Install directory variables need prefix. 🤖 Prompt for AI Agents
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe we are no longer using project prefix uppercase style. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed:
Length of output: 5224 @PacificViking You're absolutely correct! The codebase search confirms that the project has indeed standardized on lowercase
The variables ✏️ Learnings added
⛔ Learnings removed
🧠 Learnings used
|
||
|
||
add_subdirectory(src/ystdlib) | ||
|
||
set(ystdlib_targets | ||
containers | ||
error_handling | ||
io_interface | ||
wrapped_facade_headers) | ||
|
||
foreach(install_target_name IN LISTS ystdlib_targets) | ||
|
||
install(TARGETS | ||
${install_target_name} | ||
EXPORT ystdlib-targets | ||
LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR} | ||
ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR} | ||
RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR} | ||
FILE_SET HEADERS | ||
) | ||
|
||
endforeach() | ||
|
||
|
||
install(EXPORT ystdlib-targets NAMESPACE ystdlib:: DESTINATION ${ystdlib_INSTALL_CONFIG_DIR}) | ||
|
||
configure_package_config_file( | ||
${CMAKE_CURRENT_LIST_DIR}/CMake/ystdlib-config.cmake.in | ||
${CMAKE_CURRENT_BINARY_DIR}/ystdlib-config.cmake | ||
INSTALL_DESTINATION ${ystdlib_INSTALL_CONFIG_DIR} | ||
PATH_VARS | ||
ystdlib_INSTALL_INCLUDE_DIR | ||
) | ||
Comment on lines
+114
to
+120
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Include all relevant path variables. 🤖 Prompt for AI Agents
|
||
|
||
Comment on lines
+114
to
+121
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Include config dir in 🤖 Prompt for AI Agents
|
||
write_basic_package_version_file( | ||
${CMAKE_CURRENT_BINARY_DIR}/ystdlib-config-version.cmake | ||
COMPATIBILITY SameMajorVersion | ||
) | ||
|
||
install( | ||
FILES | ||
${CMAKE_CURRENT_BINARY_DIR}/ystdlib-config.cmake | ||
${CMAKE_CURRENT_BINARY_DIR}/ystdlib-config-version.cmake | ||
DESTINATION ${ystdlib_INSTALL_CONFIG_DIR} | ||
) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,10 +4,13 @@ An open-source C++ library developed and used at YScope. | |
# Usage | ||
|
||
## Via CMake's add_subdirectory() | ||
> Note: this is not accurate in this branch, which is a work in progress to use ystdlib-cpp as a | ||
CMake library | ||
Comment on lines
+7
to
+8
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Clarify library import note 🤖 Prompt for AI Agents
|
||
|
||
Clone `ystdlib-cpp` into your project. Then, in your project's `CMakeLists.txt`, add the following: | ||
```cmake | ||
Comment on lines
10
to
11
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Surround fenced code blocks with blank lines 🧰 Tools🪛 markdownlint-cli2 (0.17.2)11-11: Fenced code blocks should be surrounded by blank lines (MD031, blanks-around-fences) 🤖 Prompt for AI Agents
|
||
# Set `YSTDLIB_CPP_BUILD_TESTING` to an accepted `FALSE` class value to skip building unit tests. | ||
# option(YSTDLIB_CPP_BUILD_TESTING "" OFF) | ||
# Set `ystdlib_BUILD_TESTING` to an accepted `FALSE` class value to skip building unit tests. | ||
# option(ystdlib_BUILD_TESTING "" OFF) | ||
add_subdirectory(/path/to/ystdlib-cpp EXCLUDE_FROM_ALL) | ||
target_link_libraries(<target_name> <link_options> | ||
ystdlib::<lib_1> ystdlib::<lib_2> ... ystdlib::<lib_N> | ||
|
@@ -64,7 +67,7 @@ task test-<lib_name> | |
``` | ||
|
||
When generating a testing target, the CMake variable `BUILD_TESTING` is followed (unless overruled | ||
by setting `YSTDLIB_CPP_BUILD_TESTING` to false). By default, if built as a top-level project, | ||
by setting `ystdlib_BUILD_TESTING` to false). By default, if built as a top-level project, | ||
`BUILD_TESTING` is set to true and unit tests are built. | ||
|
||
## Linting | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,30 @@ | ||||||
cmake_minimum_required(VERSION 3.22.1) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Consider updating CMake version requirement for consistency. The examples project requires CMake 3.22.1, but the main project now requires 3.23. For consistency and to ensure all features work properly, consider updating to match the main project requirement. -cmake_minimum_required(VERSION 3.22.1)
+cmake_minimum_required(VERSION 3.23) 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||
|
||||||
project(ystdlib-examples) | ||||||
|
||||||
include("${CMAKE_SOURCE_DIR}/../build/deps/cmake-settings/settings.cmake" REQUIRED) | ||||||
|
||||||
if(NOT DEFINED ystdlib_ROOT) | ||||||
set(ystdlib_ROOT "${CMAKE_SOURCE_DIR}/../build/build_install") | ||||||
endif() | ||||||
find_package(ystdlib REQUIRED) | ||||||
|
||||||
add_library( | ||||||
common | ||||||
OBJECT | ||||||
types.cpp | ||||||
types.hpp | ||||||
constants.hpp | ||||||
) | ||||||
target_link_libraries(common PRIVATE ystdlib::error_handling) | ||||||
add_executable(main main.cpp) | ||||||
|
||||||
target_compile_features(main PRIVATE cxx_std_20) | ||||||
target_compile_features(common PRIVATE cxx_std_20) | ||||||
|
||||||
target_link_libraries( | ||||||
main | ||||||
PRIVATE | ||||||
ystdlib::error_handling | ||||||
common | ||||||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
cmake .. | ||
|
||
Run above command from examples/build | ||
|
||
Dependencies: | ||
|
||
After building main project, cd to build and run | ||
|
||
cmake --install . --prefix=build_install to have an install location |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
#ifndef EXAMPLE_CONSTANTS_HPP | ||
#define EXAMPLE_CONSTANTS_HPP | ||
|
||
#include <array> | ||
#include <string_view> | ||
#include <system_error> | ||
|
||
namespace ystdlib::error_handling::test { | ||
constexpr std::string_view cAlwaysSuccessErrorCategoryName{"Always Success Error Code"}; | ||
constexpr std::string_view cBinaryTestErrorCategoryName{"Binary Error Code"}; | ||
constexpr std::string_view cSuccessErrorMsg{"Success"}; | ||
constexpr std::string_view cFailureErrorMsg{"Failure"}; | ||
constexpr std::string_view cUnrecognizedErrorCode{"Unrecognized Error Code"}; | ||
constexpr std::array cFailureConditions{std::errc::not_connected, std::errc::timed_out}; | ||
constexpr std::array cNoneFailureConditions{std::errc::broken_pipe, std::errc::address_in_use}; | ||
} // namespace ystdlib::error_handling::test | ||
|
||
#endif // EXAMPLE_CONSTANTS_HPP |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
#include <algorithm> | ||
#include <string_view> | ||
#include <system_error> | ||
|
||
#include <ystdlib/error_handling/ErrorCode.hpp> | ||
|
||
#include "constants.hpp" | ||
#include "types.hpp" | ||
|
||
#include <cassert> | ||
#include <iostream> | ||
|
||
using namespace ystdlib::error_handling::test; | ||
|
||
int main() { | ||
// Test error codes within the same error category | ||
BinaryErrorCode const success{BinaryErrorCodeEnum::Success}; | ||
std::error_code const success_error_code{success}; | ||
assert((success == success_error_code)); | ||
assert((cSuccessErrorMsg == success_error_code.message())); | ||
assert((BinaryErrorCode::get_category() == success_error_code.category())); | ||
assert((cBinaryTestErrorCategoryName == success_error_code.category().name())); | ||
|
||
BinaryErrorCode const failure{BinaryErrorCodeEnum::Failure}; | ||
std::error_code const failure_error_code{failure}; | ||
assert((failure == failure_error_code)); | ||
assert((cFailureErrorMsg == failure_error_code.message())); | ||
assert((BinaryErrorCode::get_category() == failure_error_code.category())); | ||
assert((cBinaryTestErrorCategoryName == failure_error_code.category().name())); | ||
|
||
assert((success_error_code != failure_error_code)); | ||
assert((success_error_code.category() == failure_error_code.category())); | ||
|
||
AlwaysSuccessErrorCode const always_success{AlwaysSuccessErrorCodeEnum::Success}; | ||
std::error_code const always_success_error_code{always_success}; | ||
assert((always_success_error_code == always_success)); | ||
assert((cSuccessErrorMsg == always_success_error_code.message())); | ||
assert((AlwaysSuccessErrorCode::get_category() == always_success_error_code.category())); | ||
assert((cAlwaysSuccessErrorCategoryName == always_success_error_code.category().name())); | ||
|
||
// Compare error codes from different error category | ||
// Error codes that have the same value or message won't be the same with each other if they are | ||
// from different error categories. | ||
assert((success_error_code.value() == always_success_error_code.value())); | ||
assert((success_error_code.message() == always_success_error_code.message())); | ||
assert((success_error_code.category() != always_success_error_code.category())); | ||
assert((success_error_code != always_success_error_code)); | ||
assert((AlwaysSuccessErrorCode{AlwaysSuccessErrorCodeEnum::Success} != success_error_code)); | ||
assert((BinaryErrorCode{BinaryErrorCodeEnum::Success} != always_success_error_code)); | ||
|
||
std::cout << "test is good!" << std::endl; | ||
return 0; | ||
} |
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.
🧹 Nitpick (assertive)
Include CMakePackageConfigHelpers for config helpers
To ensure the macros
set_and_check
andcheck_required_components
are available, addimmediately after
include(CMakeFindDependencyMacro)
, or verify that@PACKAGE_INIT@
injects it.🤖 Prompt for AI Agents