Skip to content

Commit 2066564

Browse files
davidliondavidlion
committed
Address review comments.
1 parent 7687b09 commit 2066564

File tree

13 files changed

+80
-92
lines changed

13 files changed

+80
-92
lines changed

‎cmake/libs/containers-config.cmake.in‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
# gersemi: off
21
if(NOT TARGET ystdlib::containers)
32
include("${CMAKE_CURRENT_LIST_DIR}/containers-target.cmake")
43
endif()
5-
# gersemi: on

‎cmake/libs/error_handling-config.cmake.in‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,7 @@
1-
# gersemi: off
21
if(@Boost_FOUND@)
32
find_dependency(Boost @boost_find_package_args@)
43
endif()
54

65
if(NOT TARGET ystdlib::error_handling)
76
include("${CMAKE_CURRENT_LIST_DIR}/error_handling-target.cmake")
87
endif()
9-
# gersemi: on

‎cmake/libs/io_interface-config.cmake.in‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,7 @@
1-
# gersemi: off
21
if(NOT TARGET ystdlib::wrapped_facade_headers)
32
include("${CMAKE_CURRENT_LIST_DIR}/wrapped_facade_headers-target.cmake")
43
endif()
54

65
if(NOT TARGET ystdlib::io_interface)
76
include("${CMAKE_CURRENT_LIST_DIR}/io_interface-target.cmake")
87
endif()
9-
# gersemi: on

‎cmake/libs/wrapped_facade_headers-config.cmake.in‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
# gersemi: off
21
if(NOT TARGET ystdlib::wrapped_facade_headers)
32
include("${CMAKE_CURRENT_LIST_DIR}/wrapped_facade_headers-target.cmake")
43
endif()
5-
# gersemi: on

‎cmake/ystdlib-config.cmake.in‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
# gersemi: off
2-
# This is a template file and shouldn't be linted like a CMake file
31
include(CMakeFindDependencyMacro)
42

53
@PACKAGE_INIT@
@@ -26,4 +24,3 @@ else()
2624
endif()
2725

2826
check_required_components(ystdlib)
29-
# gersemi: on

‎cmake/ystdlib-helpers.cmake‎

Lines changed: 69 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,18 @@
11
include(CMakePackageConfigHelpers)
22

3-
# @param {string[]} REQUIRED_ARGS The list of arguments to check.
4-
# @param {string[]} ARG_KEYWORDS_MISSING_VALUES The list of arguments with missing values.
5-
function(require_argument_values REQUIRED_ARGS ARG_KEYWORDS_MISSING_VALUES)
6-
foreach(ARG IN LISTS REQUIRED_ARGS)
7-
if("${ARG}" IN_LIST ARG_KEYWORDS_MISSING_VALUES)
8-
message(FATAL_ERROR "Missing values for argument: '${ARG}'")
3+
# Checks that each argument name is defined and non-empty. Requires a prior call to
4+
# `cmake_parse_arguments` with `ARG` as the prefix to access the values of the arguments in
5+
# `REQUIRED_ARG_NAMES`.
6+
#
7+
# @param {string[]} REQUIRED_ARG_NAMES
8+
macro(require_argument_values REQUIRED_ARG_NAMES)
9+
set(_REQUIRED_ARGS "${REQUIRED_ARG_NAMES}")
10+
foreach(_REQUIRED_ARG IN LISTS _REQUIRED_ARGS)
11+
if(NOT DEFINED ARG_${_REQUIRED_ARG} OR ARG_${_REQUIRED_ARG} STREQUAL "")
12+
message(FATAL_ERROR "Non-empty value for argument: '${VAR_NAME}'")
913
endif()
1014
endforeach()
11-
endfunction()
15+
endmacro()
1216

