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

Generate MSCV solution with static CRT? #776

Closed
thomthom opened this issue Jan 25, 2016 · 24 comments
Closed

Generate MSCV solution with static CRT? #776

thomthom opened this issue Jan 25, 2016 · 24 comments

Comments

@thomthom
Copy link
Contributor

Is it possible to have CMake generate a MSVC solution that uses static CRT? (/MT)

I'm admittedly not that familiar with CMake and I'm not sure if this is something that is generic to using CMake or if the project has to build in support for it.

My use case is that I'm writing an extension for an application where I'm implementing OpenSubdiv. The extensions for this application normally doesn't use installers but instead are packaged as simple ZIP packages that is unpacked. Hence I'm trying to avoid the need to depend on CRT.

@jcowles
Copy link
Contributor

jcowles commented Jan 25, 2016

A cursory search seems to suggest it's something you must configure manually, but I haven't checked our cmake setup to see if it was implemented previously.

Jeremy

Sent from my phone

On Jan 24, 2016, at 7:00 PM, Thomas Thomassen notifications@github.com wrote:

Is it possible to have CMake generate a MSVC solution that uses static CRT? (/MT)

I'm admittedly not that familiar with CMake and I'm not sure if this is something that is generic to using CMake or if the project has to build in support for it.

My use case is that I'm writing an extension for an application where I'm implementing OpenSubdiv. The extensions for this application normally doesn't use installers but instead are packaged as simple ZIP packages that is unpacked. Hence I'm trying to avoid the need to depend on CRT.


Reply to this email directly or view it on GitHub.

@thomthom
Copy link
Contributor Author

Configured in Visual Studio after CMake generation, or can I configure CMake to generate the solution up front?

@c64kernal
Copy link
Contributor

Hi Thomas, I'd recommend you configure this in cmake and not modify the generated projects:

https://cmake.org/Wiki/CMake_FAQ#How_can_I_build_my_MSVC_application_with_a_static_runtime.3F

@jcowles
Copy link
Contributor

jcowles commented Jan 25, 2016

+1 and if you get it working via CMake, please send a pull request!

Jeremy

Sent from my phone

On Jan 24, 2016, at 8:06 PM, George ElKoura notifications@github.com wrote:

Hi Thomas, I'd recommend you configure this in cmake and not modify the generated projects:

https://cmake.org/Wiki/CMake_FAQ#How_can_I_build_my_MSVC_application_with_a_static_runtime.3F


Reply to this email directly or view it on GitHub.

@jcowles
Copy link
Contributor

jcowles commented Jan 25, 2016

Oh, it looks like the doc George sent is explains how to setup a one-off CMake override, which is probably not something we would distribute.

But if you get it working for real with a config option, then a pull request would be great.

Jeremy

Sent from my phone

On Jan 25, 2016, at 7:36 AM, Jeremy Cowles jeremy.cowles@gmail.com wrote:

+1 and if you get it working via CMake, please send a pull request!

Jeremy

Sent from my phone

On Jan 24, 2016, at 8:06 PM, George ElKoura notifications@github.com wrote:

Hi Thomas, I'd recommend you configure this in cmake and not modify the generated projects:

https://cmake.org/Wiki/CMake_FAQ#How_can_I_build_my_MSVC_application_with_a_static_runtime.3F


Reply to this email directly or view it on GitHub.

@thomthom
Copy link
Contributor Author

Will do!

@manuelk
Copy link

manuelk commented Jan 26, 2016

I vaguely remember early releases of OSD having the /Mt option, but it looks like it's disappeared.

This should be as easy as inserting /Mt here:
https://github.com/PixarAnimationStudios/OpenSubdiv/blob/master/CMakeLists.txt#L272

Although : why not link statically both OSD and CRT with your shipping binary ?

@thomthom
Copy link
Contributor Author

Although : why not link statically both OSD and CRT with your shipping binary ?

That's what I'm trying to do - but then I need to build the OSD libs with /Mt to match.

Thanks for the hint at where to change the CMakeList.txt - I'll be digging into it as soon as I get a spare moment.

@thomthom
Copy link
Contributor Author

thomthom commented Feb 2, 2016

This should be as easy as inserting /Mt here:

That would change it permanently to be static CRT, right? Wouldn't it be desired to have this be configurable when running cmake?

@thomthom
Copy link
Contributor Author

thomthom commented Feb 2, 2016

I had a quick look into this - and that's not as simple as just adding /MT because in debug builds we want /MTd. I'm not familiar enough with CMake to know exactly how to do this right now - but I just wanted to check in if it would be desired to have this configuration as an option of not when running CMake.

@nyue
Copy link
Contributor

nyue commented Feb 2, 2016

Don't have CMake + Visual Studio in front of me to verify the following but something like this is how I would construct them

IF (WIN32)
IF ( CMAKE_BUILD_TYPE MATCHES "Debug" )
ADD_DEFINITIONS ( /MTd )
ELSE ( CMAKE_BUILD_TYPE MATCHES "Debug" )
ADD_DEFINITIONS ( /MT )
ENDIF ( CMAKE_BUILD_TYPE MATCHES "Debug" )
ENDIF (WIN32)

@thomthom
Copy link
Contributor Author

thomthom commented Feb 3, 2016

Correct me if I'm wrong, but it appear to me that that would not work for multi-configuration solutions. CMAKE_BUILD_TYPE doesn't seem to be set by default and if you did you'd only get one config.

I got an alternative which works with multi-config solution:

