Replace CMAKE_SOURCE_DIR with CMAKE_CURRENT_SOURCE_DIR #220
Conversation
This is because when using yosys-slang as a library in other CMake projects CMAKE_SOURCE_DIR will point to the wrong directory causing the config to fail. CMAKE_CURRENT_SOURCE_DIR will point to the directory that the current CMake is in.
Did this to allow other libs to override where find_package looks by setting CMAKE_MODULE_PATH
|
Thanks, I'll let @whitequark take a look |
This comment was marked as off-topic.
This comment was marked as off-topic.
| @@ -1,4 +1,4 @@ | |||
| if (CMAKE_BINARY_DIR STREQUAL CMAKE_SOURCE_DIR) | |||
| if (CMAKE_BINARY_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR) | |||
There was a problem hiding this comment.
This doesn't look right to me.
There was a problem hiding this comment.
This says that if the CMAKE_BINARY_DIR is the directory of yosys-slang then you cant build. This makes sense because you could have
./top/CMakeLists.txt # user cmake
./top/yosys-slang/CMakeLists.txt # yosys-slang cmake
when in top/yosys-slang/CMakeLists.txt
CMAKE_SOURCE_DIR is set to "./top"
CMAKE_CURRENT_SOURCE_DIR is set to "./top/yosys-slang"
if they set CMAKE_BINARY_DIR to "./top/yosys-slang" and we are comparing CMAKE_SOURCE_DIR then it would allow the build to be "In-tree". Which is not supported according to the fatal error.
Comparing CMAKE_CURRENT_SOURCE_DIR instead the makes the STREQUAL true and calls the FATAL_ERROR stopping the build from being "In-tree".
When CMAKE_SOURCE_DIR is equal to CMAKE_CURRENT_SOURCE_DIR this will have the same behavior as before the change.
There was a problem hiding this comment.
I think it should be CMAKE_CURRENT_BINARY_DIR, since we don't support in-tree builds as a subproject either
There was a problem hiding this comment.
Great, I'll leave the change in.
|
|
||
| project(yosys-slang CXX) | ||
| set(CMAKE_MODULE_PATH "${CMAKE_SOURCE_DIR}/cmake" ${CMAKE_MODULE_PATH}) | ||
| set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} "${CMAKE_CURRENT_SOURCE_DIR}/cmake") |
There was a problem hiding this comment.
I don't think we want the modules to be overridden; this makes accidental name collisions possible where previously they were not
There was a problem hiding this comment.
I need it because the default FindYosys.cmake does not work for me. If you think swapping them could break other projects I will remove it from the PR.
There was a problem hiding this comment.
If it doesn't work for you then that should be fixed.
3 participants
Replacing CMAKE_SOURCE_DIR will allow it to be using as a library in other CMake projects
This is because when yosys-slang is included/added as a subdirectory CMAKE_SOURCE_DIR will point to the wrong directory (The top CMakeLists.txt) causing the config to fail.
CMAKE_CURRENT_SOURCE_DIR will point to the directory that the current CMake is in and so should not change functionality when building regularly.
Edit:
Also swapped ${CMAKE_SOURCE_DIR}/cmake" and ${CMAKE_MODULE_PATH} because this allows other libs to override where find_package looks by setting CMAKE_MODULE_PATH.
This should pr should probably have a different name