Copilot commented on code in PR #2168:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2168#discussion_r3180835868
##########
cmake/ssl/FindOpenSSL.cmake:
##########
@@ -15,31 +15,52 @@
# specific language governing permissions and limitations
# under the License.
-# Dummy OpenSSL find for when we use bundled version
-
-if(NOT OPENSSL_FOUND)
- set(OPENSSL_FOUND "YES" CACHE STRING "" FORCE)
- set(OPENSSL_INCLUDE_DIR "${EXPORTED_OPENSSL_INCLUDE_DIR}" CACHE STRING ""
FORCE)
- set(OPENSSL_CRYPTO_LIBRARY "${EXPORTED_OPENSSL_CRYPTO_LIBRARY}" CACHE
STRING "" FORCE)
- set(OPENSSL_SSL_LIBRARY "${EXPORTED_OPENSSL_SSL_LIBRARY}" CACHE STRING ""
FORCE)
- set(OPENSSL_LIBRARIES "${EXPORTED_OPENSSL_LIBRARIES}" CACHE STRING ""
FORCE)
- set(OPENSSL_VERSION "${EXPORTED_OPENSSL_VERSION}" CACHE STRING "" FORCE)
-endif()
-
-if(NOT TARGET OpenSSL::Crypto)
- add_library(OpenSSL::Crypto UNKNOWN IMPORTED)
- set_target_properties(OpenSSL::Crypto PROPERTIES
- INTERFACE_INCLUDE_DIRECTORIES "${OPENSSL_INCLUDE_DIR}")
- set_target_properties(OpenSSL::Crypto PROPERTIES
- IMPORTED_LINK_INTERFACE_LANGUAGES "C"
- IMPORTED_LOCATION "${OPENSSL_CRYPTO_LIBRARY}")
-endif()
-
-if(NOT TARGET OpenSSL::SSL)
- add_library(OpenSSL::SSL UNKNOWN IMPORTED)
- set_target_properties(OpenSSL::SSL PROPERTIES
- INTERFACE_INCLUDE_DIRECTORIES "${OPENSSL_INCLUDE_DIR}")
- set_target_properties(OpenSSL::SSL PROPERTIES
- IMPORTED_LINK_INTERFACE_LANGUAGES "C"
- IMPORTED_LOCATION "${OPENSSL_SSL_LIBRARY}")
-endif()
+
+if (NOT OPENSSL_ROOT_DIR)
+ message(FATAL_ERROR "Strict bundled OpenSSL requires OPENSSL_ROOT_DIR to
be passed to this CMake scope!")
+endif ()
+
+if (WIN32)
+ set(LIB_EXT ".lib")
+elseif (APPLE AND CMAKE_SYSTEM_PROCESSOR MATCHES "^(x86_64|amd64)$")
+ set(LIB_EXT ".dylib")
+else ()
+ set(LIB_EXT ".a")
+endif ()
+
+if (APPLE OR WIN32 OR CMAKE_SIZEOF_VOID_P EQUAL 4 OR CMAKE_SYSTEM_PROCESSOR
MATCHES "^(arm64|aarch64|armv8)$")
+ set(OSSL_LIBDIR "lib")
+else ()
+ set(OSSL_LIBDIR "lib64")
+endif ()
+
+set(OPENSSL_INCLUDE_DIR "${OPENSSL_ROOT_DIR}/include" CACHE STRING "" FORCE)
+set(OPENSSL_CRYPTO_LIBRARY
"${OPENSSL_ROOT_DIR}/${OSSL_LIBDIR}/libcrypto${LIB_EXT}" CACHE STRING "" FORCE)
+set(OPENSSL_SSL_LIBRARY "${OPENSSL_ROOT_DIR}/${OSSL_LIBDIR}/libssl${LIB_EXT}"
CACHE STRING "" FORCE)
+set(OPENSSL_LIBRARIES ${OPENSSL_SSL_LIBRARY} ${OPENSSL_CRYPTO_LIBRARY} CACHE
STRING "" FORCE)
Review Comment:
`OPENSSL_LIBRARIES` used to include `${CMAKE_DL_LIBS}` for bundled OpenSSL,
but this strict finder now drops it. `cmake/KubernetesClientC.cmake` forwards
`OPENSSL_LIBRARIES` into libwebsockets via `LWS_OPENSSL_LIBRARIES`, so static
Linux builds can start failing with unresolved `dlopen`/`dlsym` symbols in that
nested build.
##########
thirdparty/aws-sdk-cpp/aws-c-cal.patch:
##########
@@ -0,0 +1,42 @@
+diff --git a/CMakeLists.txt b/CMakeLists.txt
+index 5d30c6d..dd20fb4 100644
+--- a/CMakeLists.txt
++++ b/CMakeLists.txt
+@@ -128,7 +128,7 @@ endif()
+ if (NOT BYO_CRYPTO)
+ if ((NOT WIN32 AND NOT APPLE) OR
AWS_USE_LIBCRYPTO_TO_SUPPORT_ED25519_EVERYWHERE)
+ if (USE_OPENSSL AND NOT ANDROID)
+- find_package(OpenSSL REQUIRED)
++ include(${FIND_OPENSSL_PATH})
+ find_package(Threads REQUIRED)
+ list(APPEND PLATFORM_LIBS OpenSSL::Crypto Threads::Threads)
+ message(STATUS "Using libcrypto from system:
${OPENSSL_CRYPTO_LIBRARY}")
Review Comment:
This status line is now misleading: after switching to
`include(${FIND_OPENSSL_PATH})`, the build is no longer necessarily using the
system OpenSSL, but the message still says it is. That will send future
debugging in the wrong direction when we're chasing bundled-vs-system
resolution problems.
##########
cmake/ssl/FindOpenSSL.cmake:
##########
@@ -15,31 +15,52 @@
# specific language governing permissions and limitations
# under the License.
-# Dummy OpenSSL find for when we use bundled version
-
-if(NOT OPENSSL_FOUND)
- set(OPENSSL_FOUND "YES" CACHE STRING "" FORCE)
- set(OPENSSL_INCLUDE_DIR "${EXPORTED_OPENSSL_INCLUDE_DIR}" CACHE STRING ""
FORCE)
- set(OPENSSL_CRYPTO_LIBRARY "${EXPORTED_OPENSSL_CRYPTO_LIBRARY}" CACHE
STRING "" FORCE)
- set(OPENSSL_SSL_LIBRARY "${EXPORTED_OPENSSL_SSL_LIBRARY}" CACHE STRING ""
FORCE)
- set(OPENSSL_LIBRARIES "${EXPORTED_OPENSSL_LIBRARIES}" CACHE STRING ""
FORCE)
- set(OPENSSL_VERSION "${EXPORTED_OPENSSL_VERSION}" CACHE STRING "" FORCE)
-endif()
-
-if(NOT TARGET OpenSSL::Crypto)
- add_library(OpenSSL::Crypto UNKNOWN IMPORTED)
- set_target_properties(OpenSSL::Crypto PROPERTIES
- INTERFACE_INCLUDE_DIRECTORIES "${OPENSSL_INCLUDE_DIR}")
- set_target_properties(OpenSSL::Crypto PROPERTIES
- IMPORTED_LINK_INTERFACE_LANGUAGES "C"
- IMPORTED_LOCATION "${OPENSSL_CRYPTO_LIBRARY}")
-endif()
-
-if(NOT TARGET OpenSSL::SSL)
- add_library(OpenSSL::SSL UNKNOWN IMPORTED)
- set_target_properties(OpenSSL::SSL PROPERTIES
- INTERFACE_INCLUDE_DIRECTORIES "${OPENSSL_INCLUDE_DIR}")
- set_target_properties(OpenSSL::SSL PROPERTIES
- IMPORTED_LINK_INTERFACE_LANGUAGES "C"
- IMPORTED_LOCATION "${OPENSSL_SSL_LIBRARY}")
-endif()
+
+if (NOT OPENSSL_ROOT_DIR)
+ message(FATAL_ERROR "Strict bundled OpenSSL requires OPENSSL_ROOT_DIR to
be passed to this CMake scope!")
+endif ()
+
+if (WIN32)
+ set(LIB_EXT ".lib")
+elseif (APPLE AND CMAKE_SYSTEM_PROCESSOR MATCHES "^(x86_64|amd64)$")
+ set(LIB_EXT ".dylib")
+else ()
+ set(LIB_EXT ".a")
+endif ()
+
+if (APPLE OR WIN32 OR CMAKE_SIZEOF_VOID_P EQUAL 4 OR CMAKE_SYSTEM_PROCESSOR
MATCHES "^(arm64|aarch64|armv8)$")
+ set(OSSL_LIBDIR "lib")
+else ()
+ set(OSSL_LIBDIR "lib64")
+endif ()
Review Comment:
The rest of the build treats `ARM64` the same as `arm64`/`aarch64` when
choosing OpenSSL's library directory (`cmake/BundledOpenSSL.cmake` does), but
this regex dropped the uppercase spelling. On Linux ARM64 toolchains that
report `CMAKE_SYSTEM_PROCESSOR=ARM64`, this finder will switch to `lib64` even
though the bundled install uses `lib`, so the imported targets point at files
that do not exist.
##########
thirdparty/aws-sdk-cpp/s2n.patch:
##########
@@ -0,0 +1,51 @@
+diff --git a/CMakeLists.txt b/CMakeLists.txt
+index 2acb62d91..6470de162 100644
+--- a/CMakeLists.txt
++++ b/CMakeLists.txt
+@@ -264,9 +264,9 @@ if (TARGET crypto)
+ message(STATUS "S2N found target: crypto")
+ set(LINK_LIB "crypto")
+ else()
+- find_package(crypto REQUIRED)
++ include(${FIND_OPENSSL_PATH})
+ message(STATUS "Using libcrypto from the cmake path")
+- set(LINK_LIB "AWS::crypto")
++ set(LINK_LIB "OpenSSL::Crypto")
+ endif()
+
+ if (S2N_INTERN_LIBCRYPTO)
+@@ -375,11 +375,11 @@ endif()
+
+ if (S2N_INTERN_LIBCRYPTO)
+
+- # Check if the AWS::crypto target has been added and handle it
+- if (TARGET AWS::crypto)
++ # Check if the OpenSSL::Crypto target has beeen added and handle it
Review Comment:
Typo: 'beeen' should be 'been'.
##########
cmake/ssl/FindOpenSSL.cmake:
##########
@@ -15,31 +15,52 @@
# specific language governing permissions and limitations
# under the License.
-# Dummy OpenSSL find for when we use bundled version
-
-if(NOT OPENSSL_FOUND)
- set(OPENSSL_FOUND "YES" CACHE STRING "" FORCE)
- set(OPENSSL_INCLUDE_DIR "${EXPORTED_OPENSSL_INCLUDE_DIR}" CACHE STRING ""
FORCE)
- set(OPENSSL_CRYPTO_LIBRARY "${EXPORTED_OPENSSL_CRYPTO_LIBRARY}" CACHE
STRING "" FORCE)
- set(OPENSSL_SSL_LIBRARY "${EXPORTED_OPENSSL_SSL_LIBRARY}" CACHE STRING ""
FORCE)
- set(OPENSSL_LIBRARIES "${EXPORTED_OPENSSL_LIBRARIES}" CACHE STRING ""
FORCE)
- set(OPENSSL_VERSION "${EXPORTED_OPENSSL_VERSION}" CACHE STRING "" FORCE)
-endif()
-
-if(NOT TARGET OpenSSL::Crypto)
- add_library(OpenSSL::Crypto UNKNOWN IMPORTED)
- set_target_properties(OpenSSL::Crypto PROPERTIES
- INTERFACE_INCLUDE_DIRECTORIES "${OPENSSL_INCLUDE_DIR}")
- set_target_properties(OpenSSL::Crypto PROPERTIES
- IMPORTED_LINK_INTERFACE_LANGUAGES "C"
- IMPORTED_LOCATION "${OPENSSL_CRYPTO_LIBRARY}")
-endif()
-
-if(NOT TARGET OpenSSL::SSL)
- add_library(OpenSSL::SSL UNKNOWN IMPORTED)
- set_target_properties(OpenSSL::SSL PROPERTIES
- INTERFACE_INCLUDE_DIRECTORIES "${OPENSSL_INCLUDE_DIR}")
- set_target_properties(OpenSSL::SSL PROPERTIES
- IMPORTED_LINK_INTERFACE_LANGUAGES "C"
- IMPORTED_LOCATION "${OPENSSL_SSL_LIBRARY}")
-endif()
+
+if (NOT OPENSSL_ROOT_DIR)
+ message(FATAL_ERROR "Strict bundled OpenSSL requires OPENSSL_ROOT_DIR to
be passed to this CMake scope!")
+endif ()
+
+if (WIN32)
+ set(LIB_EXT ".lib")
+elseif (APPLE AND CMAKE_SYSTEM_PROCESSOR MATCHES "^(x86_64|amd64)$")
Review Comment:
`BundledOpenSSL.cmake` builds macOS x86_64/AMD64 as shared `.dylib`, but
this matcher no longer recognizes `AMD64`. On toolchains that report that
uppercase value, the strict finder will look for `libcrypto.a`/`libssl.a` even
though the bundled install produced `.dylib` files.
##########
cmake/BundledLibcURL.cmake:
##########
@@ -16,91 +16,57 @@
# under the License.
function(use_bundled_curl SOURCE_DIR BINARY_DIR)
- # Define patch step
- set(PATCH_FILE_1 "${SOURCE_DIR}/thirdparty/curl/module-path.patch")
- set(PC ${Bash_EXECUTABLE} -c "set -x && \
- (\"${Patch_EXECUTABLE}\" -p1 -R -s -f --dry-run -i
\"${PATCH_FILE_1}\" || \"${Patch_EXECUTABLE}\" -p1 -N -i \"${PATCH_FILE_1}\")")
- # Define byproducts
- string(TOLOWER "${CMAKE_BUILD_TYPE}" build_type)
- if (WIN32)
- if (build_type MATCHES relwithdebinfo OR build_type MATCHES release)
- set(BYPRODUCT "lib/libcurl.lib")
- else()
- set(BYPRODUCT "lib/libcurl-d.lib")
- endif()
- else()
- include(GNUInstallDirs)
- string(REPLACE "/" ";" LIBDIR_LIST ${CMAKE_INSTALL_LIBDIR})
- list(GET LIBDIR_LIST 0 LIBDIR)
- if (build_type MATCHES relwithdebinfo OR build_type MATCHES release)
- set(BYPRODUCT "${LIBDIR}/libcurl.a")
- else()
- set(BYPRODUCT "${LIBDIR}/libcurl-d.a")
- endif()
- endif()
+ message(STATUS "Using bundled curl via FetchContent")
+
+ find_package(OpenSSL REQUIRED)
+ find_package(ZLIB REQUIRED)
+ find_package(Threads REQUIRED)
- # Set build options
- set(CURL_CMAKE_ARGS ${PASSTHROUGH_CMAKE_ARGS}
- "-DCMAKE_INSTALL_PREFIX=${BINARY_DIR}/thirdparty/curl-install"
- -DBUILD_CURL_EXE=OFF
- -DBUILD_TESTING=OFF
- -DBUILD_SHARED_LIBS=OFF
- -DHTTP_ONLY=ON
- -DCURL_CA_PATH=none
- -DCURL_USE_LIBSSH2=OFF
- -DUSE_LIBIDN2=OFF
- -DCURL_USE_LIBPSL=OFF
- -DCURL_USE_OPENSSL=ON
- -DUSE_NGHTTP2=OFF
- -DCURL_ZSTD=OFF
- -DCURL_BROTLI=OFF
- )
+ include(FetchContent)
- append_third_party_passthrough_args(CURL_CMAKE_ARGS "${CURL_CMAKE_ARGS}")
+ set(PATCH_FILE "${SOURCE_DIR}/thirdparty/curl/module-path.patch")
+ set(PC "${Patch_EXECUTABLE}" -p1 -i "${PATCH_FILE}")
- # Build project
- ExternalProject_Add(
- curl-external
- URL
"https://github.com/curl/curl/releases/download/curl-8_18_0/curl-8.18.0.tar.gz";
- URL_HASH
"SHA256=e9274a5f8ab5271c0e0e6762d2fce194d5f98acc568e4ce816845b2dcc0cf88f"
- SOURCE_DIR "${BINARY_DIR}/thirdparty/curl-src"
- LIST_SEPARATOR % # This is needed for passing semicolon-separated
lists
- CMAKE_ARGS ${CURL_CMAKE_ARGS}
+ FetchContent_Declare(
+ curl
+ URL
"https://github.com/curl/curl/releases/download/curl-8_19_0/curl-8.19.0.tar.gz";
+ URL_HASH
"SHA256=2a2c11db4c122691aa23b4363befda1bfd801770bfebf41e1d21cee4f2ab0f71"
PATCH_COMMAND ${PC}
- BUILD_BYPRODUCTS
"${BINARY_DIR}/thirdparty/curl-install/${BYPRODUCT}"
- EXCLUDE_FROM_ALL TRUE
- DOWNLOAD_NO_PROGRESS TRUE
- TLS_VERIFY TRUE
+ SYSTEM
+ OVERRIDE_FIND_PACKAGE
)
- # Set dependencies
- add_dependencies(curl-external ZLIB::ZLIB OpenSSL::SSL OpenSSL::Crypto)
+ set(BUILD_CURL_EXE OFF CACHE BOOL "" FORCE)
+ set(BUILD_TESTING OFF CACHE BOOL "" FORCE)
+ set(BUILD_SHARED_LIBS OFF CACHE BOOL "" FORCE)
+ set(HTTP_ONLY ON CACHE BOOL "" FORCE)
+ set(CURL_CA_PATH "none" CACHE STRING "" FORCE)
+ set(CURL_USE_LIBSSH2 OFF CACHE BOOL "" FORCE)
+ set(USE_LIBIDN2 OFF CACHE BOOL "" FORCE)
+ set(CURL_USE_LIBPSL OFF CACHE BOOL "" FORCE)
+ set(CURL_USE_OPENSSL ON CACHE BOOL "" FORCE)
+ set(USE_NGHTTP2 OFF CACHE BOOL "" FORCE)
+ set(CURL_ZSTD OFF CACHE BOOL "" FORCE)
+ set(CURL_BROTLI OFF CACHE BOOL "" FORCE)
- # Set variables
- set(CURL_FOUND "YES" CACHE STRING "" FORCE)
- set(CURL_INCLUDE_DIR "${BINARY_DIR}/thirdparty/curl-install/include" CACHE
STRING "" FORCE)
- set(CURL_INCLUDE_DIRS "${CURL_INCLUDE_DIR}" CACHE STRING "" FORCE)
- set(CURL_LIBRARY "${BINARY_DIR}/thirdparty/curl-install/${BYPRODUCT}"
CACHE STRING "" FORCE)
- set(CURL_LIBRARIES "${CURL_LIBRARY}" CACHE STRING "" FORCE)
+ FetchContent_MakeAvailable(curl)
- # Set exported variables for FindPackage.cmake
- set(PASSTHROUGH_VARIABLES ${PASSTHROUGH_VARIABLES}
"-DEXPORTED_CURL_INCLUDE_DIR=${CURL_INCLUDE_DIR}" CACHE STRING "" FORCE)
- set(PASSTHROUGH_VARIABLES ${PASSTHROUGH_VARIABLES}
"-DEXPORTED_CURL_LIBRARY=${CURL_LIBRARY}" CACHE STRING "" FORCE)
+ if (TARGET libcurl_static)
+ target_compile_definitions(libcurl_static PUBLIC CURL_STATICLIB)
- # Create imported targets
- file(MAKE_DIRECTORY ${CURL_INCLUDE_DIRS})
-
- add_library(CURL::libcurl STATIC IMPORTED)
- set_target_properties(CURL::libcurl PROPERTIES IMPORTED_LOCATION
"${CURL_LIBRARY}")
- add_dependencies(CURL::libcurl curl-external)
- target_include_directories(CURL::libcurl INTERFACE ${CURL_INCLUDE_DIRS})
- target_link_libraries(CURL::libcurl INTERFACE ZLIB::ZLIB Threads::Threads
OpenSSL::SSL OpenSSL::Crypto)
- target_compile_definitions(CURL::libcurl INTERFACE CURL_STATICLIB)
- if (APPLE)
- target_link_libraries(CURL::libcurl INTERFACE "-framework
CoreFoundation")
- target_link_libraries(CURL::libcurl INTERFACE "-framework
SystemConfiguration")
- target_link_libraries(CURL::libcurl INTERFACE "-framework
CoreServices")
- elseif (WIN32)
- target_link_libraries(CURL::libcurl INTERFACE Iphlpapi.lib)
+ if (APPLE)
+ target_link_libraries(libcurl_static INTERFACE "-framework
CoreFoundation -framework SystemConfiguration -framework CoreServices")
+ elseif (WIN32)
+ target_link_libraries(libcurl_static INTERFACE Iphlpapi.lib)
+ endif ()
+ else()
+ message(WARNING "libcurl_static target not found; bundled curl may not
link correctly.")
endif()
-endfunction(use_bundled_curl SOURCE_DIR BINARY_DIR)
+
+ # --- EXPORT LEGACY VARIABLES ---
+ set(CURL_FOUND "YES" CACHE INTERNAL "")
+ set(CURL_INCLUDE_DIR "${curl_SOURCE_DIR}/include" CACHE INTERNAL "")
+ set(CURL_INCLUDE_DIRS "${curl_SOURCE_DIR}/include" CACHE INTERNAL "")
+ set(CURL_LIBRARY CURL::libcurl CACHE INTERNAL "")
+ set(CURL_LIBRARIES CURL::libcurl CACHE INTERNAL "")
Review Comment:
This no longer exports any concrete location for the bundled curl.
ExternalProject children still get `CMAKE_MODULE_PATH`/`PASSTHROUGH_VARIABLES`,
and both `BundledLibArchive.cmake` and `BundledAwsSdkCpp.cmake` now consume
`CURL_ROOT_DIR`; because this code never sets `CURL_ROOT_DIR` or the old
`EXPORTED_CURL_*` values, those nested builds can fall back to the host curl
again or fail to find curl at all.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
