build: add libcurl to contrib build system #173
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| # Design: Integrate curl into Arrow's dependency chain | ||
|
|
||
| ## Problem | ||
|
|
||
| jpfeuffer's review on PR #173 asks that the dependency chain | ||
| curl -> aws-sdk -> arrow/parquet uses the same curl built by contrib. | ||
|
|
||
| Currently: | ||
| - CURL is built after Arrow (wrong order) | ||
| - Arrow has no S3/curl options enabled | ||
| - Arrow does not bundle curl — it requires it via `find_package(CURL)` | ||
|
|
||
| ## Approach | ||
|
|
||
| Build CURL before ARROW and configure Arrow to use it via the existing | ||
| `CMAKE_PREFIX_PATH` mechanism (same pattern used for boost, zlib, bzip2). | ||
|
|
||
| ## Changes | ||
|
|
||
| ### 1. CMakeLists.txt — Reorder build | ||
|
|
||
| Move the CURL build block before ARROW. New order: | ||
| ... -> BOOST -> CURL -> ARROW -> ... | ||
|
|
||
| ### 2. libraries.cmake/arrow.cmake — Enable S3 with bundled AWS SDK | ||
|
|
||
| Add to both Windows and Linux/macOS cmake invocations: | ||
| - `-DARROW_S3=ON` — enables S3 filesystem, triggers AWS SDK + curl | ||
| - `-DAWSSDK_SOURCE=BUNDLED` — Arrow builds AWS SDK from source, using our curl | ||
|
|
||
| Curl is found automatically via `CMAKE_PREFIX_PATH=${PROJECT_BINARY_DIR}` | ||
| which is already set in arrow.cmake. No extra flags needed. | ||
|
|
||
| ### 3. No other changes needed | ||
|
|
||
| - curl.cmake already installs to `${PROJECT_BINARY_DIR}` | ||
| - Dockerfiles already have `libcurl-dev` | ||
| - No new dependency resolution mechanism required | ||
|
|
||
| ## Key facts | ||
|
|
||
| - Arrow 23.0.0 does NOT support `CURL_SOURCE=BUNDLED` — curl must be a system/pre-built dependency | ||
| - Arrow finds curl via standard `find_package(CURL REQUIRED)` in its `find_curl()` macro | ||
| - AWS SDK can be BUNDLED by Arrow, but still requires external curl |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| ################################################## | ||
| ### libcurl ### | ||
| ################################################## | ||
|
|
||
| MACRO( OPENMS_CONTRIB_BUILD_CURL ) | ||
| OPENMS_LOGHEADER_LIBRARY("curl") | ||
|
|
||
| if(MSVC) | ||
| set(ZIP_ARGS x -y -osrc) | ||
| else() | ||
| set(ZIP_ARGS xzf) | ||
| endif() | ||
| OPENMS_SMARTEXTRACT(ZIP_ARGS ARCHIVE_CURL "CURL" "CMakeLists.txt") | ||
|
|
||
| # curl doesn't allow insource builds | ||
| set(_CURL_BUILD_DIR "${CURL_DIR}/build") | ||
| file(TO_NATIVE_PATH "${_CURL_BUILD_DIR}" _CURL_NATIVE_BUILD_DIR) | ||
| execute_process(COMMAND ${CMAKE_COMMAND} -E make_directory ${_CURL_NATIVE_BUILD_DIR}) | ||
|
|
||
| # Platform-specific TLS backend | ||
| if(MSVC) | ||
| set(_CURL_TLS_OPTION "-DCURL_USE_SCHANNEL=ON") | ||
| elseif(APPLE) | ||
| set(_CURL_TLS_OPTION "-DCURL_USE_SECTRANSP=ON") | ||
| else() | ||
| set(_CURL_TLS_OPTION "-DCURL_USE_OPENSSL=ON") | ||
| endif() | ||
|
|
||
| if(APPLE) | ||
| set(_CURL_EXTRA_ARGS | ||
| "-DCMAKE_OSX_DEPLOYMENT_TARGET=${CMAKE_OSX_DEPLOYMENT_TARGET}" | ||
| "-DCMAKE_OSX_SYSROOT=${CMAKE_OSX_SYSROOT}" | ||
| "-DCMAKE_MACOSX_RPATH=TRUE" | ||
| ) | ||
| else() | ||
| set(_CURL_EXTRA_ARGS "") | ||
| endif() | ||
|
|
||
| message(STATUS "Generating curl build system .. ") | ||
| execute_process(COMMAND ${CMAKE_COMMAND} | ||
| -G "${CMAKE_GENERATOR}" | ||
| ${ARCHITECTURE_OPTION_CMAKE} | ||
| -D CMAKE_BUILD_TYPE=Release | ||
| -D CMAKE_INSTALL_PREFIX=${PROJECT_BINARY_DIR} | ||
| -D BUILD_SHARED_LIBS=${BUILD_SHARED_LIBRARIES} | ||
| -D BUILD_CURL_EXE=OFF | ||
|
Comment on lines
+43
to
+46
Contributor
There was a problem hiding this comment. Do not force shared curl builds unconditionally. This overrides the global shared/static policy and can desync curl artifacts from the rest of contrib. Suggested fix- -D BUILD_SHARED_LIBS=ON
+ -D BUILD_SHARED_LIBS=${BUILD_SHARED_LIBRARIES}🤖 Prompt for AI Agents |
||
| -D BUILD_TESTING=OFF | ||
| -D CURL_DISABLE_LDAP=ON | ||
| -D CURL_USE_LIBPSL=OFF | ||
| ${_CURL_TLS_OPTION} | ||
| ${_CURL_EXTRA_ARGS} | ||
| ${CURL_DIR} | ||
| WORKING_DIRECTORY ${_CURL_NATIVE_BUILD_DIR} | ||
| OUTPUT_VARIABLE _CURL_CMAKE_OUT | ||
| ERROR_VARIABLE _CURL_CMAKE_ERR | ||
| RESULT_VARIABLE _CURL_CMAKE_SUCCESS) | ||
|
|
||
| # output to logfile | ||
| file(APPEND ${LOGFILE} ${_CURL_CMAKE_OUT}) | ||
| file(APPEND ${LOGFILE} ${_CURL_CMAKE_ERR}) | ||
|
|
||
| if (NOT _CURL_CMAKE_SUCCESS EQUAL 0) | ||
| message(FATAL_ERROR "Generating curl build system .. failed") | ||
| else() | ||
| message(STATUS "Generating curl build system .. done") | ||
| endif() | ||
|
|
||
| # the install target on windows has a different name than on mac/lnx | ||
| if(MSVC) | ||
| set(_CURL_INSTALL_TARGET "INSTALL") | ||
| else() | ||
| set(_CURL_INSTALL_TARGET "install") | ||
| endif() | ||
|
|
||
| message(STATUS "Building curl (Release) .. ") | ||
| execute_process(COMMAND ${CMAKE_COMMAND} --build ${_CURL_NATIVE_BUILD_DIR} --target ${_CURL_INSTALL_TARGET} --config Release | ||
| WORKING_DIRECTORY ${_CURL_NATIVE_BUILD_DIR} | ||
| OUTPUT_VARIABLE _CURL_BUILD_OUT | ||
| ERROR_VARIABLE _CURL_BUILD_ERR | ||
| RESULT_VARIABLE _CURL_BUILD_SUCCESS) | ||
|
|
||
| # output to logfile | ||
| file(APPEND ${LOGFILE} ${_CURL_BUILD_OUT}) | ||
| file(APPEND ${LOGFILE} ${_CURL_BUILD_ERR}) | ||
|
|
||
| if (NOT _CURL_BUILD_SUCCESS EQUAL 0) | ||
| message(FATAL_ERROR "Building curl (Release) .. failed") | ||
| else() | ||
| message(STATUS "Building curl (Release) .. done") | ||
| endif() | ||
|
|
||
| ENDMACRO( OPENMS_CONTRIB_BUILD_CURL ) | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: OpenMS/contrib
Length of output: 1855
🏁 Script executed:
Repository: OpenMS/contrib
Length of output: 825
🏁 Script executed:
rg -n "set\(ZIP_ARGS" libraries.cmake/ -A 1 -B 1Repository: OpenMS/contrib
Length of output: 3461
🏁 Script executed:
Repository: OpenMS/contrib
Length of output: 3165
Remove quotes from ZIP_ARGS to match project conventions and ensure correct flag parsing.
curl.cmake uses quoted strings for ZIP_ARGS (
"x -y -osrc"and"xzf"), making them single-element lists in CMake. All 14 other library files in libraries.cmake/ use the unquoted tokenized form (x -y -osrcandxzf), which correctly splits extraction flags as separate arguments. The quoted form risks incorrect argument passing to 7z on MSVC.Suggested fix
🤖 Prompt for AI Agents