search

SyneRBI / SIRF

Main repository for the CCP SynerBI software
http://www.ccpsynerbi.ac.uk
Other
58 stars 29 forks source link

travis and updating STIR/Gadgetron dependency #366

Open KrisThielemans opened 5 years ago

KrisThielemans commented 5 years ago

A recent STIR update needed changes in SIRF. This causes a chicken-and-egg problem for Travis as it's using the SIRF-SuperBuild version_config.cmake. We cannot change the latter yet, until the SIRF update has been tested on Travis, but without changing version_config.cmake, the SIRF Travis builds failed.

The solution was to change SIRF .travis.yml to explicitly set STIR_TAG for all builds (I'm setting it to master for now).

Sadly, this is ugly/error-prone as now both SIRF's .travis.yml and version_config.cmake contain hard-coded STIR versions. At some point, that'll create trouble...

any other ideas?

ashgillman commented 5 years ago

I'm not sure of a concrete solution, but it quite possibly makes sense for the default dependency versions to be somewhere in the SIRF repo, and have SIRF-SuperBuild collect them?

Alternatively, I think this is why the likes of FB, Microsoft, etc. purport to use monorepos.

rijobro commented 5 years ago

Not sure what solution is, but just to chime in that I've struggled with this problem in the past, too.

ashgillman commented 5 years ago

I should add, I'm not convinced a monorepo (i.e., everything in one big repo) is a good idea.

Could CMAKE variables in SIRF be set for recommended versions, and picked up in SIRF-SuperBuild's version_config.cmake? Would this actually even effect the Travis machine?

KrisThielemans commented 4 years ago

We are breaking SIRF Travis again because STIR master is ahead of SIRF, see https://github.com/CCPPETMR/SIRF/pull/608#issuecomment-607049333

I believe that we can restore backwards compatibility in STIR, see https://github.com/UCL/STIR/issues/479. However, I think that our current line forcing Travis to use STIR master https://github.com/CCPPETMR/SIRF/blob/7ebe03a35a534501cd443278a648a9dbedf32cd9/.travis.yml#L62 should be removed again, and we should rely on SIRF-SuperBuild and its version_config.cmake most of the time, certainly after a SIRF PR is merged.

Currently, I see only the following painful way to get Travis green once a STIR update breaks SIRF

  1. create a SIRF PR fixing it. During the PR, set STIR_TAG to master in SIRF's .travis.yml
  2. merge SIRF PR
  3. adapt version_config.cmake to update default (i.e. non-devel) tags for SIRF and STIR
  4. change SIRF .travis.yml to remove the hard-wired STIR_TAG

(obviously the same holds for an update to any of the dependencies).

Until step 3 is completed, the SIRF-SuperBuild will go red, for either the non-devel or the devel build. That seems unavoidable.

casperdcl commented 4 years ago

explicitly set STIR_TAG for all builds

Maybe just do this for some builds? This way some should pass, giving confidence...

casperdcl commented 4 years ago

Until step 3 is completed, the SIRF-SuperBuild will go red, for either the non-devel or the devel build. That seems unavoidable.

This is, er, surely the point of the devel builds? Feature-not-a-bug?

KrisThielemans commented 4 years ago

Until step 3 is completed, the SIRF-SuperBuild will go red, for either the non-devel or the devel build. That seems unavoidable.

This is, er, surely the point of the devel builds? Feature-not-a-bug?

:-) yes and no. Travis for SIRF-SuperBuild should give us some overall status, but when we're updating SIRF-SuperBuild code (as @paskino was doing), you don't want to be bothered by bugs in SIRF/STIR. Tricky balance!

KrisThielemans commented 4 years ago

explicitly set STIR_TAG for all builds

Maybe just do this for some builds? This way some should pass, giving confidence...

Possibly, but SIRF Travis checks SIRF master of course. We can try to let SIRF master cope with different STIR versions, but it's already hard enough! (We do check STIR version in SIRF CMake, but STIR versions move far far too slowly. @rijobro sometimes copes with this by checking for existence of new files, but in this case that wouldn't help. It's of course difficult to keep 2 actively developed projects in sync).