fix travis for osx builds
https://github.com/CCPPETMR/SIRF/pull/79
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.
@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.
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.
Thanks for looking into this. It confirms what I had said during the earlier tcons. You can see the mismatched bin amd lib versions (2.7.10 and 2.7.14 from what I remember) in the cmake output on travis. What I had requested is of anyone knows of a way of installing matching bin & lib versions on osx. AFAIK travis doesn't have matched versions. I'm tempted to just go with anaconda...
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.
we've briefly looked at this. we're still using python-config
to fix PYTHON_LIBRARY
presumably. That'll create conflicts.
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.
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 CCPPETMR/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?)
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).
@rijobro, can you merge your SuperBuild changes here to SIRF as well?
Done, and have pull requested. Travis currently fails because SIRF clones the master SuperBuild which does not have this fix yet.
Merging #79 into master will increase coverage by
0.97%
.
The diff coverage isn/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.
Merging #79 into master will increase coverage by
0.97%
.
The diff coverage isn/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.
—
@casperdcl pushed 1 commit.
—
You are receiving this because you are subscribed to this thread.
View it on GitHub or mute the thread.
@KrisThielemans commented on this pull request.
In .travis.yml:
> @@ -143,12 +181,12 @@ before_install: - export BUILD_FLAGS="$BUILD_FLAGS -DPYVER=$PYMVER -DSIRF_URL=$PWD -DSIRF_TAG=$TRAVIS_COMMIT" # get SuperBuild - cd .. - - git clone https://github.com/CCPPETMR/SIRF-SuperBuild --recursive -b master + - git clone https://github.com/CCPPETMR/SIRF-SuperBuild --recursive -b OSX_abs_paths
do we want this? we then will have to remember to remove it later.
@casperdcl commented on this pull request.
In .travis.yml:
> @@ -143,12 +181,12 @@ before_install: - export BUILD_FLAGS="$BUILD_FLAGS -DPYVER=$PYMVER -DSIRF_URL=$PWD -DSIRF_TAG=$TRAVIS_COMMIT" # get SuperBuild - cd .. - - git clone https://github.com/CCPPETMR/SIRF-SuperBuild --recursive -b master + - git clone https://github.com/CCPPETMR/SIRF-SuperBuild --recursive -b OSX_abs_paths
yes, have it in my local merge, will push when approved
@KrisThielemans approved this pull request.