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

Several cmake improvements #75

Closed
wants to merge 12 commits into from
Closed

Conversation

emmenlau
Copy link

Still work in progress.

@papadop
Copy link
Contributor

papadop commented Nov 30, 2017

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
Copy link
Author

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
Copy link
Author

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
Copy link
Contributor

papadop commented Nov 30, 2017

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
Copy link
Contributor

papadop commented Nov 30, 2017

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
Copy link
Author

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

@coveralls
Copy link

coveralls commented Nov 30, 2017

Coverage Status

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

4 similar comments
@coveralls
Copy link

Coverage Status

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

@coveralls
Copy link

Coverage Status

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

@coveralls
Copy link

Coverage Status

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

@coveralls
Copy link

Coverage Status

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

@coveralls
Copy link

coveralls commented Nov 30, 2017

Coverage Status

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

@coveralls
Copy link

coveralls commented Nov 30, 2017

Coverage Status

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

@tbeu tbeu force-pushed the master branch 2 times, most recently from d982732 to 5dfe2db Compare December 20, 2017 07:45
@tbeu
Copy link
Owner

tbeu commented Feb 3, 2018

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

@papadop
Copy link
Contributor

papadop commented Feb 3, 2018

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

@henryiii
Copy link

henryiii commented Sep 5, 2018

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
Copy link
Owner

tbeu commented Sep 5, 2018

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

@tbeu tbeu force-pushed the master branch 3 times, most recently from 4ff7b1e to 4f964f8 Compare September 12, 2018 18:34
@tbeu tbeu force-pushed the master branch 5 times, most recently from 3846a94 to 0904d31 Compare September 24, 2018 05:26
@tbeu tbeu force-pushed the master branch 2 times, most recently from b97f68d to be83cfb Compare October 16, 2018 15:16
@tbeu tbeu mentioned this pull request Jan 29, 2019
@JohanMabille JohanMabille mentioned this pull request Mar 6, 2019
5 tasks
@massich
Copy link
Contributor

massich commented Apr 2, 2019

I would close this one in favor of #107

@tbeu
Copy link
Owner

tbeu commented Apr 2, 2019

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

@tbeu
Copy link
Owner

tbeu commented Mar 6, 2020

I would close this one in favor of #107

Doing so.

@tbeu tbeu closed this Mar 6, 2020
@tbeu tbeu added the invalid label Mar 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants