Skip to content

Commit 6e77351

Browse files
majetideepakmeta-codesync[bot]
authored andcommitted
feat: Remove cpr dependency and CMake variable leaks (#14160)
Summary: cpr requires a specific libcurl version and is not portable. The current usage can be replaced by libcurl which is present on most distros. When cuDF is enabled, curl is bundled. Use the system or bundled curl otherwise for testing. When a dependency uses `option` for a variable, it gets cached and becomes global. However, if the variable is defined before, it does not get cached in some instances. This PR unsets commonly used CMake variables BUILD_TESTING and BUILD_SHARED_LIBS if set by a dependency. Pull Request resolved: #14160 Reviewed By: amitkdutta Differential Revision: D86775547 Pulled By: peterenescu fbshipit-source-id: f1a76ada3a86d47024f87b7b10a7e49092d002c9
1 parent de1cf35 commit 6e77351

File tree

16 files changed

+148
-230
lines changed

16 files changed

+148
-230
lines changed

CMake/resolve_dependency_modules/README.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ by Velox. See details on bundling below.
3939
| fbthrift | v2025.04.28.00 | No ||
4040
| libstemmer | 2.2.0 | Yes ||
4141
| DuckDB (testing) | 0.8.1 | Yes ||
42-
| cpr (testing) | 1.10.15 | Yes ||
4342
| arrow | 15.0.0 | Yes ||
4443
| geos | 3.10.7 | Yes ||
4544
| fast_float | v8.0.2 | Yes ||

CMake/resolve_dependency_modules/cpr.cmake

Lines changed: 0 additions & 51 deletions
This file was deleted.

CMake/resolve_dependency_modules/cpr/cpr-libcurl-compatible.patch

Lines changed: 0 additions & 41 deletions
This file was deleted.

CMake/resolve_dependency_modules/cpr/cpr-remove-sancheck.patch

Lines changed: 0 additions & 24 deletions
This file was deleted.

CMake/resolve_dependency_modules/cudf.cmake

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,4 +114,5 @@ block(SCOPE_FOR VARIABLES)
114114
)
115115

116116
unset(BUILD_SHARED_LIBS)
117+
unset(BUILD_TESTING CACHE)
117118
endblock()

CMake/resolve_dependency_modules/curl.cmake

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -13,19 +13,4 @@
1313
# limitations under the License.
1414
include_guard(GLOBAL)
1515

16-
set(VELOX_CURL_VERSION 8.4.0)
17-
string(REPLACE "." "_" VELOX_CURL_VERSION_UNDERSCORES ${VELOX_CURL_VERSION})
18-
set(
19-
VELOX_CURL_BUILD_SHA256_CHECKSUM
20-
16c62a9c4af0f703d28bda6d7bbf37ba47055ad3414d70dec63e2e6336f2a82d
21-
)
22-
string(
23-
CONCAT
24-
VELOX_CURL_SOURCE_URL
25-
"https://github.com/curl/curl/releases/download/"
26-
"curl-${VELOX_CURL_VERSION_UNDERSCORES}/curl-${VELOX_CURL_VERSION}.tar.xz"
27-
)
28-
29-
velox_resolve_dependency_url(CURL)
30-
31-
FetchContent_Declare(curl URL ${VELOX_CURL_SOURCE_URL} URL_HASH ${VELOX_CURL_BUILD_SHA256_CHECKSUM})
16+
add_subdirectory(${CMAKE_CURRENT_LIST_DIR}/curl)
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
# Copyright (c) Facebook, Inc. and its affiliates.
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
set(VELOX_CURL_VERSION 8.4.0)
16+
string(REPLACE "." "_" VELOX_CURL_VERSION_UNDERSCORES ${VELOX_CURL_VERSION})
17+
set(
18+
VELOX_CURL_BUILD_SHA256_CHECKSUM
19+
16c62a9c4af0f703d28bda6d7bbf37ba47055ad3414d70dec63e2e6336f2a82d
20+
)
21+
string(
22+
CONCAT
23+
VELOX_CURL_SOURCE_URL
24+
"https://github.com/curl/curl/releases/download/"
25+
"curl-${VELOX_CURL_VERSION_UNDERSCORES}/curl-${VELOX_CURL_VERSION}.tar.xz"
26+
)
27+
28+
set(BUILD_TESTING OFF)
29+
set(BUILD_SHARED_LIBS ON)
30+
31+
velox_resolve_dependency_url(CURL)
32+
33+
message(STATUS "Building CURL from source")
34+
35+
FetchContent_Declare(curl URL ${VELOX_CURL_SOURCE_URL} URL_HASH ${VELOX_CURL_BUILD_SHA256_CHECKSUM})
36+
37+
FetchContent_MakeAvailable(curl)
38+
39+
# Curl uses CMake option for BUILD_TESTING and BUILD_SHARED_LIBS
40+
# See CMake option semantics.
41+
unset(BUILD_TESTING CACHE)
42+
unset(BUILD_SHARED_LIBS CACHE)

CMake/resolve_dependency_modules/duckdb.cmake

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ FetchContent_Declare(
4444
# that.
4545
set(GIT_COMMIT_HASH "6536a77")
4646
set(BUILD_UNITTESTS OFF)
47+
set(BUILD_TESTING OFF)
4748
set(ENABLE_SANITIZER OFF)
4849
set(ENABLE_UBSAN OFF)
4950
set(BUILD_SHELL OFF)
@@ -65,3 +66,5 @@ endif()
6566

6667
set(CMAKE_CXX_FLAGS ${PREVIOUS_CMAKE_CXX_FLAGS})
6768
set(CMAKE_BUILD_TYPE ${PREVIOUS_BUILD_TYPE})
69+
# Some DuckDB third-party package sets this flags. We cannot control that.
70+
unset(BUILD_TESTING)

CMake/resolve_dependency_modules/faiss.cmake

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,14 @@ FetchContent_Declare(
6060
# Set build options
6161
block()
6262
set(BUILD_SHARED_LIBS OFF)
63+
set(BUILD_TESTING OFF)
6364
set(CMAKE_BUILD_TYPE Release)
6465
set(FAISS_ENABLE_GPU OFF)
6566
set(FAISS_ENABLE_PYTHON OFF)
6667
set(FAISS_ENABLE_GPU_TESTS OFF)
6768
# Make FAISS available
6869
FetchContent_MakeAvailable(faiss)
70+
add_library(FAISS::faiss ALIAS faiss)
71+
unset(BUILD_TESTING CACHE)
72+
unset(BUILD_SHARED_LIBS CACHE)
6973
endblock()

CMake/resolve_dependency_modules/re2.cmake

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,3 +60,5 @@ set(re2_INCLUDE_DIRS ${re2_SOURCE_DIR})
6060

6161
set(RE2_ROOT ${re2_BINARY_DIR})
6262
set(re2_ROOT ${re2_BINARY_DIR})
63+
64+
unset(BUILD_TESTING CACHE)

0 commit comments

Comments
 (0)