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

Update arrow to include https://github.com/apache/arrow/pull/3392 #3765

Merged
merged 3 commits into from
Jan 14, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions cmake/Modules/ArrowExternalProject.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,17 @@
# - PLASMA_SHARED_LIB

set(arrow_URL https://github.com/ray-project/arrow.git)
# This commit is based on https://github.com/apache/arrow/pull/3197. We
# This commit is based on https://github.com/apache/arrow/pull/3392. We
# include the link here to make it easier to find the right commit because
# Arrow often rewrites git history and invalidates certain commits.
# It has been patched to fix an upstream symbol clash with TensorFlow,
# the patch is available at
# https://github.com/ray-project/arrow/commit/c347cd571e51723fc8512922f1b3a8e45e45b169
# https://github.com/ray-project/arrow/commit/f8c990e4165012123e03d822c95526eaea5cbfab
# See the discussion in https://github.com/apache/arrow/pull/3177
# WARNING: If the arrow version is updated, you need to also update the
# SETUPTOOLS_SCM_PRETEND_VERSION version string in the ThirdpartyToolchain.cmake
# file
set(arrow_TAG c347cd571e51723fc8512922f1b3a8e45e45b169)
set(arrow_TAG f8c990e4165012123e03d822c95526eaea5cbfab)
Copy link
Contributor

Choose a reason for hiding this comment

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

Line 23 with # https://github.com/ray-project/arrow/commit/c347cd571e51723fc8512922f1b3a8e45e45b169 should also be changed, right? Is there a time line to merge apache/arrow#3177 , current status seems very inconvenient.

Copy link
Contributor Author

@pcmoritz pcmoritz Jan 14, 2019

Choose a reason for hiding this comment

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

Ok, I changed the link to the patch (originally I didn't because it's the same changes, but could be confusing I agree).

As for an upstream fix, I don't think it will happen any time soon. Also pytorch/tensorflow won't be fixed any time soon, see the discussion (they essentially can't do it on their own I think). It's really annoying, but there is not much we can do about it. The arguments from all sides make a lot of sense to me, probably what actually needs to happen is people need either to switch away from pip to conda, or nvidia needs to make sure that cuda is compatible with manylinux1 (or manylinux2010 in the future) to really fix this for everybody (I assume you followed the discussion in that issue and the mailing list).

Copy link
Contributor

Choose a reason for hiding this comment

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

Understood.


set(ARROW_INSTALL_PREFIX ${CMAKE_CURRENT_BINARY_DIR}/external/arrow-install)
set(ARROW_HOME ${ARROW_INSTALL_PREFIX})
Expand Down
2 changes: 1 addition & 1 deletion cmake/Modules/ThirdpartyToolchain.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ if ("${CMAKE_RAY_LANG_PYTHON}" STREQUAL "YES")

# PYARROW_PARALLEL= , so it will add -j to pyarrow build
set(pyarrow_ENV
"SETUPTOOLS_SCM_PRETEND_VERSION=0.11.1-RAY"
"SETUPTOOLS_SCM_PRETEND_VERSION=0.12.0-RAY"
"PKG_CONFIG_PATH=${ARROW_LIBRARY_DIR}/pkgconfig"
"PYARROW_WITH_PLASMA=1"
"PYARROW_WITH_TENSORFLOW=1"
Expand Down
2 changes: 2 additions & 0 deletions src/ray/object_manager/object_buffer_pool.cc
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
#include "ray/object_manager/object_buffer_pool.h"

#include "arrow/util/logging.h"

namespace ray {

ObjectBufferPool::ObjectBufferPool(const std::string &store_socket_name,
Expand Down
3 changes: 2 additions & 1 deletion src/ray/object_manager/object_store_notification_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@
#include <boost/bind.hpp>
#include <boost/function.hpp>

#include "ray/common/common_protocol.h"
#include "arrow/util/logging.h"

#include "ray/common/common_protocol.h"
#include "ray/object_manager/object_store_notification_manager.h"
#include "ray/util/util.h"

Expand Down
2 changes: 2 additions & 0 deletions src/ray/object_manager/test/object_manager_stress_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@

#include "gtest/gtest.h"

#include "arrow/util/logging.h"

#include "ray/object_manager/object_manager.h"

namespace ray {
Expand Down
2 changes: 2 additions & 0 deletions src/ray/object_manager/test/object_manager_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

#include "gtest/gtest.h"

#include "arrow/util/logging.h"

#include "ray/object_manager/object_manager.h"

namespace ray {
Expand Down
2 changes: 2 additions & 0 deletions src/ray/raylet/node_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

#include <fstream>

#include "arrow/util/logging.h"

#include "ray/common/common_protocol.h"
#include "ray/id.h"
#include "ray/raylet/format/node_manager_generated.h"
Expand Down
2 changes: 2 additions & 0 deletions src/ray/raylet/object_manager_integration_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

#include "gtest/gtest.h"

#include "arrow/util/logging.h"

#include "ray/raylet/raylet.h"

namespace ray {
Expand Down