From ae97acbd974a2551301531910d89242c0c7e7f60 Mon Sep 17 00:00:00 2001 From: doodspav Date: Fri, 16 Jun 2023 09:12:54 +0200 Subject: [PATCH 01/18] GHI #18 Move cmake helpers out of `utils` subdir --- CMakeLists.txt | 8 ++++---- cmake/{util => }/InstallConfig.cmake | 0 cmake/{util => }/InstallRules.cmake | 4 ++-- cmake/{util => }/OptionVariables.cmake | 0 cmake/{util => }/PreventInSourceBuilds.cmake | 0 cmake/{util => }/ProjectIsTopLevel.cmake | 0 6 files changed, 6 insertions(+), 6 deletions(-) rename cmake/{util => }/InstallConfig.cmake (100%) rename cmake/{util => }/InstallRules.cmake (93%) rename cmake/{util => }/OptionVariables.cmake (100%) rename cmake/{util => }/PreventInSourceBuilds.cmake (100%) rename cmake/{util => }/ProjectIsTopLevel.cmake (100%) diff --git a/CMakeLists.txt b/CMakeLists.txt index 2ec1eb7be..fd59bed58 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,6 +1,6 @@ cmake_minimum_required(VERSION 3.14) -include(cmake/util/PreventInSourceBuilds.cmake) +include(cmake/PreventInSourceBuilds.cmake) project( patomic @@ -11,8 +11,8 @@ project( ) # don't change include order, OptionVariables checks if project is top level -include(cmake/util/ProjectIsTopLevel.cmake) -include(cmake/util/OptionVariables.cmake) +include(cmake/ProjectIsTopLevel.cmake) +include(cmake/OptionVariables.cmake) # ---- Declare Library ---- @@ -77,7 +77,7 @@ target_compile_features(patomic_patomic PUBLIC c_std_90) # ---- Install Rules ---- if(NOT CMAKE_SKIP_INSTALL_RULES) - include(cmake/util/InstallRules.cmake) + include(cmake/InstallRules.cmake) endif() diff --git a/cmake/util/InstallConfig.cmake b/cmake/InstallConfig.cmake similarity index 100% rename from cmake/util/InstallConfig.cmake rename to cmake/InstallConfig.cmake diff --git a/cmake/util/InstallRules.cmake b/cmake/InstallRules.cmake similarity index 93% rename from cmake/util/InstallRules.cmake rename to cmake/InstallRules.cmake index 486f3336c..1df99939c 100644 --- a/cmake/util/InstallRules.cmake +++ b/cmake/InstallRules.cmake @@ -7,7 +7,7 @@ set(package patomic) # copy header files to CMAKE_INSTALL_INCLUDEDIR install( DIRECTORY - "${PROJECT_SOURCE_DIR}/include/" + "../include" "${PROJECT_BINARY_DIR}/include/" DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}" COMPONENT patomic_Development @@ -30,7 +30,7 @@ install( # copy config file for find_package to find install( - FILES "${PROJECT_SOURCE_DIR}/cmake/util/InstallConfig.cmake" + FILES "InstallConfig.cmake" DESTINATION "${PATOMIC_INSTALL_CMAKEDIR}" RENAME "${package}Config.cmake" COMPONENT patomic_Development diff --git a/cmake/util/OptionVariables.cmake b/cmake/OptionVariables.cmake similarity index 100% rename from cmake/util/OptionVariables.cmake rename to cmake/OptionVariables.cmake diff --git a/cmake/util/PreventInSourceBuilds.cmake b/cmake/PreventInSourceBuilds.cmake similarity index 100% rename from cmake/util/PreventInSourceBuilds.cmake rename to cmake/PreventInSourceBuilds.cmake diff --git a/cmake/util/ProjectIsTopLevel.cmake b/cmake/ProjectIsTopLevel.cmake similarity index 100% rename from cmake/util/ProjectIsTopLevel.cmake rename to cmake/ProjectIsTopLevel.cmake From de41e2ebc0309a649ad6de22b65aca089a284785 Mon Sep 17 00:00:00 2001 From: doodspav Date: Sat, 21 Oct 2023 10:21:36 +0100 Subject: [PATCH 02/18] GHI #18 CMake fixes and introduction of `package_name` - introduce `package_name` and `target_name` variables and use these where appropriate (i.e. without confusing reviewers about what values some variables hold) - replace `InstallConfig.cmake` with `in/patomic-config.cmake.in` - only include targets file in config file if target isn't already defined - add summary table in `OptionVariables.cmake` - change PATH cache variables to STRING so that there aren't issues if they're set on the command line - changed installed CMake names to kebab-case --- CMakeLists.txt | 48 +++++++++++++++++++----------- cmake/InstallConfig.cmake | 1 - cmake/InstallRules.cmake | 51 +++++++++++++++++--------------- cmake/OptionVariables.cmake | 48 +++++++++++++++++++++++++----- cmake/in/patomic-config.cmake.in | 12 ++++++++ 5 files changed, 110 insertions(+), 50 deletions(-) delete mode 100644 cmake/InstallConfig.cmake create mode 100644 cmake/in/patomic-config.cmake.in diff --git a/CMakeLists.txt b/CMakeLists.txt index fd59bed58..1103053c1 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -2,8 +2,15 @@ cmake_minimum_required(VERSION 3.14) include(cmake/PreventInSourceBuilds.cmake) + +# ---- Initialize Project ---- + +# used to support find_package +set(package_name "patomic") + +# create base project project( - patomic + ${package_name} VERSION 0.5.1 DESCRIPTION "Portable C90 Atomics Library" HOMEPAGE_URL "https://github.com/doodspav/patomic" @@ -17,29 +24,35 @@ include(cmake/OptionVariables.cmake) # ---- Declare Library ---- -add_library( - patomic_patomic - ${build_type} +# target that we can modify (can't modify ALIAS targets) +# target name should not be the same as ${PROJECT_NAME}, causes add_subdirectory issues +set(target_name "patomic_patomic") +add_library(${target_name} ${build_type}) + +# alias to cause error at configuration time instead of link time if target is missing +add_library(patomic::patomic ALIAS ${target_name}) + +# add sources to target +target_sources( + ${target_name} PRIVATE # include include/patomic/patomic.h # src src/patomic.c ) -# alias to cause error at configuration time instead of link time if target is missing -add_library(patomic::patomic ALIAS patomic_patomic) - # ---- Generate Build Info Headers ---- # used in export header generated below if(NOT PATOMIC_BUILD_SHARED) - target_compile_definitions(patomic_patomic PUBLIC PATOMIC_STATIC_DEFINE) + target_compile_definitions(${target_name} PUBLIC PATOMIC_STATIC_DEFINE) endif() +# generate header file with export macros prefixed with BASE_NAME include(GenerateExportHeader) generate_export_header( - patomic_patomic + ${target_name} BASE_NAME patomic EXPORT_FILE_NAME include/patomic/patomic_export.h ) @@ -47,31 +60,32 @@ generate_export_header( # ---- Library Properties ---- +# hide all symbols by default +# use SameMajorVersion versioning for shared library lookup set_target_properties( - patomic_patomic PROPERTIES + ${target_name} PROPERTIES C_VISIBILITY_PRESET hidden VISIBILITY_INLINES_HIDDEN YES VERSION "${PROJECT_VERSION}" SOVERSION "${PROJECT_VERSION_MAJOR}" - EXPORT_NAME patomic - OUTPUT_NAME patomic + EXPORT_NAME "patomic" + OUTPUT_NAME "patomic" ) # header files generated by CMake target_include_directories( - patomic_patomic SYSTEM - PUBLIC + ${target_name} SYSTEM PUBLIC "$" ) # header files from /include target_include_directories( - patomic_patomic ${warning_guard} - PUBLIC + ${target_name} ${warning_guard} PUBLIC "$" ) -target_compile_features(patomic_patomic PUBLIC c_std_90) +# require C90 compiler support +target_compile_features(${target_name} PUBLIC c_std_90) # ---- Install Rules ---- diff --git a/cmake/InstallConfig.cmake b/cmake/InstallConfig.cmake deleted file mode 100644 index 30d8d5813..000000000 --- a/cmake/InstallConfig.cmake +++ /dev/null @@ -1 +0,0 @@ -include("${CMAKE_CURRENT_LIST_DIR}/patomicTargets.cmake") diff --git a/cmake/InstallRules.cmake b/cmake/InstallRules.cmake index 1df99939c..b245a22a5 100644 --- a/cmake/InstallRules.cmake +++ b/cmake/InstallRules.cmake @@ -1,60 +1,63 @@ include(CMakePackageConfigHelpers) -include(GNUInstallDirs) - -# find_package() call for consumers to find this project -set(package patomic) # copy header files to CMAKE_INSTALL_INCLUDEDIR install( DIRECTORY - "../include" - "${PROJECT_BINARY_DIR}/include/" + "${PROJECT_SOURCE_DIR}/include" # our header files + "${PROJECT_BINARY_DIR}/include" # generated header files DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}" - COMPONENT patomic_Development + COMPONENT ${package_name}-development ) # copy target build output artifacts to OS dependent locations +# (except includes) install( - TARGETS patomic_patomic - EXPORT patomicTargets + TARGETS ${target_name} + EXPORT ${package_name}-targets RUNTIME # - COMPONENT patomic_Runtime + COMPONENT ${package_name}-runtime LIBRARY # - COMPONENT patomic_Runtime - NAMELINK_COMPONENT patomic_Development + COMPONENT ${package_name}-runtime + NAMELINK_COMPONENT ${package_name}-development ARCHIVE # - COMPONENT patomic_Development + COMPONENT ${package_name}-development INCLUDES # DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}" ) +# create config file that points to targets file +configure_file( + "${PROJECT_SOURCE_DIR}/cmake/in/patomic-config.cmake.in" + "${PROJECT_BINARY_DIR}/cmake/${package_name}-config.cmake" + @ONLY +) + # copy config file for find_package to find install( - FILES "InstallConfig.cmake" + FILES "${PROJECT_BINARY_DIR}/cmake/${package_name}-config.cmake" DESTINATION "${PATOMIC_INSTALL_CMAKEDIR}" - RENAME "${package}Config.cmake" - COMPONENT patomic_Development + COMPONENT ${package_name}-development ) # create version file for consumer to check version in CMake write_basic_package_version_file( - "${package}ConfigVersion.cmake" - COMPATIBILITY SameMajorVersion + "${package_name}-config-version.cmake" + COMPATIBILITY SameMajorVersion # a.k.a. SemVer ) -# copy version file for find_package to find +# copy version file for find_package to find for version check install( - FILES "${PROJECT_BINARY_DIR}/${package}ConfigVersion.cmake" + FILES "${PROJECT_BINARY_DIR}/${package_name}-config-version.cmake" DESTINATION "${PATOMIC_INSTALL_CMAKEDIR}" - COMPONENT patomic_Development + COMPONENT ${package_name}-development ) -# create core configuration file detailing targets for consumer +# create targets file included by config file with targets for consumers install( - EXPORT patomicTargets + EXPORT ${package_name}-targets NAMESPACE patomic:: DESTINATION "${PATOMIC_INSTALL_CMAKEDIR}" - COMPONENT patomic_Development + COMPONENT ${package_name}-development ) # support packaging library diff --git a/cmake/OptionVariables.cmake b/cmake/OptionVariables.cmake index 0b5528f08..2e95642d1 100644 --- a/cmake/OptionVariables.cmake +++ b/cmake/OptionVariables.cmake @@ -1,3 +1,22 @@ +include(GNUInstallDirs) + + +# ---- Options Summary ---- + +# ------------------------------------------------------------------------------------------------ +# | Option | Availability | Default | +# |==============================|===============|===============================================| +# | BUILD_SHARED_LIBS | Top-Level | OFF | +# | BUILD_TESTING | Top-Level | OFF | +# | CMAKE_INSTALL_INCLUDEDIR | Top-Level | include/${package_name}-${PROJECT_VERSION} | +# |------------------------------|---------------|-----------------------------------------------| +# | PATOMIC_BUILD_SHARED | Always | ${BUILD_SHARED_LIBS} | +# | PATOMIC_BUILD_TESTING | Always | ${BUILD_TESTING} | +# | PATOMIC_INCLUDES_WITH_SYSTEM | Not Top-Level | ON | +# | PATOMIC_INSTALL_CMAKEDIR | Always | ${CMAKE_INSTALL_LIBDIR}/cmake/${package_name} | +# ------------------------------------------------------------------------------------------------ + + # ---- Build Shared ---- # Sometimes it's useful to be able to single out a dependency to be built as @@ -7,7 +26,7 @@ if(PROJECT_IS_TOP_LEVEL) endif() option( PATOMIC_BUILD_SHARED - "Override BUILD_SHARED_LIBS for patomic library" + "Override BUILD_SHARED_LIBS for ${package_name} library" ${BUILD_SHARED_LIBS} ) mark_as_advanced(PATOMIC_BUILD_SHARED) @@ -27,7 +46,7 @@ set(warning_guard "") if(NOT PROJECT_IS_TOP_LEVEL) option( PATOMIC_INCLUDES_WITH_SYSTEM - "Use SYSTEM modifier for patomic's includes, disabling warnings" + "Use SYSTEM modifier for ${package_name}'s includes, disabling warnings" ON ) mark_as_advanced(PATOMIC_INCLUDES_WITH_SYSTEM) @@ -48,7 +67,7 @@ if(PROJECT_IS_TOP_LEVEL) endif() option( PATOMIC_BUILD_TESTING - "Override BUILD_TESTING for patomic library" + "Override BUILD_TESTING for ${package_name} library" ${BUILD_TESTING} ) mark_as_advanced(PATOMIC_BUILD_TESTING) @@ -57,13 +76,20 @@ mark_as_advanced(PATOMIC_BUILD_TESTING) # ---- Install Include Directory ---- # Adds an extra directory to the include path by default, so that when you link -# against the target, you get `/include/patomic-X.Y.Z` added to your +# against the target, you get `/include/-X.Y.Z` added to your # include paths rather than ` Date: Sat, 21 Oct 2023 11:27:12 +0100 Subject: [PATCH 03/18] GHI #18 Fix cache variable issue with `CMAKE_INSTALL_INCLUDEDIR` - include `GNUInstallDirs` after setting `CMAKE_INSTALL_INCLUDEDIR` cache variable to avoid interfering with pre-existing one (previously changes didn't take effect) - add `/` at the end of the directory paths in `install(...)`, or CMake is unhappy --- cmake/InstallRules.cmake | 6 +++--- cmake/OptionVariables.cmake | 8 +++++++- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/cmake/InstallRules.cmake b/cmake/InstallRules.cmake index b245a22a5..b8c683c91 100644 --- a/cmake/InstallRules.cmake +++ b/cmake/InstallRules.cmake @@ -3,14 +3,14 @@ include(CMakePackageConfigHelpers) # copy header files to CMAKE_INSTALL_INCLUDEDIR install( DIRECTORY - "${PROJECT_SOURCE_DIR}/include" # our header files - "${PROJECT_BINARY_DIR}/include" # generated header files + "${PROJECT_SOURCE_DIR}/include/" # our header files + "${PROJECT_BINARY_DIR}/include/" # generated header files DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}" COMPONENT ${package_name}-development ) # copy target build output artifacts to OS dependent locations -# (except includes) +# (except includes, that just sets a compiler flag with the path) install( TARGETS ${target_name} EXPORT ${package_name}-targets diff --git a/cmake/OptionVariables.cmake b/cmake/OptionVariables.cmake index 2e95642d1..a7480567a 100644 --- a/cmake/OptionVariables.cmake +++ b/cmake/OptionVariables.cmake @@ -1,4 +1,5 @@ -include(GNUInstallDirs) +# included further down to avoid interfering with our cache variables +# include(GNUInstallDirs) # ---- Options Summary ---- @@ -93,6 +94,11 @@ if(PROJECT_IS_TOP_LEVEL) endif() +# do not include earlier or we can't set CMAKE_INSTALL_INCLUDEDIR above +# required for CMAKE_INSTALL_LIBDIR below +include(GNUInstallDirs) + + # ---- Install CMake Directory ---- # This allows package maintainers to freely override the installation path for From 93279cc8bd1aa5f14d372c8520b89ddb30680eaa Mon Sep 17 00:00:00 2001 From: doodspav Date: Sat, 21 Oct 2023 13:36:42 +0100 Subject: [PATCH 04/18] GHI #18 Improved documentation in test `OptionVariables.cmake` --- test/cmake/OptionVariables.cmake | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/test/cmake/OptionVariables.cmake b/test/cmake/OptionVariables.cmake index e2c8211e4..fb3f9a9d2 100644 --- a/test/cmake/OptionVariables.cmake +++ b/test/cmake/OptionVariables.cmake @@ -1,3 +1,16 @@ +# ---- Options Summary ---- + +# -------------------------------------------------------------------- +# | Option | Availability | Default | +# |======================================|==============|============| +# | CMAKE_INSTALL_TESTDIR (unofficial) | Always | share/test | +# |--------------------------------------|--------------|------------| +# | PATOMIC_CREATE_TEST_TARGETS_MATCHING | Always | ^(.*)$ | +# | PATOMIC_WINDOWS_SET_CTEST_PATH_ENV | Always | ON | +# | PATOMIC_WINDOWS_CREATE_PATH_ENV_FILE | Always | OFF | +# -------------------------------------------------------------------- + + # ---- Test Install Directory ---- # Normally tests would be install in CMAKE_INSTALL_BINDIR by default since @@ -15,8 +28,8 @@ set( # ---- Test Build Selection ---- -# This option provides a way to selectively disable/enable tests based on target -# name (not suite/case name, or name passed to create_test). +# This option provides a way to selectively disable/enable building tests based +# on target name (not suite/case name, or name passed to create_test). # Regex must be written according to CMake Regex Specification. # By default all test targets are enabled. set( @@ -30,8 +43,9 @@ set( # By default we set PATH for tests run with CTest on Windows in order to prevent # linker errors. # Due to limitations in CMake, we can only completely override the PATH, rather -# than prepend to it. +# than prepend or append to it. # This gives users the option to disable this behaviour. +# This option has no effect when not running on Windows. option( PATOMIC_WINDOWS_SET_CTEST_PATH_ENV "Set PATH environment variable for tests when run CTest on Windows" @@ -42,13 +56,16 @@ mark_as_advanced(PATOMIC_WINDOWS_SET_CTEST_PATH_ENV) # ---- Windows Path File ---- -# By default on Windows we generate a file per test kind that contains a string -# that can be prepended to PATH before running tests, in order to ensure that +# On Windows we need to set PATH for tests but may not want to have the PATH be +# completely overridden, like with PATOMIC_WINDOWS_SET_CTEST_PATH_ENV. +# Instead we can generated a file per test kind that contains a string that can +# be manually prepended to PATH before running tests, in order to ensure that # runtime dependencies can be found. # Most of the time we don't need this file (since CTest will take care of that -# for us), so we don't need to generate it. +# for us), so we don't generate it by default. # Additionally disabled by default because it contains potentially private # information about the target platform. +# This option has no effect when not running on Windows. option( PATOMIC_WINDOWS_CREATE_PATH_ENV_FILE "Create file with PATH environment variables for tests on Windows" From 66daac3d4ef4fa2edb47092a4b06b19ebee9a849 Mon Sep 17 00:00:00 2001 From: doodspav Date: Sat, 21 Oct 2023 13:37:51 +0100 Subject: [PATCH 05/18] GHI #18 Fix bug and improved documentation in test `WindowsDependenciesPath.cmake` - previously would do stuff if HOST was Windows even if TARGET wasn't windows - fixed by checking TARGET system --- test/cmake/WindowsDependenciesPath.cmake | 25 +++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/test/cmake/WindowsDependenciesPath.cmake b/test/cmake/WindowsDependenciesPath.cmake index 4671253a7..c3cedb418 100644 --- a/test/cmake/WindowsDependenciesPath.cmake +++ b/test/cmake/WindowsDependenciesPath.cmake @@ -1,33 +1,39 @@ # ---- Windows Path Prefix ---- # Windows doesn't support rpath, so when linking dynamically the libraries need -# to either be in the same directory or on PATH. -# This function sets a variable to a GENERATOR string that may be prepended to -# path in order to find linked dependencies +# to either be in the same directory or in PATH. +# This function sets a variable to a list of GENERATOR strings which resolve to +# the paths of the linked dependencies. # See: https://docs.microsoft.com/en-us/windows/win32/dlls/dynamic-link-library-search-order -# Usage: windows_deps_path( ...) -function(windows_deps_path ARG_VAR) - if(NOT CMAKE_HOST_SYSTEM_NAME STREQUAL "Windows") +# Usage: windows_deps_paths( ...) +function(windows_deps_paths ARG_VAR) + + # check that we're on Windows + if(NOT CMAKE_SYSTEM_NAME STREQUAL "Windows") set(${ARG_VAR} "" PARENT_SCOPE) return() endif() + # create a list of paths to dependency shared library locations set(paths "") foreach(target IN LISTS ARGN) - # This will fail if passed a link option that isn't a target - # This is intentional; don't do that + # This will fail if passed a link option that isn't a target. + # This is intentional; don't do that. # Instead, create an IMPORTED library, and set its target properties # such as IMPORTED_LOCATION for the library (.a .so etc.) path and # set INTERFACE_INCLUDE_DIRECTORIES to the directory containing any - # necessary header files + # necessary header files. get_target_property(type "${target}" TYPE) if(type STREQUAL "SHARED_LIBRARY") list(APPEND paths "$") endif() endforeach() + # remove duplicates + # makes generated file more human readable (if generated) list(REMOVE_DUPLICATES paths) + # concatenate list into string with same format as PATH on Windows set(path "") set(glue "") foreach(p IN LISTS paths) @@ -35,5 +41,6 @@ function(windows_deps_path ARG_VAR) set(glue "\;") # backslash is important endforeach() + # "return" string with PATH data set(${ARG_VAR} "${path}" PARENT_SCOPE) endfunction() From dd9139932f5396798395c2b6a6a86d8367d83750 Mon Sep 17 00:00:00 2001 From: doodspav Date: Sat, 21 Oct 2023 13:41:00 +0100 Subject: [PATCH 06/18] GHI #18 Reworking internals of `create_test` with some bug fixes --- CMakeLists.txt | 2 +- test/bt/CMakeLists.txt | 7 +- test/cmake/CreateTest.cmake | 185 ++++++++++++++++++++++-------------- test/ut/CMakeLists.txt | 7 +- 4 files changed, 122 insertions(+), 79 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 1103053c1..0e1dbac7a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -26,7 +26,7 @@ include(cmake/OptionVariables.cmake) # target that we can modify (can't modify ALIAS targets) # target name should not be the same as ${PROJECT_NAME}, causes add_subdirectory issues -set(target_name "patomic_patomic") +set(target_name "patomic-patomic") add_library(${target_name} ${build_type}) # alias to cause error at configuration time instead of link time if target is missing diff --git a/test/bt/CMakeLists.txt b/test/bt/CMakeLists.txt index 7469ab097..66794e7e0 100644 --- a/test/bt/CMakeLists.txt +++ b/test/bt/CMakeLists.txt @@ -1,12 +1,11 @@ -# create_test requires windows_deps_path include(../cmake/CreateTest.cmake) include(../cmake/WindowsDependenciesPath.cmake) # ---- Setup Tests ---- -add_custom_target(patomic_bt) -add_dependencies(patomic_test patomic_bt) +add_custom_target(patomic-test-bt) +add_dependencies(patomic-test patomic-test-bt) create_test(BT example_add SOURCE example_add.cpp) create_test(BT example_sub SOURCE example_sub.cpp) @@ -14,4 +13,4 @@ create_test(BT example_sub SOURCE example_sub.cpp) # ---- Windows Path Issues ---- -create_test_win_deps_path_file(BT) +create_test_win_deps_paths_file(BT) diff --git a/test/cmake/CreateTest.cmake b/test/cmake/CreateTest.cmake index 97110406b..71af34c5a 100644 --- a/test/cmake/CreateTest.cmake +++ b/test/cmake/CreateTest.cmake @@ -1,11 +1,21 @@ +include(WindowsDependenciesPath.cmake) + + # ---- Create Test ---- # Creates a target to build a test executable and registers it with CTest. -# Expects a target named patomic_${kind} to exist. -# E.g. if you call it as create_test(BT ...) then patomic_bt must exist. -# The target is named patomic_${kind}_${name} but the name of the executable -# produced is just ${name}. -# When the executable is installed, it's installed as patomic/${kind}/${name}. +# Expects a target named patomic-test-${kind} to exist. +# E.g. if you call it as create_test(BT ...) then patomic-test-bt must exist. +# +# Naming: +# - created target -> patomic-test-${kind}-${name} (e.g. patomic-test-bt-SomeExample) +# - executable name -> ${name} (e.g. SomeExample on Unix or SomeExample.exe on Windows) +# - install directory -> ${CMAKE_INSTALL_TESTDIR}/${kind} (e.g. share/test/bt) +# +# Hierarchy: +# - patomic-test -> base custom target & component (build/install) for all tests +# - patomic-test-${kind} -> custom target & component (build install) for all tests of a specific kind +# - patomic-test-${kind}-${name} -> executable target for a single test # # create_test( # BT|UT @@ -17,8 +27,8 @@ function(create_test) # setup what arguments we expect - set(all_kinds "BT;UT") # list we can iterate over - set(all_kinds_option "BT|UT") # string to use in debug message + set(all_kinds "BT;UT") # list of all kinds we can iterate over + set(all_kinds_opt_msg "BT|UT") # string to use in debug message cmake_parse_arguments( "ARG" @@ -32,10 +42,10 @@ function(create_test) # check what test kinds are passed set(kind "") # -> "bt" - set(name "") # -> "${ARG_BT}" - # we turn kind into a list so that we can check its length (should be 1) - # and name is just the last name we process - foreach(ak IN LISTS all_kinds) + set(name "") # -> value of "${ARG_BT}" + # we turn 'kind' into a list so that we can check its length (which should be 1) + # 'name' is just the last name we process (there should only be 1 name) + foreach(ak in LISTS all_kinds) # if(ARG_BT) if (ARG_${ak}) string(TOLOWER ${ak} ak_lower) @@ -49,63 +59,91 @@ function(create_test) # validate arguments + # setup set(args_valid TRUE) - set(func_name "create_test") - set(LENGTH kind kinds_count) + set(func_name "create_test") # CMAKE_CURRENT_FUNCTION is CMake 3.17+ + set(LENGTH kind kinds_count) # expected value is 1 + # go through all possible issues with arguments if(kinds_count EQUAL 0) - message(WARNING "${all_kinds_option} option needs to be specified when invoking '${func_name}'") + message(WARNING "'${all_kinds_opt_msg}' option must be specified invoking '${func_name}'") set(args_valid FALSE) elseif(kinds_count GREATER 1) - message(WARNING "Only a single ${all_kinds_option} option may be specified when invoking '${func_name}'") + message(WARNING "Only a single '${all_kinds_opt_msg}' option may be specified when invoking '${func_name}'") set(args_valid FALSE) elseif(TARGET ${name}) - message(WARNING "Test name must not be an existing target when invoking '${func_name}', was passed: ${name}") + message(WARNING "Test name must not be an existing target when invoking '${func_name}', was passed: '${name}'") set(args_valid FALSE) elseif("${name}" STREQUAL "") message(WARNING "Test name must not be empty when invoking '${func_name}'") + set(args_valid FALSE) endif() + # check there are no leftover arguments if(DEFINED ARG_UNPARSED_ARGUMENTS) message(WARNING "The following arguments were not recognised when invoking '${func_name}': ${ARG_UNPARSED_ARGUMENTS}") set(args_valid FALSE) endif() - if (NOT args_valid) + # abort if validation failed + if(NOT args_valid) message(FATAL_ERROR "Aborting '${func_name}' due to invalid arguments") endif() - # setup target + # create test target - set(parent_target patomic_${kind}) - set(target ${parent_target}_${name}) - set(target_deps patomic::patomic GTest::gtest_main ${ARG_LINK}) - set(output_name ${name}) + # setup + set(base_target patomic-test) # patomic-test + set(parent_target ${base_target}-${kind}) # patomic-test-bt + set(target ${parent_target}-${name}) # patomic-test-bt-SomeExample - if (NOT "${target}" MATCHES ${PATOMIC_CREATE_TEST_TARGETS_MATCHING}) - message(DEBUG "Skipping creation of test target ${target} (matches ${PATOMIC_CREATE_TEST_TARGETS_MATCHING})") - return() + # check target name matches pattern + if(NOT "${target}" MATCHES "${PATOMIC_CREATE_TEST_TARGETS_MATCHING}") + message(VERBOSE "Skipping creation of test target '${target}' (does not match ${PATOMIC_CREATE_TEST_TARGETS_MATCHING})") endif() + # create target with sources add_executable( ${target} - ${ARG_INCLUDE} ${ARG_SOURCE} ) + # add include directories + target_include_directories( + ${target} PRIVATE + ${ARG_INCLUDE} + ) + + # link dependencies (all tests use GTest framework) + # save list for Windows PATH issues later + set(target_deps patomic::patomic GTest::gtest GTest::gmock ${ARG_LINK}) target_link_libraries( - ${target} - PRIVATE + ${target} PRIVATE ${target_deps} ) - target_compile_features(${target} PRIVATE cxx_std_14) + # require C++14 as minimum + target_compile_features( + ${target} PRIVATE + cxx_std_14 + ) + + # set macro to know which test kind code is part of + string(TOUPPER "${kind}" kind_upper) + target_compile_definitions( + ${target} PRIVATE + "PATOMIC_${kind_upper}" + ) - set_target_properties(${target} PROPERTIES OUTPUT_NAME ${output_name}) + # set binary name instead of using default + set_target_properties( + ${target} PROPERTIES + OUTPUT_NAME "${name}" + ) - # register tests with CTest + # register test with GTest/CTest and parent target # must be run in same directory scope as target gtest_add_tests( @@ -113,84 +151,88 @@ function(create_test) TEST_LIST added_tests ) + add_dependencies(${parent_target} ${target}) + - # deal with Windows runtime linker issues for tests (with and without CTest) + # deal with Windows runtime linker issues - if (CMAKE_HOST_SYSTEM_NAME STREQUAL "Windows") + if(CMAKE_SYSTEM_NAME STREQUAL "Windows") # check we actually care about Windows PATH stuff - if (NOT PATOMIC_WINDOWS_SET_CTEST_PATH_ENV AND NOT PATOMIC_WINDOWS_CREATE_PATH_ENV_FILE) + if(NOT PATOMIC_WINDOWS_SET_CTEST_PATH_ENV AND + NOT PATOMIC_WINDOWS_CREATE_PATH_ENV_FILE) return() endif() # get paths to all shared library dependencies (DLLs) - windows_deps_path( - deps_path + windows_deps_paths( + deps_paths ${target_deps} ) - # set environment variable for the each test so that CTest works automatically - if (deps_path AND PATOMIC_WINDOWS_SET_CTEST_PATH_ENV) + # set environment variable for each test so that CTest works + if(deps_paths AND PATOMIC_WINDOWS_SET_CTEST_PATH_ENV) foreach(test IN LISTS added_tests) set_property( TEST "${test}" - PROPERTY ENVIRONMENT "PATH=${deps_path}" + PROPERTY ENVIRONMENT "PATH=${deps_paths}" ) endforeach() endif() # make dependencies accessible from parent target - if (PATOMIC_WINDOWS_CREATE_PATH_ENV_FILE) + # so that we can create single file for all tests in kind + if(PATOMIC_WINDOWS_CREATE_PATH_ENV_FILE) set_property( TARGET ${parent_target} - APPEND PROPERTY WIN_DEP_TARGETS ${target_deps} + APPEND PROPERTY WIN_DEPS_TARGETS "${target_deps}" ) endif() + endif() # setup install of target if(NOT CMAKE_SKIP_INSTALL_RULES) - # install as part of patomic_${kind} component + + # install as part of patomic-test-${kind} component install( TARGETS ${target} RUNTIME # - COMPONENT "${parent_target}" - DESTINATION "${CMAKE_INSTALL_TESTDIR}/patomic/${kind}" + COMPONENT ${parent_target} + DESTINATION "${CMAKE_INSTALL_TESTDIR}/patomic/${kind}/" EXCLUDE_FROM_ALL ) - # install as part of patomic_test component + # install as part of patomic-test component install( TARGETS ${target} RUNTIME # - COMPONENT patomic_test - DESTINATION "${CMAKE_INSTALL_TESTDIR}/patomic/${kind}" + COMPONENT ${base_target} + DESTINATION "${CMAKE_INSTALL_TESTDIR}/patomic/${kind}/" EXCLUDE_FROM_ALL ) - endif() - - # attach to parent target - - add_dependencies(${parent_target} ${target}) + endif() endfunction() # ---- Create Test Dependency File ---- -# Creates a file containing the output of windows_deps_path for all tests of +# Creates a file containing the output of windows_deps_paths for all tests of # the given kind registered so far. -# Expects a target named patomic_${kind} to exist -# E.g. if you call it as create_test_win_deps_path_file(BT) then patomic_bt must +# The file path will be "${CMAKE_CURRENT_BINARY_DIR}/windows_dependencies_path.txt". +# Expects a target named patomic-test-${kind} to exist +# E.g. if you call it as create_test_win_deps_paths_file(BT) then patomic-bt must # exist. +# This function has no effect when not running on Windows. # -# create_test_win_deps_path_file( +# create_test_win_deps_paths_file( # BT|UT # ) -function(create_test_win_deps_path_file ARG_KIND) +function(create_test_win_deps_paths_file ARG_KIND) # check we actually want to generate file @@ -201,10 +243,10 @@ function(create_test_win_deps_path_file ARG_KIND) # check KIND is valid - set(all_kinds_option "BT|UT") - set(func_name "create_test_win_deps_path_file") + set(all_kinds_opt_msg "BT|UT") + set(func_name "create_test_win_deps_paths_file") - if (NOT ARG_KIND MATCHES "^(${all_kinds_option})$") + if(NOT ARG_KIND MATCHES "^(${all_kinds_option})$") message(WARNING "${all_kinds_option} option needs to be specified when invoking '${func_name}'") message(FATAL_ERROR "Aborting '${func_name}' due to invalid arguments") endif() @@ -214,19 +256,19 @@ function(create_test_win_deps_path_file ARG_KIND) # create and install file with dependencies path - if(CMAKE_HOST_SYSTEM_NAME STREQUAL "Windows") + if(CMAKE_SYSTEM_NAME STREQUAL "Windows") # get dependencies set by create_test from target - get_target_property(dep_targets patomic_${kind} WIN_DEP_TARGETS) + get_target_property(dep_targets patomic-test-${kind} WIN_DEP_TARGETS) if("${dep_targets}" STREQUAL "dep_targets-NOTFOUND") - message(DEBUG "Skipping creation of Windows dependencies PATH file for ${ARG_KIND}; no relevant test targets created") + message(VERBOSE "Skipping creation of Windows dependencies PATH file for ${ARG_KIND}; no relevant test targets created") return() endif() list(REMOVE_DUPLICATES dep_targets) # get paths to all shared library dependencies (DLLs) - windows_deps_path( - deps_path + windows_deps_paths( + deps_paths ${dep_targets} ) @@ -234,27 +276,30 @@ function(create_test_win_deps_path_file ARG_KIND) set(file_path "${CMAKE_CURRENT_BINARY_DIR}/windows_dependencies_path.txt") file(GENERATE OUTPUT ${file_path} - CONTENT "${deps_path}" + CONTENT "${deps_paths}" ) # copy file to install location if(NOT CMAKE_SKIP_INSTALL_RULES) - # install as part of patomic_${kind} component + + # install as part of patomic-test-${kind} component install( FILES ${file_path} - COMPONENT patomic_${kind} + COMPONENT patomic-test-${kind} DESTINATION "${CMAKE_INSTALL_TESTDIR}/patomic/${kind}" EXCLUDE_FROM_ALL ) - # install as part of patomic_test component + # install as part of patomic-test component install( FILES ${file_path} - COMPONENT patomic_test + COMPONENT patomic-test DESTINATION "${CMAKE_INSTALL_TESTDIR}/patomic/${kind}" EXCLUDE_FROM_ALL ) + endif() + endif() endfunction() diff --git a/test/ut/CMakeLists.txt b/test/ut/CMakeLists.txt index 20b7cd490..b72c0b29b 100644 --- a/test/ut/CMakeLists.txt +++ b/test/ut/CMakeLists.txt @@ -1,12 +1,11 @@ -# create_test requires windows_deps_path include(../cmake/CreateTest.cmake) include(../cmake/WindowsDependenciesPath.cmake) # ---- Setup Tests ---- -add_custom_target(patomic_ut) -add_dependencies(patomic_test patomic_ut) +add_custom_target(patomic-test-ut) +add_dependencies(patomic-test patomic-test-ut) create_test(UT example_add SOURCE example_add.cpp) create_test(UT example_sub SOURCE example_sub.cpp) @@ -14,4 +13,4 @@ create_test(UT example_sub SOURCE example_sub.cpp) # ---- Windows Path Issues ---- -create_test_win_deps_path_file(UT) +create_test_win_deps_paths_file(UT) From 7797877e941ae23b0dbb10bcdbe54ad18ca37e6d Mon Sep 17 00:00:00 2001 From: doodspav Date: Sat, 21 Oct 2023 13:42:38 +0100 Subject: [PATCH 07/18] GHI #18 Fixed minor typo in docs --- test/cmake/OptionVariables.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/cmake/OptionVariables.cmake b/test/cmake/OptionVariables.cmake index fb3f9a9d2..56e9ee430 100644 --- a/test/cmake/OptionVariables.cmake +++ b/test/cmake/OptionVariables.cmake @@ -17,7 +17,7 @@ # they're executables. # This is undesirable, so this variable exists to override the install location # of test binaries separately. -# It's not prefixed with patomic_test because it's ok for it to be shared and +# It's not prefixed with PATOMIC_ because it's ok for it to be shared and # overridden by parent projects. # Note: this is not an official CMake variable set( From a795b13f05fc1a41a6ef854a0dcab3caef987b03 Mon Sep 17 00:00:00 2001 From: doodspav Date: Sat, 21 Oct 2023 14:16:40 +0100 Subject: [PATCH 08/18] GHI #18 Add `create_{bt|ut}` to replace `create_test` --- test/bt/CMakeLists.txt | 4 +- test/cmake/CreateTest.cmake | 74 +++++++++++++++++++++++++++++++++---- test/ut/CMakeLists.txt | 4 +- 3 files changed, 71 insertions(+), 11 deletions(-) diff --git a/test/bt/CMakeLists.txt b/test/bt/CMakeLists.txt index 66794e7e0..9cccd846e 100644 --- a/test/bt/CMakeLists.txt +++ b/test/bt/CMakeLists.txt @@ -7,8 +7,8 @@ include(../cmake/WindowsDependenciesPath.cmake) add_custom_target(patomic-test-bt) add_dependencies(patomic-test patomic-test-bt) -create_test(BT example_add SOURCE example_add.cpp) -create_test(BT example_sub SOURCE example_sub.cpp) +create_bt(NAME example_add SOURCE example_add.cpp) +create_bt(NAME example_sub SOURCE example_sub.cpp) # ---- Windows Path Issues ---- diff --git a/test/cmake/CreateTest.cmake b/test/cmake/CreateTest.cmake index 71af34c5a..5b800156f 100644 --- a/test/cmake/CreateTest.cmake +++ b/test/cmake/CreateTest.cmake @@ -17,13 +17,13 @@ include(WindowsDependenciesPath.cmake) # - patomic-test-${kind} -> custom target & component (build install) for all tests of a specific kind # - patomic-test-${kind}-${name} -> executable target for a single test # -# create_test( +# _create_test( # BT|UT # [INCLUDE ...] # [SOURCE ...] # [LINK ...] # ) -function(create_test) +function(_create_test) # setup what arguments we expect @@ -116,11 +116,11 @@ function(create_test) ) # link dependencies (all tests use GTest framework) - # save list for Windows PATH issues later - set(target_deps patomic::patomic GTest::gtest GTest::gmock ${ARG_LINK}) + # update list directly because we use it in Windows PATH stuff later + list(APPEND ARG_LINK GTest::gtest GTest::gmock) target_link_libraries( ${target} PRIVATE - ${target_deps} + ${ARG_LINK} ) # require C++14 as minimum @@ -167,7 +167,7 @@ function(create_test) # get paths to all shared library dependencies (DLLs) windows_deps_paths( deps_paths - ${target_deps} + ${ARG_LINK} ) # set environment variable for each test so that CTest works @@ -185,7 +185,7 @@ function(create_test) if(PATOMIC_WINDOWS_CREATE_PATH_ENV_FILE) set_property( TARGET ${parent_target} - APPEND PROPERTY WIN_DEPS_TARGETS "${target_deps}" + APPEND PROPERTY WIN_DEPS_TARGETS "${ARG_LINK}" ) endif() @@ -219,6 +219,66 @@ function(create_test) endfunction() +# Creates target patomic-test-bt-${name} corresponding to BT test executable. +# +# create_bt( +# NAME +# [INCLUDE ...] +# [SOURCE ...] +# [LINK ...] +# ) +function(create_bt) + + cmake_parse_arguments( + "ARG" + "" + "NAME" + "INCLUDE;SOURCE;LINK" + ${ARGN} + ) + + _create_test( + ${ARG_UNPARSED_ARGUMENTS} + BT ${ARG_NAME} + INCLUDE ${ARG_INCLUDE} + SOURCE ${ARG_SOURCE} + LINK + patomic::patomic + ${ARG_LINK} + ) + +endfunction() + + +# Creates target patomic-test-ut-${name} corresponding to UT test executable. +# +# create_ut( +# NAME +# [INCLUDE ...] +# [SOURCE ...] +# ) +function(create_ut) + + cmake_parse_arguments( + "ARG" + "" + "NAME" + "INCLUDE;SOURCE" + ${ARGN} + ) + + _create_test( + ${ARG_UNPARSED_ARGUMENTS} + UT ${ARG_NAME} + SOURCE ${ARG_SOURCE} + INCLUDE + "$" + ${ARG_INCLUDE} + ) + +endfunction() + + # ---- Create Test Dependency File ---- # Creates a file containing the output of windows_deps_paths for all tests of diff --git a/test/ut/CMakeLists.txt b/test/ut/CMakeLists.txt index b72c0b29b..2d2f66303 100644 --- a/test/ut/CMakeLists.txt +++ b/test/ut/CMakeLists.txt @@ -7,8 +7,8 @@ include(../cmake/WindowsDependenciesPath.cmake) add_custom_target(patomic-test-ut) add_dependencies(patomic-test patomic-test-ut) -create_test(UT example_add SOURCE example_add.cpp) -create_test(UT example_sub SOURCE example_sub.cpp) +create_ut(NAME example_add SOURCE example_add.cpp) +create_ut(NAME example_sub SOURCE example_sub.cpp) # ---- Windows Path Issues ---- From 6b0eb135a4c4c6708f7bef414679e79f148ebddb Mon Sep 17 00:00:00 2001 From: doodspav Date: Sat, 21 Oct 2023 14:34:18 +0100 Subject: [PATCH 09/18] GHI #18 Disable UTs unless `PATOMIC_SOURCE_DIR` set - expected by `patomic-test`, but always set by `patomic` if tests are enabled - if not set, UTs are disabled since we don't have access to required source files --- CMakeLists.txt | 5 +++++ test/CMakeLists.txt | 10 ++++++++-- test/cmake/CreateTest.cmake | 2 +- test/cmake/OptionVariables.cmake | 31 ++++++++++++++++++++++++++++++- 4 files changed, 44 insertions(+), 4 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 0e1dbac7a..281ab2e1d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -98,7 +98,12 @@ endif() # ---- Setup Tests ---- if(PATOMIC_BUILD_TESTING) + + # tell unit tests where our source files are + set(PATOMIC_SOURCE_DIR "${PROJECT_SOURCE_DIR}") + # need to enable testing in case BUILD_TESTING is disabled enable_testing() add_subdirectory(test) + endif() diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index a13189af9..ca892f721 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -31,9 +31,15 @@ include(GoogleTest) add_custom_target(patomic_test) -# add all test subdirectories +# add BTs unconditionally add_subdirectory(bt) -add_subdirectory(ut) + +# only add UTs if patomic source files are available +if(PATOMIC_SOURCE_DIR) + add_subdirectory(ut) +else() + message(STATUS "Variable 'PATOMIC_SOURCE_DIR' empty or not set; skipping unit tests") +endif() # ---- Support Packaging Library ---- diff --git a/test/cmake/CreateTest.cmake b/test/cmake/CreateTest.cmake index 5b800156f..08f98c64a 100644 --- a/test/cmake/CreateTest.cmake +++ b/test/cmake/CreateTest.cmake @@ -272,7 +272,7 @@ function(create_ut) UT ${ARG_NAME} SOURCE ${ARG_SOURCE} INCLUDE - "$" + "$" ${ARG_INCLUDE} ) diff --git a/test/cmake/OptionVariables.cmake b/test/cmake/OptionVariables.cmake index 56e9ee430..ee3a7c17e 100644 --- a/test/cmake/OptionVariables.cmake +++ b/test/cmake/OptionVariables.cmake @@ -5,11 +5,16 @@ # |======================================|==============|============| # | CMAKE_INSTALL_TESTDIR (unofficial) | Always | share/test | # |--------------------------------------|--------------|------------| +# | PATOMIC_SOURCE_DIR | Always | "" | # | PATOMIC_CREATE_TEST_TARGETS_MATCHING | Always | ^(.*)$ | # | PATOMIC_WINDOWS_SET_CTEST_PATH_ENV | Always | ON | # | PATOMIC_WINDOWS_CREATE_PATH_ENV_FILE | Always | OFF | # -------------------------------------------------------------------- +# Note: +# PATOMIC_SOURCE_DIR is set by patomic and available through there if +# patomic-test is built as a subproject. + # ---- Test Install Directory ---- @@ -20,10 +25,33 @@ # It's not prefixed with PATOMIC_ because it's ok for it to be shared and # overridden by parent projects. # Note: this is not an official CMake variable +# The variable type is STRING rather than PATH, because otherwise passing +# -DCMAKE_INSTALL_TESTDIR=share/test on the command line would expand to an +# absolute path with the base being the current CMake directory, leading to +# unexpected errors. set( CMAKE_INSTALL_TESTDIR "share/test" - CACHE PATH "(unofficial) Default test install location" + CACHE STRING "(unofficial) Default test install location" +) + + +# ---- Library Source Directory + +# Unit tests need access to the private header files not available from linking +# against the patomic::patomic target. +# This variable can be set to let unit tests know where to find these files. +# When building this as a subproject of patomic (as you would by just building +# patomic normally with tests enabled), this variable is set automatically. +# If this variable is empty or not set, no unit tests will be built. +# The variable type is STRING rather than PATH, because otherwise passing +# -PATOMIC_SOURCE_DIR=relative/patomic on the command line would expand to an +# absolute path with the base being the current CMake directory, leading to +# unexpected errors. +set( + PATOMIC_SOURCE_DIR "" + CACHE STRING "Path to source files of patomic::patomic target for unit tests" ) +mark_as_advanced(PATOMIC_SOURCE_DIR) # ---- Test Build Selection ---- @@ -36,6 +64,7 @@ set( PATOMIC_CREATE_TEST_TARGETS_MATCHING "^(.*)$" CACHE STRING "Only test targets matching regex are created and registered with CTest" ) +mark_as_advanced(PATOMIC_CREATE_TEST_TARGETS_MATCHING) # ---- Windows Tests Path ---- From 1edef3fb70e64c2f4f4375e2d286d719988b65c9 Mon Sep 17 00:00:00 2001 From: doodspav Date: Sat, 21 Oct 2023 14:36:52 +0100 Subject: [PATCH 10/18] GHI #18 Minor fix --- test/CMakeLists.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index ca892f721..1812a0a32 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -16,7 +16,7 @@ include(cmake/OptionVariables.cmake) include(cmake/ProjectIsTopLevel.cmake) -if(PROJECT_IS_TOP_LEVEL) +if(NOT TARGET patomic::patomic) find_package(patomic REQUIRED) enable_testing() endif() @@ -29,7 +29,7 @@ include(GoogleTest) # ---- Declare Tests ---- -add_custom_target(patomic_test) +add_custom_target(patomic-test) # add BTs unconditionally add_subdirectory(bt) From 9c9acd4e535f8bbd0d4a5c4f04a84f4f13db1866 Mon Sep 17 00:00:00 2001 From: doodspav Date: Sat, 21 Oct 2023 15:41:23 +0100 Subject: [PATCH 11/18] GHI #18 Fix BT/UT dependency checks - UT requires PATOMIC_{BINARY, SOURCE}_DIR variables are set (but these are private, so only works if patomic_test is a sub-project of patomic) - BT requires patomic::patomic target is available --- CMakeLists.txt | 13 +++++++++---- test/CMakeLists.txt | 23 ++++++++++++++++------- test/cmake/CreateTest.cmake | 9 ++++----- test/cmake/OptionVariables.cmake | 24 ------------------------ 4 files changed, 29 insertions(+), 40 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 281ab2e1d..631e3d4c8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -10,7 +10,7 @@ set(package_name "patomic") # create base project project( - ${package_name} + patomic VERSION 0.5.1 DESCRIPTION "Portable C90 Atomics Library" HOMEPAGE_URL "https://github.com/doodspav/patomic" @@ -99,11 +99,16 @@ endif() if(PATOMIC_BUILD_TESTING) - # tell unit tests where our source files are + # need to enable testing in case BUILD_TESTING is disabled + if(PROJECT_IS_TOP_LEVEL) + enable_testing() + endif() + + # tell unit tests where our files are + set(PATOMIC_BINARY_DIR "${PROJECT_BINARY_DIR}") set(PATOMIC_SOURCE_DIR "${PROJECT_SOURCE_DIR}") - # need to enable testing in case BUILD_TESTING is disabled - enable_testing() + # include test project add_subdirectory(test) endif() diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 1812a0a32..59bd6915b 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -16,11 +16,16 @@ include(cmake/OptionVariables.cmake) include(cmake/ProjectIsTopLevel.cmake) -if(NOT TARGET patomic::patomic) - find_package(patomic REQUIRED) +# need to enable testing ourselves if we're the root project +if(PROJECT_IS_TOP_LEVEL) enable_testing() endif() +# patomic is not a required dependency +if(NOT TARGET patomic::patomic) + find_package(patomic QUIET) +endif() + # for Windows: prevent overriding the parent project's compiler/linker settings set(gtest_force_shared_crt ON CACHE BOOL "" FORCE) find_package(GTest REQUIRED) @@ -31,14 +36,18 @@ include(GoogleTest) add_custom_target(patomic-test) -# add BTs unconditionally -add_subdirectory(bt) +# only add BTs if patomic target is available +if(TARGET patomic::patomic) + add_subdirectory(bt) +else() + message(STATUS "Skipping binary tests; patomic target not available") +endif() -# only add UTs if patomic source files are available -if(PATOMIC_SOURCE_DIR) +# only add UTs if patomic files are available +if(PATOMIC_BINARY_DIR AND PATOMIC_SOURCE_DIR) add_subdirectory(ut) else() - message(STATUS "Variable 'PATOMIC_SOURCE_DIR' empty or not set; skipping unit tests") + message(STATUS "Skipping unit tests; not building as direct sub-project of patomic") endif() diff --git a/test/cmake/CreateTest.cmake b/test/cmake/CreateTest.cmake index 08f98c64a..3e1254cc6 100644 --- a/test/cmake/CreateTest.cmake +++ b/test/cmake/CreateTest.cmake @@ -1,6 +1,3 @@ -include(WindowsDependenciesPath.cmake) - - # ---- Create Test ---- # Creates a target to build a test executable and registers it with CTest. @@ -45,7 +42,7 @@ function(_create_test) set(name "") # -> value of "${ARG_BT}" # we turn 'kind' into a list so that we can check its length (which should be 1) # 'name' is just the last name we process (there should only be 1 name) - foreach(ak in LISTS all_kinds) + foreach(ak IN LISTS all_kinds) # if(ARG_BT) if (ARG_${ak}) string(TOLOWER ${ak} ak_lower) @@ -117,7 +114,7 @@ function(_create_test) # link dependencies (all tests use GTest framework) # update list directly because we use it in Windows PATH stuff later - list(APPEND ARG_LINK GTest::gtest GTest::gmock) + list(APPEND ARG_LINK GTest::gtest_main GTest::gmock_main) target_link_libraries( ${target} PRIVATE ${ARG_LINK} @@ -272,6 +269,8 @@ function(create_ut) UT ${ARG_NAME} SOURCE ${ARG_SOURCE} INCLUDE + "$" + "$" "$" ${ARG_INCLUDE} ) diff --git a/test/cmake/OptionVariables.cmake b/test/cmake/OptionVariables.cmake index ee3a7c17e..81a860ad1 100644 --- a/test/cmake/OptionVariables.cmake +++ b/test/cmake/OptionVariables.cmake @@ -5,16 +5,11 @@ # |======================================|==============|============| # | CMAKE_INSTALL_TESTDIR (unofficial) | Always | share/test | # |--------------------------------------|--------------|------------| -# | PATOMIC_SOURCE_DIR | Always | "" | # | PATOMIC_CREATE_TEST_TARGETS_MATCHING | Always | ^(.*)$ | # | PATOMIC_WINDOWS_SET_CTEST_PATH_ENV | Always | ON | # | PATOMIC_WINDOWS_CREATE_PATH_ENV_FILE | Always | OFF | # -------------------------------------------------------------------- -# Note: -# PATOMIC_SOURCE_DIR is set by patomic and available through there if -# patomic-test is built as a subproject. - # ---- Test Install Directory ---- @@ -35,25 +30,6 @@ set( ) -# ---- Library Source Directory - -# Unit tests need access to the private header files not available from linking -# against the patomic::patomic target. -# This variable can be set to let unit tests know where to find these files. -# When building this as a subproject of patomic (as you would by just building -# patomic normally with tests enabled), this variable is set automatically. -# If this variable is empty or not set, no unit tests will be built. -# The variable type is STRING rather than PATH, because otherwise passing -# -PATOMIC_SOURCE_DIR=relative/patomic on the command line would expand to an -# absolute path with the base being the current CMake directory, leading to -# unexpected errors. -set( - PATOMIC_SOURCE_DIR "" - CACHE STRING "Path to source files of patomic::patomic target for unit tests" -) -mark_as_advanced(PATOMIC_SOURCE_DIR) - - # ---- Test Build Selection ---- # This option provides a way to selectively disable/enable building tests based From 56d795c3d86adb4b26e267773f7d8a1e4dd21055 Mon Sep 17 00:00:00 2001 From: doodspav Date: Sat, 21 Oct 2023 15:45:49 +0100 Subject: [PATCH 12/18] GHI #18 Fix minor typo --- test/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 59bd6915b..a8d21a945 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -47,7 +47,7 @@ endif() if(PATOMIC_BINARY_DIR AND PATOMIC_SOURCE_DIR) add_subdirectory(ut) else() - message(STATUS "Skipping unit tests; not building as direct sub-project of patomic") + message(STATUS "Skipping unit tests; not building as sub-project of patomic") endif() From b1c6cb6a07e8225db6c52ef33c47870cf9a0a5d1 Mon Sep 17 00:00:00 2001 From: doodspav Date: Sat, 21 Oct 2023 15:48:29 +0100 Subject: [PATCH 13/18] GHI #18 Fix UTs by providing source file --- test/ut/CMakeLists.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/ut/CMakeLists.txt b/test/ut/CMakeLists.txt index 2d2f66303..efdd8230e 100644 --- a/test/ut/CMakeLists.txt +++ b/test/ut/CMakeLists.txt @@ -7,8 +7,8 @@ include(../cmake/WindowsDependenciesPath.cmake) add_custom_target(patomic-test-ut) add_dependencies(patomic-test patomic-test-ut) -create_ut(NAME example_add SOURCE example_add.cpp) -create_ut(NAME example_sub SOURCE example_sub.cpp) +create_ut(NAME example_add SOURCE example_add.cpp "${PATOMIC_SOURCE_DIR}/src/patomic.c") +create_ut(NAME example_sub SOURCE example_sub.cpp "${PATOMIC_SOURCE_DIR}/src/patomic.c") # ---- Windows Path Issues ---- From 2d3cd0c7aeb9ab0ed0317595cc72c948bcf02eb7 Mon Sep 17 00:00:00 2001 From: doodspav Date: Sat, 21 Oct 2023 15:53:54 +0100 Subject: [PATCH 14/18] GHI #18 Additional comment --- test/CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index a8d21a945..5d79d5479 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -44,6 +44,7 @@ else() endif() # only add UTs if patomic files are available +# these are currently set by patomic before including this project if(PATOMIC_BINARY_DIR AND PATOMIC_SOURCE_DIR) add_subdirectory(ut) else() From c71bdeb44a0f4ac0bad5cc83412d77ce15cf7961 Mon Sep 17 00:00:00 2001 From: doodspav Date: Sat, 21 Oct 2023 16:20:30 +0100 Subject: [PATCH 15/18] GHI #18 Make our GTest stuff compliant with CMake 3.14 --- test/cmake/CreateTest.cmake | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/test/cmake/CreateTest.cmake b/test/cmake/CreateTest.cmake index 3e1254cc6..64ec2a1c8 100644 --- a/test/cmake/CreateTest.cmake +++ b/test/cmake/CreateTest.cmake @@ -112,9 +112,17 @@ function(_create_test) ${ARG_INCLUDE} ) + # update dependencies list directly because we use it in Windows PATH stuff later + if("${CMAKE_VERSION}" VERSION_GREATER_EQUAL "3.20.0") + list(APPEND ARG_LINK GTest::gtest_main) + if ("${CMAKE_VERSION}" VERSION_GREATER_EQUAL "3.23.0") + list(APPEND ARG_LINK GTest::gmock_main) + endif() + else() + list(APPEND ARG_LINK GTest::Main GTest::GTest) + endif() + # link dependencies (all tests use GTest framework) - # update list directly because we use it in Windows PATH stuff later - list(APPEND ARG_LINK GTest::gtest_main GTest::gmock_main) target_link_libraries( ${target} PRIVATE ${ARG_LINK} From 905de009980e7b562d30e5fa604a29d5479c43ab Mon Sep 17 00:00:00 2001 From: doodspav Date: Sat, 21 Oct 2023 17:28:25 +0100 Subject: [PATCH 16/18] GHI #18 Disable visibility macros for UTs - define PATOMIC_STATIC_DEFINE to disable visibility macros - otherwise will have issues of using the wrong import/export specification (because including a header file without telling CMake we're an importer so that it sets the correct defines) - not an issue since we don't link against any libraries in UTs Signed-off-by: doodspav --- test/cmake/CreateTest.cmake | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/cmake/CreateTest.cmake b/test/cmake/CreateTest.cmake index 64ec2a1c8..1b3e4b504 100644 --- a/test/cmake/CreateTest.cmake +++ b/test/cmake/CreateTest.cmake @@ -283,6 +283,10 @@ function(create_ut) ${ARG_INCLUDE} ) + # visibility macros will break UTs on Windows + set(target_name patomic-test-ut-${ARG_NAME}) + target_compile_definitions(${target_name} PRIVATE PATOMIC_STATIC_DEFINE) + endfunction() From 64bb3fdf5a8a85454dbfc1ce229937d1a510ca0e Mon Sep 17 00:00:00 2001 From: doodspav Date: Sat, 21 Oct 2023 17:34:48 +0100 Subject: [PATCH 17/18] GHI #18 Remove GMock for now - breaks test lookup on Windows - and not sure that it would work on older CMake versions anyway since there's no `GTest::GMock` target Signed-off-by: doodspav --- test/cmake/CreateTest.cmake | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/cmake/CreateTest.cmake b/test/cmake/CreateTest.cmake index 1b3e4b504..b4d18af5b 100644 --- a/test/cmake/CreateTest.cmake +++ b/test/cmake/CreateTest.cmake @@ -115,9 +115,10 @@ function(_create_test) # update dependencies list directly because we use it in Windows PATH stuff later if("${CMAKE_VERSION}" VERSION_GREATER_EQUAL "3.20.0") list(APPEND ARG_LINK GTest::gtest_main) - if ("${CMAKE_VERSION}" VERSION_GREATER_EQUAL "3.23.0") - list(APPEND ARG_LINK GTest::gmock_main) - endif() + # TODO: this prevents test lookup on Windows; fix once pipeline exists + # if ("${CMAKE_VERSION}" VERSION_GREATER_EQUAL "3.23.0") + # list(APPEND ARG_LINK GTest::gmock) + # endif() else() list(APPEND ARG_LINK GTest::Main GTest::GTest) endif() From 08311fbba3c2758cae316121757c1f85b468da47 Mon Sep 17 00:00:00 2001 From: doodspav Date: Sat, 21 Oct 2023 17:48:15 +0100 Subject: [PATCH 18/18] GHI #18 Introduce per-directory CML files - start with a CML file in `/src` - unfortunately can't put one in `/include` because it would be available to users as well --- CMakeLists.txt | 8 +++++--- src/CMakeLists.txt | 4 ++++ test/cmake/CreateTest.cmake | 1 + 3 files changed, 10 insertions(+), 3 deletions(-) create mode 100644 src/CMakeLists.txt diff --git a/CMakeLists.txt b/CMakeLists.txt index 631e3d4c8..f284e6903 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -32,15 +32,17 @@ add_library(${target_name} ${build_type}) # alias to cause error at configuration time instead of link time if target is missing add_library(patomic::patomic ALIAS ${target_name}) -# add sources to target +# add /include files to target +# unfortunately can't have CML file in /include target_sources( ${target_name} PRIVATE # include include/patomic/patomic.h - # src - src/patomic.c ) +# add /src files to target +add_subdirectory(src) + # ---- Generate Build Info Headers ---- diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt new file mode 100644 index 000000000..dd861e061 --- /dev/null +++ b/src/CMakeLists.txt @@ -0,0 +1,4 @@ +target_sources( + ${target_name} PRIVATE + "patomic.c" +) diff --git a/test/cmake/CreateTest.cmake b/test/cmake/CreateTest.cmake index b4d18af5b..d30daab9c 100644 --- a/test/cmake/CreateTest.cmake +++ b/test/cmake/CreateTest.cmake @@ -116,6 +116,7 @@ function(_create_test) if("${CMAKE_VERSION}" VERSION_GREATER_EQUAL "3.20.0") list(APPEND ARG_LINK GTest::gtest_main) # TODO: this prevents test lookup on Windows; fix once pipeline exists + # TODO: see https://github.com/google/googletest/issues/2157 # if ("${CMAKE_VERSION}" VERSION_GREATER_EQUAL "3.23.0") # list(APPEND ARG_LINK GTest::gmock) # endif()