CI: OSX

[casperdcl]

fix travis for osx builds (continues #71)

[rijobro]

@casperdcl:
The original problem for OSX was that it couldn't open rpaths to dylibs, so compilation would complete, but tests would fail. I managed to replicate this problem on my own OSX.

Using the Cmake argument, CMAKE_INSTALL_NAME_DIR=${CMAKE_INSTALL_PREFIX}/lib in SuperBuild.cmake and then passing this to all subsequent CMAKE_ARGS in the necessary External_*.cmake files, I managed to install all shared libraries with absolute paths on my own machine (and tests all passed).

However, Travis is still failing. The error is Fatal Python error: PyThreadState_Get: no current thread, and a quick google tells me that this happens when libraries are compiled against one version of python, but run with another. I know you added the python$PYMVER command, so I'm at a loss of what to do. Thoughts?

As far as I can tell from the Travis log, I have no way of telling which library was compiled against the different version of python (if that is the issue...). The branch is here: OSX_abs_paths.

[casperdcl]

Thanks for looking into this. It confirms what I had said during the earlier tcons. You can see the mismatched bin and lib versions (2.7.10 and 2.7.14 from what I remember) in the cmake output on travis. What I had requested is help with installing matching bin & lib versions on osx. AFAIK travis doesn't have matched versions. I'm tempted to just go with anaconda...

[casperdcl]

Edit: yes just looked at your log, it shows 2.7.14 (bin) and 2.7.10 (lib). Don't recall ever having issues with rpaths/dylibs.

[KrisThielemans]

we've briefly looked at this. we're still using python-config to fix PYTHON_LIBRARY presumably. That'll create conflicts.

[rijobro]

Hi all,

I've been trying to set one specific version of python. I want to get that to work before going to the general case of letting the user select their python version.

I set EXTRA_BUILD_FLAGS to include PTHON_EXECUTABLE, PYTHON_LIBRARY and PYTHON_INCLUDE_DIR which point towards bin/python2.7, lib/python2.7.dylib and include/python2.7, respectively, in /System/Library/Frameworks/Python.framework/Versions/2.7/.

I also created a variable, PYTHON_EXECUTABLE, so that instead of calling python$MPYVER -m pip, PYTHON_EXECUTABLE -m pip can be used instead.

However, I'm now getting a strange error during the ctests saying: ImportError: No module named matplotlib. This is odd because earlier in the same log, during the installation of matplotlib, there is the following message: Requirement already satisfied: matplotlib in /System/Library/Frameworks/Python.framework/Versions/2.7/Extras/lib/python

I've been trying to remedy this by messing around with pip install to no avail. Any ideas?

The branch I'm working on is here.

[KrisThielemans]

Checked the latest travis log. It seems that @rijobro got passed the python/matplotlib problems with the hard-wired path (great, good starting point!). However, now we have

ImportError: dlopen(/Users/travis/build/CCPPETMR/SIRF-SuperBuild/INSTALL/python/_pygadgetron.so, 2): Library not loaded: libismrmrd.1.3.dylib

  Referenced from: /Users/travis/build/CCPPETMR/SIRF-SuperBuild/INSTALL/python/_pygadgetron.so

  Reason: unsafe use of relative rpath libismrmrd.1.3.dylib in /Users/travis/build/CCPPETMR/SIRF-SuperBuild/INSTALL/python/_pygadgetron.so with restricted binary

This is essentially the same problem as SyneRBI/SIRF-SuperBuild#63. I believe this needs a SIRF modification to the CMakeLists.txt as in https://stackoverflow.com/questions/35377704/dlopen-error-unsafe-use-of-relative-rpath-on-os-x-el-capitan (although it seems to me that CMAKE_BUILD_WITH_INSTALL_RPATH would normally be kept to FALSE). Note also that that post and the CMake wiki set CMAKE_INSTALL_RPATH twice (I would keep only the 2nd one).

Presumably we would then need a similar modification in ISMRMRD and Gadgetron and STIR... One way to achieve this would be to set these in the SuperBuild.cmake and use mark_as_superbuild for all the relevant variables.

(Didn't we have this conversation already?)

[rijobro]

Unfortunately, options such as CMAKE_BUILD_WITH_INSTALL_RPATH and CMAKE_INSTALL_RPATH are depreciated on OSX and no longer have an effect.

I managed to avoid the unsafe use of relative rpath error with CMAKE_INSTALL_NAME_DIR=${CMAKE_INSTALL_PREFIX}/lib, which installs the shared libraries with absolute paths. I put this as an advanced option called SHARED_LIBS_ABS_PATH in SuperBuild.cmake for OSX users (default is OFF). I then passed CMAKE_INSTALL_NAME_DIR to all the CMAKE_ARGS of the necessary External_*.cmake files.

Brew's installation of Boost 1.65 uses rpaths, so caused errors. This can be avoided either by compiling our own version (with SHARED_LIBS_ABS_PATH=ON) or using brew upgrade boost (this gets version 1.66, which uses absolute paths).

Lastly, we had problems of mismatching python versions. I got around this by hard-wiring paths to PYTHON_EXECUTABLE, PYTHON_LIBRARY and PYTHON_INCLUDE_DIR. I then replaced python$MPYVER with PYTHON_EXECUTABLE to ensure everything was installed with the same version of python. This needs some work to make more flexible (or we can just leave it for Travis as it seems to work on real Macs).

[KrisThielemans]

@rijobro, can you merge your SuperBuild changes here to SIRF as well?

[rijobro]

Done, and have pull requested. Travis currently fails because SIRF clones the master SuperBuild which does not have this fix yet.

[codecov]

Codecov Report

Merging #79 into master will increase coverage by 0.97%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master      #79      +/-   ##
==========================================
+ Coverage   54.54%   55.51%   +0.97%     
==========================================
  Files           2        2              
  Lines        1674     1632      -42     
==========================================
- Hits          913      906       -7     
+ Misses        761      726      -35

Impacted Files	Coverage Δ
src/xSTIR/pSTIR/pSTIR.py	61.63% <0%> (+2.25%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 54afc3e...66fbf45. Read the comment docs.

[codecov]

Codecov Report

Merging #79 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master      #79   +/-   ##
=======================================
  Coverage   54.54%   54.54%           
=======================================
  Files           2        2           
  Lines        1674     1674           
=======================================
  Hits          913      913           
  Misses        761      761

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3a4f953...14fe89d. Read the comment docs.

[KrisThielemans]

do we want this? we then will have to remember to remove it later.

[casperdcl]

yes, have it in my local merge, will push when approved