Get OSX to compile on Travis.
https://github.com/CCPPETMR/SIRF-SuperBuild/pull/80
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.
@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)
@rijobro pushed 1 commit.
—
You are receiving this because you are subscribed to this thread.
View it on GitHub or mute the thread.
@rijobro pushed 1 commit.
—
You are receiving this because you are subscribed to this thread.
View it on GitHub or mute the thread.
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
@rijobro pushed 1 commit.
—
You are receiving this because you are subscribed to this thread.
View it on GitHub or mute the thread.
@rijobro pushed 1 commit.
—
You are receiving this because you are subscribed to this thread.
View it on GitHub or mute the thread.
changes made. there are currently 3 matrices, all of which build successfully.
closes #72
@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:
The rest lgtm
@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
@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
@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
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.
@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.
@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.
@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).
Merged #80.