Add export config file for assigner package #13
Conversation
b8203be to
24e8b0c
Compare
f56d6f9 to
760ca8c
Compare
ae4bd55 to
1a1d22f
Compare
b0bbc98 to
3199b5e
Compare
CMakeLists.txt
Outdated
| @@ -142,4 +142,21 @@ install(TARGETS ${install_targets} EXPORT evmoneTargets | |||
| install(DIRECTORY ${include_dir}/ | |||
| DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}) | |||
|
|
|||
| set(CONFIG_PATH ${CMAKE_INSTALL_LIBDIR}/cmake/evmone-assigner) | |||
| install(EXPORT evmoneTargets DESTINATION ${CONFIG_PATH}) | |||
| install(EXPORT assignerTargets DESTINATION ${CONFIG_PATH}) | |||
There was a problem hiding this comment.
I think, we should use NAMESPACE here (it will be added to IMPORTED library target). As a result, you will use something like evm_assigner::evm_assigner instead of just evm_assigner.
The reason for this is that CMake won't treat namespaced target as a "system library" and will immediately fail if the target is not found.
https://stackoverflow.com/a/62490462
The root of the problem is actually a questionable syntax of target_link_libraries(), which allows you to pass even linker flags to it.
I believe right now, that using namespaces when exporting targets is the best practice, since in that case you will be sure later, that you are linking exactly this one target rather than some rubbish.
There was a problem hiding this comment.
Agree, will add evm_assigner:: namespace for both evm_assigner and evmone targets.
FYI I've found LINK_LIBRARIES_ONLY_TARGETS property related to this problem. But I was unable to make it work when I tried it.
.github/workflows/build.yml
Outdated
| CXX=$(nix develop -c bash -c 'which g++' | tail -1) | ||
| echo $CXX | ||
| $CMAKE_BINARY -GNinja -DCMAKE_MAKE_PROGRAM=$NINJA_BINARY -B build -DCMAKE_BUILD_TYPE=Release -DCMAKE_CXX_COMPILER=$CXX -DBUILD_ASSIGNER_TESTS=TRUE | ||
| $NINJA_BINARY -C build |
There was a problem hiding this comment.
How about we move this into some shell script like scripts/non-nix-build.sh so one can run this locally?
There was a problem hiding this comment.
Moved to script as is. I was thinking about a customizable script, but there are too many parameters, so it's hard to make the script useful.
No description provided.