Skip to content

Commit

Permalink
Require C++17 (facebook#9481)
Browse files Browse the repository at this point in the history
Summary:
Drop support for some old compilers by requiring C++17 standard
(or higher). See facebook#9388

First modification based on this is to remove some conditional compilation in slice.h (also
better for ODR)

Also in this PR:
* Fix some Makefile formatting that seems to affect ASSERT_STATUS_CHECKED config in
some cases
* Add c_test to NON_PARALLEL_TEST in Makefile
* Fix a clang-analyze reported "potential leak" in lru_cache_test
* Better "compatibility" definition of DEFINE_uint32 for old versions of gflags
* Fix a linking problem with shared libraries in Makefile (`./random_test: error while loading shared libraries: librocksdb.so.6.29: cannot open shared object file: No such file or directory`)
* Always set ROCKSDB_SUPPORT_THREAD_LOCAL and use thread_local (from C++11)
  * TODO in later PR: clean up that obsolete flag
* Fix a cosmetic typo in c.h (facebook#9488)

Pull Request resolved: facebook#9481

Test Plan:
CircleCI config substantially updated.

* Upgrade to latest Ubuntu images for each release
* Generally prefer Ubuntu 20, but keep a couple Ubuntu 16 builds with oldest supported
compilers, to ensure compatibility
* Remove .circleci/cat_ignore_eagain except for Ubuntu 16 builds, because this is to work
around a kernel bug that should not affect anything but Ubuntu 16.
* Remove designated gcc-9 build, because the default linux build now uses GCC 9 from
Ubuntu 20.
* Add some `apt-key add` to fix some apt "couldn't be verified" errors
* Generally drop SKIP_LINK=1; work-around no longer needed
* Generally `add-apt-repository` before `apt-get update` as manual testing indicated the
reverse might not work.

Travis:
* Use gcc-7 by default (remove specific gcc-7 and gcc-4.8 builds)
* TODO in later PR: fix s390x "Assembler messages: Error: invalid switch -march=z14" failure

AppVeyor:
* Completely dropped because we are dropping VS2015 support and CircleCI covers
VS >= 2017

Also local testing with old gflags (out of necessity when using ROCKSDB_NO_FBCODE=1).

Reviewed By: mrambacher

Differential Revision: D33946377

Pulled By: pdillinger

fbshipit-source-id: ae077c823905b45370a26c0103ada119459da6c1
  • Loading branch information
pdillinger authored and facebook-github-bot committed Feb 5, 2022
1 parent 42c8afd commit fd3e0f4
Show file tree
Hide file tree
Showing 17 changed files with 156 additions and 262 deletions.
157 changes: 74 additions & 83 deletions .circleci/config.yml

Large diffs are not rendered by default.

19 changes: 3 additions & 16 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -43,29 +43,19 @@ env:
- JOB_NAME=cmake-gcc9 # 3-5 minutes
- JOB_NAME=cmake-gcc9-c++20 # 3-5 minutes
- JOB_NAME=cmake-mingw # 3 minutes
- JOB_NAME=make-gcc4.8
- JOB_NAME=status_checked

matrix:
exclude:
- os : linux
arch: arm64
env: JOB_NAME=cmake-mingw
- os : linux
arch: arm64
env: JOB_NAME=make-gcc4.8
- os: linux
arch: ppc64le
env: JOB_NAME=cmake-mingw
- os: linux
arch: ppc64le
env: JOB_NAME=make-gcc4.8
- os: linux
arch: s390x
env: JOB_NAME=cmake-mingw
- os: linux
arch: s390x
env: JOB_NAME=make-gcc4.8
- os: linux
compiler: clang
- if: type = pull_request AND commit_message !~ /FULL_CI/
Expand Down Expand Up @@ -210,6 +200,7 @@ matrix:
env: JOB_NAME=status_checked

install:
- CC=gcc-7 && CXX=g++-7
- if [ "${JOB_NAME}" == cmake-gcc8 ]; then
sudo apt-get install -y g++-8 || exit $?;
CC=gcc-8 && CXX=g++-8;
Expand All @@ -221,9 +212,8 @@ install:
- if [ "${JOB_NAME}" == cmake-mingw ]; then
sudo apt-get install -y mingw-w64 || exit $?;
fi
- if [ "${JOB_NAME}" == make-gcc4.8 ]; then
sudo apt-get install -y g++-4.8 || exit $?;
CC=gcc-4.8 && CXX=g++-4.8;
- if [ "${CXX}" == "g++-7" ]; then
sudo apt-get install -y g++-7 || exit $?;
fi
- |
if [[ "${JOB_NAME}" == cmake* ]]; then
Expand Down Expand Up @@ -293,9 +283,6 @@ script:

mkdir build && cd build && cmake -DCMAKE_BUILD_TYPE=Release -DWITH_TESTS=0 -DWITH_GFLAGS=0 -DWITH_BENCHMARK_TOOLS=0 -DWITH_TOOLS=0 -DWITH_CORE_TOOLS=1 .. && make -j4 && cd .. && rm -rf build && mkdir build && cd build && cmake -DJNI=1 .. -DCMAKE_BUILD_TYPE=Release $OPT && make -j4 rocksdb rocksdbjni
;;
make-gcc4.8)
OPT=-DTRAVIS LIB_MODE=shared V=1 SKIP_LINK=1 make -j4 all && [ "Linking broken because libgflags compiled with newer ABI" ]
;;
status_checked)
OPT=-DTRAVIS LIB_MODE=shared V=1 ASSERT_STATUS_CHECKED=1 make -j4 check_some
;;
Expand Down
22 changes: 6 additions & 16 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
# cd build
# 3. Run cmake to generate project files for Windows, add more options to enable required third-party libraries.
# See thirdparty.inc for more information.
# sample command: cmake -G "Visual Studio 15 Win64" -DCMAKE_BUILD_TYPE=Release -DWITH_GFLAGS=1 -DWITH_SNAPPY=1 -DWITH_JEMALLOC=1 -DWITH_JNI=1 ..
# sample command: cmake -G "Visual Studio 16 2019" -DCMAKE_BUILD_TYPE=Release -DWITH_GFLAGS=1 -DWITH_SNAPPY=1 -DWITH_JEMALLOC=1 -DWITH_JNI=1 ..
# 4. Then build the project in debug mode (you may want to add /m[:<N>] flag to run msbuild in <N> parallel threads
# or simply /m to use all avail cores)
# msbuild rocksdb.sln
Expand All @@ -27,7 +27,7 @@
#
# Linux:
#
# 1. Install a recent toolchain such as devtoolset-3 if you're on a older distro. C++11 required.
# 1. Install a recent toolchain if you're on a older distro. C++17 required (GCC >= 7, Clang >= 5)
# 2. mkdir build; cd build
# 3. cmake ..
# 4. make -j
Expand Down Expand Up @@ -92,7 +92,7 @@ else()
endif()