1317
# @param {string[]} SOURCE_LIST The list of source files to check.
1418
# @param {bool} IS_HEADER_ONLY Returns TRUE if list only contains header files, FALSE otherwise.
@@ -37,7 +41,7 @@ endfunction()
3741
# @param {string[]} [PRIVATE_LINK_LIBRARIES]
3842
# @param {string[]} [BUILD_INCLUDE_DIRS="${PROJECT_SOURCE_DIR}/src"] The list of include paths for
3943
# building the library and for external projects that use ystdlib via add_subdirectory().
40-
function(cpp_library)
44+
function(add_cpp_library)
4145
set(SINGLE_VALUE_ARGS
4246
NAME
4347
NAMESPACE
@@ -54,63 +58,58 @@ function(cpp_library)
5458
NAMESPACE
5559
PUBLIC_HEADERS
5660
)
57-
cmake_parse_arguments(arg "" "${SINGLE_VALUE_ARGS}" "${MULTI_VALUE_ARGS}" ${ARGN})
58-
require_argument_values("${REQUIRED_ARGS}" "${arg_KEYWORDS_MISSING_VALUES}")
61+
cmake_parse_arguments(ARG "" "${SINGLE_VALUE_ARGS}" "${MULTI_VALUE_ARGS}" ${ARGN})
62+
require_argument_values("${REQUIRED_ARGS}")
5963

60-
if(NOT DEFINED arg_BUILD_INCLUDE_DIRS)
61-
set(arg_BUILD_INCLUDE_DIRS "${PROJECT_SOURCE_DIR}/src")
64+
if(NOT DEFINED ARG_BUILD_INCLUDE_DIRS)
65+
set(ARG_BUILD_INCLUDE_DIRS "${PROJECT_SOURCE_DIR}/src")
6266
endif()
6367

64-
set(ALIAS_TARGET_NAME "${arg_NAMESPACE}::${arg_NAME}")
68+
set(ALIAS_TARGET_NAME "${ARG_NAMESPACE}::${ARG_NAME}")
6569

66-
check_if_header_only(arg_PUBLIC_HEADERS IS_VALID_INTERFACE INVALID_HEADER_FILE)
70+
check_if_header_only(ARG_PUBLIC_HEADERS IS_VALID_INTERFACE INVALID_HEADER_FILE)
6771
if(NOT IS_VALID_INTERFACE)
6872
message(
6973
FATAL_ERROR
7074
"Invalid interface header file ${INVALID_HEADER_FILE} for ${ALIAS_TARGET_NAME}."
7175
)
7276
endif()
7377

74-
check_if_header_only(arg_PRIVATE_SOURCES IS_INTERFACE_LIB _)
78+
check_if_header_only(ARG_PRIVATE_SOURCES IS_INTERFACE_LIB _)
7579
if(IS_INTERFACE_LIB)
76-
if(arg_PRIVATE_LINK_LIBRARIES)
80+
if(ARG_PRIVATE_LINK_LIBRARIES)
7781
message(
7882
FATAL_ERROR
7983
"`PRIVATE_LINK_LIBRARIES` disabled for header-only library ${ALIAS_TARGET_NAME}."
8084
)
8185
endif()
82-
add_library(${arg_NAME} INTERFACE)
83-
target_link_libraries(${arg_NAME} INTERFACE ${arg_PUBLIC_LINK_LIBRARIES})
86+
add_library(${ARG_NAME} INTERFACE)
87+
target_link_libraries(${ARG_NAME} INTERFACE ${ARG_PUBLIC_LINK_LIBRARIES})
8488

85-
target_compile_features(${arg_NAME} INTERFACE cxx_std_20)
89+
target_compile_features(${ARG_NAME} INTERFACE cxx_std_20)
8690
else()
87-
add_library(${arg_NAME})
88-
target_sources(
89-
${arg_NAME}
90-
PRIVATE
91-
${arg_PUBLIC_HEADERS}
92-
${arg_PRIVATE_SOURCES}
93-
)
91+
add_library(${ARG_NAME})
92+
target_sources(${ARG_NAME} PRIVATE ${ARG_PRIVATE_SOURCES})
9493
target_link_libraries(
95-
${arg_NAME}
94+
${ARG_NAME}
9695
PUBLIC
97-
${arg_PUBLIC_LINK_LIBRARIES}
96+
${ARG_PUBLIC_LINK_LIBRARIES}
9897
PRIVATE
99-
${arg_PRIVATE_LINK_LIBRARIES}
98+
${ARG_PRIVATE_LINK_LIBRARIES}
10099
)
101-
target_compile_features(${arg_NAME} PUBLIC cxx_std_20)
100+
target_compile_features(${ARG_NAME} PUBLIC cxx_std_20)
102101
endif()
103102

