Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SIRF-SuperBuild build fails on Windows #197

Closed
evgueni-ovtchinnikov opened this issue Jan 24, 2019 · 35 comments · Fixed by #452, #453, #454, #456 or #461
Closed

SIRF-SuperBuild build fails on Windows #197

evgueni-ovtchinnikov opened this issue Jan 24, 2019 · 35 comments · Fixed by #452, #453, #454, #456 or #461
Milestone

Comments

@evgueni-ovtchinnikov
Copy link
Contributor

SIRF-SuperBuild build fails on Windows with the error message from CMake:

8>  -- FFTW3 WINDOWS libraries: FFTW3F_LIBRARY-NOTFOUND
8>  CMake Error at C:/Program Files/CMake/share/cmake-3.12/Modules/FindPackageHandleStandardArgs.cmake:137 (message):
8>    Could NOT find FFTW3 (missing: FFTW3F_LIBRARY FFTW3_INCLUDE_DIR)
8>  Call Stack (most recent call first):
8>    C:/Program Files/CMake/share/cmake-3.12/Modules/FindPackageHandleStandardArgs.cmake:378 (_FPHSA_FAILURE_MESSAGE)
8>    C:/Users/wps46139/Documents/GitHub/build/SIRF-SuperBuild/INSTALL/lib/cmake/ISMRMRD/FindFFTW3.cmake:114 (find_package_handle_standard_args)
8>    src/xGadgetron/CMakeLists.txt:31 (find_package)

CMake GUI however displays both 'missing' items with correct values!

@rijobro
Copy link
Contributor

rijobro commented Jan 24, 2019

It looks like the error is coming from SIRF (thus xGadgetron), but I would have guessed that when you say you're looking at the CMake GUI, you're actually looking at the CMake of the superbuild. Is that correct?

If so, it sounds like FFTW3F_LIBRARY isn't being passed from the superbuild to SIRF. You could check this by looking at builds/SIRF/build/CMakeCache.txt and looking for FFTW3F_LIBRARY there.

@rijobro
Copy link
Contributor

rijobro commented Jan 24, 2019

Here are the arguments in External_SIRF.cmake:

    CMAKE_ARGS
        -DCMAKE_PREFIX_PATH=${SUPERBUILD_INSTALL_DIR}
        -DCMAKE_LIBRARY_PATH=${SUPERBUILD_INSTALL_DIR}/lib
        -DCMAKE_INSTALL_PREFIX=${SIRF_Install_Dir}
        -DBOOST_INCLUDEDIR=${BOOST_ROOT}/include/
        -DBOOST_LIBRARYDIR=${BOOST_LIBRARY_DIR}
        -DBOOST_ROOT=${BOOST_ROOT}
        -DMatlab_ROOT_DIR=${Matlab_ROOT_DIR}
        -DMATLAB_ROOT=${Matlab_ROOT_DIR} # pass this for compatibility with old SIRF
        -DMATLAB_DEST_DIR=${MATLAB_DEST_DIR}
        -DSTIR_DIR=${STIR_DIR}
        -DHDF5_ROOT=${HDF5_ROOT}
        -DHDF5_INCLUDE_DIRS=${HDF5_INCLUDE_DIRS}
        -DISMRMRD_DIR=${ISMRMRD_DIR}
        -DSWIG_EXECUTABLE=${SWIG_EXECUTABLE}
        -DPYTHON_EXECUTABLE=${PYTHON_EXECUTABLE}
        -DPYTHON_INCLUDE_DIR=${PYTHON_INCLUDE_DIRS}
        -DPYTHON_LIBRARY=${PYTHON_LIBRARIES}
        -DPYTHON_DEST_DIR=${PYTHON_DEST_DIR}
        -DPYTHON_STRATEGY=${PYTHON_STRATEGY}
        -DNiftyReg_Binary_DIR=${NiftyReg_Binary_DIR}

It doesn't look like FFTW3F_LIBRARY is being passed, so maybe you just need to add it in.

@evgueni-ovtchinnikov
Copy link
Contributor Author

Thanks, it worked!
I added to CMAKE_ARGS

		-DFFTW3_INCLUDE_DIR=${FFTW3_ROOT_DIR}
		-DFFTW3F_LIBRARY=${FFTW3F_LIBRARY}

where

  if (WIN32)
    set(FFTW3F_LIBRARY ${FFTW3_ROOT_DIR}/libfftw3f-3.lib)
  else()
    set(FFTW3F_LIBRARY ${FFTW3_ROOT_DIR}/libfftw3f.so)
  endif()

I hope this will not break anything - any comments anyone?

@rijobro
Copy link
Contributor

rijobro commented Jan 24, 2019

I would have thought that you don't need to set any variables, since these would all be set in External_FFTW3.cmake.

When I'm debugging, I print all available CMake variables like this:

message(STATUS "print all variables...")
get_cmake_property(_variableNames VARIABLES)
list (SORT _variableNames)
foreach (_variableName ${_variableNames})
    message(STATUS "${_variableName}=${${_variableName}}")
endforeach()
message(FATAL_ERROR "all variables printed.")

If I were you, I would put this somewhere in your External_SIRF.cmake file, and check whether FFTW3F_LIBRARY is already correct (it would save you having to do an if/else).

@evgueni-ovtchinnikov
Copy link
Contributor Author

will try later

now i have run into another problem when trying to SuperBuild with DEVEL_BUILD on:

