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.

Sign up for GitHub

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

Jump to bottom

Fix pyarrow version #3760

Merged
merged 18 commits into from
Jan 13, 2019
Merged

Fix pyarrow version #3760

merged 18 commits into from
Jan 13, 2019

Conversation

pcmoritz
Copy link
Contributor

@pcmoritz pcmoritz commented Jan 12, 2019 •
edited
Loading

This fixes both problems found in #3744

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/10802/
Test PASSed.

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/10807/
Test PASSed.

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/10809/
Test PASSed.

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/10811/
Test PASSed.

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/10810/
Test PASSed.

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/10813/
Test PASSed.

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/10812/
Test PASSed.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/10815/
Test FAILed.

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/10814/
Test PASSed.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/10819/
Test FAILed.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/10822/
Test FAILed.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/10823/
Test FAILed.

robertnishihara
robertnishihara reviewed Jan 13, 2019
View reviewed changes
cmake/Modules/ThirdpartyToolchain.cmake
@@ -118,6 +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"
Copy link
Collaborator

Choose a reason for hiding this comment

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

So does this need to be updated every time we update arrow? If so, we should include a prominent comment in

ray/cmake/Modules/ArrowExternalProject.cmake

Line 17 in 8674606

set(arrow_URL https://github.com/ray-project/arrow.git)

explaining that people need to update this code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

robertnishihara
robertnishihara reviewed Jan 13, 2019
View reviewed changes
python/build-wheel-macos.sh
@@ -67,7 +67,7 @@ for ((i=0; i<${#PY_VERSIONS[@]}; ++i)); do
$PIP_CMD install --upgrade setuptools
# Install setuptools_scm because otherwise when building the wheel for
# Python 3.6, we see an error.
$PIP_CMD install -q setuptools_scm==2.1.0
$PIP_CMD install -q setuptools_scm==3.1.0
Copy link
Collaborator

Choose a reason for hiding this comment

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

What was the issue?

Copy link
Contributor Author

@pcmoritz pcmoritz Jan 13, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

On macos python 2.7 it didn't support SETUPTOOLS_SCM_PRETEND_VERSION properly (the version was just None)

robertnishihara
robertnishihara reviewed Jan 13, 2019
View reviewed changes
cmake/Modules/BoostExternalProject.cmake
@@ -64,6 +64,6 @@ if (RAY_BUILD_BOOST)
BUILD_IN_SOURCE 1
BUILD_BYPRODUCTS ${Boost_BUILD_PRODUCTS}
CONFIGURE_COMMAND ./bootstrap.sh
BUILD_COMMAND bash -c "./b2 cxxflags=-fPIC cflags=-fPIC variant=release link=static --with-filesystem --with-system --with-thread --with-atomic --with-chrono --with-date_time --with-regex -j8 install --prefix=${Boost_INSTALL_PREFIX} > /dev/null"
BUILD_COMMAND bash -c "./b2 cxxflags=\"-fPIC -D_GLIBCXX_USE_CXX11_ABI=0\" cflags=-fPIC variant=release link=static --with-filesystem --with-system --with-thread --with-atomic --with-chrono --with-date_time --with-regex -j8 install --prefix=${Boost_INSTALL_PREFIX} > /dev/null"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you say what this does and what the issue was?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it caused the linker error I posted in #3744. There was a function in boost regex that used std::string and that needs the old ABI.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/10827/
Test FAILed.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/10826/
Test FAILed.

@robertnishihara robertnishihara merged commit 00e9f8d into ray-project:master Jan 13, 2019
@robertnishihara robertnishihara deleted the new-arrow branch January 13, 2019 22:28
@pcmoritz pcmoritz mentioned this pull request Jan 14, 2019
Closed
@robertnishihara robertnishihara mentioned this pull request Jan 14, 2019
Closed
@pcmoritz pcmoritz mentioned this pull request Jan 16, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@robertnishihara robertnishihara robertnishihara approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pcmoritz @AmplabJenkins @robertnishihara