[cmake-developers] Generator expressisons in target properties (was Re: conditionals in generator expressions)

[Brad King]

On 10/22/2012 01:24 PM, Stephen Kelly wrote:
> That's in the use-generator-target-2 branch in my clone. 
> 
> git://gitorious.org/~steveire/cmake/steveires-cmake.git 
> 
> I'll rebase and submit the contents of it in several parallel branches when
> 2.8.11 opens. I'll also fix the style issues in the commits etc.

Thanks.  If possible please re-organize the topic to make as many of the
refactoring changes (e.g. moves to cmGeneratorTarget) to the front without
any functional changes.  That way we can merge/test those first.

> Also in my clone is the interface-target-properties branch. The commits there
> 'finish' the work needed to make linking work with generator expressions. 

The cmComputeTargetDepends cannot be per-config because IDE generators use
one set of inter-target dependencies for all configurations.  The line you
add in cmGlobalGenerator::ComputeTargetDepends:

+  const char *config = this->LocalGenerators[0]->GetMakefile()->GetDefinition("CMAKE_BUILD_TYPE");// TODO: Get from somewhere

is a red flag.  You may have to do it for all configurations and take the
union of the dependencies for each target.

> The way I've implemented it there is to populate a LINK_LIBRARIES property as a
> result of calling target_link_libraries. The INTERFACE_LINK_LIBRARIES property
> can also be populated on a target, and it 'chains', which means that the
> INTERFACE_LINK_LIBRARIES from the dependency tree are all processed, and the DAG
> checker ensures there are no cycles in their content.
> 
> That chaining is whitelisted in the cmGeneratorExpressionEvaluator because it is
> necessary to set up a DAG checker before evaluating. So, the properties that are
> whitelisted are the properties that always get a DAG checker set on them before
> they are evaluated.
> 
> When linking to a target, the INTERFACE_LINK_LIBRARIES are automatically used
> too. The content of the INTERFACE_LINK_LIBRARIES is assumed to be mandatory, so
> this makes sense. 

Okay.  I'd prefer the term "transitive" to "chains".  The latter is already
used in our source for a different meaning (target prop chains to dir prop).

> For backwards compatibility, the debug and optimized keywords are also processed
> into generator expressions using $<CONFIG:...>.

Nice.  However:

+  else if (llt == OPTIMIZED)
+    {
+    l = std::string("$<CONFIG:Release:") + l + std::string(">");
+    }

The "optimized" keyword is not just for Release.  It is really "not debug".
Look at how the "llt" is used elsewhere to be sure.

> A side-effect of using generator expressions in the LINK_LIBRARIES property in
> this way is that all libraries will appear to be in the link implementation, and
> may appear again when processing the link interface. I don't know the
> consequences of that, if any.

Can you explain that in more detail, please?

> For now I consider the (IMPORTED_)?LINK_INTERFACE_LIBRARIES(_<CONFIG>)?
> properties obsolete and replaced by INTERFACE_LINK_LIBRARIES. I don't have any
> magic to read the old property when asked for the new one etc. This is mostly
> for simplicity, and because the IMPORTED and non imported old-properties have
> slightly different meanings (see the comments in ), but I may investigate
> further if magic is appropriate. 

Okay.  We will need carefully written documentation for each of these names
to distinguish them.

> Something I am certain of though is that the new name is needed for consistency
> with other new properties (INTERFACE_*) and for removal of the requirement for
> the IMPORTED_ prefix on imported targets. Both properties can be generated in
> export() and install(EXPORT) anyway, and really I expect minimal impact from
> this part. 

The INTERFACE_ prefix is good.  It's also nice that we don't need _<CONFIG>
versions of everything anymore.

> I've also come around to the idea that 
> 
>  target_link_libraries(foo bar)
> 
> should be enough to make foo compile and link with bar

Your justification is reasonable.  The code that adds INTERFACE_ properties
to 'bar' in the application could be made conditional if necessary anyway
for compatibility.

> At this point, because multiple properties can use generator expressions, and
> because several properties will depend on the content of the LINK_LIBRARIES
> property, cycles in generator expressions become a problem for the first time
> (up to now they haven't been). This is to be expected whenever a declarative
> language is used. In the top commit in my clone I describe the problem more
> fully, and a way which might work to break cycles like that, but which require
> user-intervention. I don't think I have the full solution yet, so more ideas are
> welcome.

It will be difficult to define the behavior for cycles.  I think rejecting
them and requiring the user to handle it is reasonable for now.  We can
always define non-error behavior for them later in certain cases.

> As it is not source compatible to put imported targets into FOO_LIBRARIES where
> paths were used before, I'd like to make a more-established convention of using
> "ModuleName::LibraryName" which would be used by consumers of the libraries
> instead of the _LIBRARIES variables.

Okay.

The fact that the Plugin test had to be fixed for export() to be done at
generate time is a bit worrying to me.  It is possible that some projects
export info from one directory and load it in another.  This could happen
for example when a dependency is added as a subdirectory but still used
with find_package().  I didn't think about this case when you asked about
export() before.  We need to consider how to handle compatibility here,
but only with projects not using the new features enabled by delaying
export() until generate time.  Can you explain this in more detail?

Thanks,
-Brad