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

Add CMake option to link MSVC runtime statically #878

Closed
wants to merge 1 commit into from

Conversation

tbeu
Copy link
Contributor

@tbeu tbeu commented Nov 19, 2023

This fixes #849 (and the broken CRT static linkage) in a proper way. Call

cmake.exe" -DSKIP_INSTALL_FILES=ON -DZLIB_STATIC_LINK_CRT=ON -DCMAKE_STATIC_LINKER_FLAGS=/SUBSYSTEM:CONSOLE,6.01 -G"NMake Makefiles" -DCMAKE_BUILD_TYPE=RELEASE

to link the C runtime library statically. Default (still) is dynamically.

This should supersed #535.

@madler
Copy link
Owner

madler commented Jan 17, 2024

Incorporated. Thanks.

@madler madler closed this Jan 17, 2024
@nmoinvaz
Copy link
Contributor

This seems rather strange. Why can't you just pass -D CMAKE_MSVC_RUNTIME_LIBRARY=MultiThreadedDebug on the command line?

@tbeu tbeu deleted the cmake-link-crt-statically branch January 18, 2024 06:45
@Neustradamus
Copy link

Merged commit:

@nmoinvaz
Copy link
Contributor

According to the CMake docs this is wrong and the variable should be set before project(). But honestly it should be better passed on the command line instead of the CMakeLists.txt because it can apply to your entire build not to just a single project, of which zlib may be one of many.

@madler
Copy link
Owner

madler commented Jan 19, 2024

@nmoinvaz Are you recommending that I undo this commit?

@Neustradamus
Copy link

Or adding a new change to be perfect?

There is this PR too:

@nmoinvaz
Copy link
Contributor

nmoinvaz commented Jan 19, 2024

I would undo this commit yes, the docs clearly state it must be set before project().

image

Even if you were to move it, CMAKE_GENERATOR would likely be invalid before project() so it would need some rework.

I also oppose #535, as I don't think you need a specialized option to statically link with runtime library. Instead it is easy enough to do:

Windows:

cmake -S . -B build -D CMAKE_MSVC_RUNTIME_LIBRARY=MultiThreaded

Linux/Apple:

export LDFLAGS=-static
cmake -S . -B build 

Or even:

cmake -S . -B build -D CMAKE_EXE_LINKER_FLAGS=-static -D CMAKE_SHARED_LINKER_FLAGS=-static

Any user can also set(CMAKE_MSVC_RUNTIME_LIBRARY "MultiThreaded") in their parent CMake project if they wanted. Also this PR change would discard any CMAKE_MSVC_RUNTIME_LIBRARY setting that might be set by a parent project because it sets it in the else() condition always.

CMake command instructions, like the ones mentioned, are better suited for the GitHub wiki page for this project. I have edited it here. Feel free to change it.

@madler
Copy link
Owner

madler commented Jan 19, 2024

Thanks! I didn't even know there was a wiki.

@madler
Copy link
Owner

madler commented Jan 19, 2024

Sorry @tbeu. Reverted.

@nmoinvaz
Copy link
Contributor

I may have read that documentation wrong, it seems only CMP0091 needs to be set before project().

I think if you keep this PR, at the very least it should be changed to if(POLICY CMP0091 AND NOT DEFINED CMAKE_MSVC_RUNTIME_LIBRARY) . I fear it will mess up parent projects that already set this a different way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

zlib 1.3 forces /MD although I build it with /MT on Windows
4 participants