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

Feature/export cmake config package #14

Closed
wants to merge 16 commits into from
Closed

Feature/export cmake config package #14

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
e9b94ce
Export cmake config package
ClausKlein Nov 21, 2024
67a3b6a
Add find-package-test using examples
ClausKlein Nov 21, 2024
25b04eb
Add an implementation note
ClausKlein Nov 21, 2024
3a030e0
Set CMake minimum version to 3.25
ClausKlein Nov 21, 2024
1923125
Need to set CMAKE_PREFIX_PATH on Linux CI
ClausKlein Nov 21, 2024
f9c6d1a
Use Ninja Multi-Config generator
ClausKlein Nov 22, 2024
75935e9
Use environment to get compiler on CI
ClausKlein Nov 22, 2024
ee3d65b
Add find-package-test onyl for Debug builds on CI
ClausKlein Nov 22, 2024
a28d0a8
Run examples as test too
ClausKlein Nov 22, 2024
0ee9ae7
Add missing depends to test for gcovr run
ClausKlein Nov 22, 2024
7b88578
Set gcov program from environment on CI
ClausKlein Nov 23, 2024
9b16321
Add missing Windows presets
ClausKlein Nov 23, 2024
4ec37c3
Test cmake presets on Linux CI too
ClausKlein Nov 24, 2024
821293a
Fix gcov config options
ClausKlein Nov 24, 2024
a75dccb
Try to check gcov on linux CI
ClausKlein Nov 24, 2024
e7071c7
cleanup
ClausKlein Nov 24, 2024
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
19 changes: 19 additions & 0 deletions .cmake-format
View file
Open in desktop
ClausKlein marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
format:
line_width: 119
tab_size: 4
ClausKlein marked this conversation as resolved.
Show resolved Hide resolved
max_subgroups_hwrap: 4
max_rows_cmdline: 8
separate_ctrl_name_with_space: false
separate_fn_name_with_space: false
dangle_parens: true
dangle_align: prefix
line_ending: unix
keyword_case: upper
always_wrap:
- file
- install
- project
- write_basic_package_version_file

markup:
enable_markup: false
49 changes: 33 additions & 16 deletions .github/workflows/ci_tests.yml
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

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

Change reject out of scope, please move to another PR.

Original file line number Diff line number Diff line change
Expand Up @@ -11,21 +11,38 @@ on:
- cron: '30 15 * * *'

jobs:
# preset-test:
# runs-on: ubuntu-latest
# strategy:
# matrix:
# preset: []
# name: "Preset Test: ${{ matrix.preset }}"
# steps:
# - uses: actions/checkout@v4
# - name: Setup build environment
# uses: lukka/get-cmake@latest
# with:
# cmakeVersion: "~3.25.0"
# ninjaVersion: "^1.11.1"
# - name: Run preset
# run: cmake --workflow --preset ${{ matrix.preset }}
preset-test:
runs-on: ubuntu-24.04
strategy:
fail-fast: false
matrix:
preset: [debug, release, gcov, asan, lsan, usan]
compiler:
- cpp: g++-14
c: gcc-14
gcov: ""
- cpp: clang++-18
c: clang-18
gcov: llvm-cov-18
name: "Preset Test: ${{ matrix.preset }} ${{ matrix.compiler.cpp }}"
steps:
- uses: actions/checkout@v4

- name: Setup build environment
uses: aminya/setup-cpp@v1
with:
# cmake: true
ninja: true
gcovr: true

- name: Run preset ${{ matrix.preset }} ${{ matrix.compiler.cpp }}
run: |
which llvm-cov gcov gcovr || echo ignored
cmake --workflow --preset ${{ matrix.preset }}
env:
CC: ${{ matrix.compiler.c }}
CXX: ${{ matrix.compiler.cpp }}
GCOV: ${{ matrix.compiler.gcov }}

test:
strategy:
Expand Down Expand Up @@ -82,7 +99,7 @@ jobs:
ninja --version
- name: Configure CMake
run: |
cmake -B build -S . "-DCMAKE_CXX_STANDARD=${{ matrix.cpp_version }} ${{ matrix.cmake_args.args }}"
cmake -B build -S . -DCMAKE_PREFIX_PATH=/tmp/beman.inplace_vector "-DCMAKE_CXX_STANDARD=${{ matrix.cpp_version }} ${{ matrix.cmake_args.args }}"
env:
CC: ${{ matrix.compiler.c }}
CXX: ${{ matrix.compiler.cpp }}
Expand Down
44 changes: 44 additions & 0 deletions .github/workflows/macos.yml
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

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

Change rejected due to out of scope, please move to another PR.

Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
# .github/workflows/macos.yml
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

name: Macos Build

on:
push:
branches: ["main", "develop"]
pull_request:
branches: ["main", "develop"]

jobs:
build:
runs-on: macos-15
strategy:
fail-fast: false

matrix:
preset: [debug, release, gcov, asan, lsan, usan]
# TODO: compiler: [g++, clang++-19]
compiler: [clang++-19]

steps:
- uses: actions/checkout@v4

- name: Setup Cpp
uses: aminya/setup-cpp@v1
with:
# cmake: true
ninja: true
gcovr: true

- name: Install llvm-19
if: startsWith(matrix.compiler, 'clang')
run: |
brew install llvm@19

- name: macos clang++-19 ${{ matrix.preset }}
if: startsWith(matrix.compiler, 'clang')
run: GCOV=$(brew --prefix llvm@19)/bin/llvm-cov CXX=$(brew --prefix llvm@19)/bin/clang++ cmake --workflow --preset ${{ matrix.preset }}

- name: macos g++ ${{ matrix.preset }}
if: startsWith(matrix.compiler, 'g++')
run: CXX=${{ matrix.compiler }} cmake --workflow --preset ${{ matrix.preset }}
48 changes: 48 additions & 0 deletions .github/workflows/windows.yml
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

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

Change rejected due to out of scope, please move to another PR.

Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
# .github/workflows/windows.yml
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

name: Windows Build

on:
push:
branches: ["main", "develop"]
pull_request:
branches: ["main", "develop"]

jobs:
build:
runs-on: windows-latest
strategy:
fail-fast: false

matrix:
preset: [debug, release]
# TODO: compiler: [cl, clang-cl]
compiler: [cl]

steps:
- uses: actions/checkout@v4

# see https://github.com/marketplace/actions/enable-developer-command-prompt
- uses: ilammy/msvc-dev-cmd@v1
with:
vsversion: 2022

# - name: build environment
# run: pip install -r requirements.txt

- name: cmake workflow ${{ matrix.preset }}
shell: bash
run: |
cmake --version
ninja --version
CXX=${{ matrix.compiler }} cmake --workflow --preset ${{ matrix.preset }}

# - name: configure
# run: CXX=${{ matrix.compiler }} cmake --preset ${{ matrix.preset }}

# - name: build
# run: cmake --build --preset ${{ matrix.preset }}

# - name: ctest
# run: ctest --preset ${{ matrix.preset }}
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/stagedir
/build
/out
CMakeUserPresets.json
115 changes: 91 additions & 24 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,22 @@
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
# cmake-format: on

cmake_minimum_required(VERSION 3.23)
set(CMAKE_SKIP_TEST_ALL_DEPENDENCY FALSE)

cmake_minimum_required(VERSION 3.25...3.31)

project(
beman.inplace_vector
beman_inplace_vector
Copy link
Member

@wusatosi wusatosi Nov 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This does not conform to our naming tradition, why changing it to beman_inplace_vector?

We have used beman.short_name for almost all other projects.

Exemplar, utf_view, iterator_interface.

Copy link
Contributor Author

@ClausKlein ClausKlein Nov 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I changed it because I needed it for cmake export config package

And it was used like this in execution26 project.

For TBD see #14 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That will end in a very strange export package, not very common:

bash-5.2$ tree /usr/local/lib/cmake/boost_
boost_atomic-1.86.0/                    boost_graph_parallel-1.86.0/            boost_math_tr1l-1.86.0/                 boost_system-1.86.0/
boost_charconv-1.86.0/                  boost_headers-1.86.0/                   boost_nowide-1.86.0/                    boost_test_exec_monitor-1.86.0/
boost_chrono-1.86.0/                    boost_iostreams-1.86.0/                 boost_prg_exec_monitor-1.86.0/          boost_thread-1.86.0/
boost_container-1.86.0/                 boost_json-1.86.0/                      boost_process-1.86.0/                   boost_timer-1.86.0/
boost_context-1.86.0/                   boost_locale-1.86.0/                    boost_program_options-1.86.0/           boost_type_erasure-1.86.0/
boost_contract-1.86.0/                  boost_log-1.86.0/                       boost_random-1.86.0/                    boost_unit_test_framework-1.86.0/
boost_coroutine-1.86.0/                 boost_log_setup-1.86.0/                 boost_regex-1.86.0/                     boost_url-1.86.0/
boost_date_time-1.86.0/                 boost_math_c99-1.86.0/                  boost_serialization-1.86.0/             boost_wave-1.86.0/
boost_exception-1.86.0/                 boost_math_c99f-1.86.0/                 boost_stacktrace_addr2line-1.86.0/      boost_wserialization-1.86.0/
boost_fiber-1.86.0/                     boost_math_c99l-1.86.0/                 boost_stacktrace_basic-1.86.0/          
boost_filesystem-1.86.0/                boost_math_tr1-1.86.0/                  boost_stacktrace_from_exception-1.86.0/ 
boost_graph-1.86.0/                     boost_math_tr1f-1.86.0/                 boost_stacktrace_noop-1.86.0/           
bash-5.2$ tree /usr/local/lib/cmake/boost_atomic-1.86.0
/usr/local/lib/cmake/boost_atomic-1.86.0
|-- boost_atomic-config-version.cmake
|-- boost_atomic-config.cmake
|-- libboost_atomic-variant-mt-shared.cmake
`-- libboost_atomic-variant-mt-static.cmake

1 directory, 4 files
bash-5.2$ tree stagedir/lib/cmake/
stagedir/lib/cmake/
`-- beman_inplace_vector
    |-- beman_inplace_vector-config-version.cmake
    |-- beman_inplace_vector-config.cmake
    `-- beman_inplace_vector-targets.cmake

2 directories, 3 files
bash-5.2$

Copy link
Contributor Author

Choose a reason for hiding this comment

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

and it may be used like this:

#---------------------------------------------------------------------------------------
# search required packages and libs
#---------------------------------------------------------------------------------------

find_package(
    Boost 1.86 CONFIG
    COMPONENTS filesystem asio headers
    REQUIRED
)

Copy link
Member

Choose a reason for hiding this comment

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

I think you are right here. This was recommended in the weekly sync too because this format works better with compiler explorer. You can keep this.

Copy link
Member

Choose a reason for hiding this comment

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

I would also agree that using _ is more idiomatic than . within a project/target name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The most important point in this PR!

So you agree this change?

Copy link
Member

Choose a reason for hiding this comment

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

Hey, according to: bemanproject/beman#69

We cannot update the project/ target name.

VERSION 1.0.0
DESCRIPTION
"A dynamically-resizable vector with fixed capacity and embedded storage"
LANGUAGES CXX
)

if(${CMAKE_SOURCE_DIR} STREQUAL ${CMAKE_BINARY_DIR})
message(FATAL_ERROR "In-source builds are not allowed!")
endif()

# [CMAKE.SKIP_EXAMPLES]
option(
BEMAN_EXEMPLAR_BUILD_EXAMPLES
Expand All @@ -27,41 +33,102 @@ option(
${PROJECT_IS_TOP_LEVEL}
)

# includes
include(GNUInstallDirs)
include(CMakePackageConfigHelpers)

add_library(beman.inplace_vector INTERFACE)
add_library(beman_inplace_vector INTERFACE)
# [CMAKE.LIBRARY_ALIAS]
add_library(beman::inplace_vector ALIAS beman.inplace_vector)
add_library(beman::beman_inplace_vector ALIAS beman_inplace_vector)
Copy link
Member

Choose a reason for hiding this comment

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

Non conforming with [CMAKE.LIBRARY_ALIAS], https://github.com/bemanproject/beman/blob/main/docs/BEMAN_STANDARD.md#cmake . Please discard this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

see #14 (comment)


target_include_directories(
beman.inplace_vector
target_sources(
beman_inplace_vector
PUBLIC
FILE_SET inplace_vector_public_headers
TYPE HEADERS
BASE_DIRS ${CMAKE_CURRENT_SOURCE_DIR}/include
FILES
"${CMAKE_CURRENT_SOURCE_DIR}/include/beman/inplace_vector/inplace_vector.hpp"
)
set_target_properties(
beman_inplace_vector
PROPERTIES VERIFY_INTERFACE_HEADER_SETS ON
)
target_compile_features(
beman_inplace_vector
INTERFACE
$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>
$<INSTALL_INTERFACE:include>
"$<$<COMPILE_FEATURES:cxx_std_23>:cxx_std_23>"
"$<$<NOT:$<COMPILE_FEATURES:cxx_std_23>>:cxx_std_20>"
)

# Install the InplaceVector library to the appropriate destination
install(
TARGETS beman.inplace_vector
EXPORT ${TARGETS_EXPORT_NAME}
DESTINATION
${CMAKE_INSTALL_LIBDIR}
)
# export cmake config package
block()
# NOTE: copied from execution26:
# FIXME: but not yet used? CK
# set(TARGET_NAME inplace_vector)
Copy link
Member

Choose a reason for hiding this comment

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

Wdym by this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That a long odyssey:

This block was copied from execution26. But the original version use CMAKE_PROJECT_NAME and that is wrong!

If you import this project with FETCH_CONTENT the PROJECT_NAME must be used.

# set(TARGET_NAMESPACE beman)
# set(TARGET_PREFIX ${TARGET_NAMESPACE}.${TARGET_NAME})
# set(TARGET_LIBRARY ${PROJECT_NAME})
# set(TARGET_ALIAS ${TARGET_NAMESPACE}::${TARGET_LIBRARY})

# Install the header files to the appropriate destination
install(
DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/
DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/${CMAKE_PROJECT_NAME}
FILES_MATCHING
PATTERN
"${CMAKE_CURRENT_SOURCE_DIR}/include/beman/inplace_vector/inplace_vector.hpp"
)
set(TARGET_PACKAGE_NAME ${PROJECT_NAME}-config)
set(TARGETS_EXPORT_NAME ${PROJECT_NAME}-targets)
set(INSTALL_CONFIGDIR ${CMAKE_INSTALL_LIBDIR}/cmake/${PROJECT_NAME})

# Install the InplaceVector library to the appropriate destination
Copy link
Member

Choose a reason for hiding this comment

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

Need help reviewing this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, most people do this wrong or only partly ....

install(
TARGETS beman_inplace_vector
EXPORT ${TARGETS_EXPORT_NAME}
FILE_SET inplace_vector_public_headers
)

write_basic_package_version_file(
${CMAKE_CURRENT_BINARY_DIR}/${TARGET_PACKAGE_NAME}-version.cmake
VERSION ${PROJECT_VERSION}
COMPATIBILITY AnyNewerVersion
)

configure_package_config_file(
cmake/Config.cmake.in
${CMAKE_CURRENT_BINARY_DIR}/${TARGET_PACKAGE_NAME}.cmake
INSTALL_DESTINATION ${INSTALL_CONFIGDIR}
)

install(
FILES
${CMAKE_CURRENT_BINARY_DIR}/${TARGET_PACKAGE_NAME}.cmake
${CMAKE_CURRENT_BINARY_DIR}/${TARGET_PACKAGE_NAME}-version.cmake
DESTINATION ${INSTALL_CONFIGDIR}
)

install(
EXPORT ${TARGETS_EXPORT_NAME}
FILE ${TARGETS_EXPORT_NAME}.cmake
DESTINATION "${INSTALL_CONFIGDIR}"
NAMESPACE beman::
)
ClausKlein marked this conversation as resolved.
Show resolved Hide resolved

set(CPACK_GENERATOR TGZ)
include(CPack)
endblock()

if(BEMAN_INPLACE_VECTOR_BUILD_TESTS)
include(CTest)
enable_testing()
add_subdirectory(tests/beman/inplace_vector)
endif()

if(BEMAN_EXEMPLAR_BUILD_EXAMPLES)
add_subdirectory(examples)
endif()

# Coverage
ClausKlein marked this conversation as resolved.
Show resolved Hide resolved
configure_file(cmake/gcovr.cfg.in gcovr.cfg @ONLY)

add_custom_target(
process_coverage
WORKING_DIRECTORY ${CMAKE_BINARY_DIR}
DEPENDS test
COMMENT "Running gcovr to process coverage results"
COMMAND mkdir -p coverage
COMMAND gcovr --config gcovr.cfg .
)
Loading