[Gandiva] [C++] Add option to use -static-libstdc++ when building libgandiva_jni.so

[asfimport]

This commit broke gandiva packaging by removing static linking of std c+, since dremio consumes a fat jar that includes packaged gandiva native libraries we would need to statically link std c+

As suggested in the commit message will re-introduce it as a CMake Flag.

Reporter: Praveen Kumar / @praveenbingo
Assignee: Wes McKinney / @wesm

PRs and other links:

• GitHub Pull Request #3020
• GitHub Pull Request #3035

_{Note: This issue was originally created as ARROW-3860. Please see the migration documentation for further details.}

[asfimport]

Wes McKinney / @wesm:
-1. As discussed in ba2b2ea#diff-d5e5df5984ba660e999a7c657039f6af, the way to do this is to pass CMAKE_SHARED_LINKER_FLAGS when invoking CMake

cmake -DCMAKE_SHARED_LINKER_FLAGS="..." ..

Another (possibly better) way to produce portable binaries is to use Redhat's devtoolset compiler toolchain. That's how we create portable Linux binaries for Python binary packages

[asfimport]

Praveen Kumar / @praveenbingo:
In which case the fix would be in https://github.com/apache/arrow/blob/master/dev/tasks/gandiva-jars/build-cpp.sh

So keep the issue open until i fix it?

Also (i am new to the open source development workflow, so please correct me if i am wrong), Is it not incorrect to add a backward incompatible change that might break existing behavior? and our priority in that case should be parity first and then modifying it in an backward compatible manner?

[asfimport]

Wes McKinney / @wesm:
Sorry, it wasn't clear to me that the packaging you were describing is part of the repository. In part this is because the community does not have regular visibility into these packaging jobs and whether they work or not. I'd love to be able to easily run all of the tasks as part of verifying changes to the build system. You might want to add some checks to https://github.com/apache/arrow/blob/master/dev/tasks/gandiva-jars/build-cpp.sh to assert that the shared libraries do not have a runtime dependency on libstdc++

I consider hard-coding -static-libstdc++ in a C++ build system to be a bug, see ARROW-3437. Also, these packages are not fully portable because they have a minimum glibc ABI requirement equal to the version in Ubuntu 14.04.

[asfimport]

Praveen Kumar / @praveenbingo:
Thanks a lot Wes. Agreed on all points.

1. I will fix the crossbow build file and add some check to the crossbow file to check the shared libs do not have a runtime dependency on std-c++.
2. Yes we will have a dependency on glibc version of Ubuntu, but the most common versions installed at the customer base seems to be CentOS 7+ and they are good with this minimum version of glibc. We have pending work to support CentOS6 like environments which have a lower version of glibc.
3. Also when i change the crossbow script it will affect the arrow packaging of gandiva (since the shared libs with statically link to std c+) . Would it be ok to use the same for arrow distribution (or) should i make this dynamic so that arrow/crossbow still builds gandiva with dynamic dependency on std-c+ while other clients can customize as per their need?

[asfimport]

Wes McKinney / @wesm:
Could you clarify what you mean by "use the same for arrow distribution"?

[asfimport]

Praveen Kumar / @praveenbingo:
The same packaging script is used by Kristian in the crossbow repo - https://github.com/kszucs/crossbow/tree/build-355-gandiva-jar-trusty. I thought these would be used in the arrow release build to release gandiva too along with the other arrow libraries. So i was wondering if i could change the script to produce the gandiva libraries statically linked to std-c++..

[asfimport]

Wes McKinney / @wesm:
It isn't appropriate to statically link libstdc++ in general binary library distributions – that's part of why I've been making a stink about this issue because the binaries produced by this can only be used in a very narrow context.

If you use the libraries to build an application, depending on the platform where you build, you may have symbol conflicts that will yield segfaults / core dumps.

I can see the argument for building the binaries this way in a JAR distribution of the JNI bindings. Though, beware: if someone links to another JNI-wrapped C++ library with different symbols, it may crash.

[asfimport]

Praveen Kumar / @praveenbingo:
"beware: if someone links to another JNI-wrapped C++ library with different symbols, it may crash."

Yup ran into this already :) had to fix by hiding symbols from the jni shared library.

Ok will try to make this a input flag, so that arrow can continue to distribute gandiva with std-c++ dynamically linked and we can substitute that with a statically linked one.

 

[asfimport]

Praveen Kumar / @praveenbingo:
Wes,

I figured a way to customize crossbow to use the flags only in dremio build context.

But tons of tests are failing with that flag on. Here is a build link.

The tests fail even after hiding symbols so i am guessing it is a double free or something similar due to duplicate linking (the tests link dynamically to stdc++)

Would it be an easier fix to only link the jni libraries statically to stdc++? They would anyway be used in the Java context and would need it work in different environments. This would prevent the core gandiva libraries having these static links.

 

[asfimport]

Wes McKinney / @wesm:
Do you want to just add a temporary build option to enable the behavior that was hard-coded before? That seems the simplest thing to unblock yourself.

I strongly recommend that you migrate to use Redhat's devtoolset compiler toolchain so this is not necessary. Using -static-libstdc++ in shared libraries is a dangerous business; sometimes it will work, other times it won't. It is best used for producing dependency-free executables

[asfimport]

Praveen Kumar / @praveenbingo:
Sure Wes, we will investigate and start on moving to the redhat toolchain.

Thanks a ton for the PR.

[asfimport]

Wes McKinney / @wesm:
Issue resolved by pull request 3035
#3035