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.

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 CMake configuration error in snappy #803

Merged
merged 1 commit into from
Sep 1, 2022

Conversation

PragmaTwice
Copy link
Member

This closes #792.

@PragmaTwice
Copy link
Member Author

PragmaTwice commented Sep 1, 2022

@torwig @aleksraiden Hi, could you guys help me to check whether this PR really fixed the CMake error on Fedora or Debian? So thanks!

Here is a simple script for reference to check it:

git clone -b fix-792 https://github.com/PragmaTwice/incubator-kvrocks
cd incubator-kvrocks && ./x.py build

Copy link
Member

@tisonkun tisonkun left a comment

Choose a reason for hiding this comment

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

May you describe a bit how adding these two files helps resolve the issue?

I think it's about CMake resolving order while it looks like we always set GFLAGS_FOUND & GTEST_FOUND to false.

@PragmaTwice
Copy link
Member Author

PragmaTwice commented Sep 1, 2022

May you describe a bit how adding these two files helps resolve the issue?

I think it's about CMake resolving order while it looks like we always set GFLAGS_FOUND & GTEST_FOUND to false.

By default, for find_package(X), CMake will use the builtin FindX.cmake file (provided by CMake) to find the X package.
It will try to find the package in system, e.g. packages installed from package manager.
We overwrite this file to make sure CMake does not use some builtin logic to find the package, because we do not need these package in snappy.

It is actually a bug in (old version of) snappy, we defined SNAPPY_BUILD_TESTS=OFF, but snappy will always find gtest regardless of our provided option, and we do not need gtest in snappy since we do not build unit tests of snappy.

@tisonkun

Copy link
Member

@tisonkun tisonkun left a comment

Choose a reason for hiding this comment

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

Thanks for your explanation! Sounds good to me.

Although, I don't have a Fedora env to verify it :P

@torwig
Copy link
Contributor

torwig commented Sep 1, 2022

@tisonkun I can confirm that the build process goes smoothly on Fedora 35 now.

@tisonkun
Copy link
Member

tisonkun commented Sep 1, 2022

@torwig Thanks for your confirmation!

Merging...

@tisonkun tisonkun merged commit b3506ad into apache:unstable Sep 1, 2022
@aleksraiden
Copy link
Contributor

@tisonkun I can confirm too, that the build process on Debian 11 are OK.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CMake error building on Fedora 35
4 participants