[CMake] SuperBuild whoes

[Michael Hertling]

On 04/26/2011 03:19 PM, Michael Wild wrote:
> On 04/26/2011 02:48 PM, Michael Hertling wrote:
>> On 04/25/2011 05:15 PM, Michael Wild wrote:
>>> On 04/25/2011 04:51 PM, Michael Hertling wrote: [...]
>>>>> [...] The only thing that required some thinking was writing a
>>>>> relocatable XXXConfig.cmake file. I think I will update my
>>>>> tutorial on the WIKI to use a un-configured, relocatable 
>>>>> XXXConfig.cmake file.
>>>>
>>>> Just a hint for that tutorial, though off-topic: Targets may
>>>> usually not be defined multiple times, i.e. the export file
>>>> generated by INSTALL(EXPORT ...) may not be included more than
>>>> once, so the
>>>>
>>>> include("@FOOBAR_CMAKE_DIR@/FooBarLibraryDepends.cmake")
>>>>
>>>> should possibly be guarded by IF(NOT TARGET ...)/ENDIF()
>>>> constructs. Otherwise, the resulting FooBarConfig.cmake file must
>>>> not be loaded twice or more - unless one is aware of the imported
>>>> targets' feature of being, say, "half-scoped", cf. [1]. This
>>>> might be an uncomfortable limitation, in particular w.r.t.
>>>> multi-component packages. Regrettably, such IF(NOT TARGET
>>>> ...)/ENDIF() constructs can hardly be automated, so one could
>>>> perhaps consider to allow redefinitions for imported targets. Due
>>>> to the absence of sources, this should be much easier to
>>>> implement than for non-imported targets.
>>>
>>> Good point. I will do something like this:
>>>
>>> get_property(FOOBAR_INCLUDED GLOBAL PROPERTY FOOBAR_INCLUDED
>>> DEFINED) if(NOT FOOBAR_INCLUDED) # include
>>> FooBarLibraryDepends.cmake here set_property(GLOBAL PROPERTY
>>> FOOBAR_INCLUDED TRUE) endif()
>>
>> Don't do it in this way. First, the DEFINED clause of GET_PROPERTY() 
>> queries if the property has been *defined* by DEFINE_PROPERTY(); if 
>> it is *set* by SET_PROPERTY() et al. doesn't matter in this regard. 
>> See the following CMakeLists.txt:
>>
>> CMAKE_MINIMUM_REQUIRED(VERSION 2.8 FATAL_ERROR) PROJECT(PROPERTIES
>> NONE) GET_PROPERTY(d GLOBAL PROPERTY P DEFINED) GET_PROPERTY(s GLOBAL
>> PROPERTY P SET) MESSAGE("P initially: defined=${d}, set=${s}") 
>> SET_PROPERTY(GLOBAL PROPERTY P TRUE) GET_PROPERTY(d GLOBAL PROPERTY P
>> DEFINED) GET_PROPERTY(s GLOBAL PROPERTY P SET) MESSAGE("P after
>> setting: defined=${d}, set=${s}") DEFINE_PROPERTY(GLOBAL PROPERTY P
>> BRIEF_DOCS "P" FULL_DOCS "P") GET_PROPERTY(d GLOBAL PROPERTY P
>> DEFINED) GET_PROPERTY(s GLOBAL PROPERTY P SET) MESSAGE("P after
>> defining: defined=${d}, set=${s}")
>>
>> Thus, you must use GET_PROPERTY()'s SET clause to achieve your aim.
> 
> Argh, sorry. I meant SET, of course.
> 
>>
>> Moreover, imported targets - in contrast to traditional ones - are,
>> as I said, "half-scoped", i.e. they've a scope like variables, but
>> cannot be overwritten/redefined. As a consequence, you can define an
>> imported target in a subdirectory, and subsequently, i.e. after
>> returning from ADD_SUBDIRECTORY(), you can define it again the
>> current scope. If you guard an export file's inclusion by a global
>> property, the inclusion will fail if it has taken place earlier in a
>> subdirectory, so the imported targets will be undefined in the
>> current scope. See the following CMakeLists.txt files:
>>
>> # CMakeLists.txt: CMAKE_MINIMUM_REQUIRED(VERSION 2.8 FATAL_ERROR) 
>> PROJECT(FOOBAR NONE) ADD_SUBDIRECTORY(subdir) 
>> GET_PROPERTY(FOOBAR_DEFINED GLOBAL PROPERTY FOOBAR_DEFINED SET) 
>> IF(NOT FOOBAR_DEFINED) ADD_EXECUTABLE(foobar IMPORTED) 
>> SET_PROPERTY(GLOBAL PROPERTY FOOBAR_DEFINED TRUE) ENDIF() IF(TARGET
>> foobar) MESSAGE("Target foobar defined at toplevel") ELSE() 
>> MESSAGE("Target foobar NOT defined at toplevel") ENDIF()
>>
>> # subdir/CMakeLists.txt: GET_PROPERTY(FOOBAR_DEFINED GLOBAL PROPERTY
>> FOOBAR_DEFINED SET) IF(NOT FOOBAR_DEFINED) ADD_EXECUTABLE(foobar
>> IMPORTED) SET_PROPERTY(GLOBAL PROPERTY FOOBAR_DEFINED TRUE) ENDIF() 
>> IF(TARGET foobar) MESSAGE("Target foobar defined in subdir") ELSE() 
>> MESSAGE("Target foobar NOT defined in subdir") ENDIF()
>>
>> Instead, you should use either directory properties to protect
>> imported targets from being redefined - appropriate to their scoping
>> - or just ordinary variables, though none of these solutions is
>> bullet-proof.
> 
> This is truly infuriating... Inherited directory properties seem to be
> the way to go then... The reason I like to use properties is that they
> are much less used than variables, reducing the potential for naming
> conflicts.
> 
>>
>> Alternatively, one might consider to provide a function specifically 
>> designed for the accident-free inclusion of export files, e.g. like:
>>
>> FUNCTION(INCLUDE_EXPORT EXPORTFILE) SET(TARGETSDEFINED FALSE) 
>> FOREACH(i IN LISTS ARGN) IF(TARGET "${i}") SET(TARGETSDEFINED TRUE) 
>> ENDIF() ENDFOREACH() IF(NOT TARGETSDEFINED) INCLUDE(${EXPORTFILE}) 
>> ENDIF() ENDFUNCTION()
>>
>> Apparently, a function's scope does not shield an imported target
>> from the surrounding scope as a directory's scope does. However, one
>> needs to provide the targets when invoking INCLUDE_EXPORT(), but
>> probably it's quite possible to retrieve them automatically by
>> scanning the export file, e.g. with an ADD_.+\(([^ ]+)IMPORTED\) or
>> the like.
> 
> Nah, too roundabout...
> 
> 
>>
>> IMO, the interdiction of - equivalently - redefining imported
>> targets is currently the latters' main disadvantage since this means
>> exactly the above-mentioned trouble when dealing with them in config
>> files.
> 
> True. IMHO the automatically generated export files should actually
> guard against double inclusion. Perhaps something for the next release? ;-)

