Re: [cmake-developers] Setting includes, defines and other usage requirements with one command

2013-02-12 Thread Stephen Kelly
Brad King wrote:

 Please try to track down and remove any extra pieces like $LINKED
 and package_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

2013-02-12 Thread Brad King
On 02/12/2013 07:20 AM, Stephen Kelly wrote:
 Brad King wrote:
 
 Please try to track down and remove any extra pieces like $LINKED
 and package_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

2013-02-12 Thread Stephen Kelly
Stephen Kelly wrote:

 * Can the INCLUDE_DIRETORIES and COMPILE_DEFINITIONS property avoid
$LINKED:foo 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

2013-02-11 Thread Brad King
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 ? $LINKED: + lib + 
 +   : $$TARGET_DEFINED: + lib + : +
 +   $TARGET_PROPERTY: + lib +
 +   ,INTERFACE_ + property + 
 + ;

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 $LINKED 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 $LINKED 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 package_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 $TPROP:foo,INTERFACE_INCLUDE_DIRECTORIES
  themselves.  Furthermore since the usage requirements always append
  there is no compatibility issue for upstreams adding include dirs
  so we may not need package_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

2013-02-11 Thread Matthew Woehlke

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. 'dir', '-I dir', 
'--include=dir', 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

2013-02-11 Thread Stephen Kelly
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 package_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 $TPROP:foo,INTERFACE_INCLUDE_DIRECTORIES
   themselves.  Furthermore since the usage requirements always append
   there is no compatibility issue for upstreams adding include dirs
   so we may not need package_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

2013-02-11 Thread Stephen Kelly
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. 'dir', '-I dir',
 '--include=dir', 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

2013-02-11 Thread Brad King
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 $LINKED
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 $LINKED
and package_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

2013-02-08 Thread Brad King
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 ? $LINKED: + lib + 
 +   : $$TARGET_DEFINED: + lib + : +
 +   $TARGET_PROPERTY: + lib +
 +   ,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 $$CONFIG:Debug:bar)
 
 The genex for the INCLUDE_DIRECTORIES of foo needs to be (excuse the python-
 style concats):
 
  $$TARGET_DEFINED: + $$CONFIG:Debug:bar + : +
$TARGET_PROPERTY: + $$CONFIG:Debug:bar +
   ,INTERFACE_ + INCLUDE_DIRECTORIES + 
  
 
 because the result of the genex could either be  or bar.

Currently the code

  target_link_libraries(foo $$CONFIG:Debug: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 $LINKED:${whole_expression} approach?  I think it should be
possible when processing out $LINKED:... 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 $TARGET_PROPERTY:bar,INTERFACE_INCLUDE_DIRECTORIES)

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

2013-02-08 Thread Stephen Kelly
Brad King wrote:
 Currently the code
 
   target_link_libraries(foo $$CONFIG:Debug: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 $$CONFIG:Debug: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 

 $TARGET_PROPERTY:$$CONFIG:Debug:not_a_target,IFACE_INCLUDE_DIRS

wouldn't work for non-Debug. It is deliberately strict.

 Can we still use
 the $LINKED:${whole_expression} 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 $LINKED:... 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$$CONFIG:Debug:_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 $TARGET_PROPERTY:bar,INTERFACE_INCLUDE_DIRECTORIES)
 
 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 $TARGET_PROPERTY:bar,SOMETHING)
 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 $$CONFIG:Debug: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 $$CONFIG:Debug: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

2013-02-08 Thread Brad King
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 $NO_INTERFACES:$$CONFIG:Debug: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 $USE_INTERFACES:... 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

2013-02-04 Thread Stephen Kelly
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

2013-02-04 Thread Brad King
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

2013-02-02 Thread Stephen Kelly
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;$LINKED:bar and user would 
end up with COMPILE_OPTIONS $LINKED:bar;$LINKED:foo which expands 
first to -O2;-O3;$LINKED:bar and then -O2;-O3;-O2.

However the 'optimized' result would look like:

 -O2 -O3

because the first expansion would be -O2;-O3;$LINKED:bar, 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 $NO_SKIP:... 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 $NO_SKIP:bar)

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

2013-02-01 Thread Brad King
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 package_NO_INTERFACES variable in their package config
 file when it exposes the new interfaces.  Also package_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

2013-01-31 Thread Stephen Kelly
Brad King wrote:

 On 01/30/2013 12:09 PM, Stephen Kelly wrote:
 We can document that $TARGET_DEFINED 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 $$CONFIG:Debug:debuglib)
 tll(tgt $TARGET_DEFINED:${lib_genex}:$TARGET_PROPERTY:${lib_genex},FOO)

 
 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.

 $LINKED:blah
 
 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

2013-01-31 Thread Brad King
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 $$CONFIG:Debug:debuglib)
  tll(tgt $TARGET_DEFINED:${lib_genex}:$TARGET_PROPERTY:${lib_genex},FOO)

Okay.  As long as $LINKED:... 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::vectorstd::basic_stringchar )’:
 .../Source/cmExportFileGenerator.cxx:425:12: warning: unused variable ‘error’ 
[-Wunused-variable]

* I think $LINKED:... 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
  $TARGET_PROPERTY:...,... reference in the export.  This way the
  $LINKED:... expression lives only as long as needed.

* We could document $LINKED:... 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
  $LINKED:foo 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

2013-01-31 Thread Stephen Kelly
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::vectorstd::basic_stringchar )’:
  .../Source/cmExportFileGenerator.cxx:425:12: warning: unused variable
  ‘error’ [-Wunused-variable]

Fixed now, thanks.

 
 * I think $LINKED:... 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 $LINKED 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
   $TARGET_PROPERTY:...,... reference in the export.  This way the
   $LINKED:... expression lives only as long as needed.
 
 * We could document $LINKED:... 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
   $LINKED:foo 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

2013-01-31 Thread Brad King
On 01/31/2013 10:23 AM, Stephen Kelly wrote:
 I left it as $LINKED so far so that it is a shorter string, and because 
 the implementation is somewhat simpler.

Ahh, I see you were using $LINKED:... 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 $LINKED 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 $INSTALL_PREFIX evaluation error.


What is the purpose of the else case here?

+  std::string value = !isGenex ? $LINKED: + lib + 
+   : $$TARGET_DEFINED: + lib + : +
+   $TARGET_PROPERTY: + lib +
+   ,INTERFACE_ + property + 
+ ;

If the input is already a genex isn't the author responsible for
making sure it is valid?


The documentation of package_NO_INTERFACES and package_INTERFACES
reference ${CMAKE_FIND_PACKAGE_NAME}_FIND_VERSION but the corresponding
package_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 package_NO_INTERFACES variable in their package config
file when it exposes the new interfaces.  Also package_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

2013-01-30 Thread Stephen Kelly
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

2013-01-30 Thread Brad King
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
  $BUILD_INTERFACE:${FOO_SOURCE_DIR}
  $INSTALL_INTERFACE:$INSTALL_PREFIX/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 = 
[$TARGET_PROPERTY:foo,INTERFACE_INCLUDE_DIRECTORIES;$$TARGET_DEFINED:m:$TARGET_PROPERTY:m,INTERFACE_INCLUDE_DIRECTORIES]
CMake Error at CMakeLists.txt:16 (export):
  $TARGET_PROPERTY:m,prop 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

2013-01-30 Thread Stephen Kelly
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 =
 [$TARGET_PROPERTY:foo,INTERFACE_INCLUDE_DIRECTORIES;
$$TARGET_DEFINED:m:$TARGET_PROPERTY:m,INTERFACE_INCLUDE_DIRECTORIES]
 CMake Error at CMakeLists.txt:16 (export):
   $TARGET_PROPERTY:m,prop 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

2013-01-30 Thread Brad King
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 
$TARGET_PROPERTY:foo,INTERFACE_COMPILE_DEFINITIONS;$$TARGET_DEFINED:m:$TARGET_PROPERTY:m,INTERFACE_COMPILE_DEFINITIONS
 INTERFACE_INCLUDE_DIRECTORIES 
$TARGET_PROPERTY:foo,INTERFACE_INCLUDE_DIRECTORIES;$$TARGET_DEFINED:m:$TARGET_PROPERTY:m,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 $TARGET_DEFINED 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 $INSTALL_PREFIX expression should evaluate as an error if
  it is ever reached by any path except install(EXPORT).  Otherwise
  the empty string in $INSTALL_PREFIX/somewhere is unlikely to
  do the right thing.

* If exported interfaces were partially evaluated then $INSTALL_PREFIX
  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.

$LINKED:blah

  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 $TARGET_PROPERTY:... 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

2013-01-30 Thread Stephen Kelly
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
  $TARGET_PROPERTY:foo,INTERFACE_COMPILE_DEFINITIONS;
$$TARGET_DEFINED:m:$TARGET_PROPERTY:m,INTERFACE_COMPILE_DEFINITIONS
  INTERFACE_INCLUDE_DIRECTORIES
  $TARGET_PROPERTY:foo,INTERFACE_INCLUDE_DIRECTORIES;
$$TARGET_DEFINED:m:$TARGET_PROPERTY:m,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 $TARGET_DEFINED 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 $INSTALL_PREFIX expression should evaluate as an error if
   it is ever reached by any path except install(EXPORT).  Otherwise
   the empty string in $INSTALL_PREFIX/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 $INSTALL_PREFIX
   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.
 
 $LINKED:blah

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 $TARGET_PROPERTY:... 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

2013-01-30 Thread Brad King
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 $TARGET_DEFINED 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 $INSTALL_PREFIX expression should evaluate as an error if
   it is ever reached by any path except install(EXPORT).  Otherwise
   the empty string in $INSTALL_PREFIX/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.

 $LINKED:blah
 
 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

2013-01-29 Thread Stephen Kelly
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
 (PACKAGE_NAME_NO_INTERFACE_INCLUDE_DIRECTORIES, off by default).
 
 How do we know the package name?  It would have to be EXPORT_NAME, 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

2013-01-29 Thread Brad King
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 pkg_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

2013-01-29 Thread Stephen Kelly
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 pkg_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

2013-01-29 Thread Stephen Kelly
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

2013-01-29 Thread Alexander Neundorf
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

2013-01-29 Thread Alexander Neundorf
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

2013-01-29 Thread Brad King
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

2013-01-29 Thread Brad King
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

2013-01-29 Thread Brad King
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

2013-01-28 Thread Alexander Neundorf
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

2013-01-28 Thread Brad King
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 
 (PACKAGE_NAME_NO_INTERFACE_INCLUDE_DIRECTORIES, off by default).

How do we know the package name?  It would have to be EXPORT_NAME, 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

2013-01-27 Thread Stephen Kelly
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

2013-01-26 Thread Stephen Kelly
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 
(PACKAGE_NAME_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

2013-01-26 Thread Stephen Kelly
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

2013-01-25 Thread Brad King
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


[cmake-developers] Setting includes, defines and other usage requirements with one command

2013-01-24 Thread Stephen Kelly

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.

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. 
In both cases, the way to know is to use
 -DCMAKE_DEBUG_TARGET_PROPERTIES=INCLUDE_DIRECTORIES or similar.

Also feel free to respond to any parts of 

 
http://thread.gmane.org/gmane.comp.programming.tools.cmake.devel/5526/focus=5710

which are not obviously already addressed.

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

2013-01-24 Thread David Cole
On Jan 24, 2013, at 3:00 PM, Stephen Kelly wrote:

 snip

 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.
 
 /snip


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