Skip to content

Commit

Permalink
c++ client: stop requiring the old gcc ABI
Browse files Browse the repository at this point in the history
With the upgrade to clang 3.9 and the transition to libc++ for TSAN, we can
now safely remove the old gcc ABI guard rail without breaking TSAN builds.

The impact on backwards compatibility is immaterial. At least one Kudu
vendor has shipped a client package for Ubuntu 16.04 built against the old
ABI; that package will be built against the new ABI going forward. Any
application built against the old ABI will be incompatible with said
package once the ABI changes. But, c++ client consumers are few and far
between, and the major consumer of record (Apache Impala) does not yet
build on Ubuntu 16.04.

Change-Id: Ia06edb8a4699fc842a2072e1edab5cfaa14c4381
Reviewed-on: http://gerrit.cloudera.org:8080/4515
Reviewed-by: Dan Burkert <[email protected]>
Tested-by: Dan Burkert <[email protected]>
  • Loading branch information
adembo authored and danburkert committed Sep 29, 2016
1 parent 9689068 commit 33e98ce
Show file tree
Hide file tree
Showing 8 changed files with 9 additions and 51 deletions.
1 change: 0 additions & 1 deletion .ycm_extra_conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@
'-DKUDU_HEADERS_USE_SHORT_STATUS_MACROS=1',
'-DKUDU_STATIC_DEFINE',
'-Dintegration_tests_EXPORTS',
'-D_GLIBCXX_USE_CXX11_ABI=0',
'-D__STDC_FORMAT_MACROS',
'-fno-strict-aliasing',
'-msse4.2',
Expand Down
9 changes: 0 additions & 9 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -141,15 +141,6 @@ set(CXX_COMMON_FLAGS "-fno-strict-aliasing -msse4.2 -Wall -Wno-sign-compare -Wno
# We want access to the PRI* print format macros.
add_definitions(-D__STDC_FORMAT_MACROS)

# Explicitly disable the new gcc5 ABI. Until clang supports abi tags [1], Kudu's
# generated code (which always uses clang) must be built against the old ABI.
# There's no recourse for using both ABIs in the same process; gcc's advice [2]
# is to build everything against the old ABI.
#
# 1. https://llvm.org/bugs/show_bug.cgi?id=23529
# 2. https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_dual_abi.html
add_definitions(-D_GLIBCXX_USE_CXX11_ABI=0)

# We want short macros from util/status.h.
add_definitions(-DKUDU_HEADERS_USE_SHORT_STATUS_MACROS=1)

Expand Down
9 changes: 8 additions & 1 deletion docs/release_notes.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,14 @@ XXX
[[rn_1.1.0_incompatible_changes]]
== Incompatible changes in Kudu 1.1.0

XXX
=== Client APIs ({cpp}/Java/Python)

- The {cpp} client library no longer requires the
link:https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_dual_abi.html[old gcc5 ABI].
Which ABI is actually used depends on the compiler configuration. Some new distros
(e.g. Ubuntu 16.04) will use the new ABI. Your application must use the same ABI as is
used by the client library; an easy way to guarantee this is to use the same compiler
to build both.

[[known_issues_and_limitations]]
== Known Issues and Limitations
Expand Down
3 changes: 0 additions & 3 deletions python/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,6 @@ def run(self):
ext = Extension('kudu.{0}'.format(submodule_name),
['kudu/{0}.pyx'.format(submodule_name)],
libraries=['kudu_client'],
# Disable the 'new' gcc5 ABI; see the top-level
# CMakeLists.txt for details.
define_macros=[('_GLIBCXX_USE_CXX11_ABI', '0')],
include_dirs=INCLUDE_PATHS,
library_dirs=LIBRARY_DIRS,
runtime_library_dirs=RT_LIBRARY_DIRS)
Expand Down
8 changes: 0 additions & 8 deletions src/kudu/client/client.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,6 @@
#include "kudu/util/monotime.h"
#include "kudu/util/status.h"

#if _GLIBCXX_USE_CXX11_ABI
#error \
"Kudu will not function properly if built with gcc 5's new ABI. " \
"Please modify your application to set -D_GLIBCXX_USE_CXX11_ABI=0. " \
"For more information about the new ABI, see " \
"https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_dual_abi.html."
#endif

namespace kudu {

class ClientStressTest_TestUniqueClientIds_Test;
Expand Down
16 changes: 0 additions & 16 deletions src/kudu/client/client_samples-test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ popd
for include_file in $(find $LIBRARY_DIR -name \*.h) ; do
echo Checking standalone compilation of $include_file...
if ! ${CXX:-g++} \
-D_GLIBCXX_USE_CXX11_ABI=0 \
-o /dev/null \
-I$LIBRARY_DIR/usr/local/include \
$include_file ; then
Expand All @@ -76,21 +75,6 @@ for include_file in $(find $LIBRARY_DIR -name \*.h) ; do
fi
done

# Test that client.h prohibits compilation with GCC 5's new ABI. Obviously the
# macro below will only be set if building with GCC 5; we set it manually for
# the purpose of the test.
if ${CXX:-g++} \
-o /dev/null \
-D_GLIBCXX_USE_CXX11_ABI=1 \
-I$LIBRARY_DIR/usr/local/include \
$LIBRARY_DIR/usr/local/include/kudu/client/client.h ; then
echo
echo ---------------------------------------------
echo "GCC 5 new ABI conftest unexpectedly passed!"
echo ---------------------------------------------
exit 1
fi

# Prefer the cmake on the system path, since we expect our client library
# to be usable with older versions of cmake. But if it isn't there,
# use the one from thirdparty.
Expand Down
3 changes: 0 additions & 3 deletions src/kudu/client/samples/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,5 @@ cmake_minimum_required(VERSION 2.8)
find_package(kuduClient REQUIRED)
include_directories(${KUDU_CLIENT_INCLUDE_DIR})

# The Kudu client library always uses the old gcc ABI.
add_definitions(-D_GLIBCXX_USE_CXX11_ABI=0)

add_executable(sample sample.cc)
target_link_libraries(sample kudu_client)
11 changes: 1 addition & 10 deletions thirdparty/build-thirdparty.sh
Original file line number Diff line number Diff line change
Expand Up @@ -128,17 +128,8 @@ EXTRA_CXXFLAGS="-fno-omit-frame-pointer -O2 $EXTRA_CXXFLAGS"

if [[ "$OSTYPE" =~ ^linux ]]; then
OS_LINUX=1
PARALLEL=$(grep -c processor /proc/cpuinfo)

# Explicitly disable the new gcc5 ABI. Until clang supports abi tags [1],
# Kudu's generated code (which always uses clang) must be built against the
# old ABI. There's no recourse for using both ABIs in the same process; gcc's
# advice [2] is to build everything against the old ABI.
#
# 1. https://llvm.org/bugs/show_bug.cgi?id=23529
# 2. https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_dual_abi.html
EXTRA_CXXFLAGS="$EXTRA_CXXFLAGS -D_GLIBCXX_USE_CXX11_ABI=0"
DYLIB_SUFFIX="so"
PARALLEL=$(grep -c processor /proc/cpuinfo)
elif [[ "$OSTYPE" == "darwin"* ]]; then
OS_OSX=1
DYLIB_SUFFIX="dylib"
Expand Down

0 comments on commit 33e98ce

Please sign in to comment.