[cmake-developers] find_package module-only policy (was: find_package error wording)

[Alexander Neundorf]

On Monday 20 February 2012, Brad King wrote:
> On 2/19/2012 10:54 AM, Alexander Neundorf wrote:
> > I don't see a problem with needing an additional keyword.
> > 
> > find_package(Qt5)
> > vs.
> > find_package(Qt5 CONFIG_MODE)
> 
> Remember we decided on
> 
>    find_package(Qt5 CONFIG)
> 
> instead which reads better.

I don't have a strong opinion on this, but I'd prefer CONFIG_MODE over CONFIG 
for two reasons:
* find_package_handle_standard_args() already knows the keyword CONFIG_MODE, 
so I wouldn't introduce a second similar but slightly different keyword to 
mean the same
* CONFIG_MODE is more different from CONFIGS than CONFIG

> I'm not opposed outright to this syntax in the future.  However, I
> think the policy as proposed is too aggressive for transition.  It
> will create a warning on every config-mode find_package call that
> exists in any project release to date.  This penalizes everyone who
> was proactive enough to have read, understood, and used the more
> powerful config-mode.  The warning will remain in all the old releases
> of their projects forever and make it look like they did something
> wrong.  Furthermore, to avoid the warning in new versions they will
> have to add extra decoration to their calls, and during the transition
> period that decoration will be spelled NO_MODULE which is pretty ugly
> when not in a Find module.
> 
> ----------------------------------------------------------------------
> 
> We need a smoother transition plan.  It must not create warnings
> in old projects.  Here is a plan I can accept:
> 
> (1) Improve the current ambiguous error message (see below).

I don't think the modified message changes anything significantly (see below).

> (2) Add the explicit MODULE and CONFIG mode keywords.  Warn if they
> are used and the current minimum CMake version is not >= 2.8.8 so that
> people do not add them in places that will break old versions.
> 
> (3) Add two possible AUTHOR_WARNING messages in the case that no mode
> keyword or Find module is present:
> 
>   - If the current minimum CMake required version is >= 2.8.8 then
>     warn that MODULE or CONFIG should be used.
> 
>   - Otherwise if CMAKE_FIND_PACKAGE_WARN_NO_MODULE is set to true then
>     warn that NO_MODULE should be used to get the config mode.

There is now a branch FindPackage_ModeWarning on stage.
It is branched away from the FindPackage_CONFIG_MODE_MODULE_MODE branch.

- default mode is unchanged.
- MODULE_MODE uses only the Find-modules
- NO_MODULE == CONFIG_MODE uses only the config files.
- warning messages are printed, but they are not finished, to say the least

It warns if:
* the required version is < 2.8.8, and MODULE_MODE or CONFIG_MODE has been 
used
* the required version is < 2.8.8, CMAKE_FIND_PACKAGE_WARN_NO_MODULE is TRUE, 
no Find-module has been found and NO_MODULE has not been used
* the required version is >= 2.8.8, no Find-module has been found, and no mode 
keyword has been used

Is that what you meant ?


> The first one will not appear in old projects but will become more
> common over time.  The second one can be used in projects (KDE) to get
> the warning even before bumping the min required version to 2.8.8.
> 
> In either case the warning should appear even when the package
> configuration file is found.  This will convince developers to
> add the keyword because it is required to avoid the warning.
> 
> (4) In the future, e.g. CMake 3.0.0, add the policy if necessary.  At
> that point it will be able to leverage the new features added above so
> that the NEW behavior is clean.

If there is always a warning when no mode keyword is used, I think this may 
not be necessary.

> ----------------------------------------------------------------------
> 
> Now to improve the error message (step 1 above).  The current
> ambiguous error message originated here:
> 
>    http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=53d9d84f
> 
> Prior to that it only talked about the package config file.  It was
> only in an attempt to help developers debug their CMAKE_MODULE_PATH
> that the ambiguous message was added, but at the cost of making it
> unclear to users what is needed.

Ok, so I added the message :-)
And I still think the error message as it is (in the branch), is correct, it 
doesn't *create* any ambiguity. All it does is, it explains the actual 
situation to the user.
The problem that it is unclear to the user (and to cmake) what is needed, is 
already there, because due to the automatic fallback it actually is unclear 
what is needed.

> The message needs to be clear to users and developers, but with a
> preference for users because it will appear even in properly written
> projects when the package config file is not found.
> 
> We can extend what I previously proposed:
> 
>   CMake Error at CMakeLists.txt:7 (find_package):
>     No package configuration file for "ecm" found by names:
> 
>       ecmConfig.cmake
>       ecm-config.cmake
> 
>     Add the installation prefix of "ecm" to CMAKE_PREFIX_PATH or
>     set "ecm_DIR" to a directory containing one of the above files.
>     If "ecm" provides separate development package or SDK be sure
>     it has been installed.
> 
>     If "ecm" does not provide a package configuration file then
>     this project must ensure that a Findecm.cmake module lies in
>     its CMAKE_MODULE_PATH before calling find_package.

I think this doesn't change a lot compared to:

"CMake Error at CMakeLists.txt:10 (find_package):
  Could not find module Findextra-cmake-modules.cmake or a configuration file
  for package extra-cmake-modules (requested version 0.0.4) with one of the
  following names:

    extra-cmake-modulesConfig.cmake
    extra-cmake-modules-config.cmake

  Adjust CMAKE_MODULE_PATH to find Findextra-cmake-modules.cmake.  To find
  the configuration file, set CMAKE_PREFIX_PATH to the installation prefix of
  extra-cmake-modules, or set extra-cmake-modules_DIR to the directory
  containing a CMake configuration file for extra-cmake-modules."

They both say the same, just the order is different.

Alex