-
Notifications
You must be signed in to change notification settings - Fork 6.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix compile error for old gcc-4.8 #7358
Conversation
.circleci/config.yml
Outdated
steps: | ||
- checkout | ||
- run: pyenv global 3.5.2 | ||
- run: sudo apt-get install gcc-4.8 g++-4.8 && sudo update-alternatives --install /usr/bin/gcc gcc /usr/bin/gcc-4.8 50 && sudo update-alternatives --install /usr/bin/g++ g++ /usr/bin/g++-4.8 50 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should not need both update-alternatives and setting CC and CXX below. Although it offers some protection against accidental use of plain 'gcc' and 'g++' commands in build, I think it's more misleading than protective.
@@ -3180,7 +3180,8 @@ Status BlockBasedTable::GetKVPairsFromDataBlocks( | |||
} | |||
|
|||
Status BlockBasedTable::DumpTable(WritableFile* out_file) { | |||
auto out_file_wrapper = WritableFileStringStreamAdapter(out_file); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC gcc 4.8 has an incomplete / non-final implementation of C++11. I suspect that this would work:
WritableFileStringStreamAdapter out_file_wrapper(out_file);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, that works. Thanks.
.circleci/config.yml
Outdated
- run: pyenv global 3.5.2 | ||
- run: sudo apt-get install gcc-4.8 g++-4.8 && sudo update-alternatives --install /usr/bin/gcc gcc /usr/bin/gcc-4.8 50 && sudo update-alternatives --install /usr/bin/g++ g++ /usr/bin/g++-4.8 50 | ||
- run: sudo apt-get update -y && sudo apt-get install -y libgflags-dev | ||
- run: CC=gcc-4.8 CXX=g++-4.8 V=1 SKIP_LINK=1 make -j32 all | .circleci/cat_ignore_eagain |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please include this comment from the Travis configuration about why SKIP_LINK is used. It could take someone a while to reverse engineer that justification.
Linking broken because libgflags compiled with newer ABI
And add circleci test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jay-zhuang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
@jay-zhuang merged this pull request in 8a8a01c. |
Summary: gcc-4.8 returns error when using the constructor. Not sure if it's a compiler bug/limitation or code issue: ``` table/block_based/block_based_table_reader.cc:3183:67: error: use of deleted function ‘rocksdb::WritableFileStringStreamAdapter::WritableFileStringStreamAdapter(rocksdb::WritableFileStringStreamAdapter&&)’ ``` Pull Request resolved: facebook#7358 Reviewed By: pdillinger Differential Revision: D23577651 Pulled By: jay-zhuang fbshipit-source-id: b0197e3d3538da61a6f3866410d88d2047fb9695
gcc-4.8 returns error when using the constructor. Not sure if it's a compiler bug/limitation or code issue:
Workaround this by set the
WritableFile
separately.