104-
add_library(${ALIAS_TARGET_NAME} ALIAS ${arg_NAME})
103+
add_library(${ALIAS_TARGET_NAME} ALIAS ${ARG_NAME})
105104

106105
target_sources(
107-
${arg_NAME}
106+
${ARG_NAME}
108107
PUBLIC
109108
FILE_SET HEADERS
110109
BASE_DIRS
111-
"$<BUILD_INTERFACE:${arg_BUILD_INCLUDE_DIRS}>"
110+
"$<BUILD_INTERFACE:${ARG_BUILD_INCLUDE_DIRS}>"
112111
"$<INSTALL_INTERFACE:${CMAKE_INSTALL_INCLUDEDIR}>"
113-
FILES ${arg_PUBLIC_HEADERS}
112+
FILES ${ARG_PUBLIC_HEADERS}
114113
)
115114
endfunction()
116115

@@ -123,7 +122,7 @@ endfunction()
123122
# @param {string[]} [LINK_LIBRARIES]
124123
# @param {string} [UNIFIED_TEST_TARGET] If set, SOURCES and LINK_LIBRARIES are also added to
125124
# UNIFIED_TEST_TARGET.
126-
function(catch2_tests)
125+
function(add_catch2_tests)
127126
set(SINGLE_VALUE_ARGS
128127
NAME
129128
NAMESPACE
@@ -138,20 +137,20 @@ function(catch2_tests)
138137
NAMESPACE
139138
SOURCES
140139
)
141-
cmake_parse_arguments(arg "" "${SINGLE_VALUE_ARGS}" "${MULTI_VALUE_ARGS}" ${ARGN})
142-
require_argument_values(REQUIRED_ARGS arg_KEYWORDS_MISSING_VALUES)
140+
cmake_parse_arguments(ARG "" "${SINGLE_VALUE_ARGS}" "${MULTI_VALUE_ARGS}" ${ARGN})
141+
require_argument_values("${REQUIRED_ARGS}")
143142

144-
set(ALIAS_TARGET "${arg_NAMESPACE}::${arg_NAME}")
145-
set(UNIT_TEST_TARGET "unit-test-${arg_NAME}")
143+
set(ALIAS_TARGET "${ARG_NAMESPACE}::${ARG_NAME}")
144+
set(UNIT_TEST_TARGET "unit-test-${ARG_NAME}")
146145

147146
add_executable(${UNIT_TEST_TARGET})
148-
target_sources(${UNIT_TEST_TARGET} PRIVATE ${arg_SOURCES})
147+
target_sources(${UNIT_TEST_TARGET} PRIVATE ${ARG_SOURCES})
149148
target_link_libraries(
150149
${UNIT_TEST_TARGET}
151150
PRIVATE
152151
Catch2::Catch2WithMain
153152
${ALIAS_TARGET}
154-
${arg_LINK_LIBRARIES}
153+
${ARG_LINK_LIBRARIES}
155154
)
156155
target_compile_features(${UNIT_TEST_TARGET} PRIVATE cxx_std_20)
157156
set_property(
@@ -162,13 +161,13 @@ function(catch2_tests)
162161
${CMAKE_BINARY_DIR}/testbin
163162
)
164163

165-
if(arg_UNIFIED_TEST_TARGET)
166-
target_sources(${arg_UNIFIED_TEST_TARGET} PRIVATE ${arg_SOURCES})
164+
if(ARG_UNIFIED_TEST_TARGET)
165+
target_sources(${ARG_UNIFIED_TEST_TARGET} PRIVATE ${ARG_SOURCES})
167166
target_link_libraries(
168-
${arg_UNIFIED_TEST_TARGET}
167+
${ARG_UNIFIED_TEST_TARGET}
169168
PRIVATE
170169
${ALIAS_TARGET}
171-
${arg_LINK_LIBRARIES}
170+
${ARG_LINK_LIBRARIES}
172171
)
173172
endif()
174173
endfunction()
@@ -177,44 +176,43 @@ endfunction()
177176
#
178177
# @param {string} NAME
179178
# @param {string} NAMESPACE
180-
# Seems like the bottom 3 are dirs? Can we name them with a _DIR suffix?
181-
# @param {string} [CONFIG_DEST_PATH] Destination to install the generated config file
179+
# @param {string} [CONFIG_DEST_DIR] Destination to install the generated config file
182180
# (NAME-config.cmake).
183-
# @param {string} [CONFIG_INPUT_PATH] configure_package_config_file input file
181+
# @param {string} [CONFIG_INPUT_DIR] configure_package_config_file input file
184182
# (NAME-config.cmake.in).
185-
# @param {string} [CONFIG_OUTPUT_PATH] configure_package_config_file output file
183+
# @param {string} [CONFIG_OUTPUT_DIR] configure_package_config_file output file
186184
# (NAME-config.cmake).
187185
function(install_library)
188186
set(SINGLE_VALUE_ARGS
189187
NAME
190188
NAMESPACE
191-
CONFIG_DEST_PATH
192-
CONFIG_INPUT_PATH
193-
CONFIG_OUTPUT_PATH
189+
CONFIG_DEST_DIR
190+
CONFIG_INPUT_DIR
191+
CONFIG_OUTPUT_DIR
194192
)
195193
set(REQUIRED_ARGS
196194
NAME
197195
NAMESPACE
198196
)
199-
cmake_parse_arguments(arg "" "${SINGLE_VALUE_ARGS}" "" ${ARGN})
200-
require_argument_values(REQUIRED_ARGS arg_KEYWORDS_MISSING_VALUES)
197+
cmake_parse_arguments(ARG "" "${SINGLE_VALUE_ARGS}" "" ${ARGN})
198+
require_argument_values("${REQUIRED_ARGS}")
201199

202-
if(NOT DEFINED arg_CONFIG_DEST_PATH)
203-
set(arg_CONFIG_DEST_PATH "${CMAKE_INSTALL_LIBDIR}/cmake/${arg_NAMESPACE}/libs")
200+
if(NOT DEFINED ARG_CONFIG_DEST_DIR)
201+
set(ARG_CONFIG_DEST_DIR "${CMAKE_INSTALL_LIBDIR}/cmake/${ARG_NAMESPACE}/libs")
204202
endif()
205203

206-
if(NOT DEFINED arg_CONFIG_INPUT_PATH)
207-
set(arg_CONFIG_INPUT_PATH "${PROJECT_SOURCE_DIR}/cmake/libs")
204+
if(NOT DEFINED ARG_CONFIG_INPUT_DIR)
205+
set(ARG_CONFIG_INPUT_DIR "${PROJECT_SOURCE_DIR}/cmake/libs")
208206
endif()
209207

210-
if(NOT DEFINED arg_CONFIG_OUTPUT_PATH)
211-
set(arg_CONFIG_OUTPUT_PATH "${CMAKE_CURRENT_BINARY_DIR}")
208+
if(NOT DEFINED ARG_CONFIG_OUTPUT_DIR)
209+
set(ARG_CONFIG_OUTPUT_DIR "${CMAKE_CURRENT_BINARY_DIR}")
212210
endif()
213211

214212
install(
215213
TARGETS
216-
"${arg_NAME}"
217-
EXPORT "${arg_NAME}-target"
214+
"${ARG_NAME}"
215+
EXPORT "${ARG_NAME}-target"
218216
LIBRARY
219217
ARCHIVE
220218
RUNTIME
@@ -223,21 +221,21 @@ function(install_library)
223221
)
224222

