RESOLVED INVALID 176887
[CMake] Find Nghttp2
https://bugs.webkit.org/show_bug.cgi?id=176887
Summary [CMake] Find Nghttp2
Don Olmstead
Reported 2017-09-13 18:24:53 PDT
WinCairo plans to use nghttp2 once curl is updated in the requirements.
Attachments
Patch (3.21 KB, patch)
2017-09-13 18:28 PDT, Don Olmstead
achristensen: review-
Don Olmstead
Comment 1 2017-09-13 18:28:37 PDT
Created attachment 320720 [details] Patch Support detection of nghttp2
Alex Christensen
Comment 2 2017-09-13 23:21:18 PDT
Comment on attachment 320720 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=320720&action=review > Source/cmake/FindNghttp2.cmake:52 > + file(READ "${NGHTTP2_INCLUDE_DIRS}/nghttp2ver.h" _nghttp2_version_content) > + > + string(REGEX MATCH "#define +NGHTTP2_VERSION +\"([0-9]+\.[0-9]+\.[0-9]+)\"" _dummy "${_nghttp2_version_content}") > + set(NGHTTP2_VERSION "${CMAKE_MATCH_1}") It would be nice to not do this every time we run CMake. Why will we need this value from within CMake? Can't we just detect it in headers or require a certain version?
Konstantin Tokarev
Comment 3 2017-09-14 03:00:01 PDT
Comment on attachment 320720 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=320720&action=review >> Source/cmake/FindNghttp2.cmake:52 >> + set(NGHTTP2_VERSION "${CMAKE_MATCH_1}") > > It would be nice to not do this every time we run CMake. Why will we need this value from within CMake? Can't we just detect it in headers or require a certain version? Certainly, this code should be run only once. One way to do this is to save result in cache variable. However, I don't see that we make use of this version variable. So it should be enough to pass PC_NGHTTP2_VERSION instead like most our modules do, and if pkg-config is not used it will be undefined.
Note You need to log in before you can comment on or make changes to this bug.