8>  CMake Error at C:/Program Files/CMake/share/cmake-3.12/Modules/FindBoost.cmake:2048 (message):
8>    Unable to find the requested Boost libraries.
8>
8>    Boost version: 1.65.1
8>
8>    Boost include path:
8>    C:/Users/wps46139/Documents/GitHub/build/SIRF-SuperBuild/INSTALL/include/boost-1_65_1
8>
8>
8>    Could not find the following Boost libraries:
8>
8>            boost_system
8>            boost_filesystem
8>            boost_thread
8>            boost_date_time
8>            boost_chrono
8>
8>    Some (but not all) of the required Boost libraries were found.  You may
8>    need to install these additional Boost libraries.  Alternatively, set
8>    BOOST_LIBRARYDIR to the directory containing Boost libraries or BOOST_ROOT
8>    to the location of Boost.
8>  Call Stack (most recent call first):
8>    src/xGadgetron/cGadgetron/CMakeLists.txt:18 (find_package)
8>
8>
8>  CMake Deprecation Warning at C:/Program Files/CMake/share/cmake-3.12/Modules/UseSWIG.cmake:492 (message):
8>    SWIG_ADD_MODULE is deprecated.  Use SWIG_ADD_LIBRARY instead.
8>  Call Stack (most recent call first):
8>    src/xGadgetron/pGadgetron/CMakeLists.txt:37 (SWIG_ADD_MODULE)
8>
8>
8>  CMake Error at C:/Program Files/CMake/share/cmake-3.12/Modules/FindBoost.cmake:2048 (message):
8>    Unable to find the requested Boost libraries.
8>
8>    Boost version: 1.65.1
8>
8>    Boost include path:
8>    C:/Users/wps46139/Documents/GitHub/build/SIRF-SuperBuild/INSTALL/include/boost-1_65_1
8>
8>
8>    Could not find the following Boost libraries:
8>
8>            boost_system
8>            boost_thread
8>            boost_filesystem
8>
8>    Some (but not all) of the required Boost libraries were found.  You may
8>    need to install these additional Boost libraries.  Alternatively, set
8>    BOOST_LIBRARYDIR to the directory containing Boost libraries or BOOST_ROOT
8>    to the location of Boost.
8>  Call Stack (most recent call first):
8>    src/xGadgetron/mGadgetron/CMakeLists.txt:21 (find_package)
8>
8>
8>  -- setup.py:pGadgetron<-sirf.Gadgetron
8>  -- setup.py:pSTIR<-sirf.STIR
8>  -- setup.py:pUtilities<-sirf.Utilities
8>  -- setup.py:pygadgetron<-sirf.pygadgetron
8>  -- setup.py:pystir<-sirf.pystir
8>  -- setup.py:pyiutilities<-sirf.pyiutilities
8>  -- setup.py:C:/Users/wps46139/Documents/GitHub/build/SIRF-SuperBuild/INSTALL/python/setup.py
8>CUSTOMBUILD : CMake error : The following variables are used in this project, but they are set to NOTFOUND.
8>  -- Configuring incomplete, errors occurred!
8>  See also "C:/Users/wps46139/Documents/GitHub/build/SIRF-SuperBuild/builds/SIRF/build/CMakeFiles/CMakeOutput.log".
8>  See also "C:/Users/wps46139/Documents/GitHub/build/SIRF-SuperBuild/builds/SIRF/build/CMakeFiles/CMakeError.log".
8>  Please set them or make sure they are set and tested correctly in the CMake files:
8>  Boost_CHRONO_LIBRARY_DEBUG (ADVANCED)
8>      linked by target "cstir" in directory C:/Users/wps46139/Documents/GitHub/build/SIRF-SuperBuild/sources/SIRF/src/xSTIR/cSTIR
8>  Boost_CHRONO_LIBRARY_RELEASE (ADVANCED)
8>      linked by target "cstir" in directory C:/Users/wps46139/Documents/GitHub/build/SIRF-SuperBuild/sources/SIRF/src/xSTIR/cSTIR
8>  Boost_DATE_TIME_LIBRARY_DEBUG (ADVANCED)
8>      linked by target "cstir" in directory C:/Users/wps46139/Documents/GitHub/build/SIRF-SuperBuild/sources/SIRF/src/xSTIR/cSTIR
8>  Boost_DATE_TIME_LIBRARY_RELEASE (ADVANCED)
8>      linked by target "cstir" in directory C:/Users/wps46139/Documents/GitHub/build/SIRF-SuperBuild/sources/SIRF/src/xSTIR/cSTIR
8>  Boost_FILESYSTEM_LIBRARY_DEBUG (ADVANCED)
8>      linked by target "cstir" in directory C:/Users/wps46139/Documents/GitHub/build/SIRF-SuperBuild/sources/SIRF/src/xSTIR/cSTIR
8>  Boost_FILESYSTEM_LIBRARY_RELEASE (ADVANCED)
8>      linked by target "cstir" in directory C:/Users/wps46139/Documents/GitHub/build/SIRF-SuperBuild/sources/SIRF/src/xSTIR/cSTIR
8>  Boost_SYSTEM_LIBRARY_DEBUG (ADVANCED)
8>      linked by target "cstir" in directory C:/Users/wps46139/Documents/GitHub/build/SIRF-SuperBuild/sources/SIRF/src/xSTIR/cSTIR
8>  Boost_SYSTEM_LIBRARY_RELEASE (ADVANCED)
8>      linked by target "cstir" in directory C:/Users/wps46139/Documents/GitHub/build/SIRF-SuperBuild/sources/SIRF/src/xSTIR/cSTIR
8>  Boost_THREAD_LIBRARY_DEBUG (ADVANCED)
8>      linked by target "cstir" in directory C:/Users/wps46139/Documents/GitHub/build/SIRF-SuperBuild/sources/SIRF/src/xSTIR/cSTIR
8>  Boost_THREAD_LIBRARY_RELEASE (ADVANCED)
8>      linked by target "cstir" in directory C:/Users/wps46139/Documents/GitHub/build/SIRF-SuperBuild/sources/SIRF/src/xSTIR/cSTIR
8>
9>------ Skipped Build: Project: ALL_BUILD, Configuration: Release x64 ------
9>Project not selected to build for this solution configuration 

BOOST_LIBRARYDIR was set as suggested (to Boost/bin.v2/libs)

@rijobro
Copy link
Contributor

rijobro commented Jan 24, 2019

Not sure for Windows, but try setting BOOST_ROOT to C:/Users/wps46139/Documents/GitHub/build/SIRF-SuperBuild/INSTALL/. If you can't seen an option for this in your CMake GUI, then try:

cmake . -DBOOST_ROOT=C:/Users/wps46139/Documents/GitHub/build/SIRF-SuperBuild/INSTALL/

@evgueni-ovtchinnikov
Copy link
Contributor Author

no, it does not help, same error message

