Skip to content

Cleanup CMake redundancies and use FILE_SET for headers #236

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

Open
wants to merge 13 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion .github/workflows/cppcmake.yml
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ jobs:
-DCMAKE_INSTALL_PREFIX=${PWD}/install \
-DLSL_UNITTESTS=ON \
-DLSL_BENCHMARKS=ON \
-DLSL_BUILD_EXAMPLES=ON \
-DCPACK_PACKAGE_DIRECTORY=${PWD}/package \
-Dlslgitrevision=${{ github.sha }} \
-Dlslgitbranch=${{ github.ref }} \
Expand All @@ -71,6 +70,18 @@ jobs:
echo ${PWD}
- name: make
run: cmake --build build --target install --config Release -j

- name: test install using examples
run: |
# Test that the in-tree install was successful by building the examples
cmake -S examples -B examples/build \
-DLSL_INSTALL_ROOT=${PWD}/install \
-DCMAKE_INSTALL_PREFIX=examples/build/install \
-DLSL_COMFY_DEFAULTS=ON \
${{ matrix.config.cmake_extra }} \
${{ github.event.inputs.cmakeextra }}
cmake --build examples/build --target install --config Release -j
./examples/build/install/bin/HandleMetaData

- name: package
run: |
Expand Down
40 changes: 26 additions & 14 deletions .github/workflows/mingw_static.yml
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
name: MinGW Windows static test

on:
# push:
# branches: ['*']
# paths:
# - '**'
push:
branches: ['*']
paths:
- 'src'
- 'include'
- 'cmake'
# - '!docs/**'
# - '!.github/**'
# - '.github/workflows/mingw_static.yml'
# pull_request:
- '.github/workflows/mingw_static.yml'
pull_request:
workflow_dispatch:

concurrency:
Expand All @@ -18,33 +20,43 @@ concurrency:
jobs:
build:
name: MinGW batteries-included
runs-on: windows-2019

strategy:
matrix:
include:
# - { sys: mingw64, env: x86_64 }
# - { sys: mingw32, env: i686 }
- { sys: ucrt64, env: ucrt-x86_64 }
# - { sys: clang64, env: clang-x86_64 }
runs-on: windows-latest
defaults:
run:
shell: 'msys2 {0}'

steps:
- uses: actions/checkout@v4
- uses: msys2/setup-msys2@v2
with:
release: false
msystem: ${{matrix.sys}}
update: true
cache: true
# install: >-
# cmake
install: >-
git
make
pacboy: >-
toolchain:p
cmake:p
ninja:p
- name: Configure CMake
run: |
cmake --version
cmake -S . -B build \
-DCMAKE_BUILD_TYPE=Release \
-DCMAKE_INSTALL_PREFIX=${PWD}/install \
-DLSL_UNITTESTS=ON \
-DLSL_BUILD_EXAMPLES=ON \
-DLSL_BUILD_STATIC=ON \
-Dlslgitrevision=${{ github.sha }} \
-Dlslgitbranch=${{ github.ref }} \
-DLSL_OPTIMIZATIONS=OFF \
-G 'MSYS Makefiles'
-G Ninja

- name: make
run: cmake --build build --target install --config Release -j --verbose
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,4 @@
# CLion
.idea/
/cmake-build-*/
/examples/build*/
31 changes: 0 additions & 31 deletions .travis.yml

This file was deleted.

