-
Notifications
You must be signed in to change notification settings - Fork 18
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.
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
fix/simplify Python CMake arguments #473
Conversation
- Created a variable PYTHONLIBS_CMAKE_ARGS that contains all settings to be used. - No longer pass PYTHON_INCLUDE_DIR from ...DIRS, nor PYTHON_LIBRARY from ..LIBRARIES, but from the variable itself. This caused problems when there are multiple files, and in particular debug and release Python libraries present. Fixes SyneRBI#472
SuperBuild.cmake
Outdated
@@ -232,8 +247,11 @@ option(BUILD_pet_rd_tools "Build pet_rd_tools" OFF) | |||
option(BUILD_CIL "Build CCPi CIL Modules and ASTRA engine" OFF) | |||
option(BUILD_CIL_LITE "Build CCPi CIL Modules" OFF) | |||
option(BUILD_NIFTYREG "Build NIFTYREG" ON) | |||
option(BUILD_SIRF_Contribs "Build SIRF-Contribs" ON) | |||
|
|||
if (BUILD_PYTHON) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess there's no guarantee that the contributions will always be limited to python.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's true, but at the moment it does
INSTALL_COMMAND ${CMAKE_COMMAND} -E copy_directory ${${proj}_SOURCE_DIR}/src/Python/sirf/ ${PYTHON_DEST}/sirf |
which fails if
PYTHON_DOEST
isn't set.
Maybe a better fix would be to fix that line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
haven't tested it obviously, but looks good to me!
9fb3ae8
to
b35c453
Compare
@rijobro I have now used a different strategy for |
Sorry I didn't test this before, but I get:
|
bugger. I guess I was too optimistic. I believe this is due to yet another bug in
also for the include files. |
Created a variable
PYTHONLIBS_CMAKE_ARGS
that contains all settings to be used.No longer pass
PYTHON_INCLUDE_DIR
from...DIRS
, norPYTHON_LIBRARY
from..LIBRARIES
, but from the variable itself. This caused problems when there are multiple files, and in particular debug and release Python libraries present.Fixes #472