Skip to content
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.

Sign up for GitHub

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

Jump to bottom

RocksDB now requires gflags v2.2.0 #10933

Closed
wants to merge 1 commit into from
Closed

RocksDB now requires gflags v2.2.0 #10933

wants to merge 1 commit into from

Conversation

adamretter
Copy link
Collaborator

No description provided.

@adamretter adamretter added bug Confirmed RocksDB bugs and removed CLA Signed labels Nov 8, 2022
@fish2bird
Copy link
Contributor

maybe 2.2.0 is least for work, and shared library wanted

for build gflags from source

mkdir cmake-build-release/
cd cmake-build-release/
cmake -DCMAKE_BUILD_TYPE=Release -DBUILD_SHARED_LIBS=1 ..
cmake --build .
cmake --install .

gflags/gflags@eaf0534

@adamretter
Copy link
Collaborator Author

@chencang1980 I just tested and it appears that 2.2.0 works fine as well, so I have updated this PR.

@adamretter adamretter changed the title RocksDB now requires gflags v2.2.2 RocksDB now requires gflags v2.2.0 Nov 9, 2022
@anand1976
Copy link
Contributor

@adamretter Can you update the description with why this change is needed?

@adamretter
Copy link
Collaborator Author

@anand1976 It is hard to remember from so long ago. I think (but my recollection may be wrong), that it was not possible to build RocksDB still with gflags 2.0, instead you now need gflags 2.1 or newer

@adamretter
Copy link
Collaborator Author

@anand1976 rebased and once again ready for review/import.

1 similar comment
@adamretter
Copy link
Collaborator Author

@anand1976 rebased and once again ready for review/import.

jowlyzhang
jowlyzhang approved these changes Oct 2, 2023
View reviewed changes
Copy link
Contributor

@jowlyzhang jowlyzhang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you for the change! Can you try rebase and kick off the CI tests again, Thanks!

@adamretter
Copy link
Collaborator Author

adamretter commented Oct 3, 2023 •
edited
Loading

@jowlyzhang I have now rebased this again, thanks.

@adamretter
Copy link
Collaborator Author

@jowlyzhang Looks like a compilation issue with Snappy 1.10, I have rolled back to 1.1.9.

@facebook-github-bot
Copy link
Contributor

@jowlyzhang has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@jowlyzhang merged this pull request in c13569e.

@igorcanadi igorcanadi mentioned this pull request Jan 17, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jowlyzhang jowlyzhang jowlyzhang approved these changes

@jay-zhuang jay-zhuang Awaiting requested review from jay-zhuang

Assignees
No one assigned
Labels
bug Confirmed RocksDB bugs CLA Signed Merged
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@adamretter @fish2bird @anand1976 @facebook-github-bot @jowlyzhang