fix CXX not initialized early enough in Makefile on openbsd + platform version 10.14 on macos #11675
Conversation
|
Hi @nikoPLP! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
|
@ajkr has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
rocksdb uses C++17 standard. on macos, xcode needs to be configured to target platform 10.14 in order for LLVM to have the C++17 standard.
|
@nikoPLP has updated the pull request. You must reimport the pull request before landing. |
nikoPLP
changed the title
|
@ajkr has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
#11686) Summary: It seems the flag `-fno-elide-constructors` is incorrectly overwritten in Makefile by https://github.com/facebook/rocksdb/blob/9c2ebcc2c365bb89af566b3076f813d7bf11146b/Makefile#L243 Applying the change in PR #11675 shows a lot of missing status checks. This PR adds the missing status checks. Most of changes are just adding asserts in unit tests. I'll add pr comment around more interesting changes that need review. Pull Request resolved: #11686 Test Plan: change Makefile as in #11675, and run `ASSERT_STATUS_CHECKED=1 TEST_UINT128_COMPAT=1 ROCKSDB_MODIFY_NPHASH=1 LIB_MODE=static OPT="-DROCKSDB_NAMESPACE=alternative_rocksdb_ns" make V=1 -j24 J=24 check` Reviewed By: hx235 Differential Revision: D48176132 Pulled By: cbi42 fbshipit-source-id: 6758946cfb1c6ff84c4c1e0ca540d05e6fc390bd
|
@nikoPLP has updated the pull request. You must reimport the pull request before landing. |
|
@ajkr has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
…` (#11686) Summary: It seems the flag `-fno-elide-constructors` is incorrectly overwritten in Makefile by https://github.com/facebook/rocksdb/blob/9c2ebcc2c365bb89af566b3076f813d7bf11146b/Makefile#L243 Applying the change in PR facebook/rocksdb#11675 shows a lot of missing status checks. This PR adds the missing status checks. Most of changes are just adding asserts in unit tests. I'll add pr comment around more interesting changes that need review. Pull Request resolved: facebook/rocksdb#11686 Test Plan: change Makefile as in facebook/rocksdb#11675, and run `ASSERT_STATUS_CHECKED=1 TEST_UINT128_COMPAT=1 ROCKSDB_MODIFY_NPHASH=1 LIB_MODE=static OPT="-DROCKSDB_NAMESPACE=alternative_rocksdb_ns" make V=1 -j24 J=24 check` Reviewed By: hx235 Differential Revision: D48176132 Pulled By: cbi42 fbshipit-source-id: 6758946cfb1c6ff84c4c1e0ca540d05e6fc390bd
…` (#11686) Summary: It seems the flag `-fno-elide-constructors` is incorrectly overwritten in Makefile by https://github.com/facebook/rocksdb/blob/9c2ebcc2c365bb89af566b3076f813d7bf11146b/Makefile#L243 Applying the change in PR facebook/rocksdb#11675 shows a lot of missing status checks. This PR adds the missing status checks. Most of changes are just adding asserts in unit tests. I'll add pr comment around more interesting changes that need review. Pull Request resolved: facebook/rocksdb#11686 Test Plan: change Makefile as in facebook/rocksdb#11675, and run `ASSERT_STATUS_CHECKED=1 TEST_UINT128_COMPAT=1 ROCKSDB_MODIFY_NPHASH=1 LIB_MODE=static OPT="-DROCKSDB_NAMESPACE=alternative_rocksdb_ns" make V=1 -j24 J=24 check` Reviewed By: hx235 Differential Revision: D48176132 Pulled By: cbi42 fbshipit-source-id: 6758946cfb1c6ff84c4c1e0ca540d05e6fc390bd
fixes #11220
fixes #11594
CXX is not initialized early enough in Makefile.
On OpenBSD its value is
g++at first, and this results in severalcommand not found, notably during the tests for HAVE_POWER8 and HAS_ALTIVEC which results in the build problem mentionned in #11594reordering the Makefile fixes the issue, by placing the creation of make_config.mk and its import before any use of
$(CXX)Also, fixes the platofrm version for macos. it must be 10.14 now that rocksdb is using the C++17 standard