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

cmake: detect keyutils if WITH_LIBCEPHFS OR WITH_RBD #12359

Merged
merged 2 commits into from Dec 7, 2016

Conversation

tchaikov
Copy link
Contributor

@tchaikov tchaikov commented Dec 7, 2016

No description provided.

* find_package(keyutils REQUIRED) if (WITH_LIBCEPHFS OR WITH_RBD)
  prior to this change, we detect keyutils if the building platform is not
  FreeBSD, we should instead check the WITH_* options, and let the
  maintainer to decided what is the best for his/her platform, and error
  out if the building host cannot fulfill the requirement to build the
  asserts.
* build krbd.cc if (WITH_RBD)

Signed-off-by: Kefu Chai <kchai@redhat.com>
* simplify the link dependencies.
* s/keyutils/${KEYUTILS_LIBRARIES}/

Signed-off-by: Kefu Chai <kchai@redhat.com>
@tchaikov
Copy link
Contributor Author

tchaikov commented Dec 7, 2016

@wjwithagen does this change make sense to you? it reverts part of c7c07da

@wjwithagen
Copy link
Contributor

@tchaikov @bassamtabbara
I remember looking at the keyutils internals, and porting in its full scale to FreeBSD is going to take quite some lifting. So for the time being it is not going to happen. There is a version in the Linux compat modules, but in how far that is actually usable in a FreeBSD code.
And I do want to start looking at RBD stuff, but then requiring keyutils is going to be a showstopper.

Copy link

@dillaman dillaman left a comment

Choose a reason for hiding this comment

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

lgtm

@tchaikov tchaikov merged commit 899e650 into ceph:master Dec 7, 2016
@tchaikov tchaikov deleted the wip-cmake branch December 7, 2016 15:09
@tchaikov
Copy link
Contributor Author

tchaikov commented Dec 7, 2016

@wjwithagen okay, since keyutils is still required by rbd, let's worry about it once you decide to look at it.

if(LINUX)
add_library(krbd_objs OBJECT krbd.cc)
endif()

if(${WITH_RBD})
Copy link
Contributor

Choose a reason for hiding this comment

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

should this be if (WITH_RBD)

Copy link
Contributor

Choose a reason for hiding this comment

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

@bassamtabbara
Yup, that is the normal invocation in the project, yours would work also in this specific case but still without ${} is preferred.
But if you want to know, loks up variable in the cmake manual

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks, will change it next time when i happen to be changing this part.

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