On Tue, Feb 02, 2016 at 09:29:10 +0100, Antonio Perez Barrero wrote:
> From: Antonio Perez Barrero <[email protected]>
>
> Check found libraries version to match user required version, including
> EXACT match.
>
> Protobuf compiler executable version is check to be aligned with found
> libraries, raising a warning message otherwise.
>
> Module interface and private variables are renamed to honour
> recommendation to be aligned in case with module name.
Thanks, comments inline.
> diff --git a/Modules/FindProtobuf.cmake b/Modules/FindProtobuf.cmake
> index 2f13b09..34cd660 100644
> --- a/Modules/FindProtobuf.cmake
> +++ b/Modules/FindProtobuf.cmake
> @@ -6,47 +6,49 @@
> #
> # The following variables can be set and are optional:
> #
> -# ``PROTOBUF_SRC_ROOT_FOLDER``
> +# ``Protobuf_SRC_ROOT_FOLDER``
Could this rename be split out into a separate patch? Backwards
compatibility still needs to be provided though, so the uppercase
versions will still need to exist.
> -function(PROTOBUF_GENERATE_CPP SRCS HDRS)
> +function(Protobuf_GENERATE_CPP SRCS HDRS)
Function case doesn't matter, so no need to duplicate it.
> + if("${_Protobuf_COMMON_H_CONTENTS}" MATCHES "#define
> GOOGLE_PROTOBUF_VERSION ${_Protobuf_VERSION_REGEX}")
No need to dereference the content variable. Should be:
+ if(_Protobuf_COMMON_H_CONTENTS MATCHES "#define GOOGLE_PROTOBUF_VERSION
${_Protobuf_VERSION_REGEX}")
> + math(EXPR Protobuf_MAJOR_VERSION "${Protobuf_VERSION} / 1000000")
> + math(EXPR Protobuf_MINOR_VERSION "${Protobuf_VERSION} / 1000 % 1000")
> + math(EXPR Protobuf_SUBMINOR_VERSION "${Protobuf_VERSION} % 1000")
Would string manipulation be better here?
> + if(Protobuf_FIND_VERSION)
> + # Set Protobuf_FOUND based on requested version.
> + set(_Protobuf_VERSION
> "${Protobuf_MAJOR_VERSION}.${Protobuf_MINOR_VERSION}.${Protobuf_SUBMINOR_VERSION}")
The find_package_handle_standard_args function has a VERSION_VAR
argument which can do this logic.
Thanks,
--Ben
--
Powered by www.kitware.com
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Kitware offers various services to support the CMake community. For more
information on each offering, please visit:
CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers
