tbeu / matio

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

Several cmake improvements #75

Closed emmenlau closed 4 years ago

emmenlau commented 6 years ago

Still work in progress.

papadop commented 6 years ago

Can you ask a pull request to the New cmake branch ? If you can drop the getopt CMakeLists.txt that would be great as I already added the one from OpenMEEG-matio. Thank's for all the suggestions. I skimmed through them quickly and they seem to be nice improvements (which also show that we never built OpenMEEG-mtio on windows with many of the options) .

emmenlau commented 6 years ago

Dear @papadop , I just pulled in your latest changes and rebased my changes on top of yours. I also removed my getopt CMakeLists.txt. I will test now and then push the update.

emmenlau commented 6 years ago

A minor thing: your getopt library is called gnu and is installed in the target directory. Would it be ok to just call it getopt or getopt_long instead? And is it required to install the getopt library, or is it only for building the tools?

papadop commented 6 years ago

The getopt library is needed to build the tools and the testing utility test_mat. Whether it is needed to install it depends on the way it is linked into the executables. I tend to think that it is linked dynamically, which means that if you install the tools or the test_mat utility then you also need to install the getopt lib.

papadop commented 6 years ago

As for the name, I do not think I changed anything. But I do not know where I got this. Looking at the CMakeLists.txt it seems that we build a static library, so there is no need to install it and we can name it getopt which is probably better than gnu indeed.

emmenlau commented 6 years ago

Great, I agree. Then I'll make these minor changes too.

coveralls commented 6 years ago

Coverage Status

Coverage remained the same at 81.008% when pulling ce3269583e97b7ba0b285520732eb60de1e1a6d3 on emmenlau:cmake_pr into c9fa3a2029bed44fdf37eb4de8413521f635e3dc on tbeu:master.

coveralls commented 6 years ago

Coverage Status

Coverage remained the same at 81.008% when pulling ce3269583e97b7ba0b285520732eb60de1e1a6d3 on emmenlau:cmake_pr into c9fa3a2029bed44fdf37eb4de8413521f635e3dc on tbeu:master.

coveralls commented 6 years ago

Coverage Status

Coverage remained the same at 81.008% when pulling ce3269583e97b7ba0b285520732eb60de1e1a6d3 on emmenlau:cmake_pr into c9fa3a2029bed44fdf37eb4de8413521f635e3dc on tbeu:master.

coveralls commented 6 years ago

Coverage Status

Coverage remained the same at 81.008% when pulling ce3269583e97b7ba0b285520732eb60de1e1a6d3 on emmenlau:cmake_pr into c9fa3a2029bed44fdf37eb4de8413521f635e3dc on tbeu:master.

coveralls commented 6 years ago

Coverage Status

Coverage remained the same at 81.008% when pulling ce3269583e97b7ba0b285520732eb60de1e1a6d3 on emmenlau:cmake_pr into c9fa3a2029bed44fdf37eb4de8413521f635e3dc on tbeu:master.

coveralls commented 6 years ago

Coverage Status

Coverage remained the same at 81.008% when pulling 207d1193356250b3553a986047fb4309fa96c6db on emmenlau:cmake_pr into c9fa3a2029bed44fdf37eb4de8413521f635e3dc on tbeu:master.

coveralls commented 6 years ago

Coverage Status

Coverage remained the same at 81.008% when pulling 207d1193356250b3553a986047fb4309fa96c6db on emmenlau:cmake_pr into c9fa3a2029bed44fdf37eb4de8413521f635e3dc on tbeu:master.

tbeu commented 6 years ago

Is this still being worked on? If not, I propose to close it w/o merge?

papadop commented 6 years ago

I intend to continue on working on thé branch... Please do not close it.

henryiii commented 6 years ago

Any updates? If matio supported add_subdirectory() from a parent CMake project, this would be great and would mean the that a project could build matio directly if needed.

tbeu commented 6 years ago

Sorry, no news here. Autotools and VS solutions are the supported build environments. See https://github.com/tbeu/matio#20-building

massich commented 5 years ago

I would close this one in favor of #107

tbeu commented 5 years ago

Let's see it it works automatically once I merge #107.

tbeu commented 4 years ago

I would close this one in favor of #107

Doing so.