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: align cmake names of library packages #14951

Merged
merged 1 commit into from May 4, 2017

Conversation

Projects
None yet
2 participants
@smithfarm
Contributor

smithfarm commented May 4, 2017

The cmake package names of libraries are case-sensitive. The name used in
FIND_PACKAGE_HANDLE_STANDARD_ARGS - e.g. "cryptopp" - must match the name used
in the module name - "Findcryptopp.cmake" - as well as the name used in
find_package() call - "find_package(cryptopp REQUIRED)"

Note that the term "package" here refers to a cmake abstraction, not an actual
RPM or Debian package.

Fixes: http://tracker.ceph.com/issues/19853
Signed-off-by: Nathan Cutler ncutler@suse.com

@smithfarm smithfarm requested a review from tchaikov May 4, 2017

cmake: align cmake names of library packages
The cmake package names of libraries are case-sensitive. The name used in
FIND_PACKAGE_HANDLE_STANDARD_ARGS - e.g. "cryptopp" - must match the name used
in the module name - "Findcryptopp.cmake" - as well as the name used in
find_package() call - "find_package(cryptopp REQUIRED)"

Note that the term "package" here refers to a cmake abstraction, *not* an
actual RPM or Debian package.

Fixes: http://tracker.ceph.com/issues/19853
Signed-off-by: Nathan Cutler <ncutler@suse.com>
@smithfarm

This comment has been minimized.

Contributor

smithfarm commented May 4, 2017

smithfarm@wilbur:~/src/ceph/smithfarm/ceph> grep -r find_package | grep CMake | grep -i "babeltrace\|cryptopp\|fuse\|rocksdb"
CMakeLists.txt:find_package(fuse)
CMakeLists.txt:  find_package(cryptopp REQUIRED)
CMakeLists.txt:  find_package(babeltrace REQUIRED)
CMakeLists.txt:  find_package(rocksdb REQUIRED)
@smithfarm

This comment has been minimized.

Contributor

smithfarm commented May 4, 2017

@@ -16,7 +16,7 @@ find_program(BABELTRACE_EXECUTABLE
NAMES babeltrace babeltrace-ctf)
include(FindPackageHandleStandardArgs)
find_package_handle_standard_args(BABELTRACE DEFAULT_MSG
find_package_handle_standard_args(babeltrace DEFAULT_MSG

This comment has been minimized.

@tchaikov

tchaikov May 4, 2017

Contributor

see https://cmake.org/cmake/help/v3.0/module/FindPackageHandleStandardArgs.html

In this mode, the name of the result-variable can be set either to either <UPPERCASED_NAME>_FOUND or <OriginalCase_Name>_FOUND using the FOUND_VAR option. Other names for the result-variable are not allowed. So for a Find-module named FindFooBar.cmake, the two possible names are FooBar_FOUND and FOOBAR_FOUND. It is recommended to use the original case version. If the FOUND_VAR option is not used, the default is <UPPERCASED_NAME>_FOUND.

so i think it's fine to use either upper case or lower case or camel case...

This comment has been minimized.

@smithfarm

smithfarm May 4, 2017

Contributor

so the all-upper-case versions are OK, but CryptoPP is not OK, because "the result-variable can be set either to either <UPPERCASED_NAME>_FOUND or <OriginalCase_Name>_FOUND" and "Other names for the result-variable are not allowed"? (In the case of CryptoPP, the "OriginalCase_Name" is "cryptopp", right?)

This would explain the bug report.

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented May 4, 2017

The cmake package names of libraries are case-sensitive.

citation needed.

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented May 4, 2017

i just checked FindPackageHandleStandardArgs.cmake. it checks ${_NAME}_FIND_REQUIRED to see if it need to print a fatal message before exits. so if the ${_NAME} does not match, the cmake will complete with STATUS message even the required package is not found.

@tchaikov tchaikov merged commit 54ba0b4 into ceph:master May 4, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details

@smithfarm smithfarm deleted the smithfarm:wip-19853 branch May 4, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment