[CCPPETMR/SIRF-SuperBuild] Travis OSX (#80)
Richard Brown
Get OSX to compile on Travis.
- Use absolute paths
- Give full paths to python executable, library and include directory for either python2 or python3.\
- Upgrade to boost 1.66
You can view, comment on, or merge this pull request online at:
https://github.com/CCPPETMR/SIRF-SuperBuild/pull/80
Commit Summary
- Give the advanced choice to OSX users to compile shared libraries with absolute paths.
- Try updating boost to 1.66
- Try forcing the paths to one version of python
- forgot $
- Force version of python (2.7)
- split up the line that errors
- mess around with pip install
- still messing with pip install
- only-binary for pip install
- more pip install
- numpy mispelt
- Comment out `find_package(PythonInterp) etc. in SuperBuild.cmake`
- Mess with PythonInterp
- update boost to 1.66 to avoid rpath error
- USE_SYSTEM_Boost=ON
- Try compiling using absolute and rpaths (we know that asbolute paths should work)
- Try using python3 too
- try python2 and 3
- keep trying
- keep on going
- keep on keepin on
- brew install python3
- Resolved merge conflicts
- Try updating boost to 1.66
- Try forcing the paths to one version of python
- forgot $
- Force version of python (2.7)
- split up the line that errors
- mess around with pip install
- still messing with pip install
- only-binary for pip install
- more pip install
- numpy mispelt
- Comment out `find_package(PythonInterp) etc. in SuperBuild.cmake`
- Mess with PythonInterp
- update boost to 1.66 to avoid rpath error
- USE_SYSTEM_Boost=ON
- Try compiling using absolute and rpaths (we know that asbolute paths should work)
- Try using python3 too
- try python2 and 3
- keep trying
- keep on going
- keep on keepin on
- brew install python3
- Rebase OSX to master. Make sure it works for linux too
- Merge branch 'OSX_abs_paths' of https://github.com/CCPPETMR/SIRF-SuperBuild into OSX_abs_paths
- rebase
- rebase
- rebase
File Changes
- M .travis.yml (46)
- M SuperBuild.cmake (36)
- M SuperBuild/External_Boost.cmake (1)
- M SuperBuild/External_HDF5.cmake (6)
- M SuperBuild/External_ISMRMRD.cmake (1)
Patch Links:
- https://github.com/CCPPETMR/SIRF-SuperBuild/pull/80.patch
- https://github.com/CCPPETMR/SIRF-SuperBuild/pull/80.diff
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.
Kris Thielemans
@KrisThielemans requested changes on this pull request.
small changes
In SuperBuild.cmake:
> @@ -41,6 +41,15 @@ if(CMAKE_INSTALL_PREFIX_INITIALIZED_TO_DEFAULT)
set(CMAKE_INSTALL_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/INSTALL" CACHE PATH "Prefix for path for installation" FORCE)
endif()
+# If OSX give the advanced option to use absolute paths for shared libraries
+if (APPLE)
+ option(SHARED_LIBS_ABS_PATH "Force shared libraries to be installed with absolute paths (as opposed to rpaths)" OFF)
shouldn't we default this to ON?
In SuperBuild.cmake:
> @@ -59,26 +68,35 @@ mark_as_superbuild( # Attempt to make Python settings consistent set(PYVER 0 CACHE STRING "Python version") if(PYVER EQUAL 0) - find_package(PythonInterp) + find_package(PythonInterp)
any chance to get rid of trivial white-space changes?
Obviously we'll need to enforce this later.
In SuperBuild/External_Boost.cmake:
> @@ -56,6 +56,7 @@ if(NOT ( DEFINED "USE_SYSTEM_${externalProjName}" AND "${USE_SYSTEM_${externalPr
${CLANG_ARG}
-DBUILD_DIR:PATH=${CMAKE_CURRENT_BINARY_DIR}/${proj}
-DBOOST_INSTALL_DIR:PATH=${Boost_Install_Dir}
+ -DCMAKE_INSTALL_NAME_DIR=${CMAKE_INSTALL_NAME_DIR}
as opposed do doing this, it should work to add CMAKE_INSTALL_NAME_DIR to the list of variables in mark_as_superbuild. Otherwise we need to do this everywhere. It should mean we can revert the `External*.cmake' files.
In SuperBuild.cmake:
> endif()
+message("\nRichard message\n")
delete this and next few lines :-) (merge with L87)
Richard Brown
@rijobro pushed 1 commit.
- 8e9ffc1 minor changes
—
You are receiving this because you are subscribed to this thread.
View it on GitHub or mute the thread.
Richard Brown
@rijobro pushed 1 commit.
- 73e62a3 Get absolute path to install dir
—
You are receiving this because you are subscribed to this thread.
View it on GitHub or mute the thread.
Kris Thielemans
failure related to ismrmrd.dylib again on only 1 of the jobs (which is quite weird). I guess need to check if the mark_as_superbuild trick worked. Could cat the SIRF-prefix/..../SIRF-build/CMakeCache.txt I guess. Presumably should have added something to the CMAKE*FLAGS
Richard Brown
@rijobro pushed 1 commit.
- 7b6e9e9 cat the CMakeCache.txt (and see if python3 works with rpaths)
—
You are receiving this because you are subscribed to this thread.
View it on GitHub or mute the thread.
Richard Brown
@rijobro pushed 1 commit.
- 7891679 correct typo
—
You are receiving this because you are subscribed to this thread.
View it on GitHub or mute the thread.
Richard Brown
changes made. there are currently 3 matrices, all of which build successfully.
- python2, abs_paths
- python3, abs_paths
- python3, rpaths
So it looks like python3 doesn't need absolute paths, whereas python2 does.
Casper da Costa-Luis
closes #72
Casper da Costa-Luis
@KrisThielemans @rijobro I squashed all the commits (diff from master) into one commit (26a201b) attributed to @rijobro, then synced .travis.yml with SIRF#79 (osx-travis). What needs reviewing is:
- README.md
- SuperBuild/External_STIR.cmake
- version_config.cmake
The rest lgtm
Kris Thielemans
@KrisThielemans requested changes on this pull request.
This seems strange. Lots of things on master are not here. I've tried (locally) to merge master onto here but most of the new changes then didn't make it. Possibly I did something wrong, or possibly something went wrong with the squashing of previous commits. Makes me rather wary... @casperdcl, do you understand what's going on?
In .travis.yml:
> env: global: - - BUILD_FLAGS="-DCMAKE_BUILD_TYPE=Release" + - BUILD_FLAGS="-DCMAKE_BUILD_TYPE=Release -DUSE_SYSTEM_Boost=ON"
not sure why we want this for all builds. I'd prefer to have one on Linux and one on Mac with "our" Boost build. I'm not 100% sure if it'll work on Mac now though.
In README.md:
> @@ -5,46 +5,22 @@ needed to compile SIRF and Gadgetron, and automatically build SIRF and Gadgetron There is still a small number of libraries that are not installed by the SuperBuild, [see below for more info for your operating system](#os-specific-information). -## Table of Contents
why are we removing this?
In README.md:
> - 3. [Installation instructions for Docker](#Docker_installation) -5. [Advanced installation](#Advanced_installation) - 1. [SIRF and MATLAB](#SIRF_and_MATLAB) - 2. [Compiling against your own packages](#Compiling_own_packages) -6. [TODO](#TODO) - -## Dependencies <a name="Dependencies"></a> - -The SuperBuild depends on CMake >= 3.7. - -If you are building Gadgetron there are a series of [additional dependencies](https://github.com/gadgetron/gadgetron/wiki/List-of-Dependencies), which must be met. - -## Generic instructions <a name="Generic_instructions"></a> +## Dependencies + +The superBuild depends on CMake >= 3.7.
good to add, but we do need to point somewhere for Gadgetron dependencies. Those are now on our own Wiki pages as well.
In README.md:
> They can be found [here](https://github.com/CCPPETMR/SIRF/wiki/SIRF-SuperBuild-on-Docker) -## Advanced installation <a name="Advanced_installation"></a> - -### SIRF and MATLAB <a name="SIRF_and_MATLAB"></a> -SIRF can be used from MATLAB. For more information on this, see our [SIRF and MATLAB page](SIRF-and-MATLAB). - -### Compiling against your own packages <a name="Compiling_own_packages"></a>
why is this removed?
> @@ -35,7 +35,7 @@ set(Armadillo_MD5 c601f3a5ec6d50666aa3a539fa20e6ca ) if(WIN32) # Just use precompiled version # TODO would prefer the next zip file but for KT using an ftp URL times-out (firewall?) - set(FFTW3_URL https://www.ccppetmr.ac.uk/sites/www.ccppetmr.ac.uk/files/downloads/fftw-3.3.5-dll64.zip ) + set(FFTW3_URL ftp://ftp.fftw.org/pub/fftw/fftw-3.3.5-dll64.zip )
this is another unwanted revert
In SuperBuild/External_STIR.cmake:
> @@ -54,6 +54,7 @@ if(NOT ( DEFINED "USE_SYSTEM_${externalProjName}" AND "${USE_SYSTEM_${externalPr
-DCMAKE_INSTALL_PREFIX=${STIR_Install_Dir}
-DGRAPHICS=None
-DCMAKE_CXX_STANDARD=11
+ -DDISABLE_ITK=On
unwanted revert
Casper da Costa-Luis
@KrisThielemans yes I thought it was strange too which is why I asked... there were quite a few merge commits & commits with 'rebase' commented which I guess was @rijobro having a little difficulty keeping in sync with master. I ultimately only squashed into a single commit (aka setting up a squash-merge, guaranteeing that I didn't change the effect of merging) because I couldn't rebase without loads of conflicts. I guess @rijobro you could
git checkout OSX_abs_path
git fetch
git reset --hard origin/OSX_abs_path
and then manually revert the bits of 26a201b which you don't recognise
Casper da Costa-Luis
@casperdcl commented on this pull request.
In .travis.yml:
> env: global: - - BUILD_FLAGS="-DCMAKE_BUILD_TYPE=Release" + - BUILD_FLAGS="-DCMAKE_BUILD_TYPE=Release -DUSE_SYSTEM_Boost=ON"
it was in all the builds (incl the mac build fixes) so I put it into the global one
Kris Thielemans
yeah well. too scary for me. I'd rather create a new branch from master and fix only .travis.yml and SuperBuild.cmake. The rest should not change. (I'm not sure why External_HDF5.cmake was changed the way it was).
Seems that the .travis.yml on CCPPETMR/SIRF#79 is almost what we need here as well.
Kris Thielemans
@KrisThielemans commented on this pull request.
In .travis.yml:
> env: global: - - BUILD_FLAGS="-DCMAKE_BUILD_TYPE=Release" + - BUILD_FLAGS="-DCMAKE_BUILD_TYPE=Release -DUSE_SYSTEM_Boost=ON"
ok. I've created #84 to fix this later.
Richard Brown
@rijobro commented on this pull request.
In .travis.yml:
> env: global: - - BUILD_FLAGS="-DCMAKE_BUILD_TYPE=Release" + - BUILD_FLAGS="-DCMAKE_BUILD_TYPE=Release -DUSE_SYSTEM_Boost=ON"
should USE_SYSTEM_Boost=OFF should work fine for Macs. Just put it in to speed up compilation whilst I was debugging.
Richard Brown
@rijobro commented on this pull request.
I changed External_HDF5.cmake when I was adding CMAKE_INSTALL_NAME_DIR to all the dependencies (before I was aware of mark_as_superbuild. I split the options onto separate lines as, with CMAKE_INSTALL_NAME_DIR there was too much to put on one line. You can leave this as it was.
In README.md:
> @@ -5,46 +5,22 @@ needed to compile SIRF and Gadgetron, and automatically build SIRF and Gadgetron There is still a small number of libraries that are not installed by the SuperBuild, [see below for more info for your operating system](#os-specific-information). -## Table of Contents
Not sure what's happened here. Maybe I did something wrong with the merge because I didn't edit this file on this branch. I would ignore all changes to the README.md file (especially as it was me that updated it on the master).
Casper da Costa-Luis
Merged #80.