Brad King wrote:

> Steve,
> 
> I've reviewed this topic:
> 
>  4cf80cc Make sure types match exacty without conversion when making
>  std::pairs. 786aa36 Fix (hopefully) the Mac build.
>  9bb1f54 Populate the LINK_LIBRARIES property when linking to targets.
>  1381d56 Add a HEAD-target to target linking API.
> 
> I'm hesitant to use
> 
>  std::pair<cmTarget*, std::string>
> 
> as a map key.  I had to read the commit history of it to understand
> what it represents.  Please use a helper struct with a suitable name
> instead.  Then squash the fixup commits back into the main HEAD-target
> commit.

Done.

> 
> Also, I see a few uses of GetOriginalLinkLibraries left.  The only one
> we should have left is the one for the CMP0003 OLD behavior.

The other uses are in cmGlobalGenerator and in the Graphiviz generator.

In cmGlobalGenerator it is used at configure-time, so we can't evaluate 
generator expressions, and shouldn't read LINK_LIBRARIES property directly 
because it can contain generator expressions set via set_property. 

Additionally, in a future patch, I strip out generator expressions before 
adding them to the OriginalLinkLibraries, so I think leaving that call there 
makes sense to get the link libraries known at configure time:

 https://gitorious.org/~steveire/cmake/steveires-
cmake/commit/e18ddcd0fcfccdefcd8d6a5303fc809985cf9746

If you insist, I can rename the method, but I think that would be just noise 
in the history.

The uses in the Graphviz generator are also called at configure-time. 
Downstreams may be relying on that and using the generated graphs in 
add_custom_target or similar. 

Additionally, the graphiviz generator does not generate multiple config-
specific graph files (I think there is a bug for that, but I don't know 
where), nor does it put config specific information in all one graph (which 
should it do - it's an open question), so we can't use the information in 
generator expressions.

I think it's a can of worms to try to do all that now, and it's a world away 
from the branch we're really trying to get integrated, and it will require 
discussion and maybe a policy etc.

Thanks,

Steve.


--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers

Reply via email to