llhttp: fix broken static build for v9.3.1#9803
llhttp: fix broken static build for v9.3.1#9803npc1054657282 wants to merge 1 commit intoxmake-io:devfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the llhttp package configuration to handle version-specific CMake flags, specifically introducing the LLHTTP_ prefix for versions 9.3.1 and later. It also refactors the installation logic to ensure that the xmake build system is only invoked when CMake is not being used. The review feedback suggests a refactor to use a conditional prefix variable for the CMake flags to reduce code redundancy and improve maintainability.
| if package:version():ge("9.3.1") then | ||
| table.insert(configs, "-DLLHTTP_BUILD_SHARED_LIBS=" .. (package:config("shared") and "ON" or "OFF")) | ||
| table.insert(configs, "-DLLHTTP_BUILD_STATIC_LIBS=" .. (package:config("shared") and "OFF" or "ON")) | ||
| else | ||
| table.insert(configs, "-DBUILD_SHARED_LIBS=" .. (package:config("shared") and "ON" or "OFF")) | ||
| table.insert(configs, "-DBUILD_STATIC_LIBS=" .. (package:config("shared") and "OFF" or "ON")) | ||
| end |
There was a problem hiding this comment.
The logic for setting CMake flags can be simplified by determining the variable prefix once based on the version. This reduces redundancy and improves the maintainability of the code.
local prefix = package:version():ge("9.3.1") and "LLHTTP_" or ""
table.insert(configs, "-D" .. prefix .. "BUILD_SHARED_LIBS=" .. (package:config("shared") and "ON" or "OFF"))
table.insert(configs, "-D" .. prefix .. "BUILD_STATIC_LIBS=" .. (package:config("shared") and "OFF" or "ON"))
There was a problem hiding this comment.
I will insist on retaining the complete LLHTTP_BUILD_SHARED_LIBS and LLHTTP_BUILD_STATIC_LIBS to ensure explicitness, readability, searchability, and maintainability.
fix xmake-io#9802 1. nodejs/llhttp#657 replaced the `BUILD_SHARED_LIBS` and `BUILD_STATIC_LIBS` with `LLHTTP_BUILD_SHARED_LIBS` and `LLHTTP_BUILD_STATIC_LIBS`. 2. Fix double compilation bug
已提交至上游 xmake-io/xmake-repo#9803 在此之前先进行本地覆盖的修复。
fix #9802
BUILD_SHARED_LIBSandBUILD_STATIC_LIBSwithLLHTTP_BUILD_SHARED_LIBSandLLHTTP_BUILD_STATIC_LIBS.