Re: [cmake-developers] Setting includes, defines and other usage requirements with one command
Stephen Kelly wrote: >> * Can the INCLUDE_DIRETORIES and COMPILE_DEFINITIONS property avoid >>$ references if foo is linked more than once? Skip >>appending it if the same reference already exists earlier. In >>the BEFORE case, prepend it and remove later instances. > > Yes I'm sure that can be done. Do you think it needs to be done before > merging to next? Anything else to do before merging it? Just to draw a line under this - this is no longer necessary as the LINKED expression is to be removed, and the remaining code doesn't have a duplication problem as consuming the property interfaces is based on the link interfaces. 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 includes, defines and other usage requirements with one command
On 02/12/2013 07:20 AM, Stephen Kelly wrote: > Brad King wrote: > >> Please try to track down and remove any extra pieces like $ >> and _NO_INTERFACES that we added in an attempt to deal with >> the previous approach's problems. > > Done now in the linked-usage-cleanup branch. Wonderful, thanks! The topic looks good except that the new commit: "Use the link information as a source of compile definitions and includes" introduces lines in cmTarget.cxx over 79 columns. -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 includes, defines and other usage requirements with one command
Brad King wrote: > Please try to track down and remove any extra pieces like $ > and _NO_INTERFACES that we added in an attempt to deal with > the previous approach's problems. Done now in the linked-usage-cleanup branch. 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 includes, defines and other usage requirements with one command
On 02/11/2013 01:15 PM, Stephen Kelly wrote: > the tll is always to append no matter the order of it relative to other calls: [snip] > That's really just a documentation concern. Yes. >> This approach will drop all the complex generator expressions from >> the interfaces altogether. > > Yes, we won't even need to know at export-time, because the generator > expressions will have been already evaluated. Yes. > So, I think this is a good idea. I can work on it after minor-fixes is > merged probably. The only commit in there that is not needed is the one > changing LINKED. You can rebase that away or create another branch and > cherry-pick the following two commits or whatever other git incantation you > wish. They've all already been through the dashboard anyway. Great, thanks. I've merged minor-fixes to master as-is. There is already $ history in master so it won't hurt. There is now lots of history in master for interface property approaches we're about to drop, but I think that is fine because it records the progress of our thinking. Please try to track down and remove any extra pieces like $ and _NO_INTERFACES that we added in an attempt to deal with the previous approach's problems. Thanks, -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 includes, defines and other usage requirements with one command
Matthew Woehlke wrote: > I've been reading about this feature for a while, and one thing that has > been in the back of my mind is, how will code generation tools (Qt MOC > comes to mind) that depend on being able to get the list of include > directories to pass to a non-compiler executable work when all this > stuff starts converging? Hi Matthew, That is an issue I raised before here about a join-genex: http://thread.gmane.org/gmane.comp.programming.tools.cmake.devel/5835/focus=5841 https://gitorious.org/~steveire/cmake/steveires- cmake/commit/449fe543680eed9f82042ef475c240cba9590a55 So, it's something for the future. The CMAKE_AUTOMOC feature already evaluates the generator expressions from the targets. > > At some point, I think it is going to need to be possible to pass > include directives from a target to a custom command (which generates > source files used /by the same target/), ideally with the ability to > specify how to pass each include directive (e.g. '', '-I ', > '--include=', etc.). > > Is this going to be possible with the new system? (Maybe the custom > command can be given an "argument" that is a generator expression that > will expand to the appropriate arguments?) > > Also, what is going to happen to projects using qt4_wrap_cpp (which > doesn't know about targets) that start using the new stuff? The macros read the INCLUDE_DIRECTORIES directory property, not the target properties. So, to keep using the macros, keep using the include_directories() command and don't put generator expressions in it until you migrate to CMAKE_AUTOMOC, or until the join-genex exists and the macro is ported to use it (I don't know if that macro will be ported to use it). 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 includes, defines and other usage requirements with one command
Brad King wrote: > On 02/08/2013 04:56 PM, Brad King wrote: >> On 02/08/2013 03:16 PM, Stephen Kelly wrote: >>> My preference currently is to leave things as they are including the >>> long and useless string. >> >> Your analysis is quite extensive, thanks. > > I thought about this more over the weekend. I think we've made this much > harder than it needs to be. One of the main arguments for propagating > includes and defines through tll() is that all other usage requirements > like -fPIC will now be able to propagate through it too. However, why > don't those usage requirements turn into complex interface properties like > those we've been discussing for includes/defines? It's because they are > not applied until generation time. Yes, but also because it's a boolean, not a list which must be uniq'd. > Ordering does not matter for compile definitions. Ordering does matter > for includes, but the reason we added _NO_INTERFACES was due > to tll() changing the order for existing projects. I think it will be > much simpler to have usage requirements of all types simply appended > to the settings configured by the project: > > * Link libraries already work this way > > * Compile definitions order does not matter > > * Include directories order matters sometimes, but in cases the order > generated by appending usage requirements fails the project can > simply use tid() with $ > themselves. Furthermore since the usage requirements always append > there is no compatibility issue for upstreams adding include dirs > so we may not need _NO_INTERFACES. So GetIncludeDirectories() would be changed to generate the directories in this order: * The content of the INCLUDE_DIRECTORIES property * The INTERFACE_INCLUDE_DIRECTORIES of linked targets Correct? I think that would work, but implementation-wise, tll() will still populate some structures on the cmTarget for the includes anyway to keep track of backtraces. Those won't be exported though. The behavior may also be a little difficult to document because the result of the tll is always to append no matter the order of it relative to other calls: include_directories(/foo/bar) target_link_libraries(tgt other) include_directories(/bing/bat) That's really just a documentation concern. As there is no backward compatibility concern, I'm sure it's fine. > > This approach will drop all the complex generator expressions from > the interfaces altogether. Only the expressions written by the > user will ever be needed. For non-target library names passed to tll() > there is no issue because at generate time we will know they are not > targets. Yes, we won't even need to know at export-time, because the generator expressions will have been already evaluated. So, I think this is a good idea. I can work on it after minor-fixes is merged probably. The only commit in there that is not needed is the one changing LINKED. You can rebase that away or create another branch and cherry-pick the following two commits or whatever other git incantation you wish. They've all already been through the dashboard anyway. 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 includes, defines and other usage requirements with one command
On 2013-02-11 09:06, Brad King wrote: I think we should drop all that and instead have the includes and defines usage requirements added at generate time based on the link closure. We discussed this before, and the reasons for doing it immediately were: * Ordering of tll() calls relative id()/tid() and ad()/tcd() * Allow user code to manipulate the properties after tll sets them If I can jump in here... I've been reading about this feature for a while, and one thing that has been in the back of my mind is, how will code generation tools (Qt MOC comes to mind) that depend on being able to get the list of include directories to pass to a non-compiler executable work when all this stuff starts converging? At some point, I think it is going to need to be possible to pass include directives from a target to a custom command (which generates source files used /by the same target/), ideally with the ability to specify how to pass each include directive (e.g. '', '-I ', '--include=', etc.). Is this going to be possible with the new system? (Maybe the custom command can be given an "argument" that is a generator expression that will expand to the appropriate arguments?) Also, what is going to happen to projects using qt4_wrap_cpp (which doesn't know about targets) that start using the new stuff? -- 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 includes, defines and other usage requirements with one command
On 02/08/2013 04:56 PM, Brad King wrote: > On 02/08/2013 03:16 PM, Stephen Kelly wrote: >> My preference currently is to leave things as they are including the long >> and useless string. > > Your analysis is quite extensive, thanks. I thought about this more over the weekend. I think we've made this much harder than it needs to be. One of the main arguments for propagating includes and defines through tll() is that all other usage requirements like -fPIC will now be able to propagate through it too. However, why don't those usage requirements turn into complex interface properties like those we've been discussing for includes/defines? It's because they are not applied until generation time. Almost all of the complexity in + std::string value = !isGenex ? "$" + : "$<$:" + + "$" + ">"; is due to trying to update INTERFACE_INCLUDE_DIRECTORIES and INTERFACE_COMPILE_DEFINITIONS immediately instead of delaying usage requirements until generate time. Effectively all the generate-time logic then needs to be encoded in generator expressions. We used $ as a placeholder for that logic for named (potential) targets. We still have not solved the problem to my satisfaction for generator expressions, and even $ feels like a hack. I think we should drop all that and instead have the includes and defines usage requirements added at generate time based on the link closure. We discussed this before, and the reasons for doing it immediately were: * Ordering of tll() calls relative id()/tid() and ad()/tcd() * Allow user code to manipulate the properties after tll sets them Now that we have added so much complexity to the expressions, user code will not be able to reliably work with the property values. Even if it could, that would restrict us from ever changing how the expressions work without breaking existing code that depends on manipulating a particular expression. Ordering does not matter for compile definitions. Ordering does matter for includes, but the reason we added _NO_INTERFACES was due to tll() changing the order for existing projects. I think it will be much simpler to have usage requirements of all types simply appended to the settings configured by the project: * Link libraries already work this way * Compile definitions order does not matter * Include directories order matters sometimes, but in cases the order generated by appending usage requirements fails the project can simply use tid() with $ themselves. Furthermore since the usage requirements always append there is no compatibility issue for upstreams adding include dirs so we may not need _NO_INTERFACES. This approach will drop all the complex generator expressions from the interfaces altogether. Only the expressions written by the user will ever be needed. For non-target library names passed to tll() there is no issue because at generate time we will know they are not targets. -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 includes, defines and other usage requirements with one command
On 02/08/2013 03:16 PM, Stephen Kelly wrote: > My preference currently is to leave things as they are including the long > and useless string. Your analysis is quite extensive, thanks. Perhaps we can provide a way to tell tll not to add the id/cd properties for a generator expression e.g. tll(foo LINK_PUBLIC $:not_a_target>>>) where the "NO_INTERFACES" expression is valid only in tll, must be the outer-most expression, and is removed immediately by tll before the value is used. We could also invert it to require a $ expression to enable them when ... is a genex. This is starting to look like a familiar discussion though. -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 includes, defines and other usage requirements with one command
Brad King wrote: > Currently the code > > target_link_libraries(foo $<$:not_a_target>) > > will put a giant generator expression in the include directories > and compile definitions properties and it will be exported, right? Yes. > > I do not like this. Since we decided not to use a new target-only > command we need to deal with it somehow though. How would the situation be different with a new command? target_use_interfaces(foo INTERFACE $<$:not_a_target>) would have had to write the same string to the property. As far as I see, no matter what porcelain is used, the effect on the plumbing values will be the same (as TARGET_PROPERTY and LINKED are currently defined). TARGET_PROPERTY requires an actual target and errors otherwise, so $:not_a_target>,IFACE_INCLUDE_DIRS> wouldn't work for non-Debug. It is deliberately strict. > Can we still use > the $ approach? Currently, LINKED expects a literal non-generator-expression. That fact is used to extract target names while exporting. If we change LINKED to allow generator expression content, then yes we might be able to use it instead of the big string. I haven't investigated though. > I think it should be > possible when processing out $ expressions to dig > into the contained expression to see if there are any real targets. Hmm, yes, that would be needed for proper exporting as we don't generate LINKED expressions on export. I'm not sure that's easy or worth it. Especially as the genex language allows things like mylib$<$:_debug>. There may be other dark corners where that would break. If we can't determine all targets, then we'd end up exporting properties with TARGET_DEFINED anyway. That would mean longer strings on export than we currently have as currently LINKED requires literal target names - no need for TARGET_DEFINED. > Actually such digging might be useful anyway because it would help > diagnose and warn/error when users write something non-sensical > like > > tll(foo $) > > by hand. One thing that tll does know is that its arguments must > be target names or non-target libraries. > Other strings make no sense. Either I'm not clear what you're referring to, or I don't think that's true of tll. The docs say "Item names starting with '-', but not '-l' or '-framework', are " "treated as linker flags." and at least KDE uses a GSSAPI module whose _LIBRARIES variable is a string like "-Wl,-Bsymbolic-functions -Wl,-z,relro -lgssapi_krb5 -lkrb5 -lk5crypto -lcom_err". Anyway, I haven't seen any problems resulting from the length of the string. I also don't have any solution to your error code above. There are many other ways to shoot in the foot or otherwise create obfuscated code: tll(foo $) set_property(TARGET foo PROPERTY SOMETHING bing) We probably can't catch all of them without harming legitimate uses which we have not thought of. An option would be to not populate the includes and defines properties at all in tll if generator expressions are used. For actual targets, that means using generator expressions in tll is less functional (by not also populating the includes and defines properties). For non-targets it means no long and ultimately useless strings in those properties. If that route is taken, the solution to this code: tll(foo INTERFACE $<$:some_target>) becomes this in the future: add_library(myIface INTERFACE) # Special-case tll with INTERFACE_LIBRARY to also # populate the includes and defines properties as # currently, including the TARGET_DEFINED tll(myIFACE INTERFACE $<$:maybe_a_target>) tll(foo INTERFACE myIface) That at least means that if you want to use generator expressions with something that is definitely not a target, use tll directly. If using tll with a genex which may be either a target or not, use an intermediate INTERFACE_LIBRARY. Also an ugly solution imo actually. My preference currently is to leave things as they are including the long and useless string. 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 includes, defines and other usage requirements with one command
On 01/31/2013 11:35 AM, Stephen Kelly wrote: > Brad King wrote: >> What is the purpose of the else case here? >> >> + std::string value = !isGenex ? "$" >> + : "$<$:" + >> + "$> + ",INTERFACE_" + property + ">" >> + ">"; >> >> If the input is already a genex isn't the author responsible for >> making sure it is valid? > > Yes, a valid genex for linking. Consider this: > > add_library(foo ...) > add_library(bar ...) > target_link_libraries(foo $<$:bar>) > > The genex for the INCLUDE_DIRECTORIES of foo needs to be (excuse the python- > style concats): > > "$<$:bar>" + ">:" + >"$:bar>" + > ",INTERFACE_" + "INCLUDE_DIRECTORIES" + ">" > ">" > > because the result of the genex could either be "" or "bar". Currently the code target_link_libraries(foo $<$:not_a_target>) will put a giant generator expression in the include directories and compile definitions properties and it will be exported, right? I do not like this. Since we decided not to use a new target-only command we need to deal with it somehow though. Can we still use the $ approach? I think it should be possible when processing out $ expressions to dig into the contained expression to see if there are any real targets. Actually such digging might be useful anyway because it would help diagnose and warn/error when users write something non-sensical like tll(foo $) by hand. One thing that tll does know is that its arguments must be target names or non-target libraries. Other strings make no sense. -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 includes, defines and other usage requirements with one command
On 02/04/2013 04:41 AM, Stephen Kelly wrote: > My patches don't seem to have had any effect. > > Can you pick a unit test (eg RunCMake.TargetPropertyGeneratorExpressions) > and find out exactly which commit is causing the slowdown from 1m55s to > 4m28s on the affected machine? > > I think I've done all I can with guesswork and without access to an affected > machine. Ugh, it isn't your change at all. It is my change that came in the same night: http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=2e4188e3 It seems we are hitting that Delay for a whole bunch of files :( I'll have to investigate why. Meanwhile, your optimizations probably have value anyway. Sorry, -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 includes, defines and other usage requirements with one command
Stephen Kelly wrote: > Brad King wrote: >> Meanwhile, I've noticed the test run times on some dashboard machines >> have doubled since two weeks ago. Can you please run some timing >> tests comparing current 'next' to 2.8.10.2 for some existing projects? > > Actually they seem to have doubled overnight when tll-includes-defines was > merged: > > http://open.cdash.org/viewTest.php?buildid=2743648 > http://open.cdash.org/viewTest.php?buildid=2795799 My patches don't seem to have had any effect. Can you pick a unit test (eg RunCMake.TargetPropertyGeneratorExpressions) and find out exactly which commit is causing the slowdown from 1m55s to 4m28s on the affected machine? I think I've done all I can with guesswork and without access to an affected machine. 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 includes, defines and other usage requirements with one command
Brad King wrote: > Meanwhile, I've noticed the test run times on some dashboard machines > have doubled since two weeks ago. Can you please run some timing > tests comparing current 'next' to 2.8.10.2 for some existing projects? Actually they seem to have doubled overnight when tll-includes-defines was merged: http://open.cdash.org/viewTest.php?buildid=2743648 http://open.cdash.org/viewTest.php?buildid=2795799 I pushed a patch which is needed for using the new interfaces in boost, and which speeds up the cmake time in kde frameworks branch a bit: before tll-includes-defines real0m27.528s user0m16.721s sys 0m6.732s after tll-includes-defines real0m30.767s user0m19.965s sys 0m6.624s After optimize: real0m27.246s user0m16.661s sys 0m6.388s Before the optimization, callgrind shows that a lot of time is spent in GetIncludeDirectories, in particular in executing cmSystemTools::ExpandListArgument with the result of genex evaluation. Also, after optimization and with kde frameworks updated to populate and rely on target interfaces, I get this: real0m26.922s user0m16.401s sys 0m6.404s It will be interesting to see if the optimization has any affect on the dashboards. The effect of GetIncludeDirectories taking a long time is multiplied several times there for the multiconfig generators. However, the optimization should only have any effect at all if several targets are used which have dependencies between them, and where there are multiple paths to the same dependencies. I considered also doing some caching in GetIncludeDirectories similar to the caching done in the linking related methods, but it didn't seem to have a noticable difference. At least for the makefile generator, GetIncludeDirectories is called only twice with the same arguments. The optimization only works because we know that includes and defines are uniq'd after the generator expressions are evaluated. Looking into the future a bit, I think that as new transitive interfaces are added in the future (COMPILE_OPTIONS and LINK_OPTIONS), it might make sense to keep that invariant with those too. Otherwise the performance problem would likely re-appear. In the case of COMPILE_OPTIONS at least, order can be relevant in some cases. http://gcc.gnu.org/onlinedocs/gcc-3.4.4/gcc/Optimize-Options.html > If you use multiple -O options, with or without level numbers, the last > such option is the one that is effective. That would mean that if we have something like this: target_compile_options(foo INTERFACE -O3) target_compile_options(bar INTERFACE -O2) target_link_libraries(foo PRIVATE bar) target_link_libraries(user PRIVATE bar foo) then the un-optimized result would look like: -O2 -O3 -O2 because foo has INTERFACE_COMPILE_OPTIONS "-O3;$" and user would end up with COMPILE_OPTIONS "$;$" which expands first to "-O2;-O3;$" and then "-O2;-O3;-O2". However the 'optimized' result would look like: -O2 -O3 because the first expansion would be "-O2;-O3;$", but the repeated 'bar' would be ignored. If instead of target_link_libraries(user PRIVATE bar foo) it was target_link_libraries(user PRIVATE foo bar) then the optimized result would be: -O3 -O2 and the unoptimized result would be -O3 -O2 -O2 The example of -O flags is not a particularly good one as it's not likely something suitable for putting into an INTERFACE. However, other compile options which would be suitable for the INTERFACE in theory might have unexpected results with this kind of thing. Solutions I can think of include declaring that it is not supported to rely on ordering in COMPILE_OPTIONS, and adding a $ expression to temporarily turn off the optimization. For example, if we had this: target_link_libraries(foo PRIVATE bar) target_compile_options(foo INTERFACE -O3) target_compile_options(bar INTERFACE -O2) target_link_libraries(user PRIVATE foo bar) (note the order changed) the optimized result would be "-O2;-O3" even though bar is listed last in 'user', because that is skipped. This could then be used instead where needed: target_link_libraries(user PRIVATE foo $) Alternatively, in the case of COMPILE_OPTIONS, we could make 'top-level' items always NO_SKIP. Just some food for thought for when we get around to this stuff. 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 includes, defines and other usage requirements with one command
On 01/31/2013 11:35 AM, Stephen Kelly wrote: >> It should be clear that the upstream is responsible for adding the example >> code to set the _NO_INTERFACES variable in their package config >> file when it exposes the new interfaces. Also _INTERFACES is to >> be set by the downstream and used by that code in the upstream. > > Ok. Hopefully that's more clear now. Better, but I decided to take my own crack at the wording and added a commit to your topic for it. Meanwhile, I've noticed the test run times on some dashboard machines have doubled since two weeks ago. Can you please run some timing tests comparing current 'next' to 2.8.10.2 for some existing projects? Thanks, -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 includes, defines and other usage requirements with one command
Brad King wrote:
> On 01/31/2013 10:23 AM, Stephen Kelly wrote:
>> I left it as $ so far so that it is a shorter string, and because
>> the implementation is somewhat simpler.
>
> Ahh, I see you were using $ even for already-defined targets.
> I think that is fine during processing of the same project to keep the
> strings shorter. We still need to keep it out of the exported interface
> so that we can change it later without having to keep the expression
> implementation around to support old export files. This way it never
> outlives the CMake process that generated it.
Fair enough. I've squashed that patch into the commit that introduces the
LINKED genex.
>
>> Do you think it needs to be done before merging to next?
>
> De-duplication of $ references can be added later but I'd still
> like to see it before the 2.8.11 release so that it doesn't become a
> behavior change later.
Ok, I can look into that.
>
>> Anything else to do before merging it?
>
> Please add a test case for the $ evaluation error.
>
Done.
>
> What is the purpose of the else case here?
>
> + std::string value = !isGenex ? "$"
> + : "$<$:" +
> + "$ + ",INTERFACE_" + property + ">"
> + ">";
>
> If the input is already a genex isn't the author responsible for
> making sure it is valid?
Yes, a valid genex for linking. Consider this:
add_library(foo ...)
add_library(bar ...)
target_link_libraries(foo $<$:bar>)
The genex for the INCLUDE_DIRECTORIES of foo needs to be (excuse the python-
style concats):
"$<$:bar>" + ">:" +
"$:bar>" +
",INTERFACE_" + "INCLUDE_DIRECTORIES" + ">"
">"
because the result of the genex could either be "" or "bar".
> The documentation of _NO_INTERFACES and _INTERFACES
> reference ${CMAKE_FIND_PACKAGE_NAME}_FIND_VERSION but the corresponding
> _FIND_VERSION isn't documented until further down. Please move
> the new docs to the bottom, but still above the NO_POLICY_SCOPE line.
>
> Also the current wording of the documentation makes it sound like 2.8.11
> is at fault for introducing incompatibility. There is no need to mention
> the version of CMake there. It is the version of the upstream that
> starts using the new features that causes a problem.
>
> It should be clear that the upstream is responsible for adding the example
> code to set the _NO_INTERFACES variable in their package config
> file when it exposes the new interfaces. Also _INTERFACES is to
> be set by the downstream and used by that code in the upstream.
Ok. Hopefully that's more clear now.
>
>
> Then merge for testing!
Done.
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 includes, defines and other usage requirements with one command
On 01/31/2013 10:23 AM, Stephen Kelly wrote:
> I left it as $ so far so that it is a shorter string, and because
> the implementation is somewhat simpler.
Ahh, I see you were using $ even for already-defined targets.
I think that is fine during processing of the same project to keep the
strings shorter. We still need to keep it out of the exported interface
so that we can change it later without having to keep the expression
implementation around to support old export files. This way it never
outlives the CMake process that generated it.
> Do you think it needs to be done before merging to next?
De-duplication of $ references can be added later but I'd still
like to see it before the 2.8.11 release so that it doesn't become a
behavior change later.
> Anything else to do before merging it?
Please add a test case for the $ evaluation error.
What is the purpose of the else case here?
+ std::string value = !isGenex ? "$"
+ : "$<$:" +
+ "$"
+ ">";
If the input is already a genex isn't the author responsible for
making sure it is valid?
The documentation of _NO_INTERFACES and _INTERFACES
reference ${CMAKE_FIND_PACKAGE_NAME}_FIND_VERSION but the corresponding
_FIND_VERSION isn't documented until further down. Please move
the new docs to the bottom, but still above the NO_POLICY_SCOPE line.
Also the current wording of the documentation makes it sound like 2.8.11
is at fault for introducing incompatibility. There is no need to mention
the version of CMake there. It is the version of the upstream that
starts using the new features that causes a problem.
It should be clear that the upstream is responsible for adding the example
code to set the _NO_INTERFACES variable in their package config
file when it exposes the new interfaces. Also _INTERFACES is to
be set by the downstream and used by that code in the upstream.
Then merge for testing!
Thanks,
-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 includes, defines and other usage requirements with one command
Brad King wrote: >> I've made the LINKED generator expression a first-class expression, not >> just something to be preprocessed away. I think this addresses much of >> the cost concern. > > Nice! Here are a few comments: > > * I got a warning while building your branch: > > .../Source/cmExportFileGenerator.cxx: In member function ‘void > cmExportFileGenerator::ResolveTargetsInGeneratorExpression(std::string&, > cmTarget*, std::vector >&)’: > .../Source/cmExportFileGenerator.cxx:425:12: warning: unused variable > ‘error’ [-Wunused-variable] Fixed now, thanks. > > * I think $ can be removed completely on export. Yes, it can be. What you describe below was my initial implementation, and I can change it back to that. I left it as $ so far so that it is a shorter string, and because the implementation is somewhat simpler. I've pushed another patch doing the replacement. > If the > item is not a target then remove it (already done in your impl). > If it is a target then replace it with the appropriate > $ reference in the export. This way the > $ expression lives only as long as needed. > > * We could document $ it as an internal implementation > detail subject to future changes. It should never be written by > a project, only by tll. Ok. > It is transformed properly on export. > I'd like to leave room for an alternative solution to out-of-order > target interfaces in the future. > > * Can the INCLUDE_DIRETORIES and COMPILE_DEFINITIONS property avoid > $ references if foo is linked more than once? Skip > appending it if the same reference already exists earlier. In > the BEFORE case, prepend it and remove later instances. Yes I'm sure that can be done. Do you think it needs to be done before merging to next? Anything else to do before merging it? 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 includes, defines and other usage requirements with one command
On 01/31/2013 05:34 AM, Stephen Kelly wrote: > I've left it in on export because it might make sense to use it with > generator expressions even on export: > > set(lib_genex $<$:debuglib>) > tll(tgt $) Okay. As long as $ is removed for non-targets then this won't be an issue. I see your branch does this already :) > I've made the LINKED generator expression a first-class expression, not just > something to be preprocessed away. I think this addresses much of the cost > concern. Nice! Here are a few comments: * I got a warning while building your branch: .../Source/cmExportFileGenerator.cxx: In member function ‘void cmExportFileGenerator::ResolveTargetsInGeneratorExpression(std::string&, cmTarget*, std::vector >&)’: .../Source/cmExportFileGenerator.cxx:425:12: warning: unused variable ‘error’ [-Wunused-variable] * I think $ can be removed completely on export. If the item is not a target then remove it (already done in your impl). If it is a target then replace it with the appropriate $ reference in the export. This way the $ expression lives only as long as needed. * We could document $ it as an internal implementation detail subject to future changes. It should never be written by a project, only by tll. It is transformed properly on export. I'd like to leave room for an alternative solution to out-of-order target interfaces in the future. * Can the INCLUDE_DIRETORIES and COMPILE_DEFINITIONS property avoid $ references if foo is linked more than once? Skip appending it if the same reference already exists earlier. In the BEFORE case, prepend it and remove later instances. Thanks, -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 includes, defines and other usage requirements with one command
Brad King wrote: > On 01/30/2013 12:09 PM, Stephen Kelly wrote: >>> We can document that $ has scope only in the current >>> project and will be processed away during export. I do not think we >>> want an upstream interface modifying its behavior based on the mere >>> presence of an arbitrary target in the downstream anyway. > > Thoughts on this behavior? I've left it in on export because it might make sense to use it with generator expressions even on export: set(lib_genex $<$:debuglib>) tll(tgt $) > > Another thought is to have tll() only append includes/defines if the > target is already defined at the call site and otherwise do nothing. > When I previously pointed out the need for handling not-yet-defined > targets I did not realize the cost. I've made the LINKED generator expression a first-class expression, not just something to be preprocessed away. I think this addresses much of the cost concern. In the KDE frameworks case, it was very useful to be able to refer to targets which were not defined yet, which was why I implemented it. It was useful for two reasons. One was that often the use of add_subdirectory(tests) appeared before the target it was testing (which tll handles), and another was the use of targets as dependencies of other library targets before that target was defined (and where re-ordering of the top-level directories doesn't seem to be possible). So, I would prefer to keep that feature. > > We could even go as far as not adding the generator expression to > INCLUDE_DIRECTORIES if the target dependency does not have an > INTERFACE_INCLUDE_DIRECTORIES already defined too (and similarly for > COMPILE_DEFINITIONS). > > Advantages: > > * Very little overhead for non-target arguments > * Very little overhead for targets without interfaces > * Works in the motivating case of using targets imported from an upstream > > Disadvantages: > > * Does not work automatically for circular dependencies. * Does not work if the target is not defined yet, even in the non-circular case * Assumes that interface includes are set before the tll() call. (ie it does not work so well in the motivating case of non-imported targets) It would break this: add_library(foo ...) add_library(bar ...) target_link_libraries(foo bar) target_include_directories(bar INTERFACE /usr/include/bar) >>> * Transitive linking is handled in C++ code rather than in generator >>> expressions so there is no bloat in the LINK_LIBRARIES properties >>> for non-target elements like the above. Perhaps we can have a special >>> generator expression that has context-sensitive evaluation e.g. >>> >>> $ >> >> That makes sense to me. I don't have time to do it right now though. >> Maybe I can do it later. > > Perhaps there is no need for it if we use the simpler approach above. I've implemented the approach with LINKED in the tll-includes-defines branch in my clone. I think it's reasonably simple, compact and acceptable. 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 includes, defines and other usage requirements with one command
On 01/30/2013 12:09 PM, Stephen Kelly wrote: >> This is not acceptable. We need to recognize when something was added >> by tll and was not a target and remove it from the non-linking interfaces. >> Can export() and install(EXPORT) do partial evaluation to remove these? > > As preprocessing is how things like that are done so far, I think using > preprocessing is the more straightforward way to do this at the moment. At > some point in the future we can see if partial evaluation is possible. It > would be possible to do in a later release if it is possible at all. Okay, if preprocessing is sufficient to get rid of those then go for it. >> We can document that $ has scope only in the current >> project and will be processed away during export. I do not think we >> want an upstream interface modifying its behavior based on the mere >> presence of an arbitrary target in the downstream anyway. Thoughts on this behavior? Another thought is to have tll() only append includes/defines if the target is already defined at the call site and otherwise do nothing. When I previously pointed out the need for handling not-yet-defined targets I did not realize the cost. We could even go as far as not adding the generator expression to INCLUDE_DIRECTORIES if the target dependency does not have an INTERFACE_INCLUDE_DIRECTORIES already defined too (and similarly for COMPILE_DEFINITIONS). Advantages: * Very little overhead for non-target arguments * Very little overhead for targets without interfaces * Works in the motivating case of using targets imported from an upstream Disadvantages: * Does not work automatically for circular dependencies. I think the disadvantage is okay because circular dependencies are a less common case and one can always make it work by writing out the generator expression manually. We should not make everyone pay the cost for supporting circular interfaces when few are using them. We can't even define circular interfaces through tll() anyway except for static libraries. >> * The $ expression should evaluate as an error if >> it is ever reached by any path except install(EXPORT). Otherwise >> the empty string in $/somewhere is unlikely to >> do the right thing. > > Right. I've added a patch to that effect now. Thanks. >> * Transitive linking is handled in C++ code rather than in generator >> expressions so there is no bloat in the LINK_LIBRARIES properties >> for non-target elements like the above. Perhaps we can have a special >> generator expression that has context-sensitive evaluation e.g. >> >> $ > > That makes sense to me. I don't have time to do it right now though. Maybe I > can do it later. Perhaps there is no need for it if we use the simpler approach above. -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 includes, defines and other usage requirements with one command
Brad King wrote:
> On 01/30/2013 10:57 AM, Stephen Kelly wrote:
>> Now that fix-target-property-commands was merged to master, I've rebased
>> tll-includes-defines to master and your testcase works.
>
> Thanks. However, now it leaks out all non-targets referenced by tll
> as giant strings in the exported interfaces:
>
> if(NOT ${CMAKE_FIND_PACKAGE_NAME}_NO_INTERFACES)
>set_target_properties(bar PROPERTIES
> INTERFACE_COMPILE_DEFINITIONS
> "$;
$<$:$>"
> INTERFACE_INCLUDE_DIRECTORIES
> "$;
$<$:$>"
>)
> endif()
>
> This is not acceptable. We need to recognize when something was added
> by tll and was not a target and remove it from the non-linking interfaces.
> Can export() and install(EXPORT) do partial evaluation to remove these?
As preprocessing is how things like that are done so far, I think using
preprocessing is the more straightforward way to do this at the moment. At
some point in the future we can see if partial evaluation is possible. It
would be possible to do in a later release if it is possible at all.
> We can document that $ has scope only in the current
> project and will be processed away during export. I do not think we
> want an upstream interface modifying its behavior based on the mere
> presence of an arbitrary target in the downstream anyway.
>
> Other thoughts:
>
> * The $ expression should evaluate as an error if
> it is ever reached by any path except install(EXPORT). Otherwise
> the empty string in $/somewhere is unlikely to
> do the right thing.
Right. I've added a patch to that effect now.
>
> * If exported interfaces were partially evaluated then $
> could be handled with a real evaluation instead of string replacement.
>
> * Transitive linking is handled in C++ code rather than in generator
> expressions so there is no bloat in the LINK_LIBRARIES properties
> for non-target elements like the above. Perhaps we can have a special
> generator expression that has context-sensitive evaluation e.g.
>
> $
That makes sense to me. I don't have time to do it right now though. Maybe I
can do it later.
>
> tll() would add this to COMPILE_DEFINITIONS and INCLUDE_DIRECTORIES
> properties where it would be evaluated to get relevant target interface
> properties. In other contexts it would be an empty string (or error?).
> It could be preprocessed out during export and replaced by nothing for
> non-targets or the relevant $ for targets.
>
> * To optimize the large strings these interfaces generate, perhaps
> properties can be represented in general as compiled generator
> expressions that do not directly store their original strings but
> have references to pieces of them and can reconstruct them.
> This can be done later though.
Perhaps. We'll have to keep the fact that not all properties can contain
generator expressions, but that should be possible. All for the future
though.
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 includes, defines and other usage requirements with one command
On 01/30/2013 10:57 AM, Stephen Kelly wrote:
> Now that fix-target-property-commands was merged to master, I've rebased
> tll-includes-defines to master and your testcase works.
Thanks. However, now it leaks out all non-targets referenced by tll
as giant strings in the exported interfaces:
if(NOT ${CMAKE_FIND_PACKAGE_NAME}_NO_INTERFACES)
set_target_properties(bar PROPERTIES
INTERFACE_COMPILE_DEFINITIONS
"$;$<$:$>"
INTERFACE_INCLUDE_DIRECTORIES
"$;$<$:$>"
)
endif()
This is not acceptable. We need to recognize when something was added
by tll and was not a target and remove it from the non-linking interfaces.
Can export() and install(EXPORT) do partial evaluation to remove these?
We can document that $ has scope only in the current
project and will be processed away during export. I do not think we
want an upstream interface modifying its behavior based on the mere
presence of an arbitrary target in the downstream anyway.
Other thoughts:
* The $ expression should evaluate as an error if
it is ever reached by any path except install(EXPORT). Otherwise
the empty string in $/somewhere is unlikely to
do the right thing.
* If exported interfaces were partially evaluated then $
could be handled with a real evaluation instead of string replacement.
* Transitive linking is handled in C++ code rather than in generator
expressions so there is no bloat in the LINK_LIBRARIES properties
for non-target elements like the above. Perhaps we can have a special
generator expression that has context-sensitive evaluation e.g.
$
tll() would add this to COMPILE_DEFINITIONS and INCLUDE_DIRECTORIES
properties where it would be evaluated to get relevant target interface
properties. In other contexts it would be an empty string (or error?).
It could be preprocessed out during export and replaced by nothing for
non-targets or the relevant $ for targets.
* To optimize the large strings these interfaces generate, perhaps
properties can be represented in general as compiled generator
expressions that do not directly store their original strings but
have references to pieces of them and can reconstruct them.
This can be done later though.
-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 includes, defines and other usage requirements with one command
Brad King wrote: > On 01/30/2013 03:15 AM, Stephen Kelly wrote: >> Brad King wrote: >> I've implemented it in the tll-includes-defines branch in my clone. I've only tested it manually though. >>> >>> Yes, that looks good. Please add tests covering each case: >> >> I've pushed it again to my clone. >> >> Once the fix-target-property-commands topic is in master, we can rebase >> it if needed and get it merged to next. > > I just tried your branch with a simple test case: > It works, but then uncommenting the marked line to tll a non-target > produces: > > -- bar.INTERFACE_INCLUDE_DIRECTORIES = > [$; $<$:$>] > CMake Error at CMakeLists.txt:16 (export): > $ requires its first parameter to be a reachable > target. That was fixed in fix-TARGET_PROPERTY-extraction, which was merged yesterday. Now that fix-target-property-commands was merged to master, I've rebased tll-includes-defines to master and your testcase works. 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 includes, defines and other usage requirements with one command
On 01/30/2013 03:15 AM, Stephen Kelly wrote:
> Brad King wrote:
>
>>> I've implemented it in the tll-includes-defines branch in my clone. I've
>>> only tested it manually though.
>>
>> Yes, that looks good. Please add tests covering each case:
>
> I've pushed it again to my clone.
>
> Once the fix-target-property-commands topic is in master, we can rebase it
> if needed and get it merged to next.
I just tried your branch with a simple test case:
--
cmake_minimum_required(VERSION 2.8.10.20130129)
project(FOO C)
set(CMAKE_INSTALL_PREFIX "${FOO_BINARY_DIR}/root")
add_library(foo SHARED foo.c)
target_include_directories(foo PUBLIC
$
$/include>
)
add_library(bar SHARED bar.c)
#set(m m) # uncommenting breaks this
target_link_libraries(bar LINK_PUBLIC foo ${m})
get_property(iface TARGET bar PROPERTY INTERFACE_INCLUDE_DIRECTORIES)
message(STATUS "bar.INTERFACE_INCLUDE_DIRECTORIES = [${iface}]")
export(TARGETS bar foo FILE Targets.cmake)
install(TARGETS bar foo DESTINATION lib EXPORT Foo)
install(EXPORT Foo DESTINATION lib/cmake/foo)
--
It works, but then uncommenting the marked line to tll a non-target
produces:
-- bar.INTERFACE_INCLUDE_DIRECTORIES =
[$;$<$:$>]
CMake Error at CMakeLists.txt:16 (export):
$ requires its first parameter to be a reachable
target.
The TARGET_PROPERTY errors out even though TARGET_DEFINED:m should
be false.
Also, will the non-target expressions added by tll be preprocessed out
during export()/install(EXPORT)?
-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 includes, defines and other usage requirements with one command
Brad King wrote: >> I've implemented it in the tll-includes-defines branch in my clone. I've >> only tested it manually though. > > Yes, that looks good. Please add tests covering each case: I've pushed it again to my clone. Once the fix-target-property-commands topic is in master, we can rebase it if needed and get it merged to next. 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 includes, defines and other usage requirements with one command
On 01/29/2013 03:31 PM, Alexander Neundorf wrote: > On Tuesday 29 January 2013, Stephen Kelly wrote: >> Also, just to shortcut a possible discussion point - this issue is >> independent of whether tll() adds includes or a new command is added. If a >> downstream is using a new command before the upstream adds >> INTERFACE_INCLUDE_DIRECTORIES, the exact same issue comes up. Just to be >> clear. > > A new command could ... require keywords to denote whether the target is > supposed to be used only for linking or for linking and include dirs. Yes, a new command could have syntax or keywords to select which pieces of the interface to use. If a requested piece does not exist then it is an error. If a non-requested piece exists it is not used anyway. This basically brings us back to the current (pre-usage-reqs) situation where every part is its own variable/command. It just puts everything inside the new command call instead of calls to separate commands. The proposed solution is to instead have upstreams use interface version numbers to decide what parts of the interface to populate based on the version that the downstream expects. This is simpler for downstreams, which is the goal of Steve's work. -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 includes, defines and other usage requirements with one command
On 01/29/2013 03:33 PM, Alexander Neundorf wrote: > But it will also make creating proper Config.cmake files harder, there is > then > one more thing to know in order to create a good Config.cmake file. > I doubt that the average developer/package maintainer will do this right. The extra work is only needed when adding usage requirements to one's interface. If an upstream does that but does not do the compatibility pieces their downstreams will report it and then it can be fixed. -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 includes, defines and other usage requirements with one command
On 01/29/2013 03:33 PM, Alexander Neundorf wrote: >> In the worst case, the needed compile definitions will be duplicated by an >> existing call to add_definitions(), and in the best case the needed >> defintions will be added where they were absent before. > > No, adding definitions which where not there before (but the project was > building and working nevertheless) can potentially break builds. > Not likely, but possible. We've decided elsewhere in this discussion that the new interfaces will not be populated for downstreams at all unless they request it either with an explicit variable or by requesting a sufficiently new version of the upstream. That avoids any compatibility breakage for existing downstreams. -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 includes, defines and other usage requirements with one command
On Tuesday 29 January 2013, Stephen Kelly wrote:
> Brad King wrote:
...
> > the option to be more general than ..._INCLUDE_DIRECTORIES:
> > if(PACKAGE_FIND_VERSION VERSION_LESS 2.3
> >
> > AND NOT MyPkg_INTERFACES)
> >
> >set(${PACKAGE_FIND_NAME}_NO_INTERFACES 1)
> >
> > endif()
> > include("${CMAKE_CURRENT_LIST_DIR}/upstreamTargets.cmake")
>
> Yes, I'm fine with that too, though it will only affect 'new' interfaces,
> not LINK_INTERFACE_LIBRARIES.
Yes, this will work.
But it will also make creating proper Config.cmake files harder, there is then
one more thing to know in order to create a good Config.cmake file.
I doubt that the average developer/package maintainer will do this right.
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 includes, defines and other usage requirements with one command
On Saturday 26 January 2013, Stephen Kelly wrote: > Brad King wrote: > > I've longed for "usage requirements" for years and always pictured > > them propagating through linking. The huge threads of discussion > > earlier made usage requirements seem more complicated than they are > > and made it feel like we should hide it all behind new interfaces. > > Now I think a new command will actually be *more* confusing in the > > long run because the two will be different only in subtle ways and > > users will wonder which one to use. > > Yes, that's exactly my thinking too. > > > I'm almost ready to accept the proposed behavior for > > target_link_libraries. However, we still need to construct a > > recommended way for packages and their dependents to handle the > > transition. There are many, many instances of the old style usage > > since it is the only one that previously worked. > > > > How can a package author allow old dependents using the old style to > > keep working while also allowing new dependents using the new style > > to work? > > As compile definitions are uniq'd, they don't pose any backward > compatibility concerns for downstreams of a project whose upstream adds > them. > > In the worst case, the needed compile definitions will be duplicated by an > existing call to add_definitions(), and in the best case the needed > defintions will be added where they were absent before. No, adding definitions which where not there before (but the project was building and working nevertheless) can potentially break builds. Not likely, but possible. 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 includes, defines and other usage requirements with one command
On Tuesday 29 January 2013, Stephen Kelly wrote: ... > This INTERFACE_INCLUDE_DIRECTORIES case is not going to be the only time an > upstream will want to add to its interface. In theory, any time they want > to add a directory to the INTERFACE_INCLUDE_DIRECTORIES they would have to > bump the compatibility number. > > Also, just to shortcut a possible discussion point - this issue is > independent of whether tll() adds includes or a new command is added. If a > downstream is using a new command before the upstream adds > INTERFACE_INCLUDE_DIRECTORIES, the exact same issue comes up. Just to be > clear. Are you sure ? A new command could handle libraries, targets with all interfaces set and targets with only the link interface set differently, it could error out if a target without includes set is used, and require keywords to denote whether the target is supposed to be used only for linking or for linking and include dirs. That way, to make it work with a target where the includes are not set, some keyword must be added by downstream to the command (which would be source compatible, since the new command does not exist yet), and if the include interface is added later on, it still would not be used since the keyword is present. If the keyword is removed, the full interface is used. target_use_interfaces(Foo PRIVATE SomeTarget /lib/libblub.so LINK_ONLY AnotherTarget) Or could a generator expression be used for that, something like: target_use_interfaces(Foo PRIVATE SomeTarget /lib/libblub.so $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 includes, defines and other usage requirements with one command
Brad King wrote:
> On 01/29/2013 10:54 AM, Stephen Kelly wrote:
>> Stephen Kelly wrote:
>>
if(PACKAGE_FIND_VERSION VERSION_LESS 2.3
AND NOT MyPkg_INTERFACES)
set(${PACKAGE_FIND_NAME}_NO_INTERFACES 1)
endif()
include("${CMAKE_CURRENT_LIST_DIR}/upstreamTargets.cmake")
>>>
>>> Yes, I'm fine with that too, though it will only affect 'new'
>>> interfaces, not LINK_INTERFACE_LIBRARIES.
>>
>> I've implemented it in the tll-includes-defines branch in my clone. I've
>> only tested it manually though.
>
> Yes, that looks good. Please add tests covering each case:
>
> - old upstream, new downstream
> - new upstream, old downstream
> - new upstream, new downstream
I'll do that tomorrow.
> Where do you think the _NO_INTERFACES variable documentation
> belongs? It is not meant to be set directly by downstreams
> but rather in the package configuration files provided by
> upstreams before loading their target files.
>
I'd say in the find_package documentation. It already documents similar
things.
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 includes, defines and other usage requirements with one command
On 01/29/2013 10:54 AM, Stephen Kelly wrote:
> Stephen Kelly wrote:
>
>>> if(PACKAGE_FIND_VERSION VERSION_LESS 2.3
>>> AND NOT MyPkg_INTERFACES)
>>> set(${PACKAGE_FIND_NAME}_NO_INTERFACES 1)
>>> endif()
>>> include("${CMAKE_CURRENT_LIST_DIR}/upstreamTargets.cmake")
>>
>> Yes, I'm fine with that too, though it will only affect 'new' interfaces,
>> not LINK_INTERFACE_LIBRARIES.
>
> I've implemented it in the tll-includes-defines branch in my clone. I've
> only tested it manually though.
Yes, that looks good. Please add tests covering each case:
- old upstream, new downstream
- new upstream, old downstream
- new upstream, new downstream
Where do you think the _NO_INTERFACES variable documentation
belongs? It is not meant to be set directly by downstreams
but rather in the package configuration files provided by
upstreams before loading their target files.
Thanks,
-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 includes, defines and other usage requirements with one command
Stephen Kelly wrote:
>>if(PACKAGE_FIND_VERSION VERSION_LESS 2.3
>>AND NOT MyPkg_INTERFACES)
>>set(${PACKAGE_FIND_NAME}_NO_INTERFACES 1)
>>endif()
>>include("${CMAKE_CURRENT_LIST_DIR}/upstreamTargets.cmake")
>
> Yes, I'm fine with that too, though it will only affect 'new' interfaces,
> not LINK_INTERFACE_LIBRARIES.
I've implemented it in the tll-includes-defines branch in my clone. I've
only tested it manually though.
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 includes, defines and other usage requirements with one command
Brad King wrote:
> On 01/29/2013 03:53 AM, Stephen Kelly wrote:
>> This INTERFACE_INCLUDE_DIRECTORIES case is not going to be the only time
>> an upstream will want to add to its interface. In theory, any time they
>> want to add a directory to the INTERFACE_INCLUDE_DIRECTORIES they would
>> have to bump the compatibility number.
>
> Yes, so encouraging use of version numbers in find_package will help.
> OTOH this is currently the case with the _INCLUDE_DIRS variables
> and no one has bothered with it.
I had the same thought, but it is indeed essentially the same situation. I
see why some would use the feature we're discussing in this thread, but I
can also imagine many upstreams ignoring it.
>>> The upstream could require that a version be requested if the downstream
>>> wants the new interfaces to be available, but that does not allow a
>>> downstream to optionally work with older versions of the upstream.
>>
>> Why not?
>
> ...because the downstream would need to do
>
> find_package(Foo ${version_with_new_interface})
>
> to get the new interface and therefore CMake would not find an older
> version now that the new one is requested.
find_package(Foo ${oldversion})
if (NOT Foo_FOUND
OR Foo_VERSION VERSION_GREATER ${version_with_new_interface})
find_package(Foo ${version_with_new_interface})
endif()
# Now Foo_LIBRARIES might be targets with interfaces or not, which
# means include_directories is still needed anyway until the minimum
# version required is ${version_with_new_interface}.
So, really if they want their minimum version of Foo to be lower than
${version_with_new_interface} they should assume they never get interfaces
and always use include_directories until they bump that minimum requirement.
Otherwise all their doing is maintaining multiple codepaths (one with
include_directories, one without) for no reason.
>> In that case upstreamConfig.cmake would look something like this:
>>
>> include("${CMAKE_CURRENT_LIST_DIR}/upstreamTargets.cmake")
>> if (PACKAGE_FIND_VERSION VERSION_LESS 2.3)
>>foreach(_target ${maintained_list_of_targets})
>> set_target_property(${_target}
>> INTERFACE_INCLUDE_DIRECTORIES ""
>> )
>>endforeach()
>> endif()
>>
>> The maintained_list_of_targets would have to be hand-maintained
>> currently. It might be an idea to 'leak' a variable out of the targets
>> file containing the list of targets, but I'm not sure that's a good idea.
>
> I'd rather not populate the properties at all. Perhaps we can combine
> it with your option (2) and have:
>
> if(PACKAGE_FIND_VERSION VERSION_LESS 2.3
> AND NOT MyPkg_INTERFACE_INCLUDE_DIRECTORIES)
>set(${PACKAGE_FIND_NAME}_NO_INTERFACE_INCLUDE_DIRECTORIES 1)
> endif()
> include("${CMAKE_CURRENT_LIST_DIR}/upstreamTargets.cmake")
Yes, I'm sure that would be fine.
>
> ---
>
> Even though this is strictly necessary only for include_directories
> I wonder if we should do it for all usage requirements. That will
> be simpler to explain than "we are confident these other features
> could not possibly break a build". It would also allow the name of
> the option to be more general than ..._INCLUDE_DIRECTORIES:
>
> if(PACKAGE_FIND_VERSION VERSION_LESS 2.3
> AND NOT MyPkg_INTERFACES)
>set(${PACKAGE_FIND_NAME}_NO_INTERFACES 1)
> endif()
> include("${CMAKE_CURRENT_LIST_DIR}/upstreamTargets.cmake")
Yes, I'm fine with that too, though it will only affect 'new' interfaces,
not LINK_INTERFACE_LIBRARIES.
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 includes, defines and other usage requirements with one command
On 01/29/2013 03:53 AM, Stephen Kelly wrote:
> This INTERFACE_INCLUDE_DIRECTORIES case is not going to be the only time an
> upstream will want to add to its interface. In theory, any time they want to
> add a directory to the INTERFACE_INCLUDE_DIRECTORIES they would have to bump
> the compatibility number.
Yes, so encouraging use of version numbers in find_package will help.
OTOH this is currently the case with the _INCLUDE_DIRS variables
and no one has bothered with it.
> this issue is independent of whether tll() adds includes or a new command
Yes. When you realized that we don't need a policy for that the reason
is actually because the policy needs to go in the individual upstreams
for their own interface in the form of an interface version number.
>> The upstream could require that a version be requested if the downstream
>> wants the new interfaces to be available, but that does not allow a
>> downstream to optionally work with older versions of the upstream.
>
> Why not?
...because the downstream would need to do
find_package(Foo ${version_with_new_interface})
to get the new interface and therefore CMake would not find an older
version now that the new one is requested.
>> Perhaps it could work if the upstream provided an explicit variable
>> (option 1 above) that has meaning when the requested version is not
>> present or not new enough.
>
> Yes.
>
>> Then downstreams would be able to use the
>> variable to get the new interfaces if the upstream is new enough to
>> provide them but still work with old upstreams.
>
> Yes. The policy emulation using combined version check and variable is
> probably the best way forward.
Agreed.
> In that case upstreamConfig.cmake would look something like this:
>
> include("${CMAKE_CURRENT_LIST_DIR}/upstreamTargets.cmake")
> if (PACKAGE_FIND_VERSION VERSION_LESS 2.3)
>foreach(_target ${maintained_list_of_targets})
> set_target_property(${_target}
> INTERFACE_INCLUDE_DIRECTORIES ""
> )
>endforeach()
> endif()
>
> The maintained_list_of_targets would have to be hand-maintained currently.
> It might be an idea to 'leak' a variable out of the targets file containing
> the list of targets, but I'm not sure that's a good idea.
I'd rather not populate the properties at all. Perhaps we can combine
it with your option (2) and have:
if(PACKAGE_FIND_VERSION VERSION_LESS 2.3
AND NOT MyPkg_INTERFACE_INCLUDE_DIRECTORIES)
set(${PACKAGE_FIND_NAME}_NO_INTERFACE_INCLUDE_DIRECTORIES 1)
endif()
include("${CMAKE_CURRENT_LIST_DIR}/upstreamTargets.cmake")
---
Even though this is strictly necessary only for include_directories
I wonder if we should do it for all usage requirements. That will
be simpler to explain than "we are confident these other features
could not possibly break a build". It would also allow the name of
the option to be more general than ..._INCLUDE_DIRECTORIES:
if(PACKAGE_FIND_VERSION VERSION_LESS 2.3
AND NOT MyPkg_INTERFACES)
set(${PACKAGE_FIND_NAME}_NO_INTERFACES 1)
endif()
include("${CMAKE_CURRENT_LIST_DIR}/upstreamTargets.cmake")
-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 includes, defines and other usage requirements with one command
Brad King wrote:
> On 01/26/2013 06:57 AM, Stephen Kelly wrote:
>> Brad King wrote:
>>> How can a package author allow old dependents using the old style to
>>> keep working while also allowing new dependents using the new style
>>> to work?
>>
>> So, the options I see are:
>>
>> 1) Upstream introduces a find_package-time variable to evaluate whether
>> to populate the INTERFACE_INCLUDE_DIRECTORIES (similar to
>> QT4_USE_IMPORTED_TARGETS). Upstream would likely have to clear the target
>> property in that case after including the exported targets file.
>
> This will require new downstreams to explicitly activate the new
> interface, always :(
>
>> 2) CMake introduces a standard documented way to not populate the target
>> property in the exported targets file at all
>> (_NO_INTERFACE_INCLUDE_DIRECTORIES, off by default).
>
> How do we know the package name? It would have to be , no?
Nope, I meant ${PACKAGE_FIND_NAME}
>
>> 3) Upstream introduces a new set of IMPORTED targets which have the
>> INTERFACE_INCLUDE_DIRECTORIES set. CMake introduces a way to control at
>> INSTALL(EXPORT) time whether to populate it. So upstream would do this:
>>
>> INSTALL(EXPORT fooTargets NAMESPACE Foo:: ...)
>> INSTALL(EXPORT fooTargets EXPORT_INCLUDE_INTERFACE NAMESPACE FooNew::
>> ...)
>
> I think that will create confusion over the purpose of the two copies.
Probably. I was thinking something along the lines of inline namespaces
where it would be more common to have version numbers like Foo12:: for Foo
1.2, and Foo13:: for Foo 1.3, or more like SO numbers where they'd bump the
SO number (independent of the release number) and add a new set of targets
if they want to extend the interface.
This INTERFACE_INCLUDE_DIRECTORIES case is not going to be the only time an
upstream will want to add to its interface. In theory, any time they want to
add a directory to the INTERFACE_INCLUDE_DIRECTORIES they would have to bump
the compatibility number.
Also, just to shortcut a possible discussion point - this issue is
independent of whether tll() adds includes or a new command is added. If a
downstream is using a new command before the upstream adds
INTERFACE_INCLUDE_DIRECTORIES, the exact same issue comes up. Just to be
clear.
> If the upstream were CMake we could add a policy for this. We need to
> do something similar without a policy in upstream CMake for every
> project out there. If downstreams were to specify a required version
> number in their find_package call then the upstream could activate the
> new interface when the required version is high enough.
Yes.
> This does not
> help when no version is requested though, and that is very common.
Yes.
> The upstream could require that a version be requested if the downstream
> wants the new interfaces to be available, but that does not allow a
> downstream to optionally work with older versions of the upstream.
Why not? All downstream would have to do is use include_directories with the
variable still published by upstream before using tll() if relevant.
That might mean moving a include_directories() call above the tll() when
they change their find_package call to require the newer upstream version,
but it is possible.
> Perhaps it could work if the upstream provided an explicit variable
> (option 1 above) that has meaning when the requested version is not
> present or not new enough.
Yes.
> Then downstreams would be able to use the
> variable to get the new interfaces if the upstream is new enough to
> provide them but still work with old upstreams.
Yes. The policy emulation using combined version check and variable is
probably the best way forward.
In that case upstreamConfig.cmake would look something like this:
include("${CMAKE_CURRENT_LIST_DIR}/upstreamTargets.cmake")
if (PACKAGE_FIND_VERSION VERSION_LESS 2.3)
foreach(_target ${maintained_list_of_targets})
set_target_property(${_target}
INTERFACE_INCLUDE_DIRECTORIES ""
)
endforeach()
endif()
The maintained_list_of_targets would have to be hand-maintained currently.
It might be an idea to 'leak' a variable out of the targets file containing
the list of targets, but I'm not sure that's a good idea.
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 includes, defines and other usage requirements with one command
On 01/26/2013 06:57 AM, Stephen Kelly wrote: > Brad King wrote: >> How can a package author allow old dependents using the old style to >> keep working while also allowing new dependents using the new style >> to work? > > So, the options I see are: > > 1) Upstream introduces a find_package-time variable to evaluate whether to > populate the INTERFACE_INCLUDE_DIRECTORIES (similar to > QT4_USE_IMPORTED_TARGETS). Upstream would likely have to clear the target > property in that case after including the exported targets file. This will require new downstreams to explicitly activate the new interface, always :( > 2) CMake introduces a standard documented way to not populate the target > property in the exported targets file at all > (_NO_INTERFACE_INCLUDE_DIRECTORIES, off by default). How do we know the package name? It would have to be , no? > 3) Upstream introduces a new set of IMPORTED targets which have the > INTERFACE_INCLUDE_DIRECTORIES set. CMake introduces a way to control at > INSTALL(EXPORT) time whether to populate it. So upstream would do this: > > INSTALL(EXPORT fooTargets NAMESPACE Foo:: ...) > INSTALL(EXPORT fooTargets EXPORT_INCLUDE_INTERFACE NAMESPACE FooNew:: ...) I think that will create confusion over the purpose of the two copies. If the upstream were CMake we could add a policy for this. We need to do something similar without a policy in upstream CMake for every project out there. If downstreams were to specify a required version number in their find_package call then the upstream could activate the new interface when the required version is high enough. This does not help when no version is requested though, and that is very common. The upstream could require that a version be requested if the downstream wants the new interfaces to be available, but that does not allow a downstream to optionally work with older versions of the upstream. Perhaps it could work if the upstream provided an explicit variable (option 1 above) that has meaning when the requested version is not present or not new enough. Then downstreams would be able to use the variable to get the new interfaces if the upstream is new enough to provide them but still work with old upstreams. -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 includes, defines and other usage requirements with one command
On Sunday 27 January 2013, Stephen Kelly wrote:
> Alexander Neundorf wrote:
> > Does
> >
> > function(target_use_stuff _target )
> >
> > target_compile_definitions(${_target} ${ARGN})
> > target_include_directories(${_target} ${ARGN})
> > target_link_libraries(${_target} ${ARGN})
> >
> > endfunction()
> >
> > actually differ from what you want to do for tll() ?
>
> Yes. tll() takes LINK_PUBLIC and the others take PUBLIC for example.
Yes, but this is just a different keyword meaning basically the same.
> Additionally tll() can take non-targets such as library-file names and
> strings such as "-Wl,-Bsymbolic-functions -Wl,-z,relro -lgssapi_krb5 -lkrb5
> -lk5crypto -lcom_err" as is used in kdelibs (from GSSAPI).
>
> There is some clean-up to do, but it is possible to write a macro for it,
> as I wrote here:
Is it maybe a bit inconsistent that
- to setup linking, I can use tll() with targets and with paths
- to setup include dirs, I can use tll() with targets, or tid() with targets
and directories, or id() with directories
Should include_directories() maybe also be taught to recognize targets ?
I mean, it kind of makes sense that tll() does not (will not) handle
directories for setting up include dirs, but I'm still not sure I like that
tll() will handle linking completely and partly setting up the includes.
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 includes, defines and other usage requirements with one command
Alexander Neundorf wrote:
> Does
>
> function(target_use_stuff _target )
> target_compile_definitions(${_target} ${ARGN})
> target_include_directories(${_target} ${ARGN})
> target_link_libraries(${_target} ${ARGN})
> endfunction()
>
> actually differ from what you want to do for tll() ?
Yes. tll() takes LINK_PUBLIC and the others take PUBLIC for example.
Additionally tll() can take non-targets such as library-file names and
strings such as "-Wl,-Bsymbolic-functions -Wl,-z,relro -lgssapi_krb5 -lkrb5
-lk5crypto -lcom_err" as is used in kdelibs (from GSSAPI).
There is some clean-up to do, but it is possible to write a macro for it, as
I wrote here:
http://thread.gmane.org/gmane.comp.programming.tools.cmake.devel/5835/focus=5841
although the macro I wrote there doesn't handle things like
tll(foo LINK_PUBLIC bar LINK_PRIVATE blub)
but again that's fixable, but I don't think a macro is the right approach.
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 includes, defines and other usage requirements with one command
On Thursday 24 January 2013, Stephen Kelly wrote:
> Hi there,
>
> I've pushed the tll-includes-defines topic to my clone again, rebased to
> master.
>
> It allows the removal a lot of redundant use of target_include_directories
> and target_compile_definitions. I think the real value of the whole feature
> will only come when all usage requirements can be set by connecting targets
> together one time with one command, be that with tll() or with
> target_use_interfaces().
>
> One reason I think tll() should be used for that is that it has the
> implication that can be documented and easily understood - namely that
> cmake requirements usage is based on linking targets together.
Yes, but that's changing of the meaning of tll(), which exists for a long
time.
> Linking correctly requires having compiled correctly, and compiling
> correctly depends on having used the correct include directories and
> compile definitions as specified by upstream, as well as a few other
> things such as use of -fPIC. Currently in cmake master, the -fPIC compile
> flag may be added depending on what targets are used in tll() calls, but
> the compile definitions and the include directories are not. As linking
> correctly depends on compiling correctly as specified by upstream, it
> seems natural to ensure that compiling correctly is possible as a result
> of
> target_link_libraries calls.
>
> As far as I understand, the only objection is to the idea that
> target_link_libraries would be doing something other than linking, and it
> might not be obvious. It is currently used for -fPIC, so I'm not so sure.
>
> Also, the objection is not that people would have to learn or discover, in
> documentation or otherwise, that target_link_libraries could have an effect
> other than linking. The objection instead is that, even long term and for
> experienced people, reading a line of code that contains a
> target_link_libraries call alone would not inform them of whether it is
> 'only' linking or whether it has other affects. This also seems funny to
> me. Given a line containing target_use_interfaces(foo PRIVATE bar), it is
> impossible to know from reading alone whether foo INCLUDE_DIRECTORIES,
> COMPILE_DEFINITIONS, LINK_LIBRARIES, or all three, are affected by the
> line.
A new command (or macro) could warn if a target is used which doesn't have all
three properties set (or maybe only for imported targets).
Does
function(target_use_stuff _target )
target_compile_definitions(${_target} ${ARGN})
target_include_directories(${_target} ${ARGN})
target_link_libraries(${_target} ${ARGN})
endfunction()
actually differ from what you want to do for tll() ?
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 includes, defines and other usage requirements with one command
Stephen Kelly wrote: > INSTALL(EXPORT fooTargets NAMESPACE Foo:: ...) > INSTALL(EXPORT fooTargets EXPORT_INCLUDE_INTERFACE NAMESPACE FooNew:: ...) > > or some other naming change, and include() both in the config file. > > (2) is easy for us and for upstream, but source incompatible for > downstream. (3) is more awkward for all upstreams, but source compatible > for downstream. Or, of course, (3) could be switched around and NO_EXPORT_INCLUDE_INTERFACE would not populate the INTERFACE_INCLUDE_DIRECTORIES. That would be easier for upstreams, but also leave them potentially accidentally breaking downstreams by not using it when they should. 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 includes, defines and other usage requirements with one command
Brad King wrote: > I've longed for "usage requirements" for years and always pictured > them propagating through linking. The huge threads of discussion > earlier made usage requirements seem more complicated than they are > and made it feel like we should hide it all behind new interfaces. > Now I think a new command will actually be *more* confusing in the > long run because the two will be different only in subtle ways and > users will wonder which one to use. Yes, that's exactly my thinking too. > I'm almost ready to accept the proposed behavior for > target_link_libraries. However, we still need to construct a > recommended way for packages and their dependents to handle the > transition. There are many, many instances of the old style usage > since it is the only one that previously worked. > > How can a package author allow old dependents using the old style to > keep working while also allowing new dependents using the new style > to work? As compile definitions are uniq'd, they don't pose any backward compatibility concerns for downstreams of a project whose upstream adds them. In the worst case, the needed compile definitions will be duplicated by an existing call to add_definitions(), and in the best case the needed defintions will be added where they were absent before. In the case of the include directories, there could be incompatibility introduced by an upstream newly using the feature. Even if upstream has all- unique headers in multiple directories (think Qt), downstream might be also using another project which has conflicting headers (in the case of Qt, that's not likely as the 'q' prefix is well claimed). So, the options I see are: 1) Upstream introduces a find_package-time variable to evaluate whether to populate the INTERFACE_INCLUDE_DIRECTORIES (similar to QT4_USE_IMPORTED_TARGETS). Upstream would likely have to clear the target property in that case after including the exported targets file. 2) CMake introduces a standard documented way to not populate the target property in the exported targets file at all (_NO_INTERFACE_INCLUDE_DIRECTORIES, off by default). 3) Upstream introduces a new set of IMPORTED targets which have the INTERFACE_INCLUDE_DIRECTORIES set. CMake introduces a way to control at INSTALL(EXPORT) time whether to populate it. So upstream would do this: INSTALL(EXPORT fooTargets NAMESPACE Foo:: ...) INSTALL(EXPORT fooTargets EXPORT_INCLUDE_INTERFACE NAMESPACE FooNew:: ...) or some other naming change, and include() both in the config file. (2) is easy for us and for upstream, but source incompatible for downstream. (3) is more awkward for all upstreams, but source compatible for downstream. 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 includes, defines and other usage requirements with one command
On 01/24/2013 03:39 PM, David Cole wrote:
> It's not only that target_link_libraries would be doing more
> than linking, it's also that you want to change the behavior of
> something that has existed for 12+ years without this behavior.
>
> If you do re-use target_link_libraries for your glorious "one
> command to rule them all," just be aware that you are
> invalidating 12+ years worth of everything ever written about
> it on the web, and the collective experience of thousands of
> CMake users around the world.
>
> *That* is my objection to re-purposing the name
> target_link_libraries, not that you'd be doing more than
> linking. I don't think it's worth the confusion that might come
> after this.
>
> I would not object at all to any *new* command with an entirely
> new name, such that it may be written about anew, with great
> excitement, now that the one true command is about to be born.
David's wording helped me think about this more from both a
historical and future perspective, and to my surprise actually
come to the opposite conclusion.
I've longed for "usage requirements" for years and always pictured
them propagating through linking. The huge threads of discussion
earlier made usage requirements seem more complicated than they are
and made it feel like we should hide it all behind new interfaces.
Now I think a new command will actually be *more* confusing in the
long run because the two will be different only in subtle ways and
users will wonder which one to use.
The "written about anew, with great excitement" feature is actually
the usage requirements. Since no projects set them yet there is no
change in behavior by adding the feature to tll. Now we can tell
library authors that they can simplify the usage interface for their
dependents.
Alex originally raised his concerns with an example of the old and
new styles, arguing that users should be able to choose between them.
The old style uses variables and several calls:
find_package(Foo)
include_directories(${Foo_INCLUDE_DIRS})
add_definitions(${Foo_DEFINITIONS}) # needed for Foo?
link_directories(${Foo_LIBRARY_DIRS})# needed for Foo?
target_link_libraries(mytgt ${Foo_LIBRARIES}) # need all Foo libs?
# (more for conditional libraries or special compile flags)
but for every package one must read the documentation to know which
pieces are needed. The new style uses usage requirements:
find_package(Foo)
target_link_libraries(mytgt Foo::SpecificLibINeed)
It might look nicer to have a different name for the command in the
new-style usage, but that's about it. The complication of my
earlier proposals for how the new command would work was due to
trying to mix the two notions of transitive behavior of the two
commands. As Steve has said it is much easier to have only one
command to define such relationships among targets.
I'm almost ready to accept the proposed behavior for
target_link_libraries. However, we still need to construct a
recommended way for packages and their dependents to handle the
transition. There are many, many instances of the old style usage
since it is the only one that previously worked.
How can a package author allow old dependents using the old style to
keep working while also allowing new dependents using the new style
to work? One either populates the usage requirements properties or
does not. If enabling the new features requires possible breakage
for existing dependents then no one will do it. Steve?
Thanks,
-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 includes, defines and other usage requirements with one command
David Cole wrote: > If you do re-use target_link_libraries for your glorious "one command to > rule them all," just be aware that you are invalidating 12+ years worth of > everything ever written about it on the web, and the collective experience > of thousands of CMake users around the world. > > That is my objection to re-purposing the name target_link_libraries, not > that you'd be doing more than linking. I don't think it's worth the > confusion that might come after this. > Thanks for clarifying. :) I didn't mean to mis-characterize the objection. What I wrote was my understanding of Alex' objection and the assumption that yours was the same or very similar. 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 includes, defines and other usage requirements with one command
On Jan 24, 2013, at 3:00 PM, Stephen Kelly wrote: > > As far as I understand, the only objection is to the idea that > target_link_libraries would be doing something other than linking, and it > might not be obvious. It is currently used for -fPIC, so I'm not so sure. > > Also, the objection is not that people would have to learn or discover, in > documentation or otherwise, that target_link_libraries could have an effect > other than linking. The objection instead is that, even long term and for > experienced people, reading a line of code that contains a > target_link_libraries call alone would not inform them of whether it is > 'only' linking or whether it has other affects. This also seems funny to me. > Given a line containing target_use_interfaces(foo PRIVATE bar), it is > impossible to know from reading alone whether foo INCLUDE_DIRECTORIES, > COMPILE_DEFINITIONS, LINK_LIBRARIES, or all three, are affected by the line. > In both cases, the way to know is to use > -DCMAKE_DEBUG_TARGET_PROPERTIES=INCLUDE_DIRECTORIES or similar. > > It's not only that target_link_libraries would be doing more than linking, it's also that you want to change the behavior of something that has existed for 12+ years without this behavior. If you do re-use target_link_libraries for your glorious "one command to rule them all," just be aware that you are invalidating 12+ years worth of everything ever written about it on the web, and the collective experience of thousands of CMake users around the world. *That* is my objection to re-purposing the name target_link_libraries, not that you'd be doing more than linking. I don't think it's worth the confusion that might come after this. I would not object at all to any *new* command with an entirely new name, such that it may be written about anew, with great excitement, now that the one true command is about to be born. :-) 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
