Cmake issues #219
Cmake issues #219
Conversation
src/CMakeLists.txt
Outdated
| if(spdlog_FOUND) | ||
| if(PCMS_ENABLE_SPDLOG) | ||
| add_definitions(-DPCMS_SPDLOG_ENABLED) | ||
| target_compile_definitions(pcms_core INTERFACE -DPCMS_SPDLOG_ENABLED) |
There was a problem hiding this comment.
Do we need both PCMS_ENABLE_SPDLOG and PCMS_SPDLOG_ENABLED, or can we use just one of them in both CMakeLists.txt and source code?
I’m also wondering if it is redundant to use both add_definitions and target_compile_definitions. Based on my understanding of cmake, add_definitions affects all targets while target_compile_definitions affects pcms_core. Does our use case require both?
There was a problem hiding this comment.
You are correct. We should remove add_definitions and only be using the target_compile_definitions. It is probably also better to match the name of the compile definition and cmake variable as you suggest.
jacobmerson
force-pushed
the
cmake-issues
branch
from
2daf82a to
96c7398
Compare
Errors were reported with older adios2 versions. closes SCOREC#218
jacobmerson
force-pushed
the
cmake-issues
branch
from
96c7398 to
f5fae51
Compare
This commit updates PCMS configuration options to use a configuration file. This should do a better job of ensuring that downstream applications are setting up the correct configuration options if they do not build with CMake. It also has the benefit of making it easier to identify what options PCMS was configured with without parsing the CMake package files.
jacobmerson
force-pushed
the
cmake-issues
branch
from
f5fae51 to
1b0addf
Compare
|
@abhiyan123 this came up in one of conversations about merging #209. The file configuration.h.in gets configured to have all of the defines instead of including them in the build process with |
|
@Sichao25 this is ready for re-review. |
2 participants
No description provided.