-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Fixed build errors resulting from upgrading to VS2019 compilers #3894
Conversation
Merging to personal fork
# which is incompatible with the /guard:cf flag we set below | ||
# for security. So we use the default flags set by CMake | ||
# and reset /ZI with /Zi | ||
set(CMAKE_C_FLAGS_DEBUG "/MDd /Zi /Ob0 /Od /RTC1 /JMC") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where do those flags /MDd /Zi /Ob0 /Od /RTC1 /JMC
come from? Are they default flags in CMake? Could you please add more details for maintaining them in the future?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, those are the default flags in CMake. I will add a bit more detail and submit another PR soon.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks, @harishsk
Purely for educational purposes...these are the recent changes in Windows-MSVC.cmake that caused this issue:
|
@harishsk where have you found the stuff above? I've looked at the Windows-MSVC.cmake in all versions from 3.14 till 3.15 and none of this was there (looking via https://github.com/Kitware/CMake). And cmake 3.14.2 with VS2019 builds fine without your change (And the vcxproj files contain /Zi, not /ZI). |
I am experiencing the same as @janvorli. I can build successfully on VS 2019 with CMake 3.14.5 without this change. I sort of think this change should be reverted until we know more. I don't like the fact that we are clobbering the defaults. |
Where is your CMake located? And what is the version reported for CMake.exe --version Mine is here: "C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Common7\IDE\CommonExtensions\Microsoft\CMake\CMake\bin\cmake.exe" And it reports: cmake version 3.14.19050301-MSVC_2 The changes I reported above are in this file: |
My CMake is located at @janvorli informed me there might be an issue with the CMake shipped by VS 2019:
@jkotas - do you happen to know anyone on the Visual Studio team who works on the CMake they ship? I'd like to find out if this is a bug in VS 2019. |
Btw, it seems that a cleaner way to fix the problem would be to use string(REPLACE "/ZI" "/Zi" CMAKE_C_FLAGS_DEBUG ${CMAKE_C_FLAGS_DEBUG}) |
Yes, I have CMake installed in Program Files on my laptop and I don't see this bug. On my desktop, I don't have CMake installed and the one shipped in Visual Studio gets used, resulting in this bug. This is a bug in VS2019. And as my comment in the fix states, we can remove those lines after the defaults are fixed. I agree what you propose is a cleaner fix. I have tested it and submitted a new pull request. |
Take a look at https://github.com/microsoft/CMake . I believe it has the sources for the version that VS ships; and you can also find the people who work on it there. |
…et#3894) * Fixed build errors resulting from upgrade to VS2019 compilers * Added additional message describing the previous fix
Fixes #3893
The default CMAKE_C_FLAG for debug configuration sets /ZI to generate a PDB capable of edit and continue. In the new compilers, this is incompatible with /guard:cf which we set for security reasons. So to fix, we need to go back to a regular pdb generated by the /Zi flag.
Since this part of the default CMake rules, we need to override the full default set and update only that particular flag.