[CCPPETMR/SIRF-SuperBuild] add dependency version control (#24)

[Kris Thielemans]

See discussion started on the mailing list

Currently the SuperBuild uses master at our CCPPETMR forks of dependencies such as STIR and Gadgetron. This means that it is impossible to go back to a specific version with all dependencies that same as when we released.

One possibility is to create a SIRF/sirf_version_config.cmake (recording some git-tags) as that'd would be version tracked. A difficulty with this is that the Superbuild clones/builds dependencies first, and hence doesn't have SIRF and so its version info yet. I think it's therefore better to keep this version_config.cmake in the Superbuild. SIRF's CMake files would just have to set minimum requirements as usual (and would conform to how our external dependencies Gadgetron are treated).

However, hard-wiring specific SIRF git-hashes makes it inconvenient for developers. On the other hand, setting it to master means we cannot rely on this to get a specific version after all. (we could have a release branch that modifies the version_config.cmake, but that will create complications/conflicts etc). Maybe we could let these be overridden by CMake variables?

Not sure what the best option is.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

[Casper da Costa-Luis]

I think there are a few options:

1. have SuperBuild/version_config.cmake and SIRF/version_config.cmake
   • superbuild refers to SIRF tag/commit hash. Slightly inconvenient for developers
   • SIRF refers to its own dependencies
   • SIRF CI triggered by pushes to SIRF:master
   • SuperBuild/VM CI triggered by pushes to SuperBuild:master
2. have SuperBuild/version_config.cmake
   • superbuild oversteps itself a bit by referring to tags/commits for SIRF and all sub-dependencies.
   • SIRF CI triggered by pushes to SIRF:master
   • SuperBuild/VM CI triggered by pushes to SuperBuild:master
3. have SIRF/version_config.cmake
   • superbuild always refers to SIRF:master, but exposes this so the end-user can change it to a certain SIRF tag/commit.
   • SIRF and SuperBuild/VM CI triggered by pushes to SIRF:master
   • SuperBuild/VM CI also triggered by pushes to SuperBuild:master

[Casper da Costa-Luis]

I vote for 3.

[Kris Thielemans]

This was discussed at the Software tcon 15 June 2017 (minutes to get into the website). Brief summary:

• A general preference was felt for option 3 presented by Casper (specify git hashes in SIRF/version_config.cmake), however this conflicts with the current SuperBuild mechanism which builds STIR etc before it even clones SIRF. Option 2 (SuperBuild/version_config.cmake) is therefore our recommended solution.

• SIRF will use normal CMake mechanisms to ensure minimum versions of dependencies but not specify git hashes of dependencies.

• The developer can override the tags by specifying variables, and we also provide a “developer flag” that just uses “master” for everything.

[Kris Thielemans]

version_config.cmake file might look like this

Boost_VERSION=1_63_0
Boost_MD5=dsfsf

option(DEVEL_BUILD "Use current versions of major packages" OFF)

if (DEVEL_BUILD)
default_SIRF_GIT_TAG=master
else()
default_SIRF_GIT_TAG=v0.9.0
endif()
option(GIT_TAG_SIRF "git tag for SIRF" ${default_SIRF_GIT_TAG})

[Edoardo Pasca]

Do we have a list of required versions for all the dependencies?

I found the following.

Armadillo => 7.800.2 (release)
Boost => 1.63 (release)
FFTW3 => 3.3.5 (release)
Gadgetron => ?? (git)
googletest (rename to Gtest)
HDF5 => 1.10.0-patch1 (release)
ISMRMRD => ?? (git)
SIRF => release-0.9.0 (git)
STIR => ?? (git)
SWIG => 3.0.12 (release)

So I just miss STIR, Gadgetron and ISMRMRD.

[Edoardo Pasca]

would be closed with #29

[Edoardo Pasca]

Closed #24.

[Kris Thielemans]

Reopened #24.

[Kris Thielemans]

@paskino, it seems we need a few more changes after PR #29. Current version_config.cmake has

if (DEVEL_BUILD)
set (SIRF_TAG master)
else()
set(SIRF_TAG v0.9.0)

endif()
option(GIT_TAG_SIRF "git tag for SIRF" ${default_SIRF_GIT_TAG})

This means that the option GIT_TAG_SIRF actually doesn't do anything. The first few lines need to set the default tag, the option then needs to set the one to be actually used (presumably SIRF_TAG).

Also, I believe we need these options for STIR, Gadgetron and ISMRMRD as well. Otherwise we can not use the superbuild to test with a newer version. I'm not 100% sure what the prefered default would be for DEVEL_BUILD. We could either use master or keep it at the "stable" git tag. @evgueni-ovtchinnikov, what would you prefer?

Currently we set the git repos to the CCPPETMR forks of the above. That prevents a developer from using a version that isn't merged into the fork. It was suggested above to use the original repos. (commit-hashes should remain the same). To make this entirely flexible, we should make the URLs into options as well. I think it'd be fine to just say

option(STIR_URL "git repository for STIR" https://github.com/UCL/STIR)

Finally, I think we should use mark_as_advanced on all these options, except for the DEVEL_BUILD option.

[Edoardo Pasca]

I totally agree with you.
I understand the DEVEL_BUILD option would make us use the latest release (master).
But I still do not understand what is the meaning of the GIT_TAG_SIRF, and default_SIRF_GIT_TAG, which aren't used anywhere.

I can clearly add the project url as advanced variable to be set here. Is this what you mean?

[Edoardo Pasca]

I created a new branch (version_control) and with e66e17c it is now possible to use the option DEVEL_BUILD to set the URL/TAG of Gadgetron, Stir and ISMRMRD to upstream/master. I've tested on my VM and it builds on travis.

I wait the merge of the #32 before issuing a PR from version_control branch, as it branches from that.

[Kris Thielemans]

Closed #24.

[Kris Thielemans]

Fixed by PR #36