225223
install(
226-
EXPORT "${arg_NAME}-target"
227-
DESTINATION ${arg_CONFIG_DEST_PATH}
228-
NAMESPACE "${arg_NAMESPACE}::"
229-
COMPONENT "${arg_NAME}"
224+
EXPORT "${ARG_NAME}-target"
225+
DESTINATION ${ARG_CONFIG_DEST_DIR}
226+
NAMESPACE "${ARG_NAMESPACE}::"
227+
COMPONENT "${ARG_NAME}"
230228
)
231229

232230
configure_package_config_file(
233-
${arg_CONFIG_INPUT_PATH}/${arg_NAME}-config.cmake.in
234-
${arg_CONFIG_OUTPUT_PATH}/${arg_NAME}-config.cmake
235-
INSTALL_DESTINATION ${arg_CONFIG_DEST_PATH}
231+
${ARG_CONFIG_INPUT_DIR}/${ARG_NAME}-config.cmake.in
232+
${ARG_CONFIG_OUTPUT_DIR}/${ARG_NAME}-config.cmake
233+
INSTALL_DESTINATION ${ARG_CONFIG_DEST_DIR}
236234
)
237235

238236
install(
239237
FILES
240-
${arg_CONFIG_OUTPUT_PATH}/${arg_NAME}-config.cmake
241-
DESTINATION ${arg_CONFIG_DEST_PATH}
238+
${ARG_CONFIG_OUTPUT_DIR}/${ARG_NAME}-config.cmake
239+
DESTINATION ${ARG_CONFIG_DEST_DIR}
242240
)
243241
endfunction()

‎src/ystdlib/containers/CMakeLists.txt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,10 @@ if(TARGET ystdlib::containers)
22
return()
33
endif()
44

5-
cpp_library(NAME containers NAMESPACE ystdlib PUBLIC_HEADERS Array.hpp)
5+
add_cpp_library(NAME containers NAMESPACE ystdlib PUBLIC_HEADERS Array.hpp)
66

77
if(ystdlib_ENABLE_TESTS)
8-
catch2_tests(
8+
add_catch2_tests(
99
NAME containers
1010
NAMESPACE ystdlib
1111
SOURCES

‎src/ystdlib/error_handling/CMakeLists.txt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ set(boost_find_package_args
1111
find_package(Boost ${boost_find_package_args})
1212
message(STATUS "Found Boost ${Boost_VERSION}.")
1313

14-
cpp_library(
14+
add_cpp_library(
1515
NAME error_handling
1616
NAMESPACE ystdlib
1717
PUBLIC_HEADERS
@@ -24,7 +24,7 @@ cpp_library(
2424
)
2525

2626
if(ystdlib_ENABLE_TESTS)
27-
catch2_tests(
27+
add_catch2_tests(
2828
NAME error_handling
2929
NAMESPACE ystdlib
3030
SOURCES

‎src/ystdlib/io_interface/CMakeLists.txt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ if(NOT wrapped_facade_headers IN_LIST ystdlib_LIBRARIES)
99
)
1010
endif()
1111

12-
cpp_library(
12+
add_cpp_library(
1313
NAME io_interface
1414
NAMESPACE ystdlib
1515
PUBLIC_HEADERS
@@ -23,7 +23,7 @@ cpp_library(
2323
)
2424

2525
if(ystdlib_ENABLE_TESTS)
26-
catch2_tests(
26+
add_catch2_tests(
2727
NAME io_interface
2828
NAMESPACE ystdlib
2929
SOURCES

‎src/ystdlib/wrapped_facade_headers/CMakeLists.txt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,10 @@ if(TARGET ystdlib::wrapped_facade_headers)
22
return()
33
endif()
44

5-
cpp_library(NAME wrapped_facade_headers NAMESPACE ystdlib PUBLIC_HEADERS sys/types.h)
5+
add_cpp_library(NAME wrapped_facade_headers NAMESPACE ystdlib PUBLIC_HEADERS sys/types.h)
66

77
if(ystdlib_ENABLE_TESTS)
8-
catch2_tests(
8+
add_catch2_tests(
99
NAME wrapped_facade_headers
1010
NAMESPACE ystdlib
1111
SOURCES

0 commit comments

Comments
 (0)