Re: [cmake-developers] target_include_directories branch in stage
On Wed, Feb 22, 2012 at 8:36 PM, David Cole david.c...@kitware.com wrote: On Wed, Feb 22, 2012 at 3:04 PM, David Cole david.c...@kitware.com wrote: On Wed, Feb 22, 2012 at 8:05 AM, David Cole david.c...@kitware.com wrote: On Thu, Feb 16, 2012 at 4:11 PM, Stephen Kelly steve...@gmail.com wrote: David Cole wrote: Pushed down the queue again... I'll get to it soon. There are a handful of minor changes that I still need to make first. Ok. Let me know if it's anything I can do. 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 Finally pushed the most recent commits this morning... Please see the target-include-directories branch on the stage. (Not yet merged to 'next' -- but soon will be.) Question (probably for Brad): What should --help-property emit now that there are both directory and target level properties for INCLUDE_DIRECTORIES? Here's what it does: $ bin/Debug/cmake --help-property INCLUDE_DIRECTORIES cmake version 2.8.7.20120222-g56a44 INCLUDE_DIRECTORIES List of preprocessor include file search directories. This read-only property specifies the list of directories given so far to the include_directories command. It is intended for debugging purposes. So the most recent commit in the topic branch, which adds documentation for the target level property, appears useless... Should we eliminate the directory level documentation as well? Or emit both? If so, how? After we settle this one remaining point, I'll patch up the topic branch and merge it to 'next'. Thanks to everybody for your help and patience in getting this feature into CMake. Cheers, David C. Finally ready... I just force-pushed the 'target-include-directories' branch to stage again. I think it's ready for merging to 'next' -- before I do that, could I get a sanity check that the documentation in the top commit (ff44b88) is acceptable? Thanks, David All set. One more force-push after cleaning up a commit on Brad's recommendation and it is now finally merged to 'next'. Whew. Thanks for pushing on this - glad to see it get merged in and looking forward to being able to use this. Marcus -- 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] target_include_directories branch in stage
On Thu, Feb 16, 2012 at 4:11 PM, Stephen Kelly steve...@gmail.com wrote: David Cole wrote: Pushed down the queue again... I'll get to it soon. There are a handful of minor changes that I still need to make first. Ok. Let me know if it's anything I can do. 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 Finally pushed the most recent commits this morning... Please see the target-include-directories branch on the stage. (Not yet merged to 'next' -- but soon will be.) Question (probably for Brad): What should --help-property emit now that there are both directory and target level properties for INCLUDE_DIRECTORIES? Here's what it does: $ bin/Debug/cmake --help-property INCLUDE_DIRECTORIES cmake version 2.8.7.20120222-g56a44 INCLUDE_DIRECTORIES List of preprocessor include file search directories. This read-only property specifies the list of directories given so far to the include_directories command. It is intended for debugging purposes. So the most recent commit in the topic branch, which adds documentation for the target level property, appears useless... Should we eliminate the directory level documentation as well? Or emit both? If so, how? After we settle this one remaining point, I'll patch up the topic branch and merge it to 'next'. Thanks to everybody for your help and patience in getting this feature into CMake. Cheers, David C. -- 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] target_include_directories branch in stage
On 2/22/2012 8:05 AM, David Cole wrote: So the most recent commit in the topic branch, which adds documentation for the target level property, appears useless... Should we eliminate the directory level documentation as well? Or emit both? Emit both and explain their interaction. The documentation for each will be a bit different because each plays a unique role. The include_directories command should document just how it sets the dir-level and target-level properties. The dir-level property should document that it is used to init the target-level property for new targets. The target-level one should document that it is used as the actual source of include directories for compilation. -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] target_include_directories branch in stage
On Wed, Feb 22, 2012 at 8:05 AM, David Cole david.c...@kitware.com wrote: On Thu, Feb 16, 2012 at 4:11 PM, Stephen Kelly steve...@gmail.com wrote: David Cole wrote: Pushed down the queue again... I'll get to it soon. There are a handful of minor changes that I still need to make first. Ok. Let me know if it's anything I can do. 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 Finally pushed the most recent commits this morning... Please see the target-include-directories branch on the stage. (Not yet merged to 'next' -- but soon will be.) Question (probably for Brad): What should --help-property emit now that there are both directory and target level properties for INCLUDE_DIRECTORIES? Here's what it does: $ bin/Debug/cmake --help-property INCLUDE_DIRECTORIES cmake version 2.8.7.20120222-g56a44 INCLUDE_DIRECTORIES List of preprocessor include file search directories. This read-only property specifies the list of directories given so far to the include_directories command. It is intended for debugging purposes. So the most recent commit in the topic branch, which adds documentation for the target level property, appears useless... Should we eliminate the directory level documentation as well? Or emit both? If so, how? After we settle this one remaining point, I'll patch up the topic branch and merge it to 'next'. Thanks to everybody for your help and patience in getting this feature into CMake. Cheers, David C. Finally ready... I just force-pushed the 'target-include-directories' branch to stage again. I think it's ready for merging to 'next' -- before I do that, could I get a sanity check that the documentation in the top commit (ff44b88) is acceptable? Thanks, David -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] target_include_directories branch in stage
On Wed, Feb 22, 2012 at 3:04 PM, David Cole david.c...@kitware.com wrote: On Wed, Feb 22, 2012 at 8:05 AM, David Cole david.c...@kitware.com wrote: On Thu, Feb 16, 2012 at 4:11 PM, Stephen Kelly steve...@gmail.com wrote: David Cole wrote: Pushed down the queue again... I'll get to it soon. There are a handful of minor changes that I still need to make first. Ok. Let me know if it's anything I can do. 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 Finally pushed the most recent commits this morning... Please see the target-include-directories branch on the stage. (Not yet merged to 'next' -- but soon will be.) Question (probably for Brad): What should --help-property emit now that there are both directory and target level properties for INCLUDE_DIRECTORIES? Here's what it does: $ bin/Debug/cmake --help-property INCLUDE_DIRECTORIES cmake version 2.8.7.20120222-g56a44 INCLUDE_DIRECTORIES List of preprocessor include file search directories. This read-only property specifies the list of directories given so far to the include_directories command. It is intended for debugging purposes. So the most recent commit in the topic branch, which adds documentation for the target level property, appears useless... Should we eliminate the directory level documentation as well? Or emit both? If so, how? After we settle this one remaining point, I'll patch up the topic branch and merge it to 'next'. Thanks to everybody for your help and patience in getting this feature into CMake. Cheers, David C. Finally ready... I just force-pushed the 'target-include-directories' branch to stage again. I think it's ready for merging to 'next' -- before I do that, could I get a sanity check that the documentation in the top commit (ff44b88) is acceptable? Thanks, David All set. One more force-push after cleaning up a commit on Brad's recommendation and it is now finally merged to 'next'. Whew. Thx, David -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] target_include_directories branch in stage
David Cole wrote: All set. One more force-push after cleaning up a commit on Brad's recommendation and it is now finally merged to 'next'. Whew. Great! Thanks for all yor work on getting this through the last mile (or was it more? :)). 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] target_include_directories branch in stage
Pushed down the queue again... I'll get to it soon. There are a handful of minor changes that I still need to make first. On Thu, Feb 16, 2012 at 8:20 AM, Stephen Kelly steve...@gmail.com wrote: David Cole wrote: Excellent : thanks very much for trying it out. I'll try to get this first draft merged to 'next' tonight or tomorrow. Any update on this? -- 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 -- 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] target_include_directories branch in stage
On 2/9/2012 3:45 PM, Alexander Neundorf wrote: Just to make completely sure: the purpose of this target property is *not* that targets linking against a library with this property set should reuse these include dirs, right ? Correct. It's just to allow the directory-level INCLUDE_DIRECTORIES specification to be target-level. Nevertheless, did you already think about how the interface for such a feature could look like (to make sure it fits with this new target property) ? Maybe something like this ? set_target_properties(... PROPERTIES INCLUDE_DIRECTORIES a/ b/ c/ PUBLIC_BUILD_INCLUDE_DIRECTORIES d/ e/ f/ PUBLIC_INSTALL_INCLUDE_DIRECTORIES g/ h/ i/ ) It would have to be possible to separate between include dirs to be used at build time of the library, and include dirs to be used once the library has been installed (similar to the build and install RPATH) ...just to make sure the syntax for the new INCLUDE_DIRECTORIES property is ok and doesn't make problems later on. I think it could work the same way as linking versus LINK_INTERFACE_LIBRARIES. Anyway, this is not a design goal for the current work and the current work's interface is a natural extension of what is there already. -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] target_include_directories branch in stage
On Thu, Feb 9, 2012 at 3:45 PM, Alexander Neundorf neund...@kde.org wrote: On Monday 30 January 2012, Stephen Kelly wrote: David Cole wrote: On Sun, Jan 29, 2012 at 11:44 AM, Stephen Kelly steve...@gmail.com wrote: David Cole wrote: OK... nearly complete now. Please review, then reply and tell me if you object to any of the 7 commits in this topic branch. No objections. They all seem fine. Great, thanks. Steve, I've preserved your authorship for most of these commits, but have significantly re-written some of them. The 3 following 073bc421620e25fef6389b0d8b71cfad8ca79786 (CMake: Eliminate cmMakefile::IncludeDirectories) seem to have been substantially re-written. I'm not sure what is the appropriate way to deal with commits like that, but whatever you choose for the author line is fine. So I guess the question is: if somebody looks up the author at some point in the future and comes asking a question, do you want to be the one they go to, or would you rather have them come to me? ;-) If you want to send them to me, I'll change the author line, but you certainly deserve the credit for getting the ball rolling on this topic. Ok, let's leave it as-is and I'll take responsibility (blame/credit) :) snip - double-check with Alex about the changes in cmake::FindPackage -- Alex? Still pending. Sorry that it took so long, I was really busy the last few weeks. I cloned this repo: git clone https://github.com/dlrdave/CMake.git Both the Eclipse generator and cmake --find-package work fine. Excellent : thanks very much for trying it out. I'll try to get this first draft merged to 'next' tonight or tomorrow. Just to make completely sure: the purpose of this target property is *not* that targets linking against a library with this property set should reuse these include dirs, right ? Nevertheless, did you already think about how the interface for such a feature could look like (to make sure it fits with this new target property) ? Maybe something like this ? set_target_properties(... PROPERTIES INCLUDE_DIRECTORIES a/ b/ c/ PUBLIC_BUILD_INCLUDE_DIRECTORIES d/ e/ f/ PUBLIC_INSTALL_INCLUDE_DIRECTORIES g/ h/ i/ ) It would have to be possible to separate between include dirs to be used at build time of the library, and include dirs to be used once the library has been installed (similar to the build and install RPATH) ...just to make sure the syntax for the new INCLUDE_DIRECTORIES property is ok and doesn't make problems later on. 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 -- 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] target_include_directories branch in stage
On Sun, Jan 29, 2012 at 11:44 AM, Stephen Kelly steve...@gmail.com wrote:
David Cole wrote:
OK... nearly complete now. Please review, then reply and tell me if
you object to any of the 7 commits in this topic branch.
No objections. They all seem fine.
Great, thanks.
Steve, I've
preserved your authorship for most of these commits, but have
significantly re-written some of them.
The 3 following 073bc421620e25fef6389b0d8b71cfad8ca79786 (CMake: Eliminate
cmMakefile::IncludeDirectories) seem to have been substantially re-written.
I'm not sure what is the appropriate way to deal with commits like that, but
whatever you choose for the author line is fine.
So I guess the question is: if somebody looks up the author at some
point in the future and comes asking a question, do you want to be the
one they go to, or would you rather have them come to me? ;-) If you
want to send them to me, I'll change the author line, but you
certainly deserve the credit for getting the ball rolling on this
topic.
I've pushed again (force-pushed, please delete previous checkouts of
this branch) to github:
https://github.com/dlrdave/CMake/commits/tgt-inc-dirs-vs-xcode
https://github.com/dlrdave/CMake/commit/0a45e239
I've verified all tests pass on Mac with Unix Makefiles, Mac with
Xcode, Windows with VS10 and Windows with VS6. I'm confident we will
have minimal dashboard issues when merging this to 'next'.
Great. Thanks for all of your efforts.
What remains to be done:
- need ExpandVariablesInString calls for the target properties, too...
at
the same point, or is there a better time to call it?
Are you asking whether cmMakefile::ExpandVariables() should do something
like (sort-of pseudocode):
foreach(Target target, this-Targets)
{
const char *includeDirs = target-GetProperty(INCLUDE_DIRECTORIES);
if (includeDirs)
{
std::string dirs = includeDirs;
this-ExpandVariablesInString(dirs, true, true);
target-SetProperty(INCLUDE_DIRECTORIES, dirs.c_str());
}
}
I don't know enough about when ExpandVariables is supposed to be called etc,
but it seems like a sensible place to do it is always called at the correct
point.
However, it starts to look like ExpandVariablesInString should be API
elsewhere in CMake (cmSystemTools). It's not a very important point though
and doesn't necessarily need to be solved with this branch.
I'll talk this over with Brad and finalize the commits with any
changes before pushing this to 'next'.
- avoid duplicates in cmMakeDepend::SetMakefile?
Considering that it used to call
this-Makefile-GetIncludeDirectories();
which does preserve uniqueness, I think it makes sense to do so with the
ported code too.
Yes, ok, I'll spend the extra effort to make that section a unique list as well.
- double-check with Alex about the changes in cmake::FindPackage --
Alex?
- fully remove cmMakefile::GetIncludeDirectories since it has no
more callers - update documentation for include_directories and the
INCLUDE_DIRECTORIES property
I have updated my branch on gitorious with an attempt at documenting the
change (commit 8acd1c07ff299ea823837ba6268b43db92ac81f2).
Thanks -- I'll pull this and use it as a basis for the final topic.
David
--
Powered by www.kitware.com
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] target_include_directories branch in stage
David Cole wrote: On Sun, Jan 29, 2012 at 11:44 AM, Stephen Kelly steve...@gmail.com wrote: David Cole wrote: OK... nearly complete now. Please review, then reply and tell me if you object to any of the 7 commits in this topic branch. No objections. They all seem fine. Great, thanks. Steve, I've preserved your authorship for most of these commits, but have significantly re-written some of them. The 3 following 073bc421620e25fef6389b0d8b71cfad8ca79786 (CMake: Eliminate cmMakefile::IncludeDirectories) seem to have been substantially re-written. I'm not sure what is the appropriate way to deal with commits like that, but whatever you choose for the author line is fine. So I guess the question is: if somebody looks up the author at some point in the future and comes asking a question, do you want to be the one they go to, or would you rather have them come to me? ;-) If you want to send them to me, I'll change the author line, but you certainly deserve the credit for getting the ball rolling on this topic. Ok, let's leave it as-is and I'll take responsibility (blame/credit) :) snip - double-check with Alex about the changes in cmake::FindPackage -- Alex? Still pending. Thanks -- I'll pull this and use it as a basis for the final topic. Great. Let me know if there's any more I can do. 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] target_include_directories branch in stage
On Mon, Jan 23, 2012 at 3:09 PM, Stephen Kelly steve...@gmail.com wrote: David Cole wrote: On Fri, Jan 20, 2012 at 3:20 PM, Stephen Kelly steve...@gmail.com wrote: Hi, I've force pushed my branch: https://gitorious.org/~steveire/cmake/steveires-cmake/commits/target- include-directories Brad King wrote: On 1/8/2012 11:47 AM, Stephen Kelly wrote: On 12/05/2011 03:17 PM, Brad King wrote: I think the property can be stored just like any other property during configuration. Then the generators can use ExpandListArguments. Would that mean that every generator would have to ensure that the includes were unique etc? Yes but there could be an internal API provided by cmTarget to compute the expansion in one place for use by any generator. I really think that all property values should be handled in the same way and interpreted in their non-string format only as needed. Done. Initialization from the directory property value can just be added to cmTarget::SetMakefile: this-SetPropertyDefault(INCLUDE_DIRECTORIES, 0); SetPropertyDefault is for initializing a property with the content of the property with the same name with a CMAKE_ prefix. Sorry, my bad. You're right, except that it looks for a *variable* with the CMAKE_ prefix. Instead you can just spell out initialization of the target property by copying the current value of the directory property. Ported. I think the branch is ready for review again. 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 I've tacked on two more commits for supporting Visual Studio and Xcode. You can pull from here if you want to see them: https://github.com/dlrdave/CMake/commits/tgt-inc-dirs-vs-xcode Brad and I will review it and take over to get it into CMake 'next' -- Awesome, thanks. I just had a look. I assume you won't mind if we slightly re-write a few things rather than go back and forth over email about them...? Indeed. I don't mind. I can review the changes to the commits later. 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 OK... nearly complete now. Please review, then reply and tell me if you object to any of the 7 commits in this topic branch. Steve, I've preserved your authorship for most of these commits, but have significantly re-written some of them. I've pushed again (force-pushed, please delete previous checkouts of this branch) to github: https://github.com/dlrdave/CMake/commits/tgt-inc-dirs-vs-xcode https://github.com/dlrdave/CMake/commit/0a45e239 I've verified all tests pass on Mac with Unix Makefiles, Mac with Xcode, Windows with VS10 and Windows with VS6. I'm confident we will have minimal dashboard issues when merging this to 'next'. What remains to be done: - need ExpandVariablesInString calls for the target properties, too... at the same point, or is there a better time to call it? - avoid duplicates in cmMakeDepend::SetMakefile? - double-check with Alex about the changes in cmake::FindPackage -- Alex? - fully remove cmMakefile::GetIncludeDirectories since it has no more callers - update documentation for include_directories and the INCLUDE_DIRECTORIES property - address any issues raised by reviewers Thanks, David C. -- 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] target_include_directories branch in stage
On Fri, Jan 20, 2012 at 3:20 PM, Stephen Kelly steve...@gmail.com wrote: Hi, I've force pushed my branch: https://gitorious.org/~steveire/cmake/steveires-cmake/commits/target- include-directories Brad King wrote: On 1/8/2012 11:47 AM, Stephen Kelly wrote: On 12/05/2011 03:17 PM, Brad King wrote: I think the property can be stored just like any other property during configuration. Then the generators can use ExpandListArguments. Would that mean that every generator would have to ensure that the includes were unique etc? Yes but there could be an internal API provided by cmTarget to compute the expansion in one place for use by any generator. I really think that all property values should be handled in the same way and interpreted in their non-string format only as needed. Done. Initialization from the directory property value can just be added to cmTarget::SetMakefile: this-SetPropertyDefault(INCLUDE_DIRECTORIES, 0); SetPropertyDefault is for initializing a property with the content of the property with the same name with a CMAKE_ prefix. Sorry, my bad. You're right, except that it looks for a *variable* with the CMAKE_ prefix. Instead you can just spell out initialization of the target property by copying the current value of the directory property. Ported. I think the branch is ready for review again. 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 I've tacked on two more commits for supporting Visual Studio and Xcode. You can pull from here if you want to see them: https://github.com/dlrdave/CMake/commits/tgt-inc-dirs-vs-xcode Brad and I will review it and take over to get it into CMake 'next' -- I assume you won't mind if we slightly re-write a few things rather than go back and forth over email about them...? Thanks, David -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] target_include_directories branch in stage
David Cole wrote: On Fri, Jan 20, 2012 at 3:20 PM, Stephen Kelly steve...@gmail.com wrote: Hi, I've force pushed my branch: https://gitorious.org/~steveire/cmake/steveires-cmake/commits/target- include-directories Brad King wrote: On 1/8/2012 11:47 AM, Stephen Kelly wrote: On 12/05/2011 03:17 PM, Brad King wrote: I think the property can be stored just like any other property during configuration. Then the generators can use ExpandListArguments. Would that mean that every generator would have to ensure that the includes were unique etc? Yes but there could be an internal API provided by cmTarget to compute the expansion in one place for use by any generator. I really think that all property values should be handled in the same way and interpreted in their non-string format only as needed. Done. Initialization from the directory property value can just be added to cmTarget::SetMakefile: this-SetPropertyDefault(INCLUDE_DIRECTORIES, 0); SetPropertyDefault is for initializing a property with the content of the property with the same name with a CMAKE_ prefix. Sorry, my bad. You're right, except that it looks for a *variable* with the CMAKE_ prefix. Instead you can just spell out initialization of the target property by copying the current value of the directory property. Ported. I think the branch is ready for review again. 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 I've tacked on two more commits for supporting Visual Studio and Xcode. You can pull from here if you want to see them: https://github.com/dlrdave/CMake/commits/tgt-inc-dirs-vs-xcode Brad and I will review it and take over to get it into CMake 'next' -- Awesome, thanks. I just had a look. I assume you won't mind if we slightly re-write a few things rather than go back and forth over email about them...? Indeed. I don't mind. I can review the changes to the commits later. 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] target_include_directories branch in stage
David Cole wrote: How can this feature now be moved forward? Do I need to convince someone to volunteer to port the other generators? Should I just file a bug for porting the other generators and wait (possibly making the feature bitrot)? I volunteer to make sure this branch works with Xcode and Visual Studio. I'll get to it within the next week... Fantastic. It's definitely something that should be done as early in the cycle as possible. Just ask if you have any questions etc. Thanks, Steve. -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] target_include_directories branch in stage
On 09.12.2011 22:23, Brad King wrote: On 12/9/2011 3:44 PM, Peter Kümmel wrote: Maybe this is a bit late, but wouldn't it be much simpler to get this feature with a namespace inspired approach: http://public.kitware.com/Bug/view.php?id=11793 I think the namespace approach will be more intrusive to implement, and it is complementary to the much-needed per-target feature anyway. All within a namespace has the same scope as an add_subdirectory added target. This way we get more than the 'target_include_directories' without adding multiple target_* functions (which maybe will come). We don't really need a target_include_directories command. It will be just a convenience on top of setting the INCLUDE_DIRECTORIES target property with the set_property command. Ah, I thought target_include_directories is the reason for the new target property. -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] target_include_directories branch in stage
Maybe this is a bit late, but wouldn't it be much simpler to get this feature with a namespace inspired approach: http://public.kitware.com/Bug/view.php?id=11793 All within a namespace has the same scope as an add_subdirectory added target. This way we get more than the 'target_include_directories' without adding multiple target_* functions (which maybe will come). Peter -- 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] target_include_directories branch in stage
David Cole wrote: I, for one, would really like to see per-target include directories in 2.8.7, even without per-config support to start with. Then, add the per-config support / new generator expressions in a later release. That seems unlikely to happen. If RC1 is Wednesday, it would have to be clean on the dashboard by then, which means cleanup of issues reported by the nightlies on Tuesday, which means putting it in next on Monday in order to get the nightlies generated. I've pushed the branch to my gitorious clone again. https://gitorious.org/~steveire/cmake/steveires-cmake The top two commits need to be reviewed and have real fixes decided and written for them. Should the memoization be removed? What does cmMakeDepends do? Note that if the goal is to merge this into next on Monday, there is little point explaining this to me and asking me to make the necessary changes. That would be too inefficient. git reset HEAD^^ and doing the fixes would be better. Additionally, the generators for XCode and VisualStudio don't build in the branch (though I haven't tried) because I changed the signature of LocalGenerator::GetIncludeDirectories. Again, that would have to be fixed by someone with direct access to those platforms and/or generators. Finally, the Eclipse generator builds, and all tests pass, which means mostly that I don't know how to test it. I didn't port it away from cmMakefile::GetIncludeDirectories, and yet the (updated) IncludeDirectories test passes. I expected the test to fail. To run the test I did: mkdir eclipse_gen cd eclipse_gen cmake .. -G Eclipse CDT4 - Unix Makefiles make ./bin/ctest Again, asking me to fix these issues would be too inefficient because I don't have direct access to the platforms, am unfamiliar with what cmMakeDepend.cxx or the memoization should be doing, and don't know enough about the eclipse generator to fix it. All that would have to be done tomorrow in order to make the RC and I would not be able to make that happen. Given that this feedback didn't happen last week, people who can give it don't likely have time on Monday either. So the options are probably delay the RC (which wouldn't make me any more capable of fixing the remaining issues anyway. It would still need $Your help) or defer the feature. 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] target_include_directories branch in stage
On Sunday 04 December 2011, Stephen Kelly wrote: David Cole wrote: I, for one, would really like to see per-target include directories in 2.8.7, even without per-config support to start with. Then, add the per-config support / new generator expressions in a later release. That seems unlikely to happen. If RC1 is Wednesday, it would have to be clean on the dashboard by then, which means cleanup of issues reported by the nightlies on Tuesday, which means putting it in next on Monday in order to get the nightlies generated. I've pushed the branch to my gitorious clone again. https://gitorious.org/~steveire/cmake/steveires-cmake The top two commits need to be reviewed and have real fixes decided and written for them. Should the memoization be removed? What does cmMakeDepends do? Note that if the goal is to merge this into next on Monday, there is little point explaining this to me and asking me to make the necessary changes. That would be too inefficient. git reset HEAD^^ and doing the fixes would be better. Additionally, the generators for XCode and VisualStudio don't build in the branch (though I haven't tried) because I changed the signature of LocalGenerator::GetIncludeDirectories. Again, that would have to be fixed by someone with direct access to those platforms and/or generators. Finally, the Eclipse generator builds, and all tests pass, which means mostly that I don't know how to test it. I didn't port it away from cmMakefile::GetIncludeDirectories, and yet the (updated) IncludeDirectories test passes. I expected the test to fail. To run the test I did: mkdir eclipse_gen cd eclipse_gen cmake .. -G Eclipse CDT4 - Unix Makefiles make ./bin/ctest The Eclipse generator generates Makefiles, and additionally Eclipse project files. But those are not tested during the tests, only the makefiles are tested. I don't know of any way to test automatically whether an Eclipse project works (except loading it manually). What could be broken there ? 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] target_include_directories branch in stage
Alexander Neundorf wrote:
On Sunday 04 December 2011, Stephen Kelly wrote:
David Cole wrote:
I, for one, would really like to see per-target include directories in
2.8.7, even without per-config support to start with. Then, add the
per-config support / new generator expressions in a later release.
That seems unlikely to happen. If RC1 is Wednesday, it would have to be
clean on the dashboard by then, which means cleanup of issues reported by
the nightlies on Tuesday, which means putting it in next on Monday in
order to get the nightlies generated.
I've pushed the branch to my gitorious clone again.
https://gitorious.org/~steveire/cmake/steveires-cmake
The top two commits need to be reviewed and have real fixes decided and
written for them. Should the memoization be removed? What does
cmMakeDepends do? Note that if the goal is to merge this into next on
Monday, there is little point explaining this to me and asking me to make
the necessary changes. That would be too inefficient. git reset HEAD^^
and doing the fixes would be better.
Additionally, the generators for XCode and VisualStudio don't build in
the branch (though I haven't tried) because I changed the signature of
LocalGenerator::GetIncludeDirectories. Again, that would have to be fixed
by someone with direct access to those platforms and/or generators.
Finally, the Eclipse generator builds, and all tests pass, which means
mostly that I don't know how to test it. I didn't port it away from
cmMakefile::GetIncludeDirectories, and yet the (updated)
IncludeDirectories test passes. I expected the test to fail. To run the
test I did:
mkdir eclipse_gen cd eclipse_gen
cmake .. -G Eclipse CDT4 - Unix Makefiles make
./bin/ctest
The Eclipse generator generates Makefiles, and additionally Eclipse
project files. But those are not tested during the tests, only the
makefiles are tested.
I don't know of any way to test automatically whether an Eclipse project
works (except loading it manually).
What could be broken there ?
The IncludeDirectories test. The Eclipse generator calls
GetIncludeDirectories on the cmMakefile instead of asking the targets for
the include directories.
stephen@hal:~/dev/src/cmake{target-include-directories}$ git grep
GetIncludeDirec | grep Eclip
Source/cmExtraEclipseCDT4Generator.cxx: = (*it)-GetMakefile()-
GetIncludeDirectories();
See the commit 'Extract and use the INCLUDE_DIRECTORIES target properties'
in the branch, in particular the change to
Source/cmLocalUnixMakefileGenerator3.cxx. Perhaps something similar is
needed in the eclipse generator.
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] target_include_directories branch in stage
Brad King wrote: On 11/29/2011 7:34 AM, Stephen Kelly wrote: I worked on the functionality for per-config target includes, but the syntax is not right yet. Please remove per-config support for now. Do you mean I should remove the UI feature of setting the INCLUDE_DIRECTORIES_DEBUG property adding to the include directories of the debug config, or do you additionally mean that I should remove the current implementation that keeps order and keeps the specified config. The current implementation (std::vectorstd::pairstd::string, std::string ) would work for the UI syntax you suggest below. It seems like removing the implementation detail only for it to be re-added later when the UI syntax is decided would be inefficient. I'm fine with removing the INCLUDE_DIRECTORIES_config UI feature. With the approach I have in mind it will be a simple matter to add support for it after the main feature works. I want to make the approach something general so it can be applied to other things like link directories. The idea is to process the single list in the generator and look for special syntax that indicates whether each entry is meant for the current config. I haven't settled on a syntax yet but it will be similar to generator expressions from add_test and add_custom_command: $CONFIG?Debug:/dir/for/Debug This entry would be ignored if the CONFIG? condition doesn't match. The cmGeneratorExpression class can be used to evaluate it. Ok. Is this something that should aim for post-2.8.7? Should I aim to get taget specific INCLUDE_DIRECTORIES into 2.8.7 (within this week) and aim for adding this config-specific feature later (would that be source compatible?). 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] target_include_directories branch in stage
On Fri, Dec 2, 2011 at 7:54 AM, Stephen Kelly steve...@gmail.com wrote: Brad King wrote: On 11/29/2011 7:34 AM, Stephen Kelly wrote: I worked on the functionality for per-config target includes, but the syntax is not right yet. Please remove per-config support for now. Do you mean I should remove the UI feature of setting the INCLUDE_DIRECTORIES_DEBUG property adding to the include directories of the debug config, or do you additionally mean that I should remove the current implementation that keeps order and keeps the specified config. The current implementation (std::vectorstd::pairstd::string, std::string ) would work for the UI syntax you suggest below. It seems like removing the implementation detail only for it to be re-added later when the UI syntax is decided would be inefficient. I'm fine with removing the INCLUDE_DIRECTORIES_config UI feature. With the approach I have in mind it will be a simple matter to add support for it after the main feature works. I want to make the approach something general so it can be applied to other things like link directories. The idea is to process the single list in the generator and look for special syntax that indicates whether each entry is meant for the current config. I haven't settled on a syntax yet but it will be similar to generator expressions from add_test and add_custom_command: $CONFIG?Debug:/dir/for/Debug This entry would be ignored if the CONFIG? condition doesn't match. The cmGeneratorExpression class can be used to evaluate it. Ok. Is this something that should aim for post-2.8.7? Should I aim to get taget specific INCLUDE_DIRECTORIES into 2.8.7 (within this week) and aim for adding this config-specific feature later (would that be source compatible?). 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 I, for one, would really like to see per-target include directories in 2.8.7, even without per-config support to start with. Then, add the per-config support / new generator expressions in a later release. Thx, David -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] target_include_directories branch in stage
On 12/2/2011 8:35 AM, David Cole wrote: Do you mean I should remove the UI feature of setting the INCLUDE_DIRECTORIES_DEBUG property adding to the include directories of the debug config, or do you additionally mean that I should remove the current implementation that keeps order and keeps the specified config. The current implementation (std::vectorstd::pairstd::string, std::string ) would work for the UI syntax you suggest below. It seems like removing the implementation detail only for it to be re-added later when the UI syntax is decided would be inefficient. I'm fine with removing the INCLUDE_DIRECTORIES_config UI feature. Remove both. The interface will not be of that form. The implementation of the future interface I propose will only need a regular string-valued property that happens to be a semicolon-separated list. The generators will take that list and evaluate the $ syntax in each entry to see if it needs any per-config filtering. $CONFIG?Debug:/dir/for/Debug Ok. Is this something that should aim for post-2.8.7? Should I aim to get taget specific INCLUDE_DIRECTORIES into 2.8.7 (within this week) and aim for adding this config-specific feature later (would that be source compatible?). I, for one, would really like to see per-target include directories in 2.8.7, even without per-config support to start with. Then, add the per-config support / new generator expressions in a later release. Yes. There will be no compatibility problem because the $ syntax makes no sense to have as a literal include directory. -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] target_include_directories branch in stage
Brad King wrote: On 11/29/2011 7:34 AM, Stephen Kelly wrote: I've been working on this topic again, but I forgot that I had not merged it into next. I think I properly reverted it though. I added another commit to the revert-... branch to fully revert it. To avoid accidental merges of work-in-progress topics, please instead create a github.com account, visit https://github.com/Kitware/CMake and use the Fork button to get your own repo in which to publish topics. Then post a link to the topics there for us to fetch and review. That way you can rewrite the topic arbitrarily during development. Ok. I've pushed it to my gitorious repo for now. I'll remove the config stuff there and let you know when that's done. Should I push it to github too or is gitorious also ok when it's ready for review? 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] target_include_directories branch in stage
On 12/1/2011 3:47 PM, Stephen Kelly wrote: https://github.com/Kitware/CMake and use the Fork button Ok. I've pushed it to my gitorious repo for now. I'll remove the config stuff there and let you know when that's done. Should I push it to github too or is gitorious also ok when it's ready for review? Gitorious is fine. What is your URL? 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] target_include_directories branch in stage
Brad King wrote: On 12/1/2011 3:47 PM, Stephen Kelly wrote: https://github.com/Kitware/CMake and use the Fork button Ok. I've pushed it to my gitorious repo for now. I'll remove the config stuff there and let you know when that's done. Should I push it to github too or is gitorious also ok when it's ready for review? Gitorious is fine. What is your URL? git://gitorious.org/~steveire/cmake/steveires-cmake.git No useful update there yet 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] target_include_directories branch in stage
On Wednesday 30 November 2011, Brad King wrote: On 11/29/2011 2:28 PM, Alexander Neundorf wrote: ...a somewhat related idea: if it will be possible to set include directories per target, and since it is already possible to set compile flags per target, it would be nice if I could also set a property on targets which keeps them from using the global settings at all. The design for the new INCLUDE_DIRECTORIES property allows this. After the last include_directories() call in a directory one may set the INCLUDE_DIRECTORIES property of any target to anything and overwrite whatever came from the directory. Cool :-) In the current design the property is initialized when the target is created to the current directory-level value. Then each additional include_directories() call appends to the dir-level value *and* the targets in the directory. Perhaps the latter could be disabled by a property like you propose. Especially I'd be interested in having that for the compile flags. I'm not sure whether a disable all global or directory level settings would be enough or whether it should be fine-grained so that e.g. compile flags and include dirs could be disabled separately. Probably also link_directories() and link_libraries() (but they are more rarely used). 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] target_include_directories branch in stage
Brad King brad.king@... writes:
On 11/6/2011 5:45 PM, Stephen Kelly wrote:I'd prefer it to be though if
we can sort out the issues with what should be the target feature set.
Good. We can work on this and revise/rewrite the topic there first and then
merge to next for testing when the design is more mature.
I've been working on this topic again, but I forgot that I had not merged it
into next. I think I properly reverted it though.
The INCLUDE_DIRECTORIES property of a target must be the *only* list
from which the final include directory list is constructed. When a target
is created the current directory-level include directories must be used
to initialize the list. Further include_directories() calls in the same
directory must append not only to the directory-level list but also to
the property for all existing targets. At any intermediate point a
target's property can be modified independently (see the set_property
command example below).
This works for me locally now.
Now we can return to per-config include directories. To incorporate them
into the same single list, we will need some kind of markup on a per-entry
basis. This is similar to the debug/optimized keywords used by the
target_link_libraries command, but must be more well designed. Perhaps
a syntax similar to generator expressions in custom commands can work.
I'll have to think more about it.
I worked on the functionality for per-config target includes, but the syntax is
not right yet. For now it's:
set_property(TARGET foo APPEND PROPERTY INCLUDE_DIRECTORIES ${foo_INCLUDES})
set_property(TARGET foo APPEND PROPERTY INCLUDE_DIRECTORIES_DEBUG
${bar_INCLUDES})
get_property(VAR foo INCLUDE_DIRECTORIES_DEBUG) # VAR is Always empty
get_property(VAR foo INCLUDE_DIRECTORIES) # In Debug configuration, VAR is
# foo_INCLUDES and bar_INCLUDES. In non-Debug it's foo_INCLUDES only.
So before I merge this into next, I'm wondering if this feature can be
considered for inclusion in 2.8.7?
Whatever features are mature and merged to master before the release will
be in it. We won't hold up a release for this though.
At the moment, all CMake unit tests pass for me with the branch (and I've just
fixed the style), but I think not all generators are configured to build for me,
so they remain unported for now. Are special switches needed to build CodeBlocks
and Eclipse Generators?
At this point I'd like some guidance on whether the general approach I took is
the correct one, and where I should go from here. I'm not familiar with lots of
parts of CMake that I'm touching, and there are lots of classes whose
responsibility and interactions with the other classes is not clear to me
(GlobalGenerator, LocalGenerator, LocalUnixMakefileGenerator3 etc). cmMakefile
doesn't seem to represent a Makefile. Is the name historical?
Anyway, while information on those classes might be useful, eyes on the patch
and feedback on the approach would be more so. My approach so far has been to
attempt to remove the use of cmMakefile::GetIncludeDirectories from everywhere
except cmTarget (which uses it to initialize its own directories) and replace it
with calls to the target for the include directories.
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] target_include_directories branch in stage
...a somewhat related idea: if it will be possible to set include directories per target, and since it is already possible to set compile flags per target, it would be nice if I could also set a property on targets which keeps them from using the global settings at all. Something like set_target_properties(hello PROPERTIES COMPILE_FLAGS ... INCLUDE_DIRS ... IGNORE_GLOBAL_SETTINGS TRUE) which would have the effect that neither the include directories for the current directory nor the global CMAKE_(C|CXX)_FLAGS would be used, but only what has been explicitely set for this target. Sounds like a useful idea ? If you think it makes sense, I might have some time to work on it. 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] target_include_directories branch in stage
On Sunday 06 November 2011, Stephen Kelly wrote:
Stephen Kelly wrote:
Issues:
* I have only tried to implement this with the makefile generator and
have so far only tested it with Unix Makefiles. One of the bugs says
XCode can't do source-level includes. Can it do target-level includes?
Would I have to implement this for all generators before it would be
considered for acceptance?
* There's scope for refactoring the new code in my branch as well as
potentially refactoring with the cmIncludeDirectoriesCommand.
* There's scope for optimization.
* I haven't written any tests yet.
Related to Alex's remarks, there may also be scope for PUBLIC_INCLUDES and
PRIVATE_INCLUDES keywords.
I think the normal include directories should be treated as PRIVATE, while
the PUBLIC ones should have to be set explicitely.
Assuming a library would have those PUBLIC_INCLUDES set, should they be used
automatically when linking against this library using target_link_libraries(),
so that the following two lines add the PUBLIC_INCLUDES from the myLib library
to the compilation of foo ?
add_executable(foo ${srcs} )
target_link_libraries(foo myLib)
I'd say no, because it would be invisible in this code that the
target_link_libraries() call adds an include-directory to the compilation.
This will make for harder debugging/understanding CMakeLists.txt you didn't
write yourself.
The target_include_directories() command from the other email could help with
this:
add_executable(foo ${srcs} )
target_link_libraries(foo myLib)
target_include_directories(foo /some/include/dir myLib)
or maybe an extra keyword for target_link_libraries(), something like ?
add_executable(foo ${srcs} )
target_link_libraries(foo myLib USE_INCLUDE_DIRECTORIES)
Just some ideas...
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] target_include_directories branch in stage
On 11/6/2011 5:45 PM, Stephen Kelly wrote:
As discussed on the cmake user mailing list, I'm interesting in implementing
the feature of target specific and configuration specific include
directories.
Thanks for working on this.
I've pushed the target-include-directories branch to stage, which implements
the feature. I started out as prototyping, but I ended up implementing API
that I think makes sense. I have not merged it into next yet as it is not
certain if it should be in the next release. I'd prefer it to be though if
we can sort out the issues with what should be the target feature set.
Good. We can work on this and revise/rewrite the topic there first and then
merge to next for testing when the design is more mature.
David mentioned one issue is whether the include directories of a target
property should overwrite those of the directory property (added with the
command include_directories). Like others on the other thread, I would
expect the final list of includes to be determined by addition. For example:
One problem with determining by addition is the ordering, and a good deal
of your message talks about that. It is not just dir-vs.-target ordering.
Where do the per-config lists appear? Let's ignore per-config rules for
the moment and focus on dir-vs.-target, and return to per-config further down.
Unlike link rules we don't have any explicit dependencies or ordering
constraints among include directories. Ideally all include directories
would contain a disjoint set of files so that the order wouldn't matter,
but that isn't going to happen in general. Unless we can ask users to
specify ordering constraints we cannot do any automatic ordering. We
have no good way to merge/combine multiple include directory lists.
The solution has to be based on only a *single* include directory list.
We already do this for link libraries. The non-target link_directories
and link_libraries commands just append to lists kept for each directory.
When a target is created it's per-target link information is initialized
by copying the current directory-level version. After that only the
target_link_libraries command can add more rules for a target.
The INCLUDE_DIRECTORIES property of a target must be the *only* list
from which the final include directory list is constructed. When a target
is created the current directory-level include directories must be used
to initialize the list. Further include_directories() calls in the same
directory must append not only to the directory-level list but also to
the property for all existing targets. At any intermediate point a
target's property can be modified independently (see the set_property
command example below).
Now we can return to per-config include directories. To incorporate them
into the same single list, we will need some kind of markup on a per-entry
basis. This is similar to the debug/optimized keywords used by the
target_link_libraries command, but must be more well designed. Perhaps
a syntax similar to generator expressions in custom commands can work.
I'll have to think more about it.
target_include_directories(foo CONFIG_TYPE DEBUG debug_helper.h)
I'd like to get this working using only the raw target property first:
set_property(TARGET foo APPEND PROPERTY INCLUDE_DIRECTORIES ${foo_INCLUDES})
and discuss a new command separately later.
* I have only tried to implement this with the makefile generator and have
so far only tested it with Unix Makefiles. One of the bugs says XCode
can't do source-level includes. Can it do target-level includes?
I haven't checked recently but I think it can do both. The concern may be
per-source flags that vary by configuration.
Would I have to implement this for all generators before it would be
considered for acceptance?
Yes, at least someone would have to implement it for each generator.
So before I merge this into next, I'm wondering if this feature can be
considered for inclusion in 2.8.7?
Whatever features are mature and merged to master before the release will
be in it. We won't hold up a release for this 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] target_include_directories branch in stage
On 11/6/2011 5:49 PM, Stephen Kelly wrote: Stephen Kelly wrote: Issues: * I have only tried to implement this with the makefile generator and have so far only tested it with Unix Makefiles. One of the bugs says XCode can't do source-level includes. Can it do target-level includes? Would I have to implement this for all generators before it would be considered for acceptance? * There's scope for refactoring the new code in my branch as well as potentially refactoring with the cmIncludeDirectoriesCommand. * There's scope for optimization. * I haven't written any tests yet. Related to Alex's remarks, there may also be scope for PUBLIC_INCLUDES and PRIVATE_INCLUDES keywords. http://thread.gmane.org/gmane.comp.programming.tools.cmake.user/39090/focus=39200 This is a separate problem. It is about propagating include directories through usage requirements of a library, just like the link interface. Let's start with per-target/config include directories. -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] target_include_directories branch in stage
On Monday 07 November 2011, Alexander Neundorf wrote: On Sunday 06 November 2011, Stephen Kelly wrote: Hi, As discussed on the cmake user mailing list, I'm interesting in implementing the feature of target specific and configuration specific include directories. http://thread.gmane.org/gmane.comp.programming.tools.cmake.user/39090/foc us =39114 I've pushed the target-include-directories branch to stage, which implements the feature. I started out as prototyping, but I ended up implementing API that I think makes sense. I have not merged it into next yet as it is not certain if it should be in the next release. I'd prefer it to be though if we can sort out the issues with what should be the target feature set. David mentioned one issue is whether the include directories of a target property should overwrite those of the directory property (added with the command include_directories). Like others on the other thread, I would expect the final list of includes to be determined by addition. I replied over there to this question. Here: http://thread.gmane.org/gmane.comp.programming.tools.cmake.user/39090/focus=39114 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] target_include_directories branch in stage
On 11/7/2011 12:57 PM, Brad King wrote:
The INCLUDE_DIRECTORIES property of a target must be the *only* list
from which the final include directory list is constructed. When a target
is created the current directory-level include directories must be used
to initialize the list. Further include_directories() calls in the same
directory must append not only to the directory-level list but also to
the property for all existing targets. At any intermediate point a
target's property can be modified independently (see the set_property
command example below).
Alex suggested the same thing:
http://thread.gmane.org/gmane.comp.programming.tools.cmake.user/39090/focus=39227
On 11/7/2011 12:23 PM, Alexander Neundorf wrote:
I think
get_target_properties(someVar foo INCLUDE_DIRECTORIES)
should return the full list of include directories used for that target.
This would mean that they are not really additive.
Instead, the INCLUDE_DIRECTORIES target property could be initialized from the
directory-property INCLUDE_DIRECTORIES.
Then, to add include dirs, use
set_property(TARGET foo APPEND PROPERTY INCLUDE_DIRECTORIES ${bar_INCLUDES} )
To set (and ignore any directory-level include dirs):
set_property(TARGET foo PROPERTY INCLUDE_DIRECTORIES ${blub_INCLUDES} )
-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