@rijobro
Copy link
Contributor

rijobro commented Jan 24, 2019

Well again, this looks like a SIRF error (not superbuild), so check the value of BOOST_ROOT in builds/SIRF/build/CMakeCache.txt.

@KrisThielemans
Copy link
Member

KrisThielemans commented Jan 24, 2019

I never succeeded to get CMake to correctly find a Boost version that we compile. It's a very complicated moving target with boost renaming libraries in the build schemes, and CMake trying to catch-up.

I very much recommend to install the Boost binaries, and set USE_SYSTEM_BOOST=ON.

@KrisThielemans
Copy link
Member

KrisThielemans commented Jan 25, 2019

Regarding the missing FFTW stuff, @evgueni-ovtchinnikov can you give more information on how you're running this? If USE_SYSTEM_FFTW3=ON, see #147.

Otherwise, I'm surprised as it searches for the library with find_library , see for example here. I thought this would go and search in our install directory., but I admit that I cannot see anything related to that in the find_library doc (but If it doesn't, how come USE_SYSTEM_FFTW3=OFF works apparently on the VM?)

@KrisThielemans
Copy link
Member

See #7 for other remaining issues with the Windows build. Note however that I succesfully build on Windows 10 with CMake 3.13.2 with USE_SYSTEM_BOOST=ON and USE_SYSTEM_FFTW3=ON

@evgueni-ovtchinnikov
Copy link
Contributor Author

same problem with boost on windows 7

all boost libraries are actually present in INSTALL/lib, but there are no respective dlls in INSTALL/bin, which is a likely reason for the error messages

moreover, there are no boost dlls in build/SIRF-SuperBuild at all

@rijobro
Copy link
Contributor

rijobro commented Jan 31, 2019

Surely all the dlls belong in INSTALL/lib, and not in INSTALL/bin, so it's normal that there aren't any dlls in INSTALL/bin?

@KrisThielemans
Copy link
Member

Please do not waste time trying to set USE_SYSTEM_BOOST=OFF on Windows. It's going to be a struggle. Instead, install the boost binaries, set the BOOST_ROOT environment (or CMake) variable.

@evgueni-ovtchinnikov
Copy link
Contributor Author

no, it does not help, same error message

@evgueni-ovtchinnikov
Copy link
Contributor Author

evgueni-ovtchinnikov commented Feb 1, 2019

installed boost_1_65_1, boost is ok, back to fftw3 problem:

 -- FFTW3 WINDOWS libraries: FFTW3F_LIBRARY-NOTFOUND
10>  CMake Error at C:/Program Files/CMake/share/cmake-3.13/Modules/FindPackageHandleStandardArgs.cmake:137 (message):
10>    Could NOT find FFTW3 (missing: FFTW3F_LIBRARY FFTW3_INCLUDE_DIR)
10>  Call Stack (most recent call first):
10>    C:/Program Files/CMake/share/cmake-3.13/Modules/FindPackageHandleStandardArgs.cmake:378 (_FPHSA_FAILURE_MESSAGE)
10>    C:/Users/wps46139/Documents/GitHub/build/SIRF-SuperBuild/INSTALL/lib/cmake/ISMRMRD/FindFFTW3.cmake:114 (find_package_handle_standard_args)
10>    src/xGadgetron/CMakeLists.txt:36 (find_package)

cmake shows correct values for FFTW3F_LIBRARY and FFTW3_INCLUDE_DIR

@KrisThielemans
Copy link
Member

This commit makes little sense to me, given the discussion in #147. Maybe I was incorrect there.

In any case,

  • if USE_SYSTEM_FFTW3=OFF, those variables aren't set.
  • it would need similar fixes for ISMRMRD and Gadgetron presumably.

Can we do this via a PR please?

@rijobro
Copy link
Contributor

rijobro commented Feb 6, 2019

This re-closed itself automatically, not sure why.

@rijobro rijobro reopened this Feb 6, 2019
@casperdcl
Copy link
Member

because

75b7f6c Revert "added missing CMake FFTW variables to External_SIRF.cmake, fixes #197 (together with installing boost_1_65_1)"

includes the text

fixes #197

