c++17 and compiler pinning #7107
Conversation
scripts/eosio_build.sh
Outdated
| -DCMAKE_C_COMPILER="${C_COMPILER}" -DCORE_SYMBOL_NAME="${CORE_SYMBOL_NAME}" \ | ||
| -DOPENSSL_ROOT_DIR="${OPENSSL_ROOT_DIR}" -DBUILD_MONGO_DB_PLUGIN=true \ | ||
| -DENABLE_COVERAGE_TESTING="${ENABLE_COVERAGE_TESTING}" -DBUILD_DOXYGEN="${DOXYGEN}" \ | ||
| $CMAKE -DCMAKE_BUILD_TYPE="${CMAKE_BUILD_TYPE}" -DCMAKE_CXX_COMPILER="${CXX}" -DOPENSSL_ROOT_DIR="${OPENSSL_ROOT_DIR}" -DBUILD_MONGO_DB_PLUGIN=false \ |
There was a problem hiding this comment.
As far as I can tell this won't actually use llvm's libc++, for example:
opt/clang8/bin/clang++ -std=c++17 string_view.cpp -o string_view -v
#include <...> search starts here:
/usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8
/usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/x86_64-linux-gnu/c++/8
/usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/backward
/usr/local/include
/home/ubuntu/opt/clang8/lib/clang/8.0.1/include
/usr/include/x86_64-linux-gnu
/usr/include
so we only get the system lib's string_view. Also the linkage is to the system's libc++, i.e.
$ ldd string_view
linux-vdso.so.1 => (0x00007fff2a3e5000)
libstdc++.so.6 => /usr/lib/x86_64-linux-gnu/libstdc++.so.6 (0x00007fd0ec3e1000)
If we're going to pin the compiler, don't we want the stdc++ lib to be part of that pin?
| @@ -25,7 +25,7 @@ include( InstallDirectoryPermissions ) | |||
| include( MASSigning ) | |||
|
|
|||
| set( BLOCKCHAIN_NAME "EOSIO" ) | |||
| set( CMAKE_CXX_STANDARD 14 ) | |||
| set( CMAKE_CXX_STANDARD 17 ) | |||
There was a problem hiding this comment.
Also need to change the compiler check that's a few lines down from here
scripts/eosio_build_amazon.sh
Outdated
| @@ -248,6 +248,39 @@ if [ $? -ne 0 ]; then exit -1; fi | |||
| cd .. | |||
| printf "\\n" | |||
|
|
|||
| if [ $BUILD_CLANG8 ]; then | |||
There was a problem hiding this comment.
I believe this check is true no matter BUILD_CLANG8 contains true or false
scripts/eosio_build.sh
Outdated
| printf "#include <iostream>\nint main(){ std::cout << \"Hello, World!\" << std::endl; }" &> $TEMP_DIR/test.cpp | ||
| `c++ -c -std=c++17 $TEMP_DIR/test.cpp -o $TEMP_DIR/test.o &> /dev/null` | ||
| if [ $? -ne 0 ]; then | ||
| `CXX -c -std=c++17 $TEMP_DIR/test.cpp -o $TEMP_DIR/test.o &> /dev/null` |
There was a problem hiding this comment.
was this supposed to be $CXX instead of CXX?
scripts/eosio_build.sh
Outdated
| @@ -179,6 +181,31 @@ if [ $STALE_SUBMODS -gt 0 ]; then | |||
| exit 1 | |||
| fi | |||
|
|
|||
| BUILD_CLANG8=false | |||
| if [ $NONINTERACTIVE -eq 0 ]; then | |||
| printf "#include <iostream>\nint main(){ std::cout << \"Hello, World!\" << std::endl; }" &> $TEMP_DIR/test.cpp | |||
There was a problem hiding this comment.
This check won't work as is; even the gcc 5.5.0 on ubuntu 16.04 passes validation here (and then the build fails when the cmake 6.0 or greater check fails).
scripts/eosio_build.sh
Outdated
| @@ -71,6 +71,8 @@ export BOOST_LINK_LOCATION=${OPT_LOCATION}/boost | |||
| export LLVM_VERSION=release_40 | |||
| export LLVM_ROOT=${OPT_LOCATION}/llvm | |||
| export LLVM_DIR=${LLVM_ROOT}/lib/cmake/llvm | |||
| export CLANG8_ROOT=${OPT_LOCATION}/clang8 | |||
| export PINNED_COMPILER_VERSION=release_80 | |||
There was a problem hiding this comment.
I'm pretty sure this isn't a pin to a released version. The current compiler version I have after running the script is
clang version 8.0.1 (https://git.llvm.org/git/clang.git a03da8be08a208122e292016cb6cea1f30229677)
|
I am unable to determine why building zlib is required. I made a branch where I removed that: and the builds seemed to be fine: (mac build failed for something unrelated; I didn't change mac builds) |
That is fine, we can change it after we get this merged. I really don't want to keep tinkering with this right now. It builds fine currently, but it might be a little bit aggressive in terms of dependencies. |
Shouldn't be dangerous, as the mac build by default is libcxx and we are statically linking libcxx so even though we don't build llvm4 I don't see it as an issue. Secondly, the plan is that llvm4 is a temporary dependency at this point. Either we replace WAVM or port it up to use llvm 8 which we are already building. |
Change Description
Update libraries and CMake to build with C++ 17 instead of C++ 14. And pin all noninteractive builds (buildkite builds) to use clang 8.
Consensus Changes
API Changes
Documentation Additions