kou commented on code in PR #48971: URL: https://github.com/apache/arrow/pull/48971#discussion_r2726116451
########## cpp/thirdparty/versions.txt: ########## @@ -56,6 +56,9 @@ ARROW_AWSSDK_BUILD_SHA256_CHECKSUM=b9944ba9905a68d6e53abb4f36ab2b3bd18ac88d85716 # Despite the confusing version name this is still the whole Azure SDK for C++ including core, keyvault, storage-common, etc. ARROW_AZURE_SDK_BUILD_VERSION=azure-identity_1.9.0 ARROW_AZURE_SDK_BUILD_SHA256_CHECKSUM=97065bfc971ac8df450853ce805f820f52b59457bd7556510186a1569502e4a1 +# WIL (Windows Implementation Libraries) is required by Azure SDK on Windows for WinHTTP transport +ARROW_WIL_BUILD_VERSION=v1.0.250325.1 +ARROW_WIL_BUILD_SHA256_CHECKSUM=c9e667d5f86ded43d17b5669d243e95ca7b437e3a167c170805ffd4aa8a9a786 Review Comment: Could you keep this list in alphabetical order? Could you also update https://github.com/apache/arrow/blob/64ce4bdc41bae1c7a3461b1a86977cf44287cd6d/cpp/thirdparty/versions.txt#L129-L174 ? ########## cpp/cmake_modules/ThirdpartyToolchain.cmake: ########## @@ -4053,6 +4061,27 @@ endif() function(build_azure_sdk) message(STATUS "Building Azure SDK for C++ from source") + + # On Windows, Azure SDK's WinHTTP transport requires WIL (Windows Implementation Libraries). + # Fetch WIL before Azure SDK so the WIL::WIL target is available. + if(WIN32) + message(STATUS "Fetching WIL (Windows Implementation Libraries) for Azure SDK") + fetchcontent_declare(wil + ${FC_DECLARE_COMMON_OPTIONS} + URL ${ARROW_WIL_URL} + URL_HASH "SHA256=${ARROW_WIL_BUILD_SHA256_CHECKSUM}") + set(WIL_BUILD_PACKAGING OFF) Review Comment: Could you call `prepare_fetchcontent()` that sets common variables for FetchContent? ```suggestion prepare_fetchcontent() set(WIL_BUILD_PACKAGING OFF) ``` ########## cpp/cmake_modules/ThirdpartyToolchain.cmake: ########## @@ -4053,6 +4061,27 @@ endif() function(build_azure_sdk) message(STATUS "Building Azure SDK for C++ from source") + + # On Windows, Azure SDK's WinHTTP transport requires WIL (Windows Implementation Libraries). + # Fetch WIL before Azure SDK so the WIL::WIL target is available. + if(WIN32) + message(STATUS "Fetching WIL (Windows Implementation Libraries) for Azure SDK") + fetchcontent_declare(wil + ${FC_DECLARE_COMMON_OPTIONS} + URL ${ARROW_WIL_URL} + URL_HASH "SHA256=${ARROW_WIL_BUILD_SHA256_CHECKSUM}") + set(WIL_BUILD_PACKAGING OFF) + set(WIL_BUILD_TESTS OFF) + fetchcontent_makeavailable(wil) + # Create a minimal config file so Azure SDK's find_package(wil CONFIG) succeeds. + # The WIL::WIL target already exists from FetchContent above. + file(WRITE "${CMAKE_CURRENT_BINARY_DIR}/wil-config/wilConfig.cmake" + "# WIL loaded via FetchContent - target WIL::WIL already exists\n") + set(wil_DIR + "${CMAKE_CURRENT_BINARY_DIR}/wil-config" + CACHE PATH "" FORCE) Review Comment: Could you add `OVERRIDE_FIND_PACKAGE` https://cmake.org/cmake/help/latest/module/FetchContent.html#command:fetchcontent_declare to `fetchcontent_declare()` like https://github.com/apache/arrow/blob/64ce4bdc41bae1c7a3461b1a86977cf44287cd6d/cpp/cmake_modules/ThirdpartyToolchain.cmake#L1062 instead of this? -- 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]
