[cmake-developers] User vs CMake include mismatch handling

[David Cole]

The really robust solution here is the simple one.

It is to implement CMAKE_CURRENT_LIST_DIR and for files that must explicitly
reference a file "in the same directory as me", use that explicitly.

Otherwise, behavior is exactly the same as it is now.

Explicit reference to the file you mean is the only guaranteed way to get
the file you mean. Anything else is a guess and will be wrong some of the
time.


2010/10/6 Alexander Neundorf <neundorf at kde.org>

> On Tuesday 05 October 2010, David Cole wrote:
> > On Tue, Oct 5, 2010 at 4:48 PM, Alexander Neundorf <neundorf at kde.org>
> wrote:
> > > On Tuesday 05 October 2010, James Bigler wrote:
> > > > On Tue, Oct 5, 2010 at 2:08 PM, Alexander Neundorf <neundorf at kde.org
> >
> > >
> > > wrote:
> > > ...
> > >
> > > > > 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.
> > >
> > > I am aware that it can potentially break builds.
> > > But, it only breaks compatibility where cmake *hopes* that nobody else
> > > breaks
> > > it instead of making sure that it gets what it expects.
> > > Also I assume that there are very rare, maybe no projects which rely on
> > > this
> > > very special behaviour.
> > > At least for kdelibs it works.
> > > Will check the other KDE modules and what we have at work tomorrow.
> > >
> > > > 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
> > >
> > > You still can override them and create your own copies of
> cmake-supplied
> > > modules and use them, no change there. It only changes something for a
> > > module
> > > shipped with cmake which tries to include another module shipped with
> > > cmake.
> > > In this case the patch ensures that the including module also gets the
> > > module
> > > included it expects.
> > >
> > > IOW, it makes cmake 2.8.3 NOT break the build of applications using
> > > installed
> > > KDE 4.5.[01] kdelibs.
> > >
> > > > 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.
> > >
> > > Alex
> > > _______________________________________________
> > > cmake-developers mailing list
> > > cmake-developers at cmake.org
> > > http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
> >
> > Another solution to this whole fiasco to consider... rather than what we
> > have now, or one of your two proposals (the CMAKE_CURRENT_LIST_DIR or the
> > backwards-compatibility breakage) ... is as follows:
> >
> > Revert the FindPackageHandleStandardArgs.cmake to how it used to be. Add
> a
> > new module with the new functionality (and a new name, perhaps
> > FindPackageHandleStandardArguments, just to make FPHSA even longer... :-)
> > ). Then include the new module, instead of the old one in built-in files
> in
> > CMake.
> >
> > This is still clean from both CMake and KDE points of view, does not
> break
> > backwards compatibility, and solves the problem. At least in this
> instance.
> >
> > Now, it would still be possible to introduce this problem again, in
> another
> > case, but consider the whole cause of this in the first place: we added
> new
> > stuff to an existing function, and then started using it, not considering
> > that it itself was the first break in backwards compatibility.
>
> As far as I understand, the module itself doesn't break compatibility.
> Old syntax:
> FPHSA( Foo  DEFAULT_MSG FOO_LIBRARY FOO_DIR)
> New Syntax:
> FPHSA( Foo REQUIRED_VARS FOO_LIBRARY_FOO_DIR
>           VERSION_VAR FOO_VERSION)
>
> The new syntax works, and the old syntax also still works the same way it
> always did. So from that POV there is no compatibility breakage I think.
>
> The breakage occurs because some FindFoo.cmake modules coming with cmake
> are "linked" against FPHSA.cmake, but at runtime, the "loader" finds
> FPHSA.cmake via a user-set "LD_LIBRARY_PATH", since FindFoo.cmake doesn't
> have an "RPATH", which would have avoided the problem.
>
> My patch basically implements an "RPATH" for the all files in CMAKE_ROOT,
> so
> they always get what we (the CMake developers) expect them to get.
>
>
> Attached is a file "log". I created this using the attached script and
> edited
> it a bit.
>
> It shows (at the end) that inside CMAKE_ROOT 128 files or include()d or
> find_package()d without full path.
> One could argue both ways:
> these are 128 chances to break existing builds with the "RPATH"
> OR
> these are 128 chances that cmake can break existing builds in the future
> when
> enhancing its own cmake files if we don't add "RPATH"
>
> I split the 128 files into groups:
> * 12 files in Modules/Compiler/
> * 42 files in Modules/Platform/
> * 34 Find-Modules
> * 40 other files
>
> I would feel better if I could be sure that in these 128 cases always the
> expected file is loaded, and not maybe a different one.
>
>
> The situation is this: projects would be still perfectly able to override
> modules shipped with cmake.
>
> Example: FindZLIB.cmake
> This file is used by FindPNG.cmake and by FindQt4.cmake
>
> So a project Bar would break if it uses FindPNG.cmake but relies on a
> custom
> FindZLIB.cmake.
> The right way to handle this for the project is to include the full
> dependencies, i.e. if they rely on a FindPNG.cmake which works only
> properly
> with their custom FindZLIB.cmake, they also need to ship their own copy of
> FindPNG.cmake. This will avoid "diving" into CMAKE_ROOT and by that their
> custom FindPNG.cmake will get their custom FindZLIB.cmake
>
> Advantage for CMake: Let's assume we enhance FindZLIB.cmake in cmake 2.8.5
> to
> make use of imported targets, and let's assume that we make use of this new
> feature in FindPNG.cmake also in the same version. This is a compatible
> change, assuming we followed all rules, i.e. the old variables stay working
> and we basically only added things.
>
> With the current behaviour, the build of the project Bar would break
> because
> the cmake-supplied FindPNG.cmake would get the project-supplied
> FindZLIB.cmake loaded, which doesn't have the imported targets yet, so
> stuff
> breaks.
>
> With the new behaviour, if the project just has a custom FindZLIB.cmake,
> the
> enhanced cmake-supplied FindPNG.cmake will get the new enhanced
> cmake-supplied FindZLIB.cmake, so it works.
> If the project calls find_package(ZLIB), they get their own custom version.
>
> If the project ships both a FindPNG.cmake and a FindZLIB.cmake, it doesn't
> matter to them what happens to those files in cmake.
>
>
> I think we should really take time to come to a really good and robust
> solution for this issue.
>
> Alex
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://public.kitware.com/pipermail/cmake-developers/attachments/20101006/dee83547/attachment.html>