if( NOT DEFINED CMAKE_CXX_STANDARD )
set(CMAKE_CXX_STANDARD 11)
set(CMAKE_CXX_STANDARD 17)
endif()

include(CMakeDependentOption)
Expand Down Expand Up @@ -342,7 +342,7 @@ endif()

# Check if -latomic is required or not
if (NOT MSVC)
set(CMAKE_REQUIRED_FLAGS "--std=c++11")
set(CMAKE_REQUIRED_FLAGS "--std=c++17")
CHECK_CXX_SOURCE_COMPILES("
#include <atomic>
std::atomic<uint64_t> x(0);
Expand All @@ -369,18 +369,8 @@ endif()
# Reset the required flags
set(CMAKE_REQUIRED_FLAGS ${OLD_CMAKE_REQUIRED_FLAGS})

CHECK_CXX_SOURCE_COMPILES("
#if defined(_MSC_VER) && !defined(__thread)
#define __thread __declspec(thread)
#endif
int main() {
static __thread int tls;
(void)tls;
}
" HAVE_THREAD_LOCAL)
if(HAVE_THREAD_LOCAL)
add_definitions(-DROCKSDB_SUPPORT_THREAD_LOCAL)
endif()
# thread_local is part of C++11 and later (TODO: clean up this define)
add_definitions(-DROCKSDB_SUPPORT_THREAD_LOCAL)

option(WITH_IOSTATS_CONTEXT "Enable IO stats context" ON)
if (NOT WITH_IOSTATS_CONTEXT)
Expand Down
1 change: 1 addition & 0 deletions HISTORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* Fixed more cases of EventListener::OnTableFileCreated called with OK status, file_size==0, and no SST file kept. Now the status is Aborted.

### Public API changes
* Require C++17 compatible compiler (GCC >= 7, Clang >= 5, Visual Studio >= 2017). See #9388.
* Remove HDFS support from main repo.
* Remove librados support from main repo.
* Remove obsolete backupable_db.h and type alias `BackupableDBOptions`. Use backup_engine.h and `BackupEngineOptions`. Similar renamings are in the C and Java APIs.
Expand Down
22 changes: 10 additions & 12 deletions INSTALL.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ than release mode.

RocksDB's library should be able to compile without any dependency installed,
although we recommend installing some compression libraries (see below).
We do depend on newer gcc/clang with C++11 support.
We do depend on newer gcc/clang with C++17 support (GCC >= 7, Clang >= 5).

There are few options when compiling RocksDB:

Expand Down Expand Up @@ -50,7 +50,7 @@ to build a portable binary, add `PORTABLE=1` before your make commands, like thi
## Supported platforms

* **Linux - Ubuntu**
* Upgrade your gcc to version at least 4.8 to get C++11 support.
* Upgrade your gcc to version at least 7 to get C++17 support.
* Install gflags. First, try: `sudo apt-get install libgflags-dev`
If this doesn't work and you're using Ubuntu, here's a nice tutorial:
(http://askubuntu.com/questions/312173/installing-gflags-12-04)
Expand All @@ -62,8 +62,7 @@ to build a portable binary, add `PORTABLE=1` before your make commands, like thi
* Install zstandard: `sudo apt-get install libzstd-dev`.

* **Linux - CentOS / RHEL**
* Upgrade your gcc to version at least 4.8 to get C++11 support:
`yum install gcc48-c++`
* Upgrade your gcc to version at least 7 to get C++17 support
* Install gflags:

git clone https://github.com/gflags/gflags.git
Expand Down Expand Up @@ -113,11 +112,11 @@ to build a portable binary, add `PORTABLE=1` before your make commands, like thi
make && sudo make install

* **OS X**:
* Install latest C++ compiler that supports C++ 11:
* Install latest C++ compiler that supports C++ 17:
* Update XCode: run `xcode-select --install` (or install it from XCode App's settting).
* Install via [homebrew](http://brew.sh/).
* If you're first time developer in MacOS, you still need to run: `xcode-select --install` in your command line.
* run `brew tap homebrew/versions; brew install gcc48 --use-llvm` to install gcc 4.8 (or higher).
* run `brew tap homebrew/versions; brew install gcc7 --use-llvm` to install gcc 7 (or higher).
* run `brew install rocksdb`

* **FreeBSD** (11.01):
Expand Down Expand Up @@ -160,7 +159,7 @@ to build a portable binary, add `PORTABLE=1` before your make commands, like thi

* Install the dependencies for RocksDB:

pkg_add gmake gflags snappy bzip2 lz4 zstd git jdk bash findutils gnuwatch
pkg_add gmake gflags snappy bzip2 lz4 zstd git jdk bash findutils gnuwatch

* Build RocksDB from source:

Expand All @@ -182,13 +181,13 @@ to build a portable binary, add `PORTABLE=1` before your make commands, like thi
* **Windows**:
* For building with MS Visual Studio 13 you will need Update 4 installed.
* Read and follow the instructions at CMakeLists.txt
* Or install via [vcpkg](https://github.com/microsoft/vcpkg)
* Or install via [vcpkg](https://github.com/microsoft/vcpkg)
* run `vcpkg install rocksdb:x64-windows`

* **AIX 6.1**
* Install AIX Toolbox rpms with gcc
* Use these environment variables:

export PORTABLE=1
export CC=gcc
export AR="ar -X64"
Expand All @@ -199,9 +198,9 @@ to build a portable binary, add `PORTABLE=1` before your make commands, like thi
export LIBPATH=/opt/freeware/lib
export JAVA_HOME=/usr/java8_64
export PATH=/opt/freeware/bin:$PATH

* **Solaris Sparc**
* Install GCC 4.8.2 and higher.
* Install GCC 7 and higher.
* Use these environment variables:

export CC=gcc
Expand All @@ -210,4 +209,3 @@ to build a portable binary, add `PORTABLE=1` before your make commands, like thi
export EXTRA_LDFLAGS=-m64
export PORTABLE=1
export PLATFORM_LDFLAGS="-static-libstdc++ -static-libgcc"

28 changes: 15 additions & 13 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ am__v_AR_0 = @echo " AR " $@;
am__v_AR_1 =

AM_LINK = $(AM_V_CCLD)$(CXX) -L. $(patsubst lib%.a, -l%, $(patsubst lib%.$(PLATFORM_SHARED_EXT), -l%, $^)) $(EXEC_LDFLAGS) -o $@ $(LDFLAGS) $(COVERAGEFLAGS)
AM_SHARE = $(AM_V_CCLD) $(CXX) $(PLATFORM_SHARED_LDFLAGS)$@ -L. $(patsubst lib%.$(PLATFORM_SHARED_EXT), -l%, $^) $(LDFLAGS) -o $@
AM_SHARE = $(AM_V_CCLD) $(CXX) $(PLATFORM_SHARED_LDFLAGS)$@ -L. $(patsubst lib%.$(PLATFORM_SHARED_EXT), -l%, $^) $(EXEC_LDFLAGS) $(LDFLAGS) -o $@

# Detect what platform we're building on.
# Export some common variables that might have been passed as Make variables
Expand Down Expand Up @@ -574,27 +574,29 @@ check-headers: $(HEADER_OK_FILES)

# options_settable_test doesn't pass with UBSAN as we use hack in the test
ifdef COMPILE_WITH_UBSAN
TESTS := $(shell echo $(TESTS) | sed 's/\boptions_settable_test\b//g')
TESTS := $(shell echo $(TESTS) | sed 's/\boptions_settable_test\b//g')
endif
ifdef ASSERT_STATUS_CHECKED
# TODO: finish fixing all tests to pass this check
TESTS_FAILING_ASC = \
c_test \
env_test \
range_locking_test \
testutil_test \
# TODO: finish fixing all tests to pass this check
TESTS_FAILING_ASC = \
c_test \
env_test \
range_locking_test \
testutil_test \

# Since we have very few ASC exclusions left, excluding them from
# the build is the most convenient way to exclude them from testing
TESTS := $(filter-out $(TESTS_FAILING_ASC),$(TESTS))
# Since we have very few ASC exclusions left, excluding them from
# the build is the most convenient way to exclude them from testing
TESTS := $(filter-out $(TESTS_FAILING_ASC),$(TESTS))
endif

ROCKSDBTESTS_SUBSET ?= $(TESTS)

# c_test - doesn't use gtest
# env_test - suspicious use of test::TmpDir
# deletefile_test - serial because it generates giant temporary files in
# its various tests. Parallel can fill up your /dev/shm
NON_PARALLEL_TEST = \
c_test \
env_test \
deletefile_test \

Expand Down Expand Up @@ -852,7 +854,7 @@ endif

parallel_tests = $(patsubst %,parallel_%,$(PARALLEL_TEST))
.PHONY: gen_parallel_tests $(parallel_tests)
$(parallel_tests): $(PARALLEL_TEST)
$(parallel_tests):
$(AM_V_at)TEST_BINARY=$(patsubst parallel_%,%,$@); \
TEST_NAMES=` \
(./$$TEST_BINARY --gtest_list_tests || echo " $${TEST_BINARY}__list_tests_failure") \
Expand Down Expand Up @@ -2236,7 +2238,7 @@ rocksdbjavastaticosx_ub: rocksdbjavastaticosx_archs
lipo -create -output ./java/target/$(ROCKSDBJNILIB) java/target/librocksdbjni-osx-x86_64.jnilib java/target/librocksdbjni-osx-arm64.jnilib
$(MAKE) rocksdbjavastatic_jar

rocksdbjavastaticosx_archs:
rocksdbjavastaticosx_archs:
$(MAKE) rocksdbjavastaticosx_arch_x86_64
$(MAKE) rocksdbjavastaticosx_arch_arm64

Expand Down
74 changes: 0 additions & 74 deletions appveyor.yml

This file was deleted.

24 changes: 4 additions & 20 deletions build_tools/build_detect_platform
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,11 @@ if test -z "$OUTPUT"; then
exit 1
fi

# we depend on C++11, but should be compatible with newer standards
# we depend on C++17, but should be compatible with newer standards
if [ "$ROCKSDB_CXX_STANDARD" ]; then
PLATFORM_CXXFLAGS="-std=$ROCKSDB_CXX_STANDARD"
else
PLATFORM_CXXFLAGS="-std=c++11"
PLATFORM_CXXFLAGS="-std=c++17"
fi

# we currently depend on POSIX platform
Expand Down Expand Up @@ -794,24 +794,8 @@ if [ "$?" = 0 ]; then
COMMON_FLAGS="$COMMON_FLAGS -DHAVE_UINT128_EXTENSION"
fi

# iOS doesn't support thread-local storage, but this check would erroneously
# succeed because the cross-compiler flags are added by the Makefile, not this
# script.
if [ "$PLATFORM" != IOS ]; then
$CXX $COMMON_FLAGS -x c++ - -o test.o 2>/dev/null <<EOF
#if defined(_MSC_VER) && !defined(__thread)
#define __thread __declspec(thread)
#endif
int main() {
static __thread int tls;
(void)tls;
}
EOF
if [ "$?" = 0 ]; then
COMMON_FLAGS="$COMMON_FLAGS -DROCKSDB_SUPPORT_THREAD_LOCAL"
fi
fi

# thread_local is part of C++11 and later (TODO: clean up this define)
COMMON_FLAGS="$COMMON_FLAGS -DROCKSDB_SUPPORT_THREAD_LOCAL"

if [ "$FBCODE_BUILD" != "true" -a "$PLATFORM" = OS_LINUX ]; then
$CXX $COMMON_FLAGS $PLATFORM_SHARED_CFLAGS -x c++ -c - -o test_dl.o 2>/dev/null <<EOF
Expand Down
Loading

0 comments on commit fd3e0f4

Please sign in to comment.