[cmake-developers] Depends information in buildsystem files

[Stephen Kelly]

Brad King wrote:

> On 02/06/2013 08:27 PM, Stephen Kelly wrote:
>> I've pushed a branch with some addtional fixes for various issues in
>> generator expressions.
> 
> Thanks, those all look good.  I presume you're finding these by
> trying to use the new features for Qt5, KDE, and Boost?  How else?

Yes. Many of them I had a on a mental todo list (eg replace 
isGeneratorExpression with something - but what? static bool 
cmGeneratorExpression::Contains? Something else?). Others I just found by 
reviewing the code again and trying things out. 

The LINK_LIBRARIES fix for the DagChecker I found while testing 
https://codereview.qt-
project.org/#patch,sidebyside,46437,8,src/corelib/Qt5CoreConfigExtras.cmake.in 
because multiple-config properties are populated there. 

I checked FindQt4, and we only populate the noconfig variant. I'm not sure 
if that's a bug or not. As we set the IMPORTED_CONFIGURATIONS in FindQt4 
too, it might be better to loop over them there too.

> 
>> One of the issues relates to depends information in the build dir. I
>> fixed the situation in the makefile generator by expanding the
>> COMPILE_DEFINITIONS before writing out the DependsInfo.cmake file.
> 
> Good catch.
> 
>> The Ninja generator doesn't
>> use such a file, but from my porcelain test it seems to already detect
>> the INTERFACE content appropriately. I didn't check how that works.
> 
> The Ninja generator depends on the toolchain to support "gcc -M" style
> flags so it always computes the dependencies that the compiler sees.
> 
> The Makefile generators are the only ones that implement their own
> dependency scanning rather than relying on the native build tools.

Great.

> 
>> It occurs to me though that this could be not just one bug but a class of
>> bugs. A similar issue could exist for the other generators, and anywhere
>> else that cmake generates 'buildsystem artifacts' which make need to be
>> evaluated as generator expressions, or may need to take INTERFACE content
>> of dependencies into account.
> 
> Every generator expression should be evaluated as it passes through the
> generators before being written out to disk anywhere for the native
> build system to see.  Are you asking whether there could be places that
> the expressions still leak through by accident?  If so, I guess users
> will discover them soon enough ;)

Yes, that's what I'm asking. More accurately, I was wondering about possible 
situations similar to the DependsInfo.cmake issue in other generators. If 
other generators don't do depends tracking like the makefile generator 
though I'm sure that's fine and users can find the rest.

> 
> The most likely source of such bugs would be interfaces whose generation
> is re-implemented by each generator rather than passing through a central
> evaluation point.  If such a case is discovered then the best fix would
> be to re-factor things to centralize the relevant evaluations.

Yes, that's something to be careful of in the future as genexes are used 
more.

For now (and in the future), I think a more likely source of bugs is reading 
a property which didn't have generator expressions before and now does (like 
the recent automoc bug fix). From grepping in Source it looks like relevant 
properties like LINK_INTERFACE_LIBRARIES are fine, as are properties like 
COMPILE_DEFINTIONS.

A concern though is this in CMakeExpandImportedTargets.cmake:            

 get_target_property(_linkInterfaceLibs "${_CURRENT_LIB}" 
   IMPORTED_LINK_INTERFACE_LIBRARIES_${_importedConfigToUse} )

That macro won't work if the IMPORTED_LINK_INTERFACE_LIBRARIES_ has 
generator expressions. Wherever that macro is needed though, cmake should 
probably be updated to have built-in support for imported targets probably.

While we can update the cmake code for that, we'd probably have to update 
the macro to be able to handle generator expressions too somehow. 

> 
>> Another possible candidate is <target>_LIB_DEPENDS in the cache.
>> Currently, cmTarget::AddLinkLibrary aborts if a generator expression is
>> found before populating that cache variable. Is that appropriate?
> 
> Those variables are purely historical and should not be populated with
> any new interfaces.

Great, that's what I thought.

Thanks,

Steve.