-
Notifications
You must be signed in to change notification settings - Fork 191
PARQUET-847: Fix CMake compilation error on Debian #231
Conversation
xhochy
left a comment
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.
+1, LGTM
@meox Can you open an issue on https://issues.apache.org/jira/browse/PARQUET/ and prefix the PR with the issue id (PARQUET-XXX: ) ?
|
In my PC now I'm not able to reproduce it :( |
|
Ok ... I try. This morning I downloaded the master branch of parquet-cpp and Thrift-0.10.0. Then I compiled parquet-cpp against this version of Thrift and I noticed a relocation error, So I recompiled again libThrift but the error still there (I tried even to remove the entire BUILD dir several times). After that I started to dig inside CMakeLists in order to find smthg more usfull and I made this changes. But now if I try to recompile the entire project starting from the master (as this moring) I'm able to compile it without any problem. So (honestly) I don't know if there is some dependency problem with the current CMake project or I did somenthing that broke it in some way. |
wesm
left a comment
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.
@meox you may have run into issues with a stale `CMakeCache.txt. Whenever you change anything about your toolchain it's a good idea to clear out all the temporary CMake files
| if (PARQUET_BUILD_SHARED) | ||
| add_library(parquet_thrift_shared SHARED | ||
| ${THRIFT_SRCS} | ||
| ) |
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.
Leaf libraries that are linked into the final .so should not be compiled as shared -- in fact, I don't see a lot of benefit to having a libparquet_thrift.a. I'm going to submit a PR that removes this
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.
Ok, but consider the fact that I removed several times my BUILD_DIRECTORY so all CMakeCache files should be removed.
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 the problem isn't reproducible starting from a new build of Thrift with -fPIC, setting THRIFT_HOME (clearing out any old Thrift installations before build/install), and a clean build, then I'm not sure how to proceed. Here is a build recipe that I know to produce the right libthrift.a -- I have not tested it with Thrift 0.10 yet: https://github.com/conda-forge/thrift-cpp-feedstock/blob/master/recipe/build.sh. I just created https://issues.apache.org/jira/browse/PARQUET-849 to upgrade the Thrift in our default toolchain
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.
ok let me know if I have to close this PullRequest
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.
Yes -- if you run into a reproducible failure, please open a new JIRA. Sorry about the trouble, build toolchains are always a bit of a pain to set up
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.
Trying to compile it with Debian jessie, I have exactly the following error, even with the compilation mentioned in https://github.com/conda-forge/thrift-cpp-feedstock/blob/master/recipe/build.sh.
I also tried to edit CMakeCache.txt and Change the "CMAKE_CXX_FLAGS:STRING=-fPIC" re-compile and install, but the same error occurred:
[ 78%] Linking CXX shared library ../../../debug/libarrow_io.so
/usr/bin/ld: /usr/lib/x86_64-linux-gnu/libboost_system-mt.a(error_code.o): relocation R_X86_64_32 against `.rodata.str1.1' can not be used when making a shared object; recompile with -fPIC
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.
The problem here is that your system's boost was build without -fPIC, you need to rebuild with ./bootstrap.sh && ./b2 "cxxflags=-fPIC"
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.
@xhochy is right. It's really very unfortunate that the Linux distributions do not add the proper compiler flags to allow static linking in shared libraries -- you may consider creating a bug report to the Debian developers to fix their boost libraries.
@olivier-bezet to work around your issue without rebuilding boost, you can pass -DARROW_BOOST_USE_SHARED=on in https://github.com/apache/parquet-cpp/blob/master/cmake_modules/ThirdpartyToolchain.cmake#L326 -- @xhochy maybe we should add a Parquet-cpp option to use shared boost libs when building Arrow automatically in the Thirdparty toolchain?
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.
see https://issues.apache.org/jira/browse/PARQUET-853
@xhochy we are not statically linking the Arrow pieces when we're building Arrow via ExternalProject. This is like to be a source of problems when the user does make install. see https://issues.apache.org/jira/browse/PARQUET-854
patches would be welcome
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,
I had the same problem with thrift, so I installed it using the similar fPIC option:
./boostrap.sh && ./configure --without-php_extension --without-tests --without-qt4 CPPFLAGS=-fPIC
And it is ok now.
|
@meox can you close the PR? Thanks |
|
done :)
…On 1 February 2017 at 21:50, Wes McKinney ***@***.***> wrote:
@meox <https://github.com/meox> can you close the PR? Thanks
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#231 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AA0CREBXGFrPeL9lcZ0f9gC8RSZ-MWTYks5rYPAUgaJpZM4Lwh0Q>
.
--
GL
http://www.meocci.it
|
this fix a problem during the linking of thrift library for shared version of parquet