8 changes: 1 addition & 7 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
cmake_minimum_required (VERSION 3.16)
cmake_minimum_required (VERSION 3.23)
project (liblsl
VERSION 1.16.2
LANGUAGES C CXX
Expand All @@ -17,18 +17,12 @@ include(cmake/SourceFiles.cmake) #
include(cmake/TargetObjLib.cmake) #
include(cmake/TargetLib.cmake) #
include(cmake/Installation.cmake) #
include(cmake/LSLCMake.cmake)
include(cmake/TargetOther.cmake)

if(LSL_UNITTESTS)
add_subdirectory(testing)
endif()

if(LSL_BUILD_EXAMPLES)
set(LSL_INSTALL_ROOT ${CMAKE_CURRENT_BINARY_DIR})
add_subdirectory(examples)
endif()

# Config for packaging
# TODO: Config for packaging for the library itself is likely to diverge from config for packing applications. e.g.,
# -> Optionally install to system directories
Expand Down
1 change: 0 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
[![GitHub Actions Status](https://github.com/sccn/liblsl/workflows/C%2FC++%20CI/badge.svg)](https://github.com/sccn/liblsl/actions)
[![Travis Build Status](https://travis-ci.org/sccn/liblsl.svg?branch=master)](https://travis-ci.org/sccn/liblsl)
[![Azure Build Status](https://dev.azure.com/labstreaminglayer/liblsl/_apis/build/status/sccn.liblsl?branchName=master)](https://dev.azure.com/labstreaminglayer/liblsl/_build/latest?definitionId=1&branchName=master)
[![DOI](https://zenodo.org/badge/123265865.svg)](https://zenodo.org/badge/latestdoi/123265865)

Expand Down
11 changes: 6 additions & 5 deletions cmake/Dependencies.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,15 @@ endif()
add_library(lslboost INTERFACE)
if(LSL_BUNDLED_BOOST)
message(STATUS "Using bundled header-only Boost")
target_include_directories(lslboost SYSTEM INTERFACE
$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/lslboost>)
target_include_directories(lslboost
INTERFACE
$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/lslboost>
)
else()
message(STATUS "Using system Boost")
find_package(Boost REQUIRED)
target_compile_definitions(lslboost INTERFACE
lslboost=boost # allows the LSL code base to work with the original Boost namespace/headers
)
# Map `lslboost` namespace, which LSL code base uses, to system `boost` namespace/headers.
target_compile_definitions(lslboost INTERFACE lslboost=boost)
target_link_libraries(lslboost INTERFACE Boost::boost Boost::disable_autolinking)
endif()
target_compile_definitions(lslboost INTERFACE BOOST_ALL_NO_LIB)
Expand Down
31 changes: 3 additions & 28 deletions cmake/Installation.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -25,24 +25,17 @@ endif()

# Install the targets and store configuration information.
install(TARGETS ${LSLTargets}
EXPORT LSLTargets # generates a CMake package config; TODO: Why the same name as the list of targets?
EXPORT LSLTargets
COMPONENT liblsl
RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}
LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}
INCLUDES DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}
# If we use CMake 3.23 FILE_SET, replace INCLUDES line with: FILE_SET HEADERS DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}
)

# TODO: What does this do? Why do we need LSLTargets.cmake in the build dir?
export(EXPORT LSLTargets
FILE "${CMAKE_CURRENT_BINARY_DIR}/LSLTargets.cmake"
NAMESPACE LSL::
FILE_SET HEADERS DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}
)

# Generate the LSLConfig.cmake file and mark it for installation
install(EXPORT LSLTargets
FILE LSLTargets.cmake # TODO: I think we can use this to generate LSLConfig.cmake, no?
FILE LSLConfig.cmake
COMPONENT liblsl
NAMESPACE "LSL::"
DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/LSL
Expand All @@ -53,28 +46,10 @@ install(EXPORT LSLTargets
# INSTALL_DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/lsl)
# If we use this method, then we need a corresponding install(FILES ...) command to install the generated file.

# Copy hardcoded CMake files to the build directory.
# TODO: Why bother with this copy? Is it not enough to install (into cmake/LSL)?
configure_file(cmake/LSLCMake.cmake "${CMAKE_CURRENT_BINARY_DIR}/LSLCMake.cmake" COPYONLY)
# TODO: Why bother with this copy? Is it not enough to install (into cmake/LSL)?
# TODO: Why use hardcoded files? We can generate the LSLConfig.cmake.
configure_file(cmake/LSLConfig.cmake "${CMAKE_CURRENT_BINARY_DIR}/LSLConfig.cmake" COPYONLY)

# Install the public headers.
# TODO: Verify that this is necessary, given that we already installed the INCLUDES above.
# TODO: Verify if it is still necessary to install the headers if we use FILE_SET.
install(DIRECTORY include/
COMPONENT liblsl
DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}
)

# Install the version file and the helper CMake script.
install(
FILES
# TODO: Keep this. But why does the configure_file(... COPYONLY) above exist?
cmake/LSLCMake.cmake
# TODO: Next line shouldn't be necessary if install(EXPORT...) uses LSLConfig.cmake instead of LSLTargets.cmake
${CMAKE_CURRENT_BINARY_DIR}/LSLConfig.cmake
${CMAKE_CURRENT_BINARY_DIR}/LSLConfigVersion.cmake
COMPONENT liblsl
DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/LSL
Expand Down
8 changes: 0 additions & 8 deletions cmake/LSLConfig.cmake

This file was deleted.

1 change: 0 additions & 1 deletion cmake/ProjectOptions.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ option(LSL_OPTIMIZATIONS "Enable some more compiler optimizations" ON)
option(LSL_BUNDLED_BOOST "Use the bundled Boost by default" ON)
option(LSL_BUNDLED_PUGIXML "Use the bundled pugixml by default" ON)
option(LSL_TOOLS "Build some experimental tools for in-depth tests" OFF)
option(LSL_BUILD_EXAMPLES "Build example programs in examples/" OFF)
option(LSL_UNITTESTS "Build LSL library unit tests" OFF)
option(LSL_FORCE_FANCY_LIBNAME "Add library name decorations (32/64/-debug)" OFF)
mark_as_advanced(LSL_FORCE_FANCY_LIBNAME)
25 changes: 13 additions & 12 deletions cmake/TargetLib.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ set_source_files_properties("src/buildinfo.cpp"
LSL_LIBRARY_INFO_STR="${LSL_VERSION_INFO}/link:${LSL_LIB_TYPE}"
)
add_library(lsl ${LSL_LIB_TYPE} src/buildinfo.cpp)
add_library(LSL::lsl ALIAS lsl)

# Configure main library

Expand All @@ -23,19 +24,19 @@ if(LSL_FORCE_FANCY_LIBNAME)
)
endif()

# Includes. TODO: Can we not inherit these from lslobj?
target_include_directories(lsl
INTERFACE
$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>
$<INSTALL_INTERFACE:include>
)
# Link dependencies. The only dependency is lslobj, which contains the bulk of the library code and linkages.
# Note: We link PRIVATE because lslobj exposes extra symbols that are not part of the public API
# but are used by the internal tests.
target_link_libraries(lsl PRIVATE lslobj)

# Link dependencies. The biggest dependency is lslobj, which contains the bulk of the library code.
target_link_libraries(lsl
PRIVATE
lslobj # TODO: If this is public, does that improve inheritance of includes and compile definitions?
lslboost # TODO: Shouldn't be needed -- lslobj already links it
${PUGIXML_LIBRARIES}
# Set the include directories for the lsl target.
# Note: We had to link lslobj as a PRIVATE dependency, therefore we must manually expose the include directories
get_target_property(LSLOBJ_HEADERS lslobj HEADER_SET)
target_sources(lsl
INTERFACE
FILE_SET HEADERS
BASE_DIRS include
FILES ${LSLOBJ_HEADERS}
)

# Set compile definitions for lsl
Expand Down
57 changes: 37 additions & 20 deletions cmake/TargetObjLib.cmake
Original file line number Diff line number Diff line change
@@ -1,32 +1,22 @@
# Create object library so all files are only compiled once
add_library(lslobj OBJECT
${lslsources}
${lslheaders}
# ${lslheaders} # Headers are added later using FILE_SET
)

# Set the includes/headers for the lslobj target
# Note: We cannot use the PUBLIC_HEADER property of the target, because
# it flattens the include directories.
# We could use the new FILE_SET feature that comes with CMake 3.23.
# This is how it would look. Note that HEADERS is a special set name and implies its type.
#target_sources(lslobj
# PUBLIC
# FILE_SET HEADERS
# BASE_DIRS include
# FILES ${lslheaders}
#)
# We settle on the older and more common target_include_directories PUBLIC approach.
# If we used the FILET_SET approach then we would remove the PUBLIC includes below.
target_include_directories(lslobj
PUBLIC
$<BUILD_INTERFACE:${CMAKE_CURRENT_LIST_DIR}/include>
$<INSTALL_INTERFACE:include>
# Note: We cannot use the PUBLIC_HEADER property of the target,
# because it flattens the include directories.
# Note: IME, this approach is less error prone than target_include_directories
target_sources(lslobj
INTERFACE
# Propagate include directories to consumers
$<BUILD_INTERFACE:${CMAKE_CURRENT_LIST_DIR}/src> # for unit tests
FILE_SET HEADERS # special set name; implies TYPE.
BASE_DIRS include
FILES ${lslheaders}
)

# Link system libs
# (boost might be bundled or system)
target_link_libraries(lslobj PRIVATE lslboost Threads::Threads)
if(MINGW)
target_link_libraries(lslobj PRIVATE bcrypt)
Expand All @@ -45,6 +35,30 @@ elseif(WIN32)
target_link_libraries(lslobj PRIVATE iphlpapi winmm mswsock ws2_32)
endif()

# Compiler settings
target_compile_definitions(lslobj
PRIVATE
LIBLSL_EXPORTS
LOGURU_DEBUG_LOGGING=$<BOOL:${LSL_DEBUGLOG}>
PUBLIC
ASIO_NO_DEPRECATED
$<$<CXX_COMPILER_ID:MSVC>:LSLNOAUTOLINK> # don't use #pragma(lib) in CMake builds
)
if(WIN32)
target_compile_definitions(lslobj
PRIVATE
_CRT_SECURE_NO_WARNINGS
PUBLIC
_WIN32_WINNT=${LSL_WINVER}
)
if(BUILD_SHARED_LIBS)
# set_target_properties(lslobj
# PROPERTIES
# WINDOWS_EXPORT_ALL_SYMBOLS ON
# )
endif(BUILD_SHARED_LIBS)
endif()

# Compiler settings
target_compile_definitions(lslobj
PRIVATE
Expand Down Expand Up @@ -72,6 +86,8 @@ endif(WIN32)
# Link in 3rd party dependencies
# - loguru and asio header-only
target_include_directories(lslobj
# Note: We use `SYSTEM` to suppress warnings from 3rd party headers and put these at the end of the include path.
# Note: We use `PUBLIC` because 'internal tests' import individual source files and link lslobj.
SYSTEM PUBLIC
$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/thirdparty/loguru>
$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/thirdparty/asio>
Expand All @@ -88,10 +104,11 @@ if(NOT LSL_OPTIMIZATIONS)
endif()

# - pugixml
# Note: We use `PUBLIC` because 'internal tests' import individual source files and link lslobj.
if(LSL_BUNDLED_PUGIXML)
target_sources(lslobj PRIVATE thirdparty/pugixml/pugixml.cpp)
target_include_directories(lslobj
SYSTEM PUBLIC
PUBLIC
$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/thirdparty/pugixml>
)
else()
Expand Down
Loading
Loading