add new supported MSVC version#20831
Conversation
alalek
left a comment
There was a problem hiding this comment.
Thank you!
Commit with touching of linker flags is the most questionable here (need to investigate in which configuration these flags are used).
CMakeLists.txt
Outdated
|
|
||
| if(POLICY CMP0069) | ||
| cmake_policy(SET CMP0069 NEW) # CMake 3.9+: target property INTERPROCEDURAL_OPTIMIZATION | ||
| endif() |
There was a problem hiding this comment.
Please keep sorted order for policies
| elseif(MSVC_VERSION MATCHES "^192[0-9]$") | ||
| set(OpenCV_RUNTIME vc16) | ||
| elseif(MSVC_VERSION MATCHES "^193[0-9]$") | ||
| set(OpenCV_RUNTIME vc17) |
There was a problem hiding this comment.
Need to update compiler detection here too: https://github.com/opencv/opencv/blob/3.4.15/cmake/templates/OpenCVConfig.root-WIN32.cmake.in#L122-L139
|
|
||
| if(MSVC) | ||
| if(MSVC AND NOT BUILD_SHARED_LIBS) | ||
| set_target_properties(${the_module} PROPERTIES LINK_FLAGS "/NODEFAULTLIB:atlthunk.lib /NODEFAULTLIB:atlsd.lib /NODEFAULTLIB:libcmt.lib /DEBUG") |
There was a problem hiding this comment.
We lost /DEBUG options here too. Is it intended behavior?
There was a problem hiding this comment.
I could set -DEBUG here unconditionally but it shouldn't be set here at all. Should all of this be moved to ocv_define_module?
There was a problem hiding this comment.
How about conditionalizing -NODEFAULTLIB for 3.4 keeping -DEBUG, all the while cleaning it up in the pull request for master? These flags only make sense when building with mismatched CRTs from different compiler versions or mixed -MD/-MT, while -DEBUG should already be passed when BUILD_WITH_DEBUG_INFO is set.
There was a problem hiding this comment.
"/DEBUG" for linker-only and "BUILD_WITH_DEBUG_INFO" are different things. The second one provides complete debug information which is heavy.
when building with mismatched CRTs from different compiler versions or mixed -MD/-MT
Right.
I believe, such build configurations are still useful for distribution of prebuilt OpenCV binaries.
|
Should I be rebasing during review, only after, or not at all? Fixup will lose existing review comments. |
|
Please rebase commits. GitHub keep review comments. |
c3acb69 to
32d9383
Compare
|
I have installed MSVS 2022 Preview (17.0.0 Preview 4.1), but I'm not able to reproduce related warnings without these patches. Compiler version is or Please left in this PR the first commit only (with vc17 handling) and extract other commits to dedicated 2 PRs with information how to reproduce observed problem (without extra toolchain). |
32d9383 to
04b40ff
Compare
|
Done. |
This is a backport of #20804 to the 3.4 branch.