Re: [cmake-developers] Setting include directories via target_link_libraries() ?
Hi,
On Saturday 29 December 2012, Stephen Kelly wrote:
Stephen Kelly wrote:
I'd like to defer the details of the
porcelain API for now and focus instead on the plumbing API and
implementation.
I have several quirks with the new command proposal, The new command can be
emulated with my branch and this macro:
include(CMakeParseArguments)
macro(target_use_interfaces target)
cmake_parse_arguments(TUI INTERFACE;PUBLIC;PRIVATE ${ARGN})
if(TUI_UNPARSED_ARGUMENTS)
message(FATAL_ERROR Unknown keywords given to
target_use_interfaces(): \${TUI_UNPARSED_ARGUMENTS}\)
endif()
foreach(lib ${TUI_INTERFACE} ${TUI_PRIVATE} ${TUI_PUBLIC})
if (NOT TARGET ${lib})
message(FATAL_ERROR This macro only accepts targets)
endif()
endforeach()
if (TUI_INTERFACE)
target_link_libraries(${target}
INTERFACE_LINK_LIBRARIES ${TUI_INTERFACE})
endif()
if (TUI_PRIVATE)
target_link_libraries(${target} LINK_PRIVATE ${TUI_PRIVATE})
endif()
if (TUI_PUBLIC)
target_link_libraries(${target} LINK_PUBLIC ${TUI_PUBLIC})
endif()
endmacro()
The quirks:
1) Restrictiveness
Only targets are allowed, which means you likely can't use ${Foo_LIBRARIES}
at all anymore, even for porting, unless you know exactly what it contains.
Even if you do know exactly what it contains, that doesn't necessarily
help.
Consider that Bar maybe doesn't provide 'rich' imported targets yet. Then
Foo will have this:
set(Foo_LIBRARIES Foo::importedlib ${Bar_LIBRARIES})
This shouldn't be necessary, should it ?
If Foo::importedlib is an exported target created by cmake, there should be no
need to add additional libraries manually to it, AFAICT.
Or worse, FooConfig.cmake will create IMPORTED targets for Bar (Bad idea,
see 5 below)
This is solvable by letting target_use_interfaces also accept library names
and library paths, but then the new command is less 'exact' and more
redundant.
It could warn if it detects a normal library...
2) Inconvenience
Assuming target_use_interfaces only allows targets, you'll have a lot of
this:
tll(foo lib1)
target_use_interfaces(foo lib2)
tll(foo lib3)
instead of just
tll(foo lib1 lib2 lib3)
Shouldn't
target_link_libraries(foo lib1 lib3)
target_use_interfaces(foo lib2)
work too ?
The imported target lib2 should know all its dependencies, right ?
3) Incompleteness
link-libraries, INTERFACE_POSITION_INDEPENDENT_CODE,
INTERFACE_WIN32_EXECUTABLE, INTERFACE_STD_CXX_11,
INTERFACE_CXX_RUNTIME_LIBCXX, INTERFACE_CXX_RUNTIME_LIBSTDCXX
will all handled by tll in both proposals (as they depend on the
LINK_LIBRARIES, which is populated by tll), but defines and includes alone
will be the INTERFACE properties which do not get populated with tll, and
need the new command instead.
4) Redundancy into the future
Some Bar packages will not create IMPORTED targets with appropriate
INTERFACE properties.
tll will not 'go away'. The extent to which a project can achieve a 'fully
ported' state depends on its upstreams. The new command may drive some (but
not most) people to read the help entry for it for a couple of releases.
Depending on how fast their cmake dependency moves, how fast they move
their upstream dependency versions and how fast their upstream
dependencies move their cmake version, the new command could be very old
news by the time they get to use it.
Yes.
And after that, there will always be two commands for very similar things.
Well, depends on whether you view those two things as very similar ;-)
Not every good idea is adopted (particularly not in a reasonable time), no
matter how good the idea is. Consider the amount and rate of adoption of
Config files to date.
5) Greater incentive to make future changes accidentally harder.
People will create their own IMPORTED targets to 'port fully' to the new
command and push themselves over the line (both in their own CMakeLists
files, and in their own Config files).
The same thing could happen even if tll does includes and defines too, but
a greater incentive is there if there's a new command and a 'port fully to
the new command' goal.
That means that if upstream introduces IMPORTED targets where they didn't
before there will be breakage.
6) API non-argument
One of the suggested benefits of a new command is a more restrictive
signature to be more typo safe.
http://thread.gmane.org/gmane.comp.programming.tools.cmake.devel/5526/focu
s=5569
If that is really such a problem for tll that it needs a solution (I don't
believe it is) we can just deprecate
tll(foo lib1 lib2)
with a policy and require people to use
tll(foo LINK_INTERFACE_LIBRARIES lib1 lib2)
tll(foo LINK_PRIVATE lib1 lib2)
tll(foo LINK_PUBLIC lib1 lib2)
instead if the policy is NEW. (and also
tll(foo INTERFACE_LINK_LIBRARIES lib1 lib2)
later, when that exists of course - The
Re: [cmake-developers] Setting include directories via target_link_libraries() ?
Stephen Kelly wrote:
I'd like to defer the details of the
porcelain API for now and focus instead on the plumbing API and
implementation.
I have several quirks with the new command proposal, The new command can be
emulated with my branch and this macro:
include(CMakeParseArguments)
macro(target_use_interfaces target)
cmake_parse_arguments(TUI INTERFACE;PUBLIC;PRIVATE ${ARGN})
if(TUI_UNPARSED_ARGUMENTS)
message(FATAL_ERROR Unknown keywords given to target_use_interfaces():
\${TUI_UNPARSED_ARGUMENTS}\)
endif()
foreach(lib ${TUI_INTERFACE} ${TUI_PRIVATE} ${TUI_PUBLIC})
if (NOT TARGET ${lib})
message(FATAL_ERROR This macro only accepts targets)
endif()
endforeach()
if (TUI_INTERFACE)
target_link_libraries(${target}
INTERFACE_LINK_LIBRARIES ${TUI_INTERFACE})
endif()
if (TUI_PRIVATE)
target_link_libraries(${target} LINK_PRIVATE ${TUI_PRIVATE})
endif()
if (TUI_PUBLIC)
target_link_libraries(${target} LINK_PUBLIC ${TUI_PUBLIC})
endif()
endmacro()
The quirks:
1) Restrictiveness
Only targets are allowed, which means you likely can't use ${Foo_LIBRARIES}
at all anymore, even for porting, unless you know exactly what it contains.
Even if you do know exactly what it contains, that doesn't necessarily help.
Consider that Bar maybe doesn't provide 'rich' imported targets yet. Then
Foo will have this:
set(Foo_LIBRARIES Foo::importedlib ${Bar_LIBRARIES})
Or worse, FooConfig.cmake will create IMPORTED targets for Bar (Bad idea,
see 5 below)
This is solvable by letting target_use_interfaces also accept library names
and library paths, but then the new command is less 'exact' and more
redundant.
2) Inconvenience
Assuming target_use_interfaces only allows targets, you'll have a lot of
this:
tll(foo lib1)
target_use_interfaces(foo lib2)
tll(foo lib3)
instead of just
tll(foo lib1 lib2 lib3)
3) Incompleteness
link-libraries, INTERFACE_POSITION_INDEPENDENT_CODE,
INTERFACE_WIN32_EXECUTABLE, INTERFACE_STD_CXX_11,
INTERFACE_CXX_RUNTIME_LIBCXX, INTERFACE_CXX_RUNTIME_LIBSTDCXX
will all handled by tll in both proposals (as they depend on the
LINK_LIBRARIES, which is populated by tll), but defines and includes alone
will be the INTERFACE properties which do not get populated with tll, and
need the new command instead.
4) Redundancy into the future
Some Bar packages will not create IMPORTED targets with appropriate
INTERFACE properties.
tll will not 'go away'. The extent to which a project can achieve a 'fully
ported' state depends on its upstreams. The new command may drive some (but
not most) people to read the help entry for it for a couple of releases.
Depending on how fast their cmake dependency moves, how fast they move their
upstream dependency versions and how fast their upstream dependencies move
their cmake version, the new command could be very old news by the time they
get to use it.
And after that, there will always be two commands for very similar things.
Not every good idea is adopted (particularly not in a reasonable time), no
matter how good the idea is. Consider the amount and rate of adoption of
Config files to date.
5) Greater incentive to make future changes accidentally harder.
People will create their own IMPORTED targets to 'port fully' to the new
command and push themselves over the line (both in their own CMakeLists
files, and in their own Config files).
The same thing could happen even if tll does includes and defines too, but a
greater incentive is there if there's a new command and a 'port fully to the
new command' goal.
That means that if upstream introduces IMPORTED targets where they didn't
before there will be breakage.
6) API non-argument
One of the suggested benefits of a new command is a more restrictive
signature to be more typo safe.
http://thread.gmane.org/gmane.comp.programming.tools.cmake.devel/5526/focus=5569
If that is really such a problem for tll that it needs a solution (I don't
believe it is) we can just deprecate
tll(foo lib1 lib2)
with a policy and require people to use
tll(foo LINK_INTERFACE_LIBRARIES lib1 lib2)
tll(foo LINK_PRIVATE lib1 lib2)
tll(foo LINK_PUBLIC lib1 lib2)
instead if the policy is NEW. (and also
tll(foo INTERFACE_LINK_LIBRARIES lib1 lib2)
later, when that exists of course - The deprecation policy can be
implemented immediately for the next CMake release).
7) Complexity non-argument
The implementation complexity argument leveled against using tll to set
includes and defines is just not true. The patch to add it is very simple
('Add includes and compile definitions with target_link_libraries') where it
touches the tll command implementation.
Implementing a new command would probably be more complex as tll may have to
be refactored for code sharing.
8) Non-discoverability
No one will use/benefit from/discover the new feature (with regard to
includes and defines) without first
Re: [cmake-developers] Setting include directories via target_link_libraries() ?
On 12/21/2012 7:18 PM, Stephen Kelly wrote: I don't think all the information is available from either side of the debate on new-command vs use-tll. I'd like to defer the details of the porcelain API for now and focus instead on the plumbing API and implementation. We will use the new command. It makes backward compatibility almost trivial. In an earlier post I explained why include dirs should be needed for everything in the link closure, but I think that will come naturally when projects switch fully to target_use_interfaces. I'll try to split up the commits in my branch a bit to put all the porcelain use-tll commits at the end, and we can focus on the plumbing commits. First I'd like to solidify and finish what's there (eg, I haven't yet documented the INTERFACE_INCLUDE_DIRECTORIES property), get it reviewed, then add COMPILE_OPTIONS and LINK_OPTIONS and their INTERFACE variants, then add new generator expressions such as $LANGUAGE:lang. After that I'd like to return to the questions around the porcelain API. Fine with me. I've been drafting a new message about how to do the policy for INTERFACE_LINK_LIBRARIES, the most critical part of this transition. Hopefully I'll have it ready to post soon. -Brad -- 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
Re: [cmake-developers] Setting include directories via target_link_libraries() ?
Brad King wrote:
On 12/18/2012 11:09 AM, Stephen Kelly wrote:
At generate time that property
will be used to accumulate values from INTERFACE_-like properties on
the named interfaces and append them to LINK_LIBRARIES, etc.
You wrote before that you don't propose using properties and generator
expressions:
Well, the raw properties (e.g. INCLUDE_DIRECTORIES) would initially
populate the generate-time C++ structures. Then the structures would
be extended after evaluating interfaces of dependencies.
In the above example the target_use_interfaces command would not do
anything except append foo to bar's USE_INTERFACES property. Then at
generate time for building bar, CMake would read its USE_INTERFACES
and see that it needs to include the interface defined by foo. It
would then go read INTERFACE_INCLUDE_DIRECTORIES from foo and append
the value to bar's include directories when generating.
When you get around to explaining more of your proposal, here are some more
things I don't like about it. You can keep them in mind when updating your
proposal, either by filling the gaps in my understanding of your proposal or
by updating it to handle them.
1) Appending the interfaces from used targets is confusing:
target_link_libraries(foo lib1)
target_use_interfaces(foo INTERFACES lib2)
target_link_libraries(foo lib3)
Warning: link order is lib1;lib3;lib2
include_directories(${lib1_INCLUDE_DIRS})
target_use_interfaces(foo INTERFACES lib2)
include_directories(${lib3_INCLUDE_DIRS})
Warning: include order is lib1/;lib3/;lib2/
2) There is no way to 'use an interface' at the same time as manipulating
the order of its use.
Same code example as above does not do so:
target_link_libraries(foo lib1)
target_use_interfaces(foo INTERFACES lib2)
target_link_libraries(foo lib3)
We would have to expand the 'use command' instead, and then maintain the
expansion:
target_link_libraries(foo lib1)
target_link_libraries(foo lib2)
# Warning: Required duplication. Is the reason for it clear to the reader?
target_use_interfaces(foo INTERFACES lib2)
target_link_libraries(foo lib3)
The same logic applies to include directories of course.
If the target_use_interfaces command did not manipulate a USE_INTERFACES
property, but instead manipulated the mulitple INTERFACE_* properties in-
place (not appending) as my branch already does, it would be more clear.
I added a patch to my wip-target-interface branch to illustrate that a bit.
Note that my branch still uses the tll() command for populating the
INTERFACE_* properties, but that does not matter as the transitivity-with-
properties question is separate to the new-command question.
3) Reading a property does not give a complete picture
include_directories(${lib1_INCLUDE_DIRS})
target_use_interfaces(foo INTERFACES lib2)
get_target_property(_foo_includes foo INCLUDE_DIRECTORIES)
# Warning: This output does not tell me what includes are actually used.
message(Foo includes: ${_foo_includes})
This problem is mostly mitigated by the debugging facility I already merged
to next, but it is nonetheless surprising.
4) Upstream can not communicate an order of dependenency includes.
target_use_interfaces(lib2 INTERFACES lib1)
target_use_interfaces(foo INTERFACES lib2)
target_use_interfaces(foo INTERFACES lib3)
lib1 and lib2 both have a common.h header. lib2 must be used with the
common.h from lib1.
Assuming the order of used interfaces is 'direct interfaces first, followed
by their dependencies' the order of resulting includes will be:
lib2/;lib3/;lib1
lib2.h includes common.h expecting that it will come from lib1, but when foo
uses it, it comes from lib3.
The top commit in my wip-target-interface branch also illustrates this
problem.
I can see the attraction of a single first-class property for 'using an
interface', but I think the way that an interface is to be used depends on
the property, and so I think it makes more sense to determine dependencies
of them on a per-property basis. I much prefer the granularity.
As far as I understand your proposal as explained so far, I think it is more
complex to understand and implement. I haven't spent any brain-cycles on
considering the export stuff. I assume that may be complex with the
USE_INTERFACES property.
Hopefully you can clarify with these kinds of cases in mind.
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
Re: [cmake-developers] Setting include directories via target_link_libraries() ?
On 12/21/2012 01:08 PM, Stephen Kelly wrote: If the target_use_interfaces command did not manipulate a USE_INTERFACES property, but instead manipulated the mulitple INTERFACE_* properties in- place (not appending) as my branch already does, it would be more clear. I added a patch to my wip-target-interface branch to illustrate that a bit. Note that my branch still uses the tll() command for populating the INTERFACE_* properties, but that does not matter as the transitivity-with- properties question is separate to the new-command question. Okay, I think the per-property approach may work with the new command. The new command can have my proposed signature but will immediately update the individual properties: target_use_interfaces(tgt [PUBLIC|PRIVATE|INTERFACE tgts...]...) For each t in tgts the command will populate target properties of tgt. For example the INCLUDE_DIRECTORIES will be updated as (pseudo-code): includes = $TARGET_PROPERTY:t,INTERFACE_INCLUDE_DIRECTORIES if PUBLIC or PRIVATE: tgt.INCLUDE_DIRECTORIES.append(includes) if PUBLIC or INTERFACE: tgt.INTERFACE_INCLUDE_DIRECTORIES.append(includes) and similar appropriate updates for COMPILE_DEFINTIONS and link libraries. Transitivity will be handled by recursive generator expression evaluation in each property. Basically this is what you have in mind for tll() but with the new command, right? -Brad -- 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
Re: [cmake-developers] Setting include directories via target_link_libraries() ?
On 12/18/2012 12:00 PM, Stephen Kelly wrote: 3. There is no change in behavior for existing use cases because the new behavior comes only from new interfaces. A new command and INTERFACE_LINK_LIBRARIES would have the same effect. But, maybe a property which is only responsible for transitivity can work. Why would you disallow cycles though? On second thought, they should be allowed. See below. Allowing them simplifies the boost case I'm sure. http://thread.gmane.org/gmane.comp.programming.tools.cmake.devel/3615/focus=5247 Also as the existing code (for computing the link closure) handles cycles, it might be easier to refactor it and keep the cycle handling. Actually, that is a killer argument for explicit interfaces instead of interfaces implied by linking. There is no problem with allowing the interface dependencies to have cycles involving any type of target. We simply define that the complete transitive closure of dependencies will be used with exactly one copy of every interface. Order is not defined if there are cycles. If a cycle causes a target to be in its own interface it is simply excluded (or not?) when building the target itself. Once the full set of interfaces needed to build is known, then we take the individual components of the interfaces (-I/-D/-l/etc.). -Brad -- 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
Re: [cmake-developers] Setting include directories via target_link_libraries() ?
Hi, Brad King wrote: I'll defer my thoughts on details of defining and exporting each part of the interface for a future message after we've discussed the overall approach. I think it would be more helpful to understanding your proposal if you detail those now instead. So far I don't understand your proposal. I don't see how it could work and satisfy the goals we've been discussing until now. 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
Re: [cmake-developers] Setting include directories via target_link_libraries() ?
Brad King wrote: On 12/18/2012 10:38 AM, Stephen Kelly wrote: Brad King wrote: I'll defer my thoughts on details of defining and exporting each part of the interface for a future message after we've discussed the overall approach. I think it would be more helpful to understanding your proposal if you detail those now instead. So far I don't understand your proposal. I don't see how it could work and satisfy the goals we've been discussing until now. I'm not sure when I'll have time to write out all the details, but here is a summary of the approach. When building a given target users will not necessarily populate the LINK_LIBRARIES, INCLUDE_DIRECTORIES, or COMPILE_DEFINITIONS properties for the target's dependencies. Instead users will use the new command to populate a USE_INTERFACES property. At generate time that property will be used to accumulate values from INTERFACE_-like properties on the named interfaces and append them to LINK_LIBRARIES, etc. You wrote before that you don't propose using properties and generator expressions: This can all be done inside C++ structures rather than with properties and generator expressions because it is only done during generation. That is the kind of thing I'm confused about. I don't see what value the USE_INTERFACES property adds. Here's some code that works with my branch (imagining a new command, and without the USE_INTERFACES property). How would it look with your proposal? add_library(foo ...) set_property(TARGET foo PROPERTY INTERFACE_INCLUDE_DIRECTORIES $$CONFIG:Debug:/foo/include ) add_executable(bar ...) target_use_interfaces(bar INTERFACES foo) The idea of a new command and the idea of a non-granular USE_INTERFACE property are not tied together. But if you clarify what you wrote about how/when *_INTERFACE properties would be used, that would at least get the understanding of your proposal off the ground. 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
Re: [cmake-developers] Setting include directories via target_link_libraries() ?
On 12/18/2012 11:09 AM, Stephen Kelly wrote: At generate time that property will be used to accumulate values from INTERFACE_-like properties on the named interfaces and append them to LINK_LIBRARIES, etc. You wrote before that you don't propose using properties and generator expressions: Well, the raw properties (e.g. INCLUDE_DIRECTORIES) would initially populate the generate-time C++ structures. Then the structures would be extended after evaluating interfaces of dependencies. This can all be done inside C++ structures rather than with properties and generator expressions because it is only done during generation. That is the kind of thing I'm confused about. I don't see what value the USE_INTERFACES property adds. 1. It puts the transitivity in one place rather than separately in each kind of build property. 2. It makes interfaces first-class rather than hidden behind linking. 3. There is no change in behavior for existing use cases because the new behavior comes only from new interfaces. Here's some code that works with my branch (imagining a new command, and without the USE_INTERFACES property). How would it look with your proposal? add_library(foo ...) set_property(TARGET foo PROPERTY INTERFACE_INCLUDE_DIRECTORIES $$CONFIG:Debug:/foo/include ) add_executable(bar ...) target_use_interfaces(bar INTERFACES foo) They keyword INTERFACES might not be the same but otherwise the example would work in my proposed approach. The idea of a new command and the idea of a non-granular USE_INTERFACE property are not tied together. But if you clarify what you wrote about how/when *_INTERFACE properties would be used, that would at least get the understanding of your proposal off the ground. In the above example the target_use_interfaces command would not do anything except append foo to bar's USE_INTERFACES property. Then at generate time for building bar, CMake would read its USE_INTERFACES and see that it needs to include the interface defined by foo. It would then go read INTERFACE_INCLUDE_DIRECTORIES from foo and append the value to bar's include directories when generating. -Brad -- 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
Re: [cmake-developers] Setting include directories via target_link_libraries() ?
On Tuesday 18 December 2012, Stephen Kelly wrote:
Alexander Neundorf wrote:
This would have the same effect, but people could simply hide the
USE_INTERFACES keyword in a variable:
set(Foo_LIBRARIES USE_INTERFACES Foo::FooLib)
and then it's again hidden:
target_link_libraries(hello ${JPEG_LIBRARIES} ${Foo_LIBRARIES})
I don't see why anyone would do that.
A buildsystem maintainer might decide that he simply wants to provide the
normal Foo_LIBRARIES variable to its users, so they can simply continue to use
it as they always did, and don't have to care about the new cmake features.
I don't say whether that's a good or bad idea, but it's possible and IMO not
too far fetched.
Alex
--
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
Re: [cmake-developers] Setting include directories via target_link_libraries() ?
Alexander Neundorf wrote:
On Tuesday 18 December 2012, Stephen Kelly wrote:
Alexander Neundorf wrote:
This would have the same effect, but people could simply hide the
USE_INTERFACES keyword in a variable:
set(Foo_LIBRARIES USE_INTERFACES Foo::FooLib)
and then it's again hidden:
target_link_libraries(hello ${JPEG_LIBRARIES} ${Foo_LIBRARIES})
I don't see why anyone would do that.
A buildsystem maintainer might decide that he simply wants to provide the
normal Foo_LIBRARIES variable to its users, so they can simply continue to
use it as they always did, and don't have to care about the new cmake
features. I don't say whether that's a good or bad idea, but it's possible
and IMO not too far fetched.
I'd certainly claim it's a bad idea :).
The upstream buildsystem maintainer has no idea whether Foo should appear in
the INTERFACE of targets that use it. It's a very bad idea.
It's not important though. That's not going to be the solution.
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
Re: [cmake-developers] Setting include directories via target_link_libraries() ?
Brad King wrote: This can all be done inside C++ structures rather than with properties and generator expressions because it is only done during generation. That is the kind of thing I'm confused about. I don't see what value the USE_INTERFACES property adds. 1. It puts the transitivity in one place rather than separately in each kind of build property. 2. It makes interfaces first-class rather than hidden behind linking. Does this have any implications now or in the future you have in mind? 3. There is no change in behavior for existing use cases because the new behavior comes only from new interfaces. A new command and INTERFACE_LINK_LIBRARIES would have the same effect. But, maybe a property which is only responsible for transitivity can work. Why would you disallow cycles though? Allowing them simplifies the boost case I'm sure. http://thread.gmane.org/gmane.comp.programming.tools.cmake.devel/3615/focus=5247 Also as the existing code (for computing the link closure) handles cycles, it might be easier to refactor it and keep the cycle handling. Here's some code that works with my branch (imagining a new command, and without the USE_INTERFACES property). How would it look with your proposal? add_library(foo ...) set_property(TARGET foo PROPERTY INTERFACE_INCLUDE_DIRECTORIES $$CONFIG:Debug:/foo/include ) add_executable(bar ...) target_use_interfaces(bar INTERFACES foo) They keyword INTERFACES might not be the same but otherwise the example would work in my proposed approach. Ok, good to know. The idea of a new command and the idea of a non-granular USE_INTERFACE property are not tied together. But if you clarify what you wrote about how/when *_INTERFACE properties would be used, that would at least get the understanding of your proposal off the ground. In the above example the target_use_interfaces command would not do anything except append foo to bar's USE_INTERFACES property. Then at generate time for building bar, CMake would read its USE_INTERFACES and see that it needs to include the interface defined by foo. It would then go read INTERFACE_INCLUDE_DIRECTORIES from foo and append the value to bar's include directories when generating. Ok good to know. That was not clear to me before. 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
Re: [cmake-developers] Setting include directories via target_link_libraries() ?
On 12/12/2012 05:32 PM, David Cole wrote: I strongly agree with Alex here. Mysteriously changing the target_link_libraries implementation to automatically do a bunch of stuff BY DEFAULT that it didn't used to do violates the principle of least surprise big time. Yes. Also I think some of the complexity, as discussed in related threads during the last couple of days, comes from trying to figure out when to propagate linking (-l), includes (-I), definitions (-D), and any other part of the usage requirements transitively. The main distinction between the two approaches is: * target_link_libraries: Specify link (-l) rules explicitly, get other usage requirements (-I/-D) implicitly. * target_use_interfaces: Specify interfaces explicitly, get all usage requirements implicitly (-l/-I/-D). I think what David and Alex are saying is that the latter is much clearer. After your (Steve's) dive into the former approach so far we've seen that implementation is difficult, especially when considering compatibility. Having thought more about how to implement either approach I think that the latter approach is not only clearer but cleaner too. I really like the name target_use_interfaces because it reads as use the interfaces defined by the following. Any of the normal target types can define interface properties. The proposed INTERFACE_LIBRARY would be distinct only in that it does not do anything other than define an interface. I think the key here is to make the interface a first-class concept that subsumes all usage requirements (-l/-I/-D/etc.). Transitive closure can be handled at the interface usage level and the results used to populate the interface components. I propose the following new command: target_use_interfaces(tgt [PUBLIC|PRIVATE|INTERFACE tgts...]...) Forcing use of the keywords handles Daniel's concern about clear argument separation. The command will populate target properties: USE_INTERFACES = to build tgt (set by PUBLIC/PRIVATE) INTERFACE_USE_INTERFACES = to use tgt (set by PUBLIC/INTERFACE) To generate the build rules for a target read its USE_INTERFACES, follow the INTERFACE_USE_INTERFACES dependency links of those, and compute the transitive interface closure. Order it by a simple topological sort that starts with the original USE_INTERFACES and adds any dependencies in topological order. This is how library ordering already works for linking, but it will be simpler for interfaces because all dependencies are explicit and cycles are not allowed. Note that this says nothing about what is *in* the interfaces. It only declares relationships. We don't even need to distinguish separate behavior for STATIC v. SHARED libraries. The distinction can be made under the hood in populating the interface components. I'll defer my thoughts on details of defining and exporting each part of the interface for a future message after we've discussed the overall approach. Now, given the interface closure computed as above to build a target, we simply take (non-transitively) the usage requirements from each interface and append them to the corresponding build rules of the target: impl.Libraries += iface.Libraries impl.Includes += iface.Includes impl.Defines += iface.Defines ... This can all be done inside C++ structures rather than with properties and generator expressions because it is only done during generation. The final impl.Libraries will still be processed to compute a final transitive link closure, but that won't add anything new to the other usage requirements anyway. So, the final build rules (-l/-I/-D/etc.) for a target will start with whatever was specified explicitly via target_link_libraries, include_directories, add_definitions, etc., which take precedence. Then we append anything from the used interfaces. The project can choose whether to use the old approach or the new approach or a mixture of the two. -Brad -- 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
Re: [cmake-developers] Setting include directories via target_link_libraries() ?
On Tuesday 11 December 2012, Stephen Kelly wrote:
Brad King wrote:
On 12/07/2012 06:10 AM, Stephen Kelly wrote:
Stephen Kelly wrote:
I haven't tried to analyse how much of the complexity is due to whether
target_use_targets or target_link_libraries is used. I think the harder
parts of this topic so far have been related to exports.
I've split out the part of the commit that changes tll(), and I think
the complexity remains in the part that would be essential even with a
new command.
Obviously there will be complexity inherent to the new capabilities.
I think the goal of this discussion is to re-design the interface that
enables the new features in order to avoid complexity related to backward
compatibility.
What kind of complexity?
Complexity of implementation, or complexity for users (having to understand
that using tll() with targets means not needing include_directories() after
the patch)?
Alex's concern is the latter only afaik.
Yes, I was looking at this only from a users perspective so far.
I still think that introducing a
new command is more complex for all users.
Here we disagree.
If these are separate commands, both examples below can be valid cmake code
for the same installation of package Foo:
find_package(Foo REQUIRED)
include_directories(${Foo_INCLUDE_DIRS})
add_executable(hello main.cpp)
target_link_libraries(hello ${Foo_LIBRARIES})
-
as well as
find_package(Foo REQUIRED)
add_executable(hello main.cpp)
target_use_targets(hello TARGETS Foo:FooLib)
IMO it is a good thing that this way a package can support both the old and
the new way at the same time, and that using a package the new way is very
obvious from just looking at the cmake code.
A similar effect can be achieved by adding a keyword to tll(), e.g.
target_link_libraries(hello ${JPEG_LIBRARIES} USE_INTERFACES Foo:FooLib)
This would have the same effect, but people could simply hide the
USE_INTERFACES keyword in a variable:
set(Foo_LIBRARIES USE_INTERFACES Foo::FooLib)
and then it's again hidden:
target_link_libraries(hello ${JPEG_LIBRARIES} ${Foo_LIBRARIES})
...
Even if we have a policy for when people use the old and new command
with the same foo, people will use both tll() and the new command in
the same project (with different targets), and that will be confusing
for them.
Why will this be confusing ?
Those two commands will have clear and separate functionality.
tll() links, the new one links, sets include dirs and defines using target
propertiesm and it can error/warn if something else that a target with all
necessary properties is used there.
Another idea is to simply not allow both commands to be used on a given
target.
Yes, that's what I means by policy with the same foo. But I guess as it's a
new command, no policy would be needed.
I do think that proposal makes the whole thing harder to use, and makes the
user need to understand a lot more about what's going on under the hood.
Since the new command does not yet exist this cannot break any
existing projects. One must either specify everything by the new command
or everything by the old command.
We could also use this to switch the default link interface to be empty
for shared library targets. If a newly created target doesn't link to
anything then of course its link interface is empty too. Once one uses
the new command to link to something then since it is a new command we
can make the link interface empty without changing existing projects.
True.
I like Daniel's name target_use_interfaces for the new command rather
than target_use_targets. The RHS might not always be targets.
Alex's proposal was that it would only accept targets, not library paths.
What else do you think would be in the RHS apart from targets?
I don't like the target_use_interfaces name because it conflicts with the
INTERFACE_LIBRARY type. The name implies that it can only be used with
targets of that type.
Anyway, let's discuss the actual name of any new command later.
For now, can we agree that the only reason to use a new command or not
depends on complexity for the user?
Then can someone (preferably not me) please make a list of the types of
users who will be affected by either choice (and in what situations) and we
can discuss that?
Eg, I don't believe most developers of KDE applications will be affected -
in practice they ignore the buildsystem
I prefer obviousness over convenience.
While this may lead to more code, it makes the code easier to understand and
easier to debug, and hopefully less likely to be considered as magic and
thus be ignored. ;-)
and it is Alex or, more likely, me
who will port them to KDE Frameworks 5 on a buildsystem level at least. Any
power users of CMake will learn about any new tll behavior from the release
notes, and non-power users probably won't notice until the notice by
accident. New
Re: [cmake-developers] Setting include directories via target_link_libraries() ?
On 2012-12-11 08:35, Brad King wrote: Another idea is to simply not allow both commands to be used on a given target. Since the new command does not yet exist this cannot break any existing projects. One must either specify everything by the new command or everything by the old command. How would this work when a target needs to link to some package that does not provide exported interfaces? (Thinking of most existing module-based find_package stuff, here...) Does it take library paths? Include paths? What about broken find modules that require linking to their libraries with link_directories() + tll(library name without path)? -- Matthew -- 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
Re: [cmake-developers] Setting include directories via target_link_libraries() ?
2012/12/7 Stephen Kelly steve...@gmail.com
Stephen Kelly wrote:
I haven't tried to analyse how much of the complexity is due to whether
target_use_targets or target_link_libraries is used. I think the harder
parts of this topic so far have been related to exports. Like I said
though, I haven't analysed how much of the exports complexity comes from
the use of tll().
I've split out the part of the commit that changes tll(), and I think the
complexity remains in the part that would be essential even with a new
command.
If target_link_libraries never sets the new properties (LINK_LIBRARIES and
INTERFACE_LINK_LIBRARIES), the question in the 'link implementation is link
interface' case remains: Should the exported link interface be populated by
iface.Libraries (as set by tll()), or by the LINK_LIBRARIES property (as
populated by a new command).
We can't really just check if LINK_LIBRARIES exists and use it if so. That
would mean that if the user starts with this:
target_link_libraries(foo bar)
bar will be in the link interface when exported. If they then add a line:
target_link_libraries(foo bar)
target_use_targets(foo bing)
bar will no longer be in the link interface when exported. Only bing will
be.
I think that would be even more complex for the user, and probably even
more
complex for us to implement and review, emit appropriate warnings etc.
Even if we have a policy for when people use the old and new command with
the same foo, people will use both tll() and the new command in the same
project (with different targets), and that will be confusing for them.
That's also a reason not to add a new command, I think.
When we add a new command for this, we might use a clearer signature.
Let's have a look at the following code:
target_link_libraries(${mytaget} lib1 lib2)
Noticed the typo in ${mytaget}? This variable may be expanded to an empty
string, which makes lib1 the first argument.
The behavior silently changes, no error is reported.
The following signature seems safer:
target_use_interfaces(target1 ... INTERFACES my::lib1 my::lib2 ...)
Here, if the target is missing, it is clearly interpreted as an error. It
may also be possible to use the same interfaces in more than one target.
--
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
Re: [cmake-developers] Setting include directories via target_link_libraries() ?
Stephen Kelly wrote: I haven't tried to analyse how much of the complexity is due to whether target_use_targets or target_link_libraries is used. I think the harder parts of this topic so far have been related to exports. Like I said though, I haven't analysed how much of the exports complexity comes from the use of tll(). I've split out the part of the commit that changes tll(), and I think the complexity remains in the part that would be essential even with a new command. If target_link_libraries never sets the new properties (LINK_LIBRARIES and INTERFACE_LINK_LIBRARIES), the question in the 'link implementation is link interface' case remains: Should the exported link interface be populated by iface.Libraries (as set by tll()), or by the LINK_LIBRARIES property (as populated by a new command). We can't really just check if LINK_LIBRARIES exists and use it if so. That would mean that if the user starts with this: target_link_libraries(foo bar) bar will be in the link interface when exported. If they then add a line: target_link_libraries(foo bar) target_use_targets(foo bing) bar will no longer be in the link interface when exported. Only bing will be. I think that would be even more complex for the user, and probably even more complex for us to implement and review, emit appropriate warnings etc. Even if we have a policy for when people use the old and new command with the same foo, people will use both tll() and the new command in the same project (with different targets), and that will be confusing for them. That's also a reason not to add a new command, I think. 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
[cmake-developers] Setting include directories via target_link_libraries() ?
Hi,
I haven't followed the long thread between Brad and Stephen about importing
and exporting targets closely...
So, if I understand correctly, in the future the following cmake code
find_package(Foo)
add_executable(hello main.cpp)
target_link_libraries(hello ${Foo_LIBRARIES})
may also set include directories ?
If so, I don't like this at all.
It changes the meaning of an existing command.
I would much prefer if instead there was a new command, e.g.
target_use_targets(hello Foo::FooLibrary)
which would be obviously different from target_link_libraries(), and it would
also only accept other targets as arguments, not library paths.
If target_link_libraries() is overloaded to do more than its name says (i.e.
not only linking, but also set include dirs and definitions), IMO this will
make for hard-to-understand/debug cmake files.
In the example above there is no visible hint that what
target_link_libraries() actually does. (There is also no hint whether the
find_package() expects a module or a Config file)
I'd much prefer the following:
find_package(Foo NO_MODULE)
add_executable(hello main.cpp)
target_use_targets(hello Foo::FooLibrary)
Alex
--
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
Re: [cmake-developers] Setting include directories via target_link_libraries() ?
On Thursday 06 December 2012, Alexander Neundorf wrote: ... I'd much prefer the following: find_package(Foo NO_MODULE) add_executable(hello main.cpp) target_use_targets(hello Foo::FooLibrary) which could even warn if a used library doesn't have include directory properties set, etc. Alex -- 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
Re: [cmake-developers] Setting include directories via target_link_libraries() ?
Alexander Neundorf wrote:
Hi,
I haven't followed the long thread between Brad and Stephen about
importing and exporting targets closely...
So, if I understand correctly, in the future the following cmake code
find_package(Foo)
add_executable(hello main.cpp)
target_link_libraries(hello ${Foo_LIBRARIES})
may also set include directories ?
For reference, here is where the idea of using target_link_libraries for
that came up:
http://thread.gmane.org/gmane.comp.programming.tools.cmake.devel/3083/focus=3093
(though Clinton later said he liked target_use_targets).
Here's a point where Alex raised a similar concern:
http://thread.gmane.org/gmane.comp.programming.tools.cmake.devel/2145/focus=2154
Here's where I changed my mind on whether we should use target_use_targets
or target_link_libraries:
http://thread.gmane.org/gmane.comp.programming.tools.cmake.devel/3615/focus=5111
I'm not sure what has changed since then.
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
Re: [cmake-developers] Setting include directories via target_link_libraries() ?
On Thu, Dec 6, 2012 at 3:37 PM, Stephen Kelly steve...@gmail.com wrote:
Alexander Neundorf wrote:
Hi,
I haven't followed the long thread between Brad and Stephen about
importing and exporting targets closely...
So, if I understand correctly, in the future the following cmake code
find_package(Foo)
add_executable(hello main.cpp)
target_link_libraries(hello ${Foo_LIBRARIES})
may also set include directories ?
For reference, here is where the idea of using target_link_libraries for
that came up:
http://thread.gmane.org/gmane.comp.programming.tools.cmake.devel/3083/focus=3093
(though Clinton later said he liked target_use_targets).
Here's a point where Alex raised a similar concern:
http://thread.gmane.org/gmane.comp.programming.tools.cmake.devel/2145/focus=2154
Here's where I changed my mind on whether we should use target_use_targets
or target_link_libraries:
http://thread.gmane.org/gmane.comp.programming.tools.cmake.devel/3615/focus=5111
I'm not sure what has changed since then.
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
What's changed since then is that we've seen how much change is needed for
the feature. And how tangled it all is...
This is going to be a venture which risks possibly breaking existing
projects for the convenience of those who are starting from scratch.
Since the risk on this one is high, we might want to reconsider whether it
would be better to use a new command entirely as the front-end for this
thing.
I'm not sure I like the name target_use_targets, but I think a new
command, whatever it be named, is better than changing
target_link_libraries substantially in a . release.
For 3.0, when we finally do go to a CMake 3.0 in the future, we could ask
the question whether it's safe to combine the two commands, once they've
both proven to work in practice.
Just my opinion,
David
--
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
Re: [cmake-developers] Setting include directories via target_link_libraries() ?
David Cole wrote: On Thu, Dec 6, 2012 at 3:37 PM, Stephen Kelly steve...@gmail.com wrote: http://thread.gmane.org/gmane.comp.programming.tools.cmake.devel/3615/focus=5111 I'm not sure what has changed since then. What's changed since then is that we've seen how much change is needed for the feature. And how tangled it all is... That's true. Well, a lot of change and complex, but not tangled. I haven't tried to analyse how much of the complexity is due to whether target_use_targets or target_link_libraries is used. I think the harder parts of this topic so far have been related to exports. Like I said though, I haven't analysed how much of the exports complexity comes from the use of tll(). This is going to be a venture which risks possibly breaking existing projects for the convenience of those who are starting from scratch. Since the risk on this one is high, we might want to reconsider whether it would be better to use a new command entirely as the front-end for this thing. I'm not sure I like the name target_use_targets, but I think a new command, whatever it be named, is better than changing target_link_libraries substantially in a . release. I was going to suggest naming the version that contains this stuff to 2.10. I don't know what has motivated similar version bumps in the past though. For 3.0, when we finally do go to a CMake 3.0 in the future, we could ask the question whether it's safe to combine the two commands, once they've both proven to work in practice. That would only be more source-incompatibility and not a good idea imo. 2.8.10 : Use target_link_libraries() 2.8.12 : Use target_use_targets() 3.0.0 : Use target_link_libraries() If tll() is not to be used for this when the feature first lands, and target_use_targets (or another name) should be used instead, then the thing to do in 3.0.0 (or earlier) is to deprecate tll(). Anyway, that's a bit of a sideline. We'd need to find out how much complexity is a result of using the existing tll(), and how much source incompatibility that risks creating. The only possible source compatibility I know of so far is what Brad mentioned, but that might exist anyway: Brad King wrote: Now even projects that have never touched LINK_INTERFACE_LIBRARIES will have to be fixed to manually set it to a copy of the link implementation in order to remain compatible with older CMake versions once they set the policy to NEW. I'm not currently sure whether this is okay. Is that right? 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
