[cmake-developers] Policy for INTERFACE_LINK_LIBRARIES

[Brad King]

On 12/02/2012 04:35 AM, Stephen Kelly wrote:
> I would really prefer if you would commit this directly to the branch 
> instead, as you had written it locally already. There is no point in having 
> to go through me. The branch can be our collaboration point. Me copying your 
> patch and pushing it myself feels like needless busy-work.

Sorry, but I wanted you to review the documentation and check your
implementation against it.  We are still missing the "LINK_PUBLIC
does not populate the old interface in NEW behavior" part of the
policy implementation.  See my last response below.

> Simliarly with the link-type keywords in the INTERFACE_LINK_LIBRARIES 
> property check, you might not like the message string that I used in the 
> error, you might ask me to change it, etc and we'll end up doing more trips 
> through me, more latency. Please feel free to update it directly if that is 
> the case.

I added these three commits to the topic:

 3a1660d GenEx: Clarify $<CONFIG_DEBUG> documentation
 977a098 Cleanup INTERFACE_LINK_LIBRARIES property implementation
 93ac463 Fix typo in CMP0019 documentation

and then rewrote it to squash things together and update the commit messages.

> I'm not sure about DEBUG_CONFIG either. I think I'd prefer a more generic 
> solution like $<COMPATIBLE_CONFIG:cfg>

The CONFIG_DEBUG property complements the "debug" keyword in tll().
That's all it does, and it exactly matches the behavior we need in
the motivating case.  Something like $<COMPATIBLE_CONFIG:cfg> may
be useful but that can be added in the future in another topic.

> If we do discuss it and insist that it's a solved-problem before add-
> INTERFACE_LINK_LIBRARIES-property can be merged, that would mean that the 
> entire add-INTERFACE_LINK_LIBRARIES-property topic would not get merged to 
> master for another week.

Don't worry about the weekly review session schedule for this topic.
We're actively interacting on it and I'll merge it to master as soon
as it's ready.

> How can we best prevent scope-creep in topics?

This isn't scope creep.  This is the minimum needed to correctly
introduce the policy.

> A few weeks ago I didn't have all the export-related stuff for 
> INTERFACE_LINK_LIBRARIES in the same commit that introduced it, but I had 
> follow-up commits in a follow-up topic to handle export() and 
> install(EXPORT) for it. 
> 
> That would have simplified the commit that actually added 
> INTERFACE_LINK_LIBRARIES I think, and I would still have been able to add 
> most of my follow-up topics once it was in. You would have probably noticed 
> that it was missing (exactly as you did with the context-target stuff and 
> the static library conversion of link implementation into link interface in 
> exports), and it might be harder for you to keep track of and review. Do you 
> have any preference?

When constructing a topic pretend that each commit is the last commit
you will ever make to CMake, without the rest of the topic after it,
or at least without future topics.  If it doesn't make sense to leave
it off at that point then the commits are organized incorrectly and
therefore harder to review.

> If I have patches or features which other pending topics depend on, should I 
> try to get a partial-implementation of limited scope in first and point to 
> the other topics in gitorious, or does that not work for you?

Following the above advice, keep dependent pieces in gitorious please.
While our workflow supports rewriting topics in 'next' it is more work.

> As a side-note, I'm no longer aiming to get the INTERFACE_LIBRARY type into 
> 2.8.11 due to time constraints and incomplete implementation. It will have 
> to wait until 2.8.12.

Okay.

> I pushed that to no-export-old-link-interface branch in my gitorious clone a 
> few days ago. I guess I should have pointed it out, but I didn't realize it 
> was a blocker for add-INTERFACE_LINK_LIBRARIES-property to get in. It also 
> might open up new questions about documentation and implementation, and 
> might introduce new failures in the dashboard. I'd prefer to discuss and 
> merge it separately after add-INTERFACE_LINK_LIBRARIES-property is in. 
> Otherwise add-INTERFACE_LINK_LIBRARIES-property will never get in.

This topic is about the new property and the policy that goes with it.
All changes needed to make the policy work belong in the topic.

> If you disagree, please cherry-pick it to the add-INTERFACE_LINK_LIBRARIES-
> property branch and merge it to next yourself so that we can move forward 
> with other things.

Please split that commit into the minimum needed to implement the new
CMP0019 documented behavior and add it to this topic.  Then add the
new INTERFACE option later.

Thanks,
-Brad