From 22442071a00ffd5c9340b70152b7ff83a4edbfee Mon Sep 17 00:00:00 2001 From: William A Rowe Jr Date: Thu, 7 Jan 2021 19:18:05 -0600 Subject: [PATCH] Resolve 14506, avoid libidn2 for our curl dependency - Reviews the proposal of @jay to resolve libidn2 feature election https://github.com/curl/curl/pull/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 --- bazel/foreign_cc/BUILD | 2 ++ bazel/foreign_cc/curl.patch | 68 ++++++++++++++++++++++++++++++++++--- bazel/repositories.bzl | 10 +++--- 3 files changed, 70 insertions(+), 10 deletions(-) diff --git a/bazel/foreign_cc/BUILD b/bazel/foreign_cc/BUILD index a374a7d905de..aa73e3bbbcac 100644 --- a/bazel/foreign_cc/BUILD +++ b/bazel/foreign_cc/BUILD @@ -126,6 +126,8 @@ envoy_cmake_external( "CURL_CA_PATH": "none", "CMAKE_USE_OPENSSL": "off", "OPENSSL_ROOT_DIR": "$EXT_BUILD_DEPS", + # Avoid libidn2 + "USE_LIBIDN2": "off", # NGHTTP2. "USE_NGHTTP2": "on", "NGHTTP2_LIBRARY": "$EXT_BUILD_DEPS/nghttp2", diff --git a/bazel/foreign_cc/curl.patch b/bazel/foreign_cc/curl.patch index e602ba03dc44..73bd8aceed71 100644 --- a/bazel/foreign_cc/curl.patch +++ b/bazel/foreign_cc/curl.patch @@ -1,14 +1,61 @@ +#commit 743021d6c7abba91c47e5be8035ff0497f2b78bd +#Author: Jay Satiro +#Date: Tue Dec 22 15:31:03 2020 -0500 +# +# cmake: Add an option to disable libidn2 +# +# 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 https://github.com/curl/curl/issues/6361 +# Closes #xxxx +# +#commit e952764adbb89f37dbf227a48a55cc57c60b537d +#Author: William A Rowe Jr +#Date: Wed Oct 7 14:32:49 2020 -0500 +# +# Correct fragile windows assumptions +# +# - Locking CMake to 3.16 breaks all features and corrections applied to +# CMake 3.17 and later, including the correction of the poorly designed +# and now abandoned Windows CRT election policy CMP0091 (see final para +# of the policy description here: +# https://cmake.org/cmake/help/v3.18/policy/CMP0091.html). Locking to +# rev 3.16 from ensures a more difficult transition to CMake-current +# +# - Windows curl builds previously only adjusted the Release and Debug +# builds, and combined with CMP0091 to break other flavors. Update any +# /MD* flags with /MT* present in the base and four alternate build +# flavors, without introducing conflicting flag values or introducing +# a CRT election where one is not present +# +# - Windows clang-cl builds of curl static libs are broken when using +# link-lld.exe because curl appended the dynamic run time flags to the +# static library lib.exe options. While these were ignored/no-op on +# Windows link.exe, they cause link-lld from LLVM/clang-cl compile +# toolchain to fail to parse the library command. +# +# Summary exists in this bazel-specific bug report; +# https://github.com/bazelbuild/rules_foreign_cc/issues/426 diff --git a/CMakeLists.txt b/CMakeLists.txt -index ec1cfa782..0c5a72f00 100644 +index 6a1a6fe8e..777ee122f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt -@@ -42,0 +42,5 @@ -+# revert CMake bug triggered by curl's defined max CMake policy version, see https://gitlab.kitware.com/cmake/cmake/-/issues/21288 +@@ -40,4 +40,9 @@ + cmake_minimum_required(VERSION 3.2...3.16 FATAL_ERROR) + ++# Revert CMake bug triggered by curl's defined max CMake policy version, see https://gitlab.kitware.com/cmake/cmake/-/issues/21288 +if(POLICY CMP0091) + cmake_policy(SET CMP0091 OLD) +endif() + -@@ -249,3 +254,6 @@ + set(CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/CMake;${CMAKE_MODULE_PATH}") + include(Utilities) +@@ -248,7 +253,10 @@ endif() + + if(CURL_STATIC_CRT) - set(CMAKE_MSVC_RUNTIME_LIBRARY "MultiThreaded$<$:Debug>") - set(CMAKE_C_FLAGS_RELEASE "${CMAKE_C_FLAGS_RELEASE} /MT") - set(CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG} /MTd") @@ -18,3 +65,16 @@ index ec1cfa782..0c5a72f00 100644 + string(REGEX REPLACE "/MD" "/MT" ${flags_var} "${${flags_var}}") + endif() + endforeach() + endif() + +@@ -619,5 +627,9 @@ endif() + + # Check for idn +-check_library_exists_concat("idn2" idn2_lookup_ul HAVE_LIBIDN2) ++option(USE_LIBIDN2 "Use libidn2 for IDN support" ON) ++set(HAVE_LIBIDN2 OFF) ++if(USE_LIBIDN2) ++ check_library_exists_concat("idn2" idn2_lookup_ul HAVE_LIBIDN2) ++endif() + + # Check for symbol dlopen (same as HAVE_LIBDL) diff --git a/bazel/repositories.bzl b/bazel/repositories.bzl index 67385d6b51f4..e1b65d3aadb1 100644 --- a/bazel/repositories.bzl +++ b/bazel/repositories.bzl @@ -633,12 +633,10 @@ def _com_github_curl(): build_file_content = BUILD_ALL_CONTENT + """ cc_library(name = "curl", visibility = ["//visibility:public"], deps = ["@envoy//bazel/foreign_cc:curl"]) """, - # Patch curl 7.72.0 due to CMake's problematic implementation of policy `CMP0091` - # introduced in CMake 3.15 and then deprecated in CMake 3.18. Curl forcing the CMake - # ruleset to 3.16 breaks the Envoy windows fastbuild target. - # Also cure a fatal assumption creating a static library using LLVM `lld-link.exe` - # adding dynamic link flags, which breaks the Envoy clang-cl library archive step. - # Upstream patch submitted: https://github.com/curl/curl/pull/6050 + # Patch curl 7.74.0 due to CMake's problematic implementation of policy `CMP0091` + # and introduction of libidn2 dependency which is inconsistently available and must + # not be a dynamic dependency on linux. + # Upstream patches submitted: https://github.com/curl/curl/pull/6050 & 6362 # TODO(https://github.com/envoyproxy/envoy/issues/11816): This patch is obsoleted # by elimination of the curl dependency. patches = ["@envoy//bazel/foreign_cc:curl.patch"],