I've proposed something similar in [1], but Brad King came up with a
reasonable point against it in [2]. Moreover, he is absolutely right
that IF(NOT TARGET ...)-ENDIF() constructs to protect an export file
from being multiply included or the contained imported targets from
being multiply defined - what I had in mind initially - might result
in subtle failures; consider the following set-up: A project defines
a target X and subsequently loads a config file which, in turn, pulls
in imported targets X and Y from an export file. Here, one definitely
wants to see a fatal error due to the X targets' naming clash. If the
export file or its targets are guarded by IF(NOT TARGET ...)-ENDIF()
constructs, this would end up with the imported targets X and/or Y
being undefined - without an error. Even if the project's target X
is an imported one, an error should be raised because such a naming
clash indicates the project's misconfiguration. Hence, the correct
approach to avoid errors due to multiply loaded config files which
involve export sets is indeed what you've intended earlier in this
thread: Remember if the export file has been included once before,
and not check if one or more/all targets have already been defined.

Having mulled over this issue, I think that there's also no actual
need to allow any redefinition of imported targets, au contraire:
Naming clashes of targets - imported or not - suit well to reveal
configurational errors. So, protecting an export file's inclusion
in a config file by a variable or directory property is perfect.

Regards,

Michael

[1] http://www.mail-archive.com/cmake@cmake.org/msg29433.html
[2] http://www.mail-archive.com/cmake@cmake.org/msg29441.html