foundationdb
foundationdb copied to clipboard
Fix rocks clang build
I tested this in our developer environment with cmk
and ccmk
after removing the following environment variables (these environment variables are not set by default in the developer container, but were inadvertently being set by something else):
- USE_VALGRIND=0
- USE_VALGRIND_FOR_CTEST=0
- USE_WERROR=0
- USE_GPERFTOOLS=0
- CXX=/opt/rh/devtoolset-8/root/usr/bin/g++
- CC=/opt/rh/devtoolset-8/root/usr/bin/gcc
- USE_LD=DEFAULT
- USE_LIBCXX=0
- # CXX=/usr/local/bin/clang++
- # CC=/usr/local/bin/clang
- # USE_LD=LLD
- # USE_LIBCXX=1
Code-Reviewer Section
The general pull request guidelines can be found here.
Please check each of the following things and check all boxes before accepting a PR.
- [ ] The PR has a description, explaining both the problem and the solution.
- [ ] The description mentions which forms of testing were done and the testing seems reasonable.
- [ ] Every function/class/actor that was touched is reasonably well documented.
For Release-Branches
If this PR is made against a release-branch, please also check the following:
- [ ] This change/bugfix is a cherry-pick from the next younger branch (younger
release-branch
ormain
if this is the youngest branch) - [ ] There is a good reason why this PR needs to go into a release branch and this reason is documented (either in the description above or in a linked GitHub issue)
Doxense CI Report for Windows 10
- Commit ID: 1501ae4940e8a61b917599cb6405fd18d0188ec0
- Result: :heavy_check_mark: SUCCEEDED
- Build Logs (available for 30 days)
Result of foundationdb-pr on Linux CentOS 7
- Commit ID: 1501ae4940e8a61b917599cb6405fd18d0188ec0
- Duration 0:47:04
- Result: :x: FAILED
- Error:
Error while executing command: if python3 -m joshua.joshua list --stopped | grep ${ENSEMBLE_ID} | grep -q 'pass=10[0-9][0-9][0-9]'; then echo PASS; else echo FAIL && exit 1; fi. Reason: exit status 1
- Build Logs (available for 30 days)
- Build Artifact (available for 30 days)
Result of foundationdb-pr-cluster-tests on Linux CentOS 7
- Commit ID: 1501ae4940e8a61b917599cb6405fd18d0188ec0
- Duration 2:09:27
- Result: :x: FAILED
- Error:
Error while executing command: if $(grep -q -- "--- FAIL:" ${CODEBUILD_SRC_DIR}/fdb-kubernetes-tests/logs/*.log); then echo "TESTS FAILED SEE THESE LOGS:"; echo ; grep -l -- "--- FAIL:" ${CODEBUILD_SRC_DIR}/fdb-kubernetes-tests/logs/*.log; exit 1; fi. Reason: exit status 1
- Build Logs (available for 30 days)
- Build Artifact (available for 30 days)
The root cause of why we are not being able to forward the corresponding flags to RocksDB is that:
- We are determining the compiler/linker flags in
ConfigureCompiler.cmake
. TheConfigureCompiler.cmake
is usingadd_compile_options
andadd_link_options
. These commands will not affectCMAKE_CXX_FLAGS
, etc. Instead, they will inject those options to each target appears after these commands. - RocksDB, unfortunately, is not one of the targets within our environment, and
CMAKE_CXX_FLAGS
, etc. are not being affected byConfigureCompiler.cmake
.
RocksDB is built using cmake. I would suggest to use FetchContent to get the RocksDB source (I think this can be done in a way so it only is downloaded if it is not available locally). Then you could just add rocksdb as a normal target.
RocksDB is built using cmake. I would suggest to use FetchContent to get the RocksDB source (I think this can be done in a way so it only is downloaded if it is not available locally). Then you could just add rocksdb as a normal target.
If we use fetch content, would our compile/link flags pollutes the RocksDB environment?
yes -- though I think that's what we want.
The cleanest solution would be to only set compiler, stdlib
flags etc in ConfigureCompiler
and add all other flags to the flow
target. That should work pretty well, though it would be some effort
Why do we need to compile RocksDB at FoundationDB build time?
We don't need to. However, we need to use the same stdlib
for rocks and for fdb. Otherwise we get in trouble. The easiest short-term solution is to compile rocks when we compile fdb. The correct solution imho is to cache the build result and reuse it.
We probably don't want to have a binary in the docker container because the number of desired build configuration could be large (we might want to be able to have a debug build, or an address sanitizer build, etc...). If we have a cache every possible configuration should only be built once.
I think a possible approach is to let the compiler/library configuration goes to CMAKE_CXX_FLAGS while the remaining can stay in COMPILE_OPTIONS.
So, we currently always recompile RocksDB, and optionally download it if the source is not available locally.
I'm not sure just passing the flags through to RocksDB will work. Their cmake configuration non-trivial.
They definitely support clang, but I couldn't figure out what their preferred approach is for using libcxx. I confirmed that the approach used (edit: to tell them to use libcxx) in this PR works under Linux with clang and g++, under windows, and in MacOS.
Yeah; that looks OK to me.
Spoke too soon. Comment in other PR.
Result of foundationdb-pr on Linux CentOS 7
- Commit ID: 1501ae4940e8a61b917599cb6405fd18d0188ec0
- Duration 0:42:37
- Result: :x: FAILED
- Error:
Error while executing command: if python3 -m joshua.joshua list --stopped | grep ${ENSEMBLE_ID} | grep -q 'pass=10[0-9][0-9][0-9]'; then echo PASS; else echo FAIL && exit 1; fi. Reason: exit status 1
- Build Logs (available for 30 days)
- Build Artifact (available for 30 days)
Result of foundationdb-pr-cluster-tests on Linux CentOS 7
- Commit ID: 1501ae4940e8a61b917599cb6405fd18d0188ec0
- Duration 2:10:29
- Result: :x: FAILED
- Error:
Error while executing command: if $(grep -q -- "--- FAIL:" ${CODEBUILD_SRC_DIR}/fdb-kubernetes-tests/logs/*.log); then echo "TESTS FAILED SEE THESE LOGS:"; echo ; grep -l -- "--- FAIL:" ${CODEBUILD_SRC_DIR}/fdb-kubernetes-tests/logs/*.log; exit 1; fi. Reason: exit status 1
- Build Logs (available for 30 days)
- Build Artifact (available for 30 days)
Can we close this PR?