github being too clever...

@rijobro
Copy link
Contributor

rijobro commented Feb 6, 2019

ah... Thanks!

@evgueni-ovtchinnikov
Copy link
Contributor Author

my standalone SIRF has CMake option SIRF_INSTALL_DEPENDENCIES, which forces copying boost dlls to INSTALL/bin, whereas SIRF-SuperBuild does not have one, and Python and Matlab fail to load SIRF Gadgetron libraries

@evgueni-ovtchinnikov
Copy link
Contributor Author

Matlab fails with the error message

    'There was an error loading the library "C:\Users\wps46139\Documents\GitHub\build\SIRF-SuperBuild\INSTALL\matlab\mgadgetron.mexw64"
     
     Missing symbol 'H5P_CLS_DATASET_CREATE_ID_g' in 'C:\Program Files\MATLAB\R2017a/bin/win64\hdf5.dll' required by 'C:\Users\wps46139\Documents\GitHub\build\SIRF-SuperBuild\INSTALL\bin\ismrmrd.dll->C:\Users\wps46139\Documents\GitHub\build\SIRF-SuperBuild\INSTALL\matlab\mgadgetron.mexw64'
     Missing symbol 'H5P_CLS_LINK_CREATE_ID_g' in 'C:\Program Files\MATLAB\R2017a/bin/win64\hdf5.dll' required by 'C:\Users\wps46139\Documents\GitHub\build\SIRF-SuperBuild\INSTALL\bin\ismrmrd.dll->C:\Users\wps46139\Documents\GitHub\build\SIRF-SuperBuild\INSTALL\matlab\mgadgetron.mexw64''

apparently there is a conflict between HDF5 libraries used by SuperBuild and matlab's ones

@evgueni-ovtchinnikov
Copy link
Contributor Author

overwriting matlab hdf5 dlls (luckily, there are only two of them) does fix the problem, but it is hardly a solution many users would like :(

@casperdcl
Copy link
Member

could prefix to matlab's path. Can also contact matlab to get them to update the dlls (I've done this before)

@KrisThielemans
Copy link
Member

This is the same problem as what we're having with Boost https://github.com/CCPPETMR/SIRF/issues/240.

Like there, I think the best thing to do is to build SIRF with the same version as what is used by Matlab. That implies that we need to check what the HDF5 version is for different Matlab versions. Fun.

@evgueni-ovtchinnikov would you be able to send an email to Mathworks support requesting the relevant information (for both HDF5 and Boost). Best to explain them why. feel free to point them to this issue.

@KrisThielemans KrisThielemans added this to the v2.1 milestone Feb 15, 2019
@KrisThielemans
Copy link
Member

HDF5 info is in #208, Boost info is in #207

@KrisThielemans
Copy link
Member

KrisThielemans commented Feb 23, 2019

Proposed FFTW fix is in #204

@KrisThielemans KrisThielemans modified the milestones: v2.1, v2.2 Oct 6, 2019
@KrisThielemans
Copy link
Member

KrisThielemans commented Dec 21, 2020

