[cmake-developers] User vs CMake include mismatch handling

[James Bigler]

On Tue, Oct 5, 2010 at 2:08 PM, Alexander Neundorf <neundorf at kde.org> wrote:

> On Tuesday 05 October 2010, Brad King wrote:
> > On 09/30/2010 04:25 PM, Alexander Neundorf wrote:
> > > New behaviour: the file from shared/cmake/Modules/ is loaded, i.e. the
> > > file with which file A was developed and tested, and not a different
> file
> > > provided by a user project. This would fix the problem currently in
> KDE.
> >
> > Dave Cole and I just talked about this.  We think this will be a subtle
> > behavioral distinction that could lead to surprising behaviors.  It may
> > also be tricky to implement because there are several ways that modules
> > can be included (enable_language, find_package, include, etc.).
> >
> > Since this approach only affects CMake files that are distributed with
> > CMake we can simply edit them.  I think your AddCMAKE_CURRENT_LIST_DIR
> > topic looks good.
>
> I just had a first try of this implemented when I received this mail.
> The patch is attached, please have a look, I did not yet have the time to
> give
> it extensive testing. Will do that in the next days.
>
> In my previous mail I forgot the case when a file is included directly from
> cmake. With the attached patch the behaviour is unchanged in this case.
>
> I think this is _really_ better than what I did in the
> AddCMAKE_CURRENT_LIST_DIR branch.
> In that branch the issue is fixed by hardcoding the include() for
> FPHSA.cmake
> in all Find-modules which are currently part of cmake.
> This has the following downsides:
>
> * this style has to be kept for all new find-modules
>
> * the problem can in the same way also occur for any other files, i.e. to
> make
> it really safe, more or less *all* include()s in cmake/Modules/ would have
> to
> use the full paths.
>
> * it looks ugly, I mean, in general the full path should only be used for
> special cases. And this would make it what has to be done for each
> include()
> in all of cmake's modules, not only the find-modules.
>
>
> This patch has the potential to break some builds, but only builds where
> the
> build relies on something like the following:
>
> src/CMakeLists.txt:
>    include (CMakeModule)
>
> -> cmake/CMakeModule.cmake:
>    include (SomeOtherCMakeModule)
>
> -> src/SomeOtherCMakeModule.cmake (overrides
> cmake/SomeOtherCMakeModule.cmake)
>
>
> I'd go so far to say if something breaks due to the change, they relied on
> an
> internal implementation detail they shouldn't have relied on.
>
> We could do an extended rc phase, especially mentioning this change.
>
> I can also add a policy, but in this case the policy would have to default
> to
> the NEW behaviour, otherwise it wouldn't have an effect.
>
> I think this new behaviour is very logical, since it has the effect that
> when
> a module in CMAKE_ROOT include()s a file, it gets the file it expects, and
> not another one, which may not do what it expects (or still does what it
> expected in previous versions).
>
> The current behaviour is really like running an executable with a shared
> library LD_PRELOADED, and hoping that the LD_PRELOADED libs will always be
> work as the correct one would.
>
> Alex
>
>
>
This patch breaks backward compatibility, because it changes the include
order.

Just like hoping that no one would want to modify the existing set of macros
in this way is like assuming that no one will make a copy of a built in
macro which will not work in a future version of cmake where the macro's api
changes.

Why can't you create a new version of the FSA macro in question?  This is
what is typically done to maintain backward compatibility with binary
libraries.


James
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://public.kitware.com/pipermail/cmake-developers/attachments/20101005/cf2c64b3/attachment.html>