tbeu / matio

MATLAB MAT File I/O Library
https://matio.sourceforge.io
BSD 2-Clause "Simplified" License
330 stars 97 forks source link

Add CMake build system #107

Closed JohanMabille closed 4 years ago

JohanMabille commented 5 years ago

Closes #53 #74 #75

massich commented 5 years ago

To replicate the build in linux ubuntu/debian

SRC_DIR=/home/sik/repos/matio
BUILD_DIR=/home/sik/Workspace/matio/build_linux
INSTALL_DIR=$BUILD_DIR/simulate_install_dir

cd $BUILD_DIR
rm $BUILD_DIR/* -Rf
cmake $SRC_DIR \
      -DCMAKE_INSTALL_PREFIX=$INSTALL_DIR

cmake --build . -- -j 8
make install

It works when hdf5 is installed using apt-get: sudo apt-get install hdf5-tools libhdf5-dev hdf5-helpers and/or conda: conda install hdf5

To run it in windows @JohanMabille and I we tested only using gitbash and conda with the following lines:

cmake -G "NMake Makefiles" -DCMAKE_INSTALL_PREFIX=%MINICONDA%/Library -DCMAKE_PREFIX_PATH=%MINICONDA%/Library/cmake ..

nmake matdump

(be aware that the windows snipped might have some typo since I didn't copy paste it)

tbeu commented 5 years ago

I believe adding the tests is optional and agree that it is beyond the scope of this PR adding CMake support for the libmatio build.

Nelson-numerical-software commented 5 years ago

How to build a dynamic library ? Current cmake configuration seems to generate only a static library

debian9:/tmp/matio$ sudo make install
[ 10%] Built target getopt
[ 70%] Built target matio
[ 80%] Built target matdump
[ 90%] Built target test_snprintf
[100%] Built target test_mat
Install the project...
-- Install configuration: "Release"
-- Up-to-date: /usr/local/lib/libmatio.a
-- Up-to-date: /usr/local/include/matio.h
-- Up-to-date: /usr/local/include/matio_pubconf.h
-- Up-to-date: /usr/local/include/matio_export.h
tbeu commented 5 years ago

There are typos in CheckHeaderSTDC.cmake: Cheking -> Checking

tbeu commented 5 years ago

Also works with CMake 3.6.2 on Cygwin:

laptop ~
$ cd /cygdrive/c/Projects/

laptop /cygdrive/c/Projects
$ git clone https://github.com/JohanMabille/matio --branch cmake matio-cmake-cygwin
Klone nach 'matio-cmake-cygwin' ...
remote: Enumerating objects: 37, done.
remote: Counting objects: 100% (37/37), done.
remote: Compressing objects: 100% (33/33), done.
remote: Total 6641 (delta 9), reused 28 (delta 4), pack-reused 6604
Empfange Objekte: 100% (6641/6641), 3.44 MiB | 686.00 KiB/s, Fertig.
Löse Unterschiede auf: 100% (4923/4923), Fertig.
Checke Dateien aus: 100% (490/490), Fertig.

laptop /cygdrive/c/Projects
$ cd matio-cmake-cygwin/

laptop /cygdrive/c/Projects/matio-cmake-cygwin
$ sed -i 's/ 3.9/ 3.6.2/' CMakeLists.txt

laptop /cygdrive/c/Projects/matio-cmake-cygwin
$ cmake --version
cmake version 3.6.2

CMake suite maintained and supported by Kitware (kitware.com/cmake).

laptop /cygdrive/c/Projects/matio-cmake-cygwin
$ cmake -G "Unix Makefiles" ./
-- The C compiler identification is GNU 7.4.0
-- The CXX compiler identification is GNU 7.4.0
-- Check for working C compiler: /usr/bin/cc
-- Check for working C compiler: /usr/bin/cc -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Detecting C compile features
-- Detecting C compile features - done
-- Check for working CXX compiler: /usr/bin/c++.exe
-- Check for working CXX compiler: /usr/bin/c++.exe -- works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Detecting CXX compile features
-- Detecting CXX compile features - done
 DEBUG Enabled
 EXTENDED_SPARSE Enabled
 MAT73 Enabled
 PROFILE Enabled
-- Looking for pthread.h
-- Looking for pthread.h - found
-- Looking for pthread_create
-- Looking for pthread_create - found
-- Found Threads: TRUE
-- Found HDF5: /usr/lib/libhdf5_cpp.dll.a;/usr/lib/libhdf5.dll.a (found version "1.8.20")
-- Found ZLIB: /usr/lib/libz.dll.a (found version "1.2.11")
-- Looking for sys/types.h
-- Looking for sys/types.h - found
-- Looking for stdint.h
-- Looking for stdint.h - found
-- Looking for stddef.h
-- Looking for stddef.h - found
-- Check size of char
-- Check size of char - done
-- Check size of double
-- Check size of double - done
-- Check size of float
-- Check size of float - done
-- Check size of int
-- Check size of int - done
-- Check size of long
-- Check size of long - done
-- Check size of long long
-- Check size of long long - done
-- Check size of short
-- Check size of short - done
-- Check size of size_t
-- Check size of size_t - done
-- Looking for vsnprintf
-- Looking for vsnprintf - found
-- Looking for snprintf
-- Looking for snprintf - found
-- Looking for vasprintf
-- Looking for vasprintf - found
-- Looking for asprintf
-- Looking for asprintf - found
-- Looking for strcasecmp
-- Looking for strcasecmp - found
-- Looking for getopt
-- Looking for getopt - found
-- Looking for inttypes.h
-- Looking for inttypes.h - found
-- Looking for strings.h
-- Looking for strings.h - found
-- Looking for ctype.h
-- Looking for ctype.h - found
-- Looking for stdlib.h
-- Looking for stdlib.h - found
-- Looking for string.h
-- Looking for string.h - found
-- Looking for stdarg.h
-- Looking for stdarg.h - found
-- Cheking for ANSI C header files
-- Cheking for ANSI C header files - found
-- Check size of uint8_t
-- Check size of uint8_t - done
-- Check size of uint16_t
-- Check size of uint16_t - done
-- Check size of uint32_t
-- Check size of uint32_t - done
-- Check size of uint64_t
-- Check size of uint64_t - done
-- Check size of int8_t
-- Check size of int8_t - done
-- Check size of int16_t
-- Check size of int16_t - done
-- Check size of int32_t
-- Check size of int32_t - done
-- Check size of int64_t
-- Check size of int64_t - done
-- Performing Test COMPILER_HAS_DEPRECATED_ATTR
-- Performing Test COMPILER_HAS_DEPRECATED_ATTR - Success
-- Configuring done
-- Generating done
-- Build files have been written to: /cygdrive/c/Projects/matio-cmake-cygwin

laptop /cygdrive/c/Projects/matio-cmake-cygwin
$ make
Scanning dependencies of target getopt
[  5%] Building C object CMakeFiles/getopt.dir/getopt/getopt_long.c.o
[ 10%] Linking C static library libgetopt.a
[ 10%] Built target getopt
Scanning dependencies of target matio
[ 15%] Building C object CMakeFiles/matio.dir/src/endian.c.o
[ 20%] Building C object CMakeFiles/matio.dir/src/mat.c.o
/cygdrive/c/Projects/matio-cmake-cygwin/src/mat.c: In Funktion »Mat_VarDelete«:
/cygdrive/c/Projects/matio-cmake-cygwin/src/mat.c:1032:5: Warnung: »mktemp« ist veraltet: the use of `mktemp' is dangerous; use `mkstemp' instead [-Wdeprecated-declarations]
     if ( (tmp_name = mktemp(temp)) != NULL ) {
     ^~
In file included from /cygdrive/c/Projects/matio-cmake-cygwin/src/mat.c:32:0:
/usr/include/stdlib.h:134:8: Anmerkung: hier deklariert
 char * mktemp (char *) _ATTRIBUTE ((__deprecated__("the use of `mktemp' is dangerous; use `mkstemp' instead")));
        ^~~~~~
[ 25%] Building C object CMakeFiles/matio.dir/src/io.c.o
[ 30%] Building C object CMakeFiles/matio.dir/src/inflate.c.o
[ 35%] Building C object CMakeFiles/matio.dir/src/mat73.c.o
[ 40%] Building C object CMakeFiles/matio.dir/src/matvar_cell.c.o
[ 45%] Building C object CMakeFiles/matio.dir/src/matvar_struct.c.o
[ 50%] Building C object CMakeFiles/matio.dir/src/mat4.c.o
[ 55%] Building C object CMakeFiles/matio.dir/src/mat5.c.o
[ 60%] Building C object CMakeFiles/matio.dir/src/snprintf.c.o
[ 65%] Building C object CMakeFiles/matio.dir/src/read_data.c.o
[ 70%] Linking C static library libmatio.a
[ 70%] Built target matio
Scanning dependencies of target matdump
[ 75%] Building C object CMakeFiles/matdump.dir/tools/matdump.c.o
[ 80%] Linking C executable matdump.exe
[ 80%] Built target matdump
Scanning dependencies of target test_mat
[ 85%] Building C object CMakeFiles/test_mat.dir/test/test_mat.c.o
[ 90%] Linking C executable test_mat.exe
[ 90%] Built target test_mat
Scanning dependencies of target test_snprintf
[ 95%] Building C object CMakeFiles/test_snprintf.dir/test/test_snprintf.c.o
[100%] Linking C executable test_snprintf.exe
[100%] Built target test_snprintf

laptop /cygdrive/c/Projects/matio-cmake-cygwin
$
tbeu commented 5 years ago

Any chance to use matio.rc for Win builds?

tbeu commented 5 years ago

Please do add yourself the Acknowledgements section of both README and README.md.

massich commented 5 years ago

Any chance to use matio.rc for Win builds?

Ok I see what the .rc file does. Is there any chance to have all this metadata in the same manner for windows/linux/osx ?

tbeu commented 5 years ago

Is there any chance to have all this metadata in the same manner for windows/linux/osx ?

https://stackoverflow.com/q/6693100/8520615 might give some clues.

massich commented 5 years ago

What I meant was: sure that we can generate the .rc from Cmake (which is what the stackoverflow link illustrates) or use the .rc to populate the Cmake.

But the question is can we agree in a manner that we use to populate autoconf, Cmake and visual? All at once?

On Tue, Mar 12, 2019, 11:17 tbeu notifications@github.com wrote:

Is there any chance to have all this metadata in the same manner for windows/linux/osx ?

https://stackoverflow.com/q/6693100/8520615 might give some clues.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/tbeu/matio/pull/107#issuecomment-471939709, or mute the thread https://github.com/notifications/unsubscribe-auth/AGt-4-ZKz9PSeYCSil62eQHINBwWhM_4ks5vV37AgaJpZM4bhWG9 .

tbeu commented 5 years ago

Even today, autoconf and VS version information are independent.

massich commented 5 years ago

@tbeu travis seems to not run in the PRs. Can you trigger it?

tbeu commented 5 years ago

Usually it did. Could it be that this is a draft PR?

tbeu commented 5 years ago

grafik

tbeu commented 5 years ago

It says: Could not parse tbeu/matio/.travis.yml@844e9935

Syntax error?

tbeu commented 5 years ago

Travis now runs, but still some command error, see https://travis-ci.org/tbeu/matio/builds/510386199 for https://github.com/tbeu/matio/tree/cmake

tbeu commented 5 years ago

If it says "Could NOT find HDF5" (as in https://travis-ci.org/tbeu/matio/jobs/514751765#L1658) then HAVE_HDF5 must not be defined (if MAT73 is defined).

massich commented 5 years ago

it builds, now what it does not do is test properly.

tbeu commented 5 years ago

it builds, now what it does not do is test properly.

See your 5 point plan above and my https://github.com/tbeu/matio/pull/107#issuecomment-470201385.

coveralls commented 5 years ago

Coverage Status

Coverage decreased (-82.8%) to 0.0% when pulling 9a3b7dadd85f5d939aa836577a37fe3601615309 on JohanMabille:cmake into 596cb3ce71038958812bd6cf9b141f12ce155ac6 on tbeu:master.

massich commented 5 years ago

If CIs turn green, this should be ready to merge. We can take whatever is left out for subsequent PRs.

@tbeu feel free to push any changes in the travis.yml

tbeu commented 5 years ago

There are typos in CheckHeaderSTDC.cmake: Cheking -> Checking

Still not fixed.

tbeu commented 5 years ago

On Win, with HDF5 installed and properly detected, I always get using cmake-gui

Configuring done
CMake Error at cmake/tools.cmake:1 (add_executable):
  Target "matdump" links to target "hdf5::hdf5-static" but the target was not
  found.  Perhaps a find_package() call is missing for an IMPORTED target, or
  an ALIAS target is missing?
Call Stack (most recent call first):
  CMakeLists.txt:29 (include)

CMake Error at cmake/tools.cmake:1 (add_executable):
  Target "matdump" links to target "hdf5::hdf5-static" but the target was not
  found.  Perhaps a find_package() call is missing for an IMPORTED target, or
  an ALIAS target is missing?
Call Stack (most recent call first):
  CMakeLists.txt:29 (include)

CMake Error at cmake/tools.cmake:1 (add_executable):
  Target "matdump" links to target "hdf5::hdf5-static" but the target was not
  found.  Perhaps a find_package() call is missing for an IMPORTED target, or
  an ALIAS target is missing?
Call Stack (most recent call first):
  CMakeLists.txt:29 (include)

CMake Error at cmake/tools.cmake:1 (add_executable):
  Target "matdump" links to target "hdf5::hdf5-static" but the target was not
  found.  Perhaps a find_package() call is missing for an IMPORTED target, or
  an ALIAS target is missing?
Call Stack (most recent call first):
  CMakeLists.txt:29 (include)

CMake Error at cmake/src.cmake:32 (add_library):
  Target "matio" links to target "hdf5::hdf5-static" but the target was not
  found.  Perhaps a find_package() call is missing for an IMPORTED target, or
  an ALIAS target is missing?
Call Stack (most recent call first):
  CMakeLists.txt:27 (include)

CMake Error at cmake/src.cmake:32 (add_library):
  Target "matio" links to target "hdf5::hdf5-static" but the target was not
  found.  Perhaps a find_package() call is missing for an IMPORTED target, or
  an ALIAS target is missing?
Call Stack (most recent call first):
  CMakeLists.txt:27 (include)

CMake Error at cmake/src.cmake:32 (add_library):
  Target "matio" links to target "hdf5::hdf5-static" but the target was not
  found.  Perhaps a find_package() call is missing for an IMPORTED target, or
  an ALIAS target is missing?
Call Stack (most recent call first):
  CMakeLists.txt:27 (include)

CMake Error at cmake/src.cmake:32 (add_library):
  Target "matio" links to target "hdf5::hdf5-static" but the target was not
  found.  Perhaps a find_package() call is missing for an IMPORTED target, or
  an ALIAS target is missing?
Call Stack (most recent call first):
  CMakeLists.txt:27 (include)

CMake Error at cmake/test.cmake:6 (add_executable):
  Target "test_snprintf" links to target "hdf5::hdf5-static" but the target
  was not found.  Perhaps a find_package() call is missing for an IMPORTED
  target, or an ALIAS target is missing?
Call Stack (most recent call first):
  CMakeLists.txt:30 (include)

CMake Error at cmake/test.cmake:6 (add_executable):
  Target "test_snprintf" links to target "hdf5::hdf5-static" but the target
  was not found.  Perhaps a find_package() call is missing for an IMPORTED
  target, or an ALIAS target is missing?
Call Stack (most recent call first):
  CMakeLists.txt:30 (include)

CMake Error at cmake/test.cmake:6 (add_executable):
  Target "test_snprintf" links to target "hdf5::hdf5-static" but the target
  was not found.  Perhaps a find_package() call is missing for an IMPORTED
  target, or an ALIAS target is missing?
Call Stack (most recent call first):
  CMakeLists.txt:30 (include)

CMake Error at cmake/test.cmake:6 (add_executable):
  Target "test_snprintf" links to target "hdf5::hdf5-static" but the target
  was not found.  Perhaps a find_package() call is missing for an IMPORTED
  target, or an ALIAS target is missing?
Call Stack (most recent call first):
  CMakeLists.txt:30 (include)

CMake Error at cmake/test.cmake:3 (add_executable):
  Target "test_mat" links to target "hdf5::hdf5-static" but the target was
  not found.  Perhaps a find_package() call is missing for an IMPORTED
  target, or an ALIAS target is missing?
Call Stack (most recent call first):
  CMakeLists.txt:30 (include)

CMake Error at cmake/test.cmake:3 (add_executable):
  Target "test_mat" links to target "hdf5::hdf5-static" but the target was
  not found.  Perhaps a find_package() call is missing for an IMPORTED
  target, or an ALIAS target is missing?
Call Stack (most recent call first):
  CMakeLists.txt:30 (include)

CMake Error at cmake/test.cmake:3 (add_executable):
  Target "test_mat" links to target "hdf5::hdf5-static" but the target was
  not found.  Perhaps a find_package() call is missing for an IMPORTED
  target, or an ALIAS target is missing?
Call Stack (most recent call first):
  CMakeLists.txt:30 (include)

CMake Error at cmake/test.cmake:3 (add_executable):
  Target "test_mat" links to target "hdf5::hdf5-static" but the target was
  not found.  Perhaps a find_package() call is missing for an IMPORTED
  target, or an ALIAS target is missing?
Call Stack (most recent call first):
  CMakeLists.txt:30 (include)

CMake Error at cmake/src.cmake:32 (add_library):
  Target "matio" links to target "hdf5::hdf5-static" but the target was not
  found.  Perhaps a find_package() call is missing for an IMPORTED target, or
  an ALIAS target is missing?
Call Stack (most recent call first):
  CMakeLists.txt:27 (include)

CMake Error at cmake/tools.cmake:1 (add_executable):
  Target "matdump" links to target "hdf5::hdf5-static" but the target was not
  found.  Perhaps a find_package() call is missing for an IMPORTED target, or
  an ALIAS target is missing?
Call Stack (most recent call first):
  CMakeLists.txt:29 (include)

CMake Error at cmake/test.cmake:6 (add_executable):
  Target "test_snprintf" links to target "hdf5::hdf5-static" but the target
  was not found.  Perhaps a find_package() call is missing for an IMPORTED
  target, or an ALIAS target is missing?
Call Stack (most recent call first):
  CMakeLists.txt:30 (include)

CMake Error at cmake/test.cmake:3 (add_executable):
  Target "test_mat" links to target "hdf5::hdf5-static" but the target was
  not found.  Perhaps a find_package() call is missing for an IMPORTED
  target, or an ALIAS target is missing?
Call Stack (most recent call first):
  CMakeLists.txt:30 (include)

Generating done

Even if I deselect MAT73 (which means HDF5 dependency is not necessary) I get the above errors.

tbeu commented 5 years ago

On Cygwin, with HDF5 not installed, and MAT73 enabled (default), it fails to compile, since HAVE_HDF5 is set, but hdf5.h cannot be found. I would prefer if the default cmake config succeeds to configure and build.

tbeu commented 5 years ago

Please do add yourself the Acknowledgements section of both README and README.md.

Still open.

tbeu commented 5 years ago

Last wish: Do you think you can add support for shared library? This would be the most asked for feature anyway.

After all, thank you for all the work and testing. It really helps to improve support of builds with different targets/platforms.

massich commented 5 years ago

Sure. But to do all that we need to add it to the appveyor to make sure we do it properly. But I had troubles to work with the current appveyor configs 'cos the build is not scripted.

On Fri, Apr 12, 2019, 17:14 tbeu notifications@github.com wrote:

Last wish: Do you think you can add support for shared library? This would be the most asked for feature anyway.

After all, thank you for all the work and testing. It really helps to improve support of builds with different targets/platforms.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/tbeu/matio/pull/107#issuecomment-482612486, or mute the thread https://github.com/notifications/unsubscribe-auth/AGt-4xEfvzPhtKGMZVW5XoXz1W71a8USks5vgKLdgaJpZM4bhWG9 .

tbeu commented 5 years ago

You mean .appveyor.yml is not enough? And what about the other issues?

tbeu commented 5 years ago

What is your plan on the open issues reported earlier?

massich commented 5 years ago

@tbeu sorry the lag. The main issue with the appveyor config is that currently we are using the sln approach rather than calling the build in a script format.

https://github.com/tbeu/matio/blob/f8cd39739f5f02cb8606ecb0f2f3aa4064a8eeb4/.appveyor.yml#L41-L46

We would need build_script rather than build. See https://www.appveyor.com/docs/appveyor-yml/


possible solution form a chat in cpplang-slack, use build in one element of the matrix and build_script in the other. https://www.appveyor.com/docs/build-configuration/#specializing-matrix-job-configuration

emmenlau commented 4 years ago

Sorry to come a bit late to the party: Is this the latest cmake integration PR? Anything I can do to help move things forward?

massich commented 4 years ago

yes this is the last one. ~I can add you to my repo so that you can push in the branch if you like~(its not my PR). It was mainly working. Can you check in your system? (that would be a good start) Then I think it was missing dynamic linking for windows.

tbeu commented 4 years ago

Anything I can do to help move things forward?

Please scroll up to see all the open issues/comments that shall be fixed before a merge.

tbeu commented 4 years ago

We would need build_script rather than build. See https://www.appveyor.com/docs/appveyor-yml/

@tbeu sorry the lag. The main issue with the appveyor config is that currently we are using the sln approach rather than calling the build in a script format.

https://github.com/tbeu/matio/blob/f8cd39739f5f02cb8606ecb0f2f3aa4064a8eeb4/.appveyor.yml#L41-L46

We would need build_script rather than build. See https://www.appveyor.com/docs/appveyor-yml/

possible solution form a chat in cpplang-slack, use build in one element of the matrix and build_script in the other. https://www.appveyor.com/docs/build-configuration/#specializing-matrix-job-configuration

Thanks for the reference. build_script is now available with 809af56b150f73e9cad590b256979ff528ea6b21. It already is prepared to be extensible by adding another build_with: cmake variable to the environment matrix. 🎄 :gift: (After all, it took me about 60 iterations to get it up and running for cygwin, cygwin64, mingw, mingw-w64. :open_mouth:)

tbeu commented 4 years ago

Anything I can do to help move things forward?

Please scroll up to see all the open issues/comments that shall be fixed before a merge.

@emmenlau Did you have a go on the mentioned issues/comments?

tbeu commented 4 years ago

grafik

Happy anniversary, #107! How to move on? I can think of the following alternatives.

  1. Move on: Some volunteer fixes the review comments from @thejohnfreeman and me, and finalizes the remaining two relevant tasks. I can happily merge it,
  2. Status quo: I can have this PR open one more year without much progress.
  3. Back: I can close with unmerged commits.

While 1. is very unlikely to happen, it would be my preferred option.

papadop commented 4 years ago

I have (at least a partial one) an update with the "cheking' correction and most of the cmake changes. Plus a merge from origin/master (I hope). But I cannot push it here and my own fork refuses it too.... Not too sure what is the best solution.

tbeu commented 4 years ago

@papadop Thanks for your offer. I tried to look for the branch in your fork but did not see any recent commits. You can simply push to your repo and I will cherry-pick your commits afterwards.

tbeu commented 4 years ago

I resolved the merge conflict with the Travis CI configuration and applied it again. Now the CMake build is running at Travis CI somehow, but I noticed other strange stings:

tbeu commented 4 years ago

Closing now, since there is no more progress to expect.

emmenlau commented 4 years ago

:-(

agramfort commented 4 years ago

:-(

Febbe commented 4 years ago

Closing now, since there is no more progress to expect.

It's sad to hear that the development is stopped. Autotools and vs-projects are really a pain. :-( May I ask, why there is no progress and what prevented a merge?

emmenlau commented 4 years ago

Closing now, since there is no more progress to expect.

It's sad to hear that the development is stopped. Autotools and vs-projects are really a pain. :-( May I ask, why there is no progress and what prevented a merge?

Well, basically you just need to scroll up to see the details...