Fix symbols missing when static linking ngtcp2,nghttp3 and nghttp2 using cmake and MSVC on Windows. #10364
Conversation
| set(CMAKE_REQUIRED_DEFINITIONS "-Dssize_t=__int64") | ||
| else() | ||
| set(CMAKE_REQUIRED_DEFINITIONS "-Dssize_t=long long") | ||
| endif() |
There was a problem hiding this comment.
This type defines seem unrelated to a static/dynamic build. Why do you need those?
There was a problem hiding this comment.
nghttp2 use ssize_t but not define it.
There was a problem hiding this comment.
The failed jobs seem not a problem of this PR, could you please help me to find out what's the problem?
There was a problem hiding this comment.
nghttp2 use
ssize_tbut not define it.
We've built curl with nghttp2 for many years already without having to add any extra defines. Why does the need start now? And if we actually do need them, they are still unrelated to the static builds and should not be done in the same PR that fixes the linking of static libs.
There was a problem hiding this comment.
ssize_t is already defined before by https://github.com/curl/curl/blob/curl-7_87_0/CMakeLists.txt#L1013-L1026 and https://github.com/curl/curl/blob/curl-7_87_0/lib/curl_config.h.cmake#L776 . It's available for all the sources of curl , but unavailable for check_symbol_exists(). I tried to add the same definitions to CMAKE_REQUIRED_DEFINITIONS just to make sure check_symbol_exists(nghttp2_version "nghttp2/nghttp2.h" HAVE_NGHTTP2_DYNAMICLIB) and check_symbol_exists(nghttp2_version "nghttp2/nghttp2.h" HAVE_NGHTTP2_STATICLIB) can work correctly, this setting will be restored by cmake_pop_check_state() after this checking is done and do not take any effect on curl building.
There was a problem hiding this comment.
thanks for explaining, I think I understand now. It is necessary for this "hack" to work.
I will leave the judgement for if this is worth merging to someone who better understands and reads cmake.
There was a problem hiding this comment.
thanks for explaining, I think I understand now. It is necessary for this "hack" to work.
I will leave the judgement for if this is worth merging to someone who better understands and reads cmake.
Thanks a lot.
owent
changed the title
…ing cmake and MSVC on Windows. Signed-off-by: owent <admin@owent.net>
|
Sorry, I see no votes or voices in favor of this PR. Maybe because not many people build static on Windows with cmake. |
Fixes #10363
NGHTTP3_STATICLIB,NGTCP2_STATICLIB,NGHTTP2_STATICLIBas needed。