[cmake-developers] User vs CMake include mismatch handling

[Brad King]

On 11/20/2010 11:30 AM, Alexander Neundorf wrote:
> CMake Warning (dev) 
> at /opt/cmake-HEAD/share/cmake-2.9/Modules/CheckCXXCompilerFlag.cmake:23 
> (INCLUDE):
>   File /opt/cmake-HEAD/share/cmake-2.9/Modules/CheckCXXCompilerFlag.cmake
> includes /home/alex/src/kde4-svn/KDE_dir/kdelibs/cmake/modules/CheckCXXSourceCompiles.cmake 
> (found via CMAKE_MODULE_PATH) which shadows
>   /opt/cmake-HEAD/share/cmake-2.9/Modules/CheckCXXSourceCompiles.cmake.  This
>   may cause errors later on .

Good.  The warning should be more specific about what "cause errors later on"
means.  It could mean both "with future versions of CMake" and "with this
version of CMake later on during *this* configuration".  The latter is more
important.

> The policy set to NEW would indeed break the build for such projects which 
> rely on their own compiler info files.

That's okay because once the project source has been edited to set the policy
to NEW then it is their responsibility to make other corresponding fixes.

> But, as above, this warning would appear, and IMO it is a very valid warning.
> Since every cmake upgrade can potentially break their build.

Yes.

>    INCLUDE(${CMAKE_USER_MAKE_RULES_OVERRIDE_C})
[snip]
> So these files could be used.

Yes, this appears at just the right point in the CMake*Information files.
The documentation of this feature needs to be improved anyway.

> Or the compiler info files could also optionally look for yet another file to 
> include, something 
> like "UserSpecific-${CMAKE_C_COMPILER_ID}-${LANGUAGE}.cmake"

Perhaps something like this:

  Custom-<id>-<lang>
  Custom-<os>-<id>-<lang>

Certainly it will help to have a better interface for overriding compiler
information than just replacing whole modules with "slightly modified"
versions.

> Or an additional cmake variable like CMAKE_MODULE_PATH_OVERRIDE could be 
> added, which would *always* be searched before CMAKE_ROOT, also when 
> including from within CMAKE_ROOT.

Then projects will just start setting CMAKE_MODULE_PATH_OVERRIDE instead
of CMAKE_MODULE_PATH and the original problem will persist.

> Or a variable which lists files for which
> CMAKE_MODULE_PATH should be preferred, or some other not too nice special 
> casing.

I think we can agree that special cases are not a solution.

> I wouldn't consider using a slightly modified copy e.g. of FindZLIB.cmake as 
> messing with internals.

...except when CMake starts providing a FindZLIB that creates imported targets
and then other find modules depend on that (your canonical example).  Then
the "slightly modified" version retroactively becomes "significantly modified".

On 11/21/2010 12:39 PM, Alexander Neundorf wrote:
> I think this would be the thing to do (didn't test, but should work):
>
> cmake_minimum_required(VERSION 2.8.4)
>
> cmake_policy(SET CMP0017 OLD)
> project(MyProject C CXX)
> cmake_policy(SET CMP0017 whatever_they_like)

Policies are for compatibility with existing projects.  They are *not*
a switch between two desirable behaviors.  All policies must be able to
eventually have support for OLD behavior removed completely in the
distant future.  We need a solution that does not make projects do
something like this.

----------------------------------------------------------------------------

The main remaining problem with this policy is that it *must* default
to WARN so the KDE releases in question will no longer build.  One
solution proposed earlier in this thread was to allow policies to be
set on the command line.  I argued that this didn't make sense due to
the scoping of policies.  However, I think a similar solution can work.

The command

  cmake_minimum_required(VERSION x.y.z) # or cmake_policy(VERSION)

sets all policies introduced in CMake x.y.z or earlier to NEW and all
others to WARN.  Perhaps instead of hard-coding the default to WARN
the command should first check for a CMAKE_POLICY_DEFAULT_CMP<NNNN>
variable which must be empty or set to NEW or OLD.  The variable
is used only for policies that would otherwise be set to WARN.

This approach has the following advantages

(a) The value of the variable can be managed like any other.
    It can be set locally, cached, etc.

(b) The value persists through all policy scopes such that every call
    to cmake_policy(VERSION) will use it consistently.  In scopes that
    specify a VERSION that is aware of the policy the variable will
    simply be ignored (but it's okay because that code is aware of the
    policy already and has been updated to work with it).

(c) Project developers can test what happens with the NEW behavior of
    a policy just by adding -DCMAKE_POLICY_DEFAULT_CMP0017=NEW to the
    command line.

(d) Users can work around breakages like the KDE v. FPHSA case by
    adding -DCMAKE_POLICY_DEFAULT_CMP0017=NEW to the command line.

(e) Users building old project releases can quiet warnings about
    specific policies by adding -DCMAKE_POLICY_DEFAULT_CMP0017=OLD.

The only problem is how to get users building KDE that see an obscure
FPHSA argument error over to instructions to add this cmd line option.
Perhaps your proposed "shadow" warning for CMP0017 that appears just
before the breakage will help.

----------------------------------------------------------------------------

I think the path forward here is:

(1) Improve documentation of CMAKE_USER_MAKE_RULES_OVERRIDE[_C] variables
(2) Add the Custom-<*> file inclusion, document it
(3) Teach cmake_policy(VERSION) to honor CMAKE_POLICY_DEFAULT_*
(4) Introduce the proposed policy CMP0017

-Brad