Simplify buildsystem #284
Simplify buildsystem #284
Conversation
Calling the macro "ExternalProject_Include_Dependencies" last ensures
all prior calls to 'mark_as_superbuild' are considered when updating
the variable '${proj}_EP_ARGS' passed to the main project.
…ect. Simply calling "mark_as_superbuild" after declaring the option is now sufficient to ensure the value will be passed to inner BRAINSTools project.
…WF_SUPPORT This variable was not used in the code base, searching through the git history of either BRAINSTools or NAMICExternalProjects didn't reveal anything. If the intended behavior of the variable was to conditionally enable/disable support for FFTWF, the ITK optiion ITK_USE_FFTWF could be set accordingly.
When including in other project, it is not desired to force this variable to any specific value.
… quote New version based on commontk/Artichoke@d453e32 This commit will avoid warning like: loading initial cache file /path/to/BRAINSTools-updated-build/teem-prefix/tmp/teem-cache-Release.cmake CMake Warning (dev) in /path/to/BRAINSTools-updated-build/teem-prefix/tmp/teem-cache-Release.cmake: Syntax Warning in cmake code at /path/to/BRAINSTools-updated-build/teem-prefix/tmp/teem-cache-Release.cmake:3:93 Argument not separated from preceding token by whitespace. This warning is for project developers. Use -Wno-dev to suppress it. $ git shortlog commontk/Artichoke@82fa9f4..commontk/Artichoke@d453e32 --no-merges Jean-Christophe Fillion-Robin (5): _sb_list_to_string: Update function to not set CMP0007 to OLD _sb_list_to_string: Ensure function works with CMake 2.8.9 Add support for specifying a different CMAKE_GENERATOR for each project Add support for specifying different toolset/platform for each project Teach mark_as_superbuild how to handle variables with double-quotes
|
@jcfr THANK YOU! I'll run one test, and then commit. |
|
@jcfr All preliminary build testing came back successful. THANK YOU. |
|
Great. Thanks for the quick feedback. |
|
Consider Slicer builds against ITK Changes that would be included are the following: |
|
Yes, Just be sure that BRAINSABC is not turned on (it has not been in the past). Everything else in C++ should be backwards compatible for a few years ago for ITK. Hans From: Jean-Christophe Fillion-Robin [email protected] Consider Slicer builds against ITK v4.10.0rc2 from 2016-05-18, is it reasonable for Slicer to use BRAINSTools master ? Changes that would be included are the following: Ali Ghayoor (34): Hans J. Johnson (70): Isaiah Norton (1): Jean-Christophe Fillion-Robin (7): Juan Carlos Prieto (3): Regina Kim (10): |
|
Roger that 👍 Have a good evening (if you are in EST timezone ..) |
|
Hi. Thanks for your commits! I've lost the ability to turn modules on and off through the superbuild. I think that this is related to your changes, I suspect this one, specifically the changes to SuperBuild.cmake: 5d27bb8
Could you please re-review your changes and see if it looks like any of them could have caused this? Thanks! |
|
@ajjl I need to assign this task to you. Currently I don't have time to investigate, and I know that JC is very very busy. Please dig in and see if you can fix it. Hans |
|
@ajjl Thanks for the detailed report. Will investigate later today. We need to get this sorted out. |
|
I'll look into it. |
|
I found the problem ... this is caused by:
and
There are few options to move forward:
Ideally I would prefer (2), but implementing (3) is also an option. Let me know what you think |
|
I definitely prefer the simplify approach Sent from my iPhone
|
Great. Will keep that in mind for future refactoring |
This PR simplifies the buildsystem and should also address the issue reported by @lassoan: