Conversation
New option USE_LIBIDN2 defaults to ON for libidn2 detection. Prior to this change libidn2 detection could not be turned off in cmake builds. Reported-by: William A Rowe Jr Fixes curl#6361 Closes #xxxx
|
That looks great @jay, TY! But why not follow the existing CURL_DISABLE_ semantic? |
|
Actually, nevermind, it's less consistent than I expected. For reference, this is the rather crazy list of overrides that the envoyproxy project is using right now... |
|
And these are the crazy lists I've used for building httpd with curl for mod_md... https://github.com/appsuite/oss-httpd-build/blob/4e7558147d7d72bce33e3273f52d0c2e2f96b113/mak/Makefile.build#L446 |
|
cmake's not my wheelhouse but IIRC CURL_DISABLE_ was supposed to be for protocols and USE_ for libraries. It looks like there's no consistent prefix for the libraries, there's USE_, CURL_ and CMAKE_USE_ depending on the library. |
- Reviews the proposal of @jay to resolve libidn2 feature election curl/curl#6362 - Uses -U2 for the patch, to ensure placement but not useless collisions - Moves extended text of the problems addressed to the patch Signed-off-by: William A Rowe Jr <wrowe@vmware.com>
|
@jay things are looking positive for this PR proposal... "Prior to this change libidn2 detection could not be turned off in cmake builds." I'd amend that to state that prior to 7.74 (AFAIK) libidn2 wasn't a dependency at all, this constitutes a regression for long-term maintainers. |
|
libidn2 has been a(n optional) dependency for libcurl since 9c91ec7 (Oct 2016, curl 7.51.0) when it replaced libidn - which was the previous library we used for IDNA hostnames. I can't find anything in the cmake build that changed for libidn2 between 7.73.0 and 7.74.0. |
bagder
left a comment
There was a problem hiding this comment.
Not a cmake expert at all but this looks fine to me!
- Reviews the proposal of @jay to resolve libidn2 feature election curl/curl#6362 - Uses -U2 for the patch, to ensure placement but not useless collisions - Moves extended text of the problems addressed to the patch This additional patch to dodge idn2 is draft, but hopefully resembles the final patch to let us pass a simple toggle to avoid libidn2 for machines deployed without libidn2 libs. In any case, any dynamic additional ld requirement goes against the static build model, and libidn2 would have to be added to the envoy build. As we expect to deprecate curl, this would be movement in the wrong direction. Risk Level: low Testing: local Docs Changes: n/a Release Notes: n/a Platform Specific Features: Linux ldd bindings Fixes: #14506 Signed-off-by: William A Rowe Jr <wrowe@vmware.com>
|
This PR looks good to the envoy project crew... we are about to adopt it for our next quarterly release. If this can make 7.75 we would enjoy dropping our patch. Thanks @jay! |
- Reviews the proposal of @jay to resolve libidn2 feature election curl/curl#6362 - Uses -U2 for the patch, to ensure placement but not useless collisions - Moves extended text of the problems addressed to the patch This additional patch to dodge idn2 is draft, but hopefully resembles the final patch to let us pass a simple toggle to avoid libidn2 for machines deployed without libidn2 libs. In any case, any dynamic additional ld requirement goes against the static build model, and libidn2 would have to be added to the envoy build. As we expect to deprecate curl, this would be movement in the wrong direction. Risk Level: low Testing: local Docs Changes: n/a Release Notes: n/a Platform Specific Features: Linux ldd bindings Fixes: #14506 Signed-off-by: William A Rowe Jr <wrowe@vmware.com>
New option USE_LIBIDN2 defaults to ON for libidn2 detection. Prior to
this change libidn2 detection could not be turned off in cmake builds.
Reported-by: William A Rowe Jr
Fixes #6361
Closes #xxxx