Skip to content
This repository was archived by the owner on May 10, 2024. It is now read-only.

PARQUET-847: Fix CMake compilation error on Debian #231

Closed
meox wants to merge 1 commit into from
Closed

PARQUET-847: Fix CMake compilation error on Debian #231

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
11 changes: 9 additions & 2 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,7 @@ set(LIBPARQUET_LINK_LIBS
)

set(BUNDLED_STATIC_LIBS
parquet_thrift
parquet_thrift_static
brotlistatic_dec
brotlistatic_enc
brotlistatic_common
Expand Down Expand Up @@ -579,7 +579,14 @@ add_subdirectory(src/parquet/util)

# Ensure that thrift compilation is done before using its generated headers
# in parquet code.
add_dependencies(parquet_objlib parquet_thrift)

if (PARQUET_BUILD_SHARED)
add_dependencies(parquet_objlib parquet_thrift_shared)
endif()

if (PARQUET_BUILD_STATIC)
add_dependencies(parquet_objlib parquet_thrift_static)
endif()

add_subdirectory(benchmarks)
add_subdirectory(examples)
Expand Down
24 changes: 20 additions & 4 deletions src/parquet/thrift/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,29 @@ set(THRIFT_SRCS
set_source_files_properties(parquet_types.cpp PROPERTIES
COMPILE_FLAGS -Wno-unused-variable)

add_library(parquet_thrift STATIC
${THRIFT_SRCS}
)
if (PARQUET_BUILD_SHARED)
add_library(parquet_thrift_shared SHARED
${THRIFT_SRCS}
)
Copy link
Member

@wesm wesm Jan 28, 2017

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

Copy link
Author

@meox meox Jan 29, 2017

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.

Copy link
Member

@wesm wesm Jan 29, 2017

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

Copy link
Author

@meox meox Jan 29, 2017

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

Copy link
Member

@wesm wesm Jan 29, 2017

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

Copy link

@olivier-bezet olivier-bezet Feb 1, 2017

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

Copy link
Member

@xhochy xhochy Feb 1, 2017

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"

Copy link
Member

@wesm wesm Feb 1, 2017

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?

Copy link
Member

@wesm wesm Feb 1, 2017 •
edited
Loading

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

Copy link

@olivier-bezet olivier-bezet Feb 1, 2017

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.


set_target_properties(parquet_thrift_shared
PROPERTIES
LIBRARY_OUTPUT_DIRECTORY "${BUILD_OUTPUT_ROOT_DIRECTORY}")
endif()

set_target_properties(parquet_thrift
if (PARQUET_BUILD_STATIC)
add_library(parquet_thrift_static STATIC
${THRIFT_SRCS}
)

set_target_properties(parquet_thrift_static
PROPERTIES
LIBRARY_OUTPUT_DIRECTORY "${BUILD_OUTPUT_ROOT_DIRECTORY}")

endif()



set_source_files_properties(${THRIFT_SRCS} PROPERTIES GENERATED TRUE)

# List of thrift output targets
Expand Down