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.

Sign up for GitHub

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

Jump to bottom

fix: export targets and cmake files, add BUILD_TESTING check #18

Merged
merged 4 commits into from
Jul 23, 2023
Merged

fix: export targets and cmake files, add BUILD_TESTING check #18

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6dc7d02
fix: export targets and cmake files, add BUILD_TESTING check
russkel Jul 22, 2023
a064faa
temp fix fmt not being found on CI
russkel Jul 23, 2023
45962cc
add libfmt-dev
netcan Jul 23, 2023
8f0c8fd
fix: don't add fmt from third_party
russkel Jul 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
48 changes: 24 additions & 24 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,11 @@ project(${TARGET_NAME}

set (CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -fno-omit-frame-pointer -fsanitize=address")
set (CMAKE_LINKER_FLAGS_DEBUG "${CMAKE_LINKER_FLAGS_DEBUG} -fno-omit-frame-pointer -fsanitize=address")
message(CMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE})

set(CMAKE_CXX_STANDARD 20)
set(CMAKE_CXX_STANDARD_REQUIRED ON)
set(CMAKE_CXX_EXTENSIONS OFF)

###############################################################################
set(ASYNCIO_INC
include/asyncio/asyncio_ns.h
include/asyncio/task.h
Expand Down Expand Up @@ -42,21 +40,19 @@ set(ASYNCIO_INC
include/asyncio/finally.h
)

include_directories(${CMAKE_SOURCE_DIR}/include)
add_library(asyncio SHARED
${ASYNC_INC}
src/event_loop.cpp)
target_link_libraries(asyncio PUBLIC fmt::fmt)

add_subdirectory(test)
add_subdirectory(third_party)
find_package(fmt REQUIRED)
Copy link
Owner

Choose a reason for hiding this comment

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

github ci can't find the fmt package, is this line still needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I had fmt in my build environment already so I didn't need to check out the submodules.

Having a look at the code, it can be removed as a dependency from the core library quite easily as it's only called in two or three places for trivial things. Do you think this is suitable?

Copy link
Owner

Choose a reason for hiding this comment

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

I have added libfmt-dev in ci, but it reports error:

install(EXPORT "asyncioTargets" ...) includes target "asyncio" which requires target "fmt" that is not in any export set.

Copy link
Contributor Author

@russkel russkel Jul 23, 2023 •
edited

Choose a reason for hiding this comment

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

Took a second to figure out what was going on there, but because this is EXPORTing the asyncio target, any dependencies to that target need to be EXPORTed as well (which we can't do for fmt as it's a dependency). CMake stuff is always annoying, the simplest fix is to use the system fmt lib (because that has been properly exported) or remove the fmt lib dep altogether as per my above suggestion.

target_link_libraries(asyncio PUBLIC fmt::fmt)

set(MODELS_CMAKE_FILES
"cmake/asyncioConfig.cmake"
"cmake/asyncioConfigVersion.cmake"
)
if (BUILD_TESTING)
add_subdirectory(test)
add_subdirectory(third_party)
endif()

add_compile_options(-fPIC)
include(GNUInstallDirs)

set_target_properties(${PROJECT_NAME}
PROPERTIES
Expand All @@ -66,20 +62,24 @@ PROPERTIES
LIBRARY_OUTPUT_DIRECTORY "${CMAKE_LIBRARY_OUTPUT_DIRECTORY}"
)

set(MODELS_LIBRARY_INSTALL_DIR "/usr/lib/")
set(MODELS_CMAKE_FILES_INSTALL_DIR "/usr/lib/cmake/asyncio/")
set(MODELS_HEADER_INSTALL_DIR "/usr/")
target_include_directories(${PROJECT_NAME} PUBLIC
$<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/include>
$<INSTALL_INTERFACE:include>)

# INSTALL CONFIGURATIONS
include(GNUInstallDirs)
install(
DIRECTORY ${CMAKE_SOURCE_DIR}/include/asyncio
DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}
)

foreach ( file ${ASYNCIO_INC} )
get_filename_component( dir ${file} DIRECTORY )
install( FILES ${file} DESTINATION "${MODELS_HEADER_INSTALL_DIR}${dir}")
endforeach()
install(TARGETS ${TARGET_NAME} EXPORT ${TARGET_NAME}Targets
LIBRARY DESTINATION lib
ARCHIVE DESTINATION lib
RUNTIME DESTINATION bin)

install(TARGETS ${TARGET_NAME}
LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR})
install(EXPORT ${TARGET_NAME}Targets
FILE ${TARGET_NAME}Config.cmake
NAMESPACE ${TARGET_NAME}::
DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/asyncio)

install(FILES ${MODELS_CMAKE_FILES}
DESTINATION ${MODELS_CMAKE_FILES_INSTALL_DIR})
# target_compile_definitions(${PROJECT_NAME} PUBLIC ${compile_definition})
target_compile_options(${PROJECT_NAME} PUBLIC -fPIC)
5 changes: 0 additions & 5 deletions cmake/asyncioConfig.cmake
View file
Open in desktop

This file was deleted.

11 changes: 0 additions & 11 deletions cmake/asyncioConfigVersion.cmake
View file
Open in desktop

This file was deleted.