Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ARROW-11836: [C++] Avoid requiring arrow_bundled_dependencies when it doesn't exist for arrow_static. #9622

Conversation

sighingnow
Copy link
Contributor

No description provided.

…arrow_static.

Signed-off-by: Tao He <sighingnow@gmail.com>
@github-actions
Copy link

github-actions bot commented Mar 3, 2021

@kou kou self-requested a review March 3, 2021 04:18
IMPORTED_LOCATION
"${arrow_lib_dir}/${CMAKE_STATIC_LIBRARY_PREFIX}arrow_bundled_dependencies${CMAKE_STATIC_LIBRARY_SUFFIX}"
)
if(@ARROW_BUNDLED_STATIC_LIBS@)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't work when we have two or more bundled static libraries. e.g.:

if(jemalloc::jemalloc;mimalloc::mimalloc)
  # ...
endif()

doesn't work:

CMake Error at /tmp/local/lib/cmake/arrow/ArrowConfig.cmake:99 (if):
  if given arguments:

    "jemalloc::jemalloc" "mimalloc::mimalloc"

  Unknown arguments specified
Call Stack (most recent call first):
  /tmp/local/lib/cmake/arrow/FindArrow.cmake:213 (find_package)
  /tmp/local/lib/cmake/arrow/FindArrow.cmake:320 (arrow_find_package_cmake_package_configuration)
  /tmp/local/lib/cmake/arrow/FindArrow.cmake:357 (arrow_find_package)
  CMakeLists.txt:24 (find_package)

How about using a variable?

diff --git a/cpp/src/arrow/ArrowConfig.cmake.in b/cpp/src/arrow/ArrowConfig.cmake.in
index a5a3619eb..6209baeec 100644
--- a/cpp/src/arrow/ArrowConfig.cmake.in
+++ b/cpp/src/arrow/ArrowConfig.cmake.in
@@ -37,6 +37,7 @@ set(ARROW_FULL_SO_VERSION "@ARROW_FULL_SO_VERSION@")
 set(ARROW_LIBRARY_PATH_SUFFIXES "@ARROW_LIBRARY_PATH_SUFFIXES@")
 set(ARROW_INCLUDE_PATH_SUFFIXES "@ARROW_INCLUDE_PATH_SUFFIXES@")
 set(ARROW_SYSTEM_DEPENDENCIES "@ARROW_SYSTEM_DEPENDENCIES@")
+set(ARROW_BUNDLED_STATIC_LIBS "@ARROW_BUNDLED_STATIC_LIBS@")
 
 include("${CMAKE_CURRENT_LIST_DIR}/ArrowOptions.cmake")
 
@@ -71,7 +72,7 @@ if(NOT (TARGET arrow_shared OR TARGET arrow_static))
     get_property(arrow_static_loc TARGET arrow_static PROPERTY LOCATION)
     get_filename_component(arrow_lib_dir ${arrow_static_loc} DIRECTORY)
 
-    if(@ARROW_BUNDLED_STATIC_LIBS@)
+    if(ARROW_BUNDLED_STATIC_LIBS)
       add_library(arrow_bundled_dependencies STATIC IMPORTED)
       set_target_properties(
         arrow_bundled_dependencies

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Thanks deinitely works!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kou Fixed.

@nealrichardson nealrichardson changed the title ARROW-11836: [C++] Aovid requiring arrow_bundled_dependencies when it doesn't exist for arrow_static. ARROW-11836: [C++] Avoid requiring arrow_bundled_dependencies when it doesn't exist for arrow_static. Mar 3, 2021
Signed-off-by: Tao He <sighingnow@gmail.com>
Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

Thanks!

@kou kou closed this in 5c3e534 Mar 4, 2021
@sighingnow sighingnow deleted the ht/fixes-find-arrow_bundled_dependencies branch March 4, 2021 05:15
GeorgeAp pushed a commit to sirensolutions/arrow that referenced this pull request Jun 7, 2021
… doesn't exist for arrow_static.

Closes apache#9622 from sighingnow/ht/fixes-find-arrow_bundled_dependencies

Authored-by: Tao He <sighingnow@gmail.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
michalursa pushed a commit to michalursa/arrow that referenced this pull request Jun 13, 2021
… doesn't exist for arrow_static.

Closes apache#9622 from sighingnow/ht/fixes-find-arrow_bundled_dependencies

Authored-by: Tao He <sighingnow@gmail.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants