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

Windows_MT.cmake has no effect anymore #3984

Closed
m1lhaus opened this issue Feb 2, 2024 · 1 comment · Fixed by #4062
Closed

Windows_MT.cmake has no effect anymore #3984

m1lhaus opened this issue Feb 2, 2024 · 1 comment · Fixed by #4062
Assignees
Labels
Component - Build CMake, Autotools Priority - 2. Medium ⏹ It would be nice to have this in the next release Type - Bug / Bugfix Please report security issues to help@hdfgroup.org instead of creating an issue on GitHub

Comments

@m1lhaus
Copy link

m1lhaus commented Feb 2, 2024

Hello,
I was building latest static hdf5 1.14.3 libs on Windows targeting static MSVCRT libs via cmake. I found out about Windows_MT.cmake from readme. So I disabled build of shared libs, enabled usage of static msvcrt libs and included mentioned Windows_MT.cmake to UserMacros.cmake. To be sure I added few message prints to mentioned cmake file to be super duper sure that the code gets executed during the configure step.

Windows_MT.cmake does simple replace of /MD to /MT for the compiler and /libs:dll to /libs:static for the linker. However, the code is not working properly, since there is nothing to be replaced in vanilla state (freshly generated cmake project). Following is the printout from inside of the cmake loops:

CMAKE_C_FLAGS_DEBUG= /Zi /Ob0 /Od /RTC1
CMAKE_C_FLAGS_RELEASE= /O2 /Ob2 /DNDEBUG
CMAKE_C_FLAGS_MINSIZEREL= /O1 /Ob1 /DNDEBUG
CMAKE_C_FLAGS_RELWITHDEBINFO= /Zi /O2 /Ob1 /DNDEBUG
CMAKE_CXX_FLAGS= /DWIN32 /D_WINDOWS /GR /EHsc
CMAKE_CXX_FLAGS_DEBUG= /Zi /Ob0 /Od /RTC1
CMAKE_CXX_FLAGS_RELEASE= /O2 /Ob2 /DNDEBUG
CMAKE_CXX_FLAGS_MINSIZEREL= /O1 /Ob1 /DNDEBUG
CMAKE_CXX_FLAGS_RELWITHDEBINFO= /Zi /O2 /Ob1 /DNDEBUG
CMAKE_Fortran_FLAGS=
CMAKE_Fortran_FLAGS_DEBUG=
CMAKE_Fortran_FLAGS_RELEASE=
CMAKE_Fortran_FLAGS_MINSIZEREL=
CMAKE_Fortran_FLAGS_RELWITHDEBINFO=

You see that there are not /MD or /libs flags to be replaced. So I just added the else branch in order to append the flags if there is nothing to replace. That also didn't work properly. I could now see proper flags being appended (/MT and /libs:static), but in my case generated Visual Studio solution was still showing me that MultiThreaded DLL library is used as Runtime library in project properties. After building many tests failed. I think the reason is the combination of (implicit) /MD and /NODEFAULTLIB:MSVCRT options. Only after adding

set(CMAKE_MSVC_RUNTIME_LIBRARY "MultiThreaded$<$<CONFIG:Debug>:Debug>")

the generated Visual Studio solution was showing me that static runtime lib is used. Then all compiled and all tests passed. Strangely enough I compiled hdf5 1.12.3 earlier that day with exactly the same tools and there just including Windows_MT.cmake was enough.

Can someone please check what changed in hdf5 cmake tooling so this feature got broken? Also can you please consider adding option BUILD_STATIC_CRT_LIBS to be a regular hdf cmake option and include Windows_MT.cmake by default? I see no reason why this option should be removed by default 🤔

I tested the same behaviour on two computers (home and at work):

  • latest up-to-date Visual Studio 2022 + 2022 compiler (v143)
  • latest up-to-date Visual Studio 2022 + 2017 compiler (v141)
  • cmake 3.28.x
@brtnfld brtnfld added Priority - 2. Medium ⏹ It would be nice to have this in the next release Component - Build CMake, Autotools Type - Bug / Bugfix Please report security issues to help@hdfgroup.org instead of creating an issue on GitHub labels Feb 5, 2024
@m1lhaus
Copy link
Author

m1lhaus commented Feb 19, 2024

I encountered the issue on other project as well recently. It turned out the behavior is linked to minimum cmake version set before project() setup in CMakeLists.txt. That lead me to https://cmake.org/cmake/help/latest/policy/CMP0091.html#policy:CMP0091 which confirmed my assumption. The change was introduced in cmake 3.15. So I guess you just need to replace the OLD behavior (manual flags handling) by NEW behavior using CMAKE_MSVC_RUNTIME_LIBRARY variable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component - Build CMake, Autotools Priority - 2. Medium ⏹ It would be nice to have this in the next release Type - Bug / Bugfix Please report security issues to help@hdfgroup.org instead of creating an issue on GitHub
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants