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

New cmake branch #74

Closed
wants to merge 21 commits into from
Closed

New cmake branch #74

wants to merge 21 commits into from

Conversation

papadop
Copy link
Contributor

@papadop papadop commented Nov 29, 2017

This is only WIP for now. All comments accepted !
The cmake build system is added. I verified it build both in old fashion and with cmake.
Tests with cmake are currently both incomplete (2018 tests instead of 3152) and sometimes wrong (180 tests out of 2018 are failing). I'm working on it.

@papadop
Copy link
Contributor Author

papadop commented Nov 29, 2017

appveyor build fails, but I do not think I'm responsible....

@emmenlau
Copy link

I'm very cuirous about this, thanks!

@tbeu
Copy link
Owner

tbeu commented Nov 29, 2017

I restarted the appveyor build.

@papadop
Copy link
Contributor Author

papadop commented Nov 29, 2017

Some options are also missing.

@tbeu
Copy link
Owner

tbeu commented Nov 29, 2017

Why did you introduce matio_noreturn.h? If there is no need to touch it, please keep it as it is.

@papadop
Copy link
Contributor Author

papadop commented Nov 29, 2017

To factor this piece of code between the matio_pubconf.h.in and matio_pubconf_cmake.in. In this way the two files only contain defines (and includres). It is also simpler to check the correspondance between the defined variables (which by the way I need to check if there were updates in this respact since I did the original cmake port).

@tbeu
Copy link
Owner

tbeu commented Nov 29, 2017

I still prefer to have a single pubconfig file and not distributed config files.

@coveralls
Copy link

coveralls commented Nov 29, 2017

Coverage Status

Coverage remained the same at 81.008% when pulling dd6566c on papadop:NewCmakeBranch into c9fa3a2 on tbeu:master.

5 similar comments
@coveralls
Copy link

Coverage Status

Coverage remained the same at 81.008% when pulling dd6566c on papadop:NewCmakeBranch into c9fa3a2 on tbeu:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 81.008% when pulling dd6566c on papadop:NewCmakeBranch into c9fa3a2 on tbeu:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 81.008% when pulling dd6566c on papadop:NewCmakeBranch into c9fa3a2 on tbeu:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 81.008% when pulling dd6566c on papadop:NewCmakeBranch into c9fa3a2 on tbeu:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 81.008% when pulling dd6566c on papadop:NewCmakeBranch into c9fa3a2 on tbeu:master.

@emmenlau
Copy link

On MSVC I get an error that the folder getopt does not contain a CMakeLists.txt file.

@emmenlau
Copy link

Ok, I found a simple CMakeLists.txt for getopt. Now I'm still stuck at:

snprintf.c.obj : error LNK2019: unresolved external symbol va_copy referenced in function rpl_vasprintf

Are there extra libraries that need to be linked on Windows? The cmake-generated matioConfig.h defines HAVE_VA_COPY and undefines HAVE___VA_COPY, so I think that its correct.

@emmenlau
Copy link

Please consider PR #75 that addresses several of the above problems.

@papadop
Copy link
Contributor Author

papadop commented Nov 30, 2017

I just pushed the one from OpenMEEG-matio... It should work out of the box as we regularly build this version of matio on windows...

@coveralls
Copy link

coveralls commented Dec 2, 2017

Coverage Status

Coverage decreased (-81.1%) to 0.0% when pulling d74d207 on papadop:NewCmakeBranch into bdb6adf on tbeu:master.

@tbeu tbeu force-pushed the master branch 6 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?

@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 28, 2019
@JohanMabille JohanMabille mentioned this pull request Mar 6, 2019
5 tasks
@tbeu tbeu force-pushed the master branch 2 times, most recently from 622a081 to aefd09e Compare September 9, 2019 18:15
@tbeu
Copy link
Owner

tbeu commented Mar 6, 2020

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

Doing so.

@tbeu tbeu closed this Mar 6, 2020
@tbeu tbeu added invalid and removed enhancement labels 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.

4 participants