-
Notifications
You must be signed in to change notification settings - Fork 6.2k
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
Changes to build to fix creation of wheels. #840
Changes to build to fix creation of wheels. #840
Conversation
Merged build finished. Test PASSed. |
Test PASSed. |
Merged build finished. Test PASSed. |
Test PASSed. |
This might not completely work yet. I just pip installed one of the wheels I built after this change and saw the following error (upon importing Ray).
|
Merged build finished. Test PASSed. |
Test PASSed. |
Merged build finished. Test PASSed. |
Test PASSed. |
Note some relevant discussion here https://issues.apache.org/jira/browse/ARROW-1368. |
Merged build finished. Test PASSed. |
Test PASSed. |
Merged build finished. Test PASSed. |
Test PASSed. |
Merged build finished. Test PASSed. |
Test PASSed. |
After compiling boost ourselves with fPIC (on Linux), things are more or less working. However, Ray still does not work on CentOS. In particular, the plasma manager crashes as soon as it starts up (there's some assertion failure somewhere in flatbuffers). |
Merged build finished. Test PASSed. |
Test PASSed. |
Merged build finished. Test PASSed. |
Test PASSed. |
The failure I'm currently seeing when starting a plasma manager on CentOS (or when using a wheel built on CentOS) is the following (an assertion failure immediately when the plasma manager starts, somewhere in flatbuffers).
Doing this in gbd and typing
Searching manually, the backtrace is something like
Actually, now that I look at it, this call to |
It looks like the problem was a flatbuffer version mismatch (I'm still a little confused about which flatbuffers library the plasma manager's plasma store client was using though). Anyway, both Arrow and Ray supposedly use flatbuffers 1.7.1, but the wheels are being built on a docker image that contains flatbuffers 1.6.0 already https://github.com/apache/arrow/blob/master/python/manylinux1/scripts/build_flatbuffers.sh, so that was the version of flatbuffers being used by Arrow (in |
Merged build finished. Test PASSed. |
Test PASSed. |
Merged build finished. Test PASSed. |
Test PASSed. |
Merged build finished. Test PASSed. |
Test PASSed. |
cc @rshin, it looks like the problem was caused at least in part by a flatbuffers version mismatch. Ray was building flatbuffers 1.7.1, but Arrow was finding flatbuffers 1.6.0 which was already in the docker image that I was using. |
@@ -81,12 +81,15 @@ def has_ext_modules(self): | |||
# The BinaryDistribution argument triggers build_ext. | |||
distclass=BinaryDistribution, | |||
install_requires=["numpy", | |||
"cython", |
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.
If you build your wheel correctly, cython should only be a build/setup_requires but not needed for installation.
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.
Thanks! Trying to fix this in #878.
This addresses #843.
It should also do the following.
$ARROW_HOME/lib64
instead of$ARROW_HOME/lib
, so in that case we just copy it to$ARROW_HOME/lib
. This is not the right solution.-fPIC
, so that we can link it statically into arrow and numbuf.Some changes we should consider making.
setup.py
, movecython
frominstall_requires
tosetup_requires
(and maybesetuptools_scm
as well, why notnumpy
as well?), see https://github.com/apache/arrow/blob/master/python/setup.py.