search

smanders / externpro

build external projects with cmake
MIT License
13 stars 12 forks source link

liblua using C++ runtime /MD instead of /MT #363

Closed smanders closed 2 years ago

smanders commented 2 years ago

I'm seeing link errors in an executable that links in liblua -- the link errors indicate that the library and executable are using different C++ runtime libraries

this wasn't an issue before externpro 22.04 the diff to patches/lua.patch made in 22.04 https://github.com/smanders/externpro/compare/22.03...22.04#diff-44bfba91cb261c3229a04535b24e11dcb49f05a7d181816f8c7bad0f6f54919f

looking at the cmake-generated lua project files, sure enough: Release is using /MD and not /MT which is dictated by the include(flags OPTIONAL)

I believe it's this change that has caused it to use /MD instead of /MT

-cmake_minimum_required ( VERSION 2.8 )
+cmake_minimum_required ( VERSION 2.8...3.21 )

combined with the fact that this project calls project() and then cmake_minimum_required(), which according to the cmake docs https://cmake.org/cmake/help/latest/command/cmake_minimum_required.html is a no-no

Note: Call the cmake_minimum_required() command at the beginning of the top-level CMakeLists.txt file even before calling the project() command.

smanders commented 2 years ago

I should have checked this for every externpro project... because I ran into this before with apr -- after doing a ton of work on the "project and patch simplification" issue https://github.com/smanders/externpro/issues/356, I went to verify that things worked on Windows, and ran into a link issue with C++ runtime libraries for apr and discovered then that I needed to swap project() and cmake_minimum_required() https://github.com/smanders/apr/commit/4e96201bd136b3d6c095d4a931c51988e1d3f94a -- you can see this commit came a few days after all the other commits that "updated" apr https://github.com/apache/apr/compare/1.5.2...smanders:apr:xp1.5.2

smanders commented 2 years ago

it appears that this same issue exists in eigen, but maybe wouldn't be seen in any projects that use externpro because eigen is a header-only library -- but I'm going to patch eigen, too, so it is more "correct"

smanders commented 2 years ago

completed with commits referenced above