option(STATIC_CRT "Statically link MSVC CRT" OFF)

if(MSVC AND STATIC_CRT)
    message(STATUS "Using static MSVC CRT")
    # http://stackoverflow.com/a/32128977/486990
    add_compile_options(
        "$<$<CONFIG:Debug>:/MTd>"
        "$<$<CONFIG:RelWithDebInfo>:/MT>"
        "$<$<CONFIG:Release>:/MT>"
        "$<$<CONFIG:MinSizeRel>:/MT>"
    )
endif()

When I added that to CMakeLists.txt I am able to control the VS solution CRT type by adding a -DSTATIC_CRT flag to cmake. This let me switch between MD(d) and MT(d)` easily.

If that sounds good to you guys I'll make a pull request.
(Though I wonder how the option is best set up - as currently the naming doesn't match the existing pattern. Perhaps make static CRT the default and name the option NO_STATIC_CRT?)

Or if this isn't satisfactory then let me know and I'll dig further.

@thomthom
Copy link
Contributor Author

thomthom commented Feb 8, 2016

Hm... I didn't actually try to build a Release with MT set until today - then I got a number of linker errors: https://gist.github.com/thomthom/5f9a23263c6a05ec5c5d

Found similar errors described here indicating there might be a mix of MT and MD being linked: http://stackoverflow.com/a/14158187/486990

Not exactly sure what is going on. (It's late and I'll resume tomorrow.)

@thomthom
Copy link
Contributor Author

thomthom commented Feb 8, 2016

Ach, obsessive mind - couldn't let it go.

I managed to make it build, though I don't comprehend exactly why. So I'm hesitant in making a pull request.

The issue was with the stringify project:

What I did to Release build after I had added the mentioned snippet to CMakeLists.txt:
2016-02-08_02h01_56

I removed the libcmt.lib reference. Not sure why I needed to do that - the linker error I got related to libcpmt.lib. (Notice the p in there.)

Futher more, the same config in Debug builds just fine.

So I'm at a loss - while I managed to build both Release and Debug with MT/MTd I'm not sure what I can use to make a pull request with a patch.

@jcowles
Copy link
Contributor

jcowles commented Feb 17, 2016

Ok, so catching up here, did you previously add that lib by hand? Does it
now build from a fresh setup with or without the flag?

Or did CMake add that lib and you had to remove it by hand from the CMake
generated project?

Jeremy

On Sun, Feb 7, 2016 at 5:07 PM, Thomas Thomassen notifications@github.com
wrote:

Ach, obsessive mind - couldn't let it go.

I managed to make it build, though I don't comprehend exactly why. So I'm
hesitant in making a pull request.

What I did to Release build after I had added the mentioned snippet to
CMakeLists.txt:
[image: 2016-02-08_02h01_56]
https://cloud.githubusercontent.com/assets/192418/12876823/5257fa48-ce08-11e5-9558-0271aef9b6d6.png

I removed the libcmt.lib reference. Not sure why I needed to do that -
the linker error I got related to libcpmt.lib. (Notice the p in there.)

Futher more, the same config in Debug builds just fine.

So I'm at a loss - while I managed to build both Release and Debug with MT
/MTd I'm not sure what I can use to make a pull request with a patch.


Reply to this email directly or view it on GitHub
#776 (comment)
.

@thomthom
Copy link
Contributor Author

I found that changing the solution after it was generated was too cumbersome because there are too many projects and configurations that all have to be updated. And it'd have to be done every time I want to update OSD.

Now I just need to a pass in MSVC_STATIC_CRT_RELEASE and/or MSVC_STATIC_CRT_DEBUG to cmake and the solution is generated with the appropriate CRT settings. Makes it easy to run a script to regenerate and build OSD whenever I pull a new update.

My main concern with the current state of this patch is the libcpmt.lib linker error I got. Where I had to remove the line that ignored libcmt.lib. This happened only in release build with static CRT. Not in debug, nor in dynamic CRT.

@thomthom
Copy link
Contributor Author

Making another attempt to bump. Don't expect anything for 3.1 which looks done. But hoping to be able to include something in next release to ease maintainability with /MT needs.

This is what I currently use: thomthom@baa04da but in #783 there was some questions about perhaps have it as build configs in the solutions. But I'm not sure there was a definitive conclusion to what was desired?

@thomthom
Copy link
Contributor Author

Any thoughts on this issue?

@davidgyu
Copy link
Member

Yes! Hoping to have a fix for you this week. We've been working through other Windows build issues including #905 and related static/dynamic dependencies...

@davidgyu
Copy link
Member

Can you take a look at PR #917 (which is a simplified version of your PR #783) and see if it works for you?

Thanks!
-David

@thomthom
Copy link
Contributor Author

The simplification is that it's one flag for both Release and Debug?
Having just one flag works for me. There wasn't any real reason to why I had different linking for Debug. The important one was Release and that PR seems to address that.
Thanks for looking into that.

@davidgyu
Copy link
Member

Right, just the single flag, and also making the "/NODEFAULTLIB:libcmt.lib" code mutually exclusive to avoid having to comment it out.

@thomthom
Copy link
Contributor Author

Yea, I never understood where that libcmt.lib error came from. Thanks for tackling that as well.

@davidgyu
Copy link
Member

davidgyu commented Feb 1, 2017

Fixed by #917 Thanks!

@davidgyu davidgyu closed this as completed Feb 1, 2017
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

No branches or pull requests

6 participants