2 updates for cmake and project structure #3
Conversation
JeffGarland
requested review from
bretbrownjr,
dietmarkuehl,
neatudarius,
steve-downey and
wusatosi
as code owners
| target_sources(${example} PRIVATE ${example}.cpp) | ||
| target_link_libraries( | ||
| beman.take_before.examples.${example} | ||
| ${example} |
There was a problem hiding this comment.
simplifies naming before
jeff@ub25-04:~/dev/take_before/build$ ls -ltr examples/
total 196
-rwxrwxr-x 1 jeff jeff 92448 Dec 25 08:13 beman.take_before.examples.take_before_as_default_projection
-rwxrwxr-x 1 jeff jeff 89344 Dec 25 08:13 beman.take_before.examples.take_before_direct_usage
after:
-rwxrwxr-x 1 jeff jeff 92448 Dec 25 08:16 take_before_as_default_projection
-rwxrwxr-x 1 jeff jeff 89344 Dec 25 08:16 take_before_direct_usage
| # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception | ||
|
|
||
| cmake_minimum_required(VERSION 3.25) | ||
| cmake_minimum_required(VERSION 3.28...4.2) |
There was a problem hiding this comment.
PR coming for examplar to update this -- 3.28 is minimum for modules and should be our standard now. We've decided range for cmake is best practice
| #if __cplusplus < 202002L | ||
| #error "C++20 or later is required" | ||
| #endif | ||
| // clang-format on |
There was a problem hiding this comment.
also needs to be added to our best practices -- give a nice user error if the CXX version isn't high enough
src/beman/take_before/CMakeLists.txt
Outdated
There was a problem hiding this comment.
Header only so removed this to simplify repo -- needed stuff moved to cmake in base directory
Pull Request Test Coverage Report for Build 20512376993Details
💛 - Coveralls |
|
The CI is mostly happy here except for MSVC. The flag passed appears to be wrong |
| #if defined(_MSVC_LANG) | ||
| #if _MSVC_LANG < 202002L | ||
| #error "C++20 or later is required" | ||
| #endif | ||
| #elif __cplusplus < 202002L | ||
| #error "C++20 or later is required" | ||
| #endif |
There was a problem hiding this comment.
MSVC does not correctly define __cplusplus by default. Added _MSVC_LANG check as a fallback
There was a problem hiding this comment.
ugh, of course it doesn't :( They do look like they set the concepts flag properly which is probably a better check anyway. I've switched it to that
There was a problem hiding this comment.
Note the CI flags still look wrong to me, but after your change it did work so hmm...
There was a problem hiding this comment.
And it worked - great. @nora77zz I think this is ready?
| add_library(beman::take_before ALIAS beman.take_before) | ||
| set_target_properties(beman.take_before PROPERTIES VERIFY_INTERFACE_HEADER_SETS ON) | ||
|
|
||
| set(CMAKE_PREFIX_PATH ${CMAKE_CURRENT_SOURCE_DIR}/infra/cmake) |
See comments for details but some general cleanup to fix cmake. One more change coming to remove src directory.