I've tried this again with latest versions.

  • I had to update the HDF5 MD5SUM for some reason (will create a PR for this). When building HDF5, VS threw up a few dialog boxes with errors. These were presumably when CMake tests some functionality. Ignoring them, or Aborting them worked fine.
  • error in the unittests for JSON (fixed by upgrading Visual Studio and JSON library, PR set JSON to 3.9.1 and don't build tests by defaults #454 )
  • STIR getopt.c complained about non-existing strings.h (Note: this disappeared after upgrading my VS 2019 to latest version)
  • some failures with ISMRMRD

I'll update this when I find more

@KrisThielemans
Copy link
Member

KrisThielemans commented Dec 21, 2020

Screenshot of error dialog when building HDF5. I'm guessing CMake really should execute this silently, but doesn't. (I have CMake 3.18.3)
image
This happens after

1>-- Checking for appropriate format for 64 bit long:

before every check like this

1>Width with l64 failed with result: 3
1>Width with l failed with result: 3
1>Width with L failed with result: 3
1>Width with q failed with result: 3

@KrisThielemans
Copy link
Member

I get warnings when building STIR via the SIRF-SuperBuild:

Warning	C4530	C++ exception handler used, but unwind semantics are not enabled. Specify /EHsc [C:\Users\krisf\Documents\devel\buildVC\SIRF-SuperBuild\builds\STIR\build\src\spatial_transformation_buildblock\spatial_transformation_buildblock.vcxproj]

Clearly, /EHsc needs to be passed, but when I build STIR directly with CMake, I don't need to do anything. Why does this happen for the SuperBuild?

@KrisThielemans
Copy link
Member

I get warnings when building STIR via the SIRF-SuperBuild:

Warning	C4530	C++ exception handler used, but unwind semantics are not enabled. Specify /EHsc [C:\Users\krisf\Documents\devel\buildVC\SIRF- SuperBuild\builds\STIR\build\src\spatial_transformation_buildblock\spatial_transformation_buildblock.vcxproj]

resolved this with 75281a5

@KrisThielemans
Copy link
Member

I've resolved all the above problems now. SIRF builds without any problems with the SIRF-SuperBuild!

Sadly, all tests fail...

@KrisThielemans
Copy link
Member

Tests failed due to path issues. I also needed SyneRBI/SIRF#840.

Setting the paths as in #460, all tests work!

C:\Users\krisf\Documents\devel\cmake\install\bin\ctest.exe -VVV -C RelWithDebInfo          Test project C:/Users/krisf/Documents/devel/buildVC/SIRF-SuperBuild/builds/SIRF/build
    Start 1: MR_TESTS_CPLUSPLUS
1/7 Test #1: MR_TESTS_CPLUSPLUS ...............   Passed    7.11 sec
    Start 2: MR_TESTS_MATLAB
2/7 Test #2: MR_TESTS_MATLAB ..................   Passed   27.02 sec
    Start 3: REG_TEST_CPLUSPLUS
3/7 Test #3: REG_TEST_CPLUSPLUS ...............   Passed   37.77 sec
    Start 4: REG_TEST_MATLAB
4/7 Test #4: REG_TEST_MATLAB ..................   Passed   25.18 sec
    Start 5: REG_TEST_MATLAB_SPM
5/7 Test #5: REG_TEST_MATLAB_SPM ..............   Passed   26.98 sec
    Start 6: PET_TESTS_CPLUSPLUS
6/7 Test #6: PET_TESTS_CPLUSPLUS ..............   Passed  156.87 sec
    Start 7: PET_TESTS_MATLAB
7/7 Test #7: PET_TESTS_MATLAB .................   Passed   15.91 sec

100% tests passed, 0 tests failed out of 7

You'll notice though that these were with MATLAB only as I currently don't have Python on my Windows laptop.

I'm 99% sure that I will have to disable MATLAB when building with Python and vice versa due to HDF5 version conflicts etc, but at least things are looking up!

@KrisThielemans
Copy link
Member

I have now tested this with miniconda's Python 3.8. I've put in a small commits but it all works for me, although I do need .SyneRBI/SIRF#844 as well. Note that I did make a separate build for Python.

@KrisThielemans
Copy link
Member

So, I believe i can close this now. There's 2 SIRF PRs pending, but as far as the SuperBuild goes, all is well.

Some info on my test environment (all very new):

  • Visual Studio 2019 version 16.8.3
  • CMake 3.19.2
  • MATLAB 2020b
  • Python 3.8 (via miniconda), and using https://github.com/SyneRBI/SIRF/wiki/SIRF-and-Python#using-anaconda (except that I didn't install h5py and libxml2. Note that h5py could potentially create conflicts with our own HDF5 build)
  • precompiled Boost 1.74.0
  • no FFT nor HDF5 libraries preinstalled

CMake Configuration:

  • STIR_TAG=release_4
  • SIRF_TAG=origin/master
  • BOOST_ROOT=c:\local\boost_1_74_0
  • either DISABLE_MATLAB or DISABLE_PYTHON=ON

run-time environment was as in #460

I built the RelWithDebInfo configuration. Debug worked for MATLAB, but wouldn't work for Python due to SyneRBI/SIRF#42

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment