|
Notes |
|
|
(0017139)
|
|
Philip Lowman
|
|
2009-08-16 23:25
|
|
I think adding this function is a good idea.
FindGTK2, FindBoost, FindOpenSceneGraph, and FindQt4 might all be able to make use of the code. |
|
|
|
(0017339)
|
|
Alex Neundorf
|
|
2009-09-08 14:19
|
|
Looks good.
What about instead of relying on <package>_VERSION being set to have the found version number as argument ?
So that instead of:
set(LibSSH_VERSION 0.3.0)
find_package_version_check(LibSSH DEFAULT_MSG)
you would do:
find_package_version_check(LibSSH 0.3.0 DEFAULT_MSG)
or
set(LibSSH_VERSION 0.3.0)
find_package_version_check(LibSSH ${LibSSH_VERSION} DEFAULT_MSG)
Alex |
|
|
|
(0017349)
|
|
Andreas Schneider.
|
|
2009-09-08 17:14
|
|
|
The cmake manpage describes that this variable should be set. So I followed it. If you prefer this way, I can change it. Both are fine for me. |
|
|
|
(0018523)
|
|
Alex Neundorf
|
|
2009-11-22 11:06
|
|
Can you change the patch so that this is done as part of find_package_handle_standard_args() if <NAME>_VERSION is set ?
Alex |
|
|
|
(0019019)
|
|
Andreas Schneider.
|
|
2010-01-03 12:35
|
|
|
Alex, I've integrated it. Check the latest uploaded file. |
|
|
|
(0019833)
|
|
Andreas Schneider.
|
|
2010-03-11 05:11
|
|
|
Here is a patch for master. |
|
|
|
(0019874)
|
|
Alex Neundorf
|
|
2010-03-14 11:41
|
|
Please give the attached findpackagehandlestandardargs-2.patch a try.
We are testing this version currently in KDE trunk.
Alex |
|
|
|
(0019886)
|
|
Andreas Schneider.
|
|
2010-03-15 12:34
|
|
|
|
|
(0019888)
|
|
Alex Neundorf
|
|
2010-03-15 19:02
|
|
New patch now also check CamelCase_VERSION, UPPERCASE_VERSION_STRING and CamelCase_VERSION_STRING.
Do you have suggestions how to improve the documentation ?
Alex |
|
|
|
(0019891)
|
|
Andreas Schneider.
|
|
2010-03-16 05:37
|
|
Improve the example and give some hints.
# Example:
# find_package(LibXml2 2.7.0)
#
# # check for the version (read the header files, pkg-config, ...)
# # and set the found version number it with:
# set(LibXml2_VERSION 2.7.1)
# FIND_PACKAGE_HANDLE_STANDARD_ARGS(LibXml2 DEFAULT_MSG LIBXML2_LIBRARIES LIBXML2_INCLUDE_DIR) |
|
|
|
(0019894)
|
|
David Cole
|
|
2010-03-16 09:35
|
|
|
Alex, seems like you're doing all the leg work on this one anyhow, so I assigned it to you... Assign back if you disagree. |
|
|
|
(0020247)
|
|
Andreas Schneider.
|
|
2010-04-18 14:24
|
|
|
Looks like it works just fine, at least in KDE now :) |
|
|
|
(0020526)
|
|
Kovarththanan Rajaratnam
|
|
2010-05-04 08:52
|
|
This is a great feature addition. I think this can be improved even more. It would be nice if FIND_PACKAGE_HANDLE_STANDARD_ARGS() could also set:
* XXX_VERSION_MAJOR
* XXX_VERSION_MINOR
* XXX_VERSION_PATCH
* XXX_VERSION_TWEAK
based on XXX_VERSION[_STRING]. This should also work in the opposite direction, since this allows maximum flexibility for the package writer.
Example 1:
FILE(READ "${XXX_INCLUDE_DIR}/XXX.h" XXX_H)
STRING(REGEX REPLACE ".*#define XXX_VERSION \"([0-9]+)\\.([0-9]+)\\.([0-9]+)\".*" "\\1.\\2.\\3" XXX_VERSION_STRING "${XXX_H}")
Example 2:
FILE(READ "${XXX_INCLUDE_DIR}/XXX.h" XXX_H)
STRING(REGEX REPLACE ".*#define XXX_VERSION_MAJOR ([0-9]+).*" "\\1" XXX_VERSION_MAJOR "${XXX_H}")
STRING(REGEX REPLACE ".*#define XXX_VERSION_MINOR ([0-9]+).*" "\\1" XXX_VERSION_MINOR "${XXX_H}") |
|
|
|
(0020704)
|
|
Alex Neundorf
|
|
2010-05-10 19:07
|
|
The attached find_package_handle_standard_args-version-checking.patch modifies the find_package_handle_standard_args() macro so that if <Package>_FIND_VERSION is set, it automatically checks whether <Package>_VERSION, <PACKAGE>_VERSION, <Package>_VERSION_STRING or <PACKAGE>_VERSION_STRING are set, and if one of them is set, compares it with the reference version given. If none of them is set, it just prints a warning and accepts what it found (otherwise too many modules break).
It also handles the EXACT keyword.
What I'm a bit unsure it whether this automatic handling is good or whether it would be better if the using module would have to specifiy the name of its version-variable. Then find_package_handle_standard_args() wouldn't have to try the 4 versions I listed above, and it would be obvious to the outside that
find_package_handle_standard_args(MyPackage DEFAULT_MSG
VERSION_VARIABLE MYPACKAGE_VERSION
MYPACKAGE_LIBRARY MYPACKAGE_INCLUDE_DIR)
would do the version checking with the given variable.
By doing this automatically, it is a bit hidden, but OTOH it works for many modules without the need to change any code in them (which is nice).
Alex |
|
|
|
(0020711)
|
|
Brad King
|
|
2010-05-11 13:13
|
|
I like the VERSION_VARIABLE option. It lets each find-module enable version checking as needed, and explicitly.
I have one high-level comment on this topic. One of the things that find_package tries to do is avoid mandating any type of version scheme on the packages. That's why in config-mode it loads for each <pkg>Config.cmake file its matching <pkg>ConfigVersion.cmake file to ask it to check whether it matches the requested version. It leaves version comparison up to each package. Trying to compare versions, perhaps other than EXACT, automatically for find-modules will almost certainly do the test wrong for some packages. That's why we need each module to enable it explicitly if it is known that the standard macro does it right for that module. |
|
|
|
(0020715)
|
|
Alex Neundorf
|
|
2010-05-11 16:40
|
|
> That's why we need each module to enable it explicitly if it is known that
> the standard macro does it right for that module.
From my experience with the modules coming with cmake and the modules in kdelibs there was not a single module which did a "wrong" versioning.
The only thing I had to do was to remove the existing version checking from the existing modules. Then it was simply working for all of them (that's in the git clone at gitorious).
So, I'm still undecided on this. On one hand, I usually don't like such automatic stuff, but OTOH it just works for all modules I encountered, and so adds a version checking to several modules without having to add any code. Which is nice. This way it also kind-of enforces the version variable naming.
Alex |
|
|
|
(0020831)
|
|
Alex Neundorf
|
|
2010-05-26 15:56
|
|
How about that:
I add a second "mode" to the command, so the syntax will be
find_package_handle_standard_args(Foo
[FAIL_MESSAGE <message>]
[SUCCESS_MESSAGE <message>]
[VERSION_VAR <var>]
[VARS <var1> ..<varN>] )
Not sure the "SUCCESS_MESSAGE" part is necessary, since the standard message is pretty good.
Also not sure whether it should be "VERSION_VAR" or "VERSION" or "VERSION_VARIABLE".
The same for "VARS" vs. "VARIABLES" vs. "REQUIRED_VARIABLES".
When used in this mode, i.e. when the first argument after the package name is "FAIL_MESSAGE" or one of the other keywords, the version checking is done only when explicitely requested.
When used with the old (current) syntax, version checking is done automatically.
This would mean it is on by default for all existing modules which use f_p_h_s_a(), and can be disabled if it goes wrong somehow.
Comments ?
Alex |
|
|
|
(0020832)
|
|
David Cole
|
|
2010-05-26 17:58
|
|
I like your suggestion of different modes for the command, with the default being the "right" (new) way as long as there is a way to achieve the old way explicitly as needed.
No harm providing an override for SUCCESS_MESSAGE.
VERSION_VAR would be ok: we use _VAR in many places in CMake already.
I like REQUIRED_VARIABLES if they are actually required. If not, then just VARIABLES is ok.
The behavior should be well defined for all combinations of these inputs, though. So the documentation should be solid. |
|
|
|
(0020945)
|
|
Alex Neundorf
|
|
2010-06-08 16:04
|
|
The attached FindPackageHandleStandardArgs.cmake-explicit-mode adds a second mode to the find_package_handle_standard_args() command, where you can enable version checking.
Documentation is not yet done.
Alex |
|
|
|
(0021536)
|
|
Alex Neundorf
|
|
2010-07-28 18:24
|
|
|
Pushed this to next now and updated FindBISON.cmake to use the new mode. |
|
|
|
(0021560)
|
|
Alex Neundorf
|
|
2010-07-29 15:57
|
|
I think I'll close this one now that it is in next.
Alex
|
|
|
|
(0022064)
|
|
Alex Neundorf
|
|
2010-09-01 12:09
|
|
|
Assigning the target version |
|
FindPackageVersionCheck.cmake (2,751) 2009-08-14 04:17
0001-Added-version-handling-in-FIND_PACKAGE_HANDLE_STANDA.patch (4,955) 2010-03-11 05:11