Repository navigation
Fix Windows nghttp2 builds while preserving older API compatibility - #905
Merged
Merged
Conversation
Fix MSVC compilation failures caused by undefined ssize_t in nghttp2 headers. Use version checks to select modern APIs for nghttp2 1.60.0 and newer, retain legacy APIs for older releases, and guard the upgrade API separately. Keep the Windows ssize_t shim scoped to the nghttp2 header. Use the installed nghttp2 CMake package when available, propagate that dependency to libhv consumers, and preserve the manual library fallback without imposing a new minimum version. Also I added HTTP Error Code 499 Request Canceled
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Static nghttp2 packages remain unsupported, and the public header unnecessarily hides legacy APIs on non-Windows platforms.
2 open findings
What changed in this PR
Improves nghttp2 compatibility across Windows and older releases while adding HTTP status 499.
Changes:
- Selects nghttp2 APIs based on version.
- Adds CMake package dependency propagation.
- Adds HTTP 499 “Request Cancelled.”
| File | Description |
|---|---|
http/httpdef.h |
Adds status 499. |
http/Http2Parser.h |
Adds nghttp2 compatibility shims. |
http/Http2Parser.cpp |
Selects legacy or modern APIs. |
CMakeLists.txt |
Discovers nghttp2 CMake packages. |
cmake/libhvConfig.cmake.in |
Propagates nghttp2 to consumers. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if(WITH_NGHTTP2) | ||
| add_definitions(-DWITH_NGHTTP2) | ||
| set(LIBS ${LIBS} nghttp2) | ||
| find_package(nghttp2 CONFIG QUIET) |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Comment on lines
+164
to
+170
| find_package(nghttp2 CONFIG QUIET) | ||
| if(TARGET nghttp2::nghttp2) | ||
| set(LIBS ${LIBS} nghttp2::nghttp2) | ||
| set(LIBHV_FIND_DEPENDENCY_NGHTTP2 TRUE) | ||
| else() | ||
| set(LIBS ${LIBS} nghttp2) | ||
| endif() |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix MSVC compilation failures caused by undefined ssize_t in nghttp2 headers. Use version checks to select modern APIs for nghttp2 1.60.0 and newer, retain legacy APIs for older releases, and guard the upgrade API separately. Keep the Windows ssize_t shim scoped to the nghttp2 header.
Use the installed nghttp2 CMake package when available, propagate that dependency to libhv consumers, and preserve the manual library fallback without imposing a new minimum version.
Also I added HTTP Error Code 499 Request Canceled