search

SyneRBI / SIRF-SuperBuild

SIRF CMake SuperBuild
http://www.ccpsynerbi.ac.uk
Apache License 2.0
15 stars 17 forks source link

switching versions of dependencies usually fails: should we remove extra cached variables in "external" projects? #618

Open KrisThielemans opened 3 years ago

KrisThielemans commented 3 years ago

Problem description

CMake caches lots of variables usually, even those that you didn't set. This makes it run fast(er) of course. However, for a SuperBuild, this leads to surprising behaviour. Example:

cmake -DPython_EXECUTABLE=python3.5 .
make
cmake -DPython_EXECUTABLE=python3.6 .
make # broken!

Why does this happen?

The reason for this is that we pass the "obvious" variables to the dependent projects (e.g. for Boost, Python, SWIG. All of these set other variables (e.g. Boost_UNIT_TEST_FRAMEWORK_LIBRARY_RELEASE, PYTHON_LIBRARY_DEBUG, SWIG_DIR, SWIG_VERSION, etc etc). However, just resetting SWIG_EXECUTABLE doesn't update the other SWIG cached variables, and I'm fairly sure that same holds for Boost.

Consequence

once you build with a particular version of a dependency, switching to another one is virtually impossible.

Current documentation

We currently indeed recommend (if you read carefully) to "start from scratch" in our wiki, in which case all this doesn't matter. Still, it keeps tripping up people (and "starting from scratch" is certainly inconvenient).

By the way, I believe this is only a problem for "old-style" CMake projects (i.e. those who do not export their own *config.cmake), as the new-style projects (including ISMRMRD, ITK and STIR) normally don't have any cached variables aside from the ${proj}_DIR variable.

Potential solutions

  1. Just document this more clearly and forget about.
  2. We could force the removal of those cached variables, e.g. set Boost_CMAKE_ARGS to -UBOOST_* -UBoost_* -DBOOST_ROOT=... etc etc. It'll of course slow down CMake a bit, but for most people, the result will be less surprising.

Of course, the downside of this approach is that if someone really wants/needs to change a cached variable, in option 2, they wouldn't be able to do anymore. I see three solutions for that problem:

  1. Just tell them it cannot be done (except by running CMake for the dependent project and never run it from the SuperBuild anymore!)
  2. Make the behaviour depend on (yet another) BOOL, e.g. REMOVE_CMAKE_CACHED_DEPENDENCIES, which we could default to ON, such that crazy developers can revert to the current behaviour.
  3. Add optional variables and do 
    set (Boost_CMAKE_ARGS` to `-UBOOST_* -UBoost_* -DBOOST_ROOT=... etc etc ${Boost_CMAKE_DEPENDENCY_FLAGS})

    (with an interesting naming confusing with e.g STIR_EXTRA_CMAKE_FLAGS which are used at build time). But who will be able to understand/remember that?

`

final notes

PS: Note that current we don't seem to be passing Matlab variables. But do it via an env variable. Not sure what that means for this issue.

KrisThielemans commented 3 years ago

@AnderBiguri @paskino any ideas?

AnderBiguri commented 3 years ago

Ah yes, this explains a lot of the problems I was having with boost.

I really don't have any ideas here. I think that the SuperBuild, while amazing that it builds, its really complex to configure to start with. However, any of the possible solutions only make it more complex, by either having another quirk documented that people would only read once they are already struggling with it (most people who will use SIRF don't know what Cmake is, even less what a dependent cache variable is), or by either adding behavior that albeit fixes the problem sometimes, makes all this behave in even more confusing ways sometimes.

So... I don't know which I prefer really. Would providing a "clean_sirf_superbuild.sh" that removes all cached variables work? With some documentation on the lines of: "If you are trying to rebuild and have changed CMake variables, and its not working, consider cleaning the entire cache and reconfiguring the SuperBuild, this will reset some of the required intermediate variables"?

In any case, I would not consider "crazy developers" anyone that wants to rebuild this with their own stuff, most of us that use it need to change something at some point, and I can tell you I am very far from being a crazy developer of SIRF hehe :)

paskino commented 2 years ago

I usually trip over this behaviour myself. I'd rather implement your solution 2, with the caveat that I would default to OFF and keep the default CMake behaviour.