Review Request: Add support for cmake target include_directories.

[Ben Wagner]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104777/
-----------------------------------------------------------

(Updated May 1, 2012, 3:30 a.m.)


Review request for KDevelop.


Changes
-------

This patch fixes some of the messy and adds a unit test. There is one caveat however, and this may be opening a can of worms. In the test there are two ProjectBaseItem for the main.cpp url. The one with the target as the parent works as expected with this change, the 'include' include directory is visible. The one with the folder as the parent does not see the 'include' include directory, as would be expected from the implementation. It appears that the parser and the like always see the target parented version, as when I look at the new 'target_includes' test project the 'include' include directory certainly is found. The unit test itself has some extra debugging code in it which may or may not be wanted.

After thinking about this for a bit, I realized that there isn't really a good way around this. It is possible that two different targets will list the same source file. Because the includes for the targets can now be different these two mentions of the same source file may have entirely different meanings. Before, if the same source file was mentioned in two CMakeLists.txt files, the file would at least be out-of-source in at least one of them. I realized that defines have the same issue, so I took a look there. It is possible to set COMPILE_DEFINITIONS on directories, targets, and source files (though I don't think KDevelop handles source files right now). It appears to have the same issue, so I suspect that this is ok.

Also, now that I look at this patch with my reviewer hat on, I wonder if CMakeManager::includeDirectories should be implemented as I have here, or if it should be more like CMakeManager::defines where it finds the first 'DefinesAttached' thing and then just asks it to resolve. Now that I stare at it, I'm not sure my new code or the old code handles nested sub-directories with includes correctly (it appears to stop at the first folder found). Perhaps this should just go in as-is with another patch for nested sub-directories, if that is needed.


Description
-------

With CMake 2.8.8, it is now possible to write something like

set_property(TARGET zlib APPEND PROPERTY INCLUDE_DIRECTORIES "include/config")

This change implements include directories on targets.

Note that I am not a committer, so if you want it, the diff I uploaded is a format-patch against current master.


This addresses bug 298859.
    http://bugs.kde.org/show_bug.cgi?id=298859


Diffs (updated)
-----

  projectmanagers/cmake/cmakemanager.cpp 9ae8f90 
  projectmanagers/cmake/cmakemodelitems.h 705a632 
  projectmanagers/cmake/cmakemodelitems.cpp 957c410 
  projectmanagers/cmake/tests/CMakeLists.txt ca8567e 
  projectmanagers/cmake/tests/cmakemanagertest.h 1316952 
  projectmanagers/cmake/tests/cmakemanagertest.cpp ca3532d 
  projectmanagers/cmake/tests/manual/target_includes/CMakeLists.txt PRE-CREATION 
  projectmanagers/cmake/tests/manual/target_includes/includes/target_include.h PRE-CREATION 
  projectmanagers/cmake/tests/manual/target_includes/main.cpp PRE-CREATION 
  projectmanagers/cmake/tests/manual/target_includes/target_includes.kdev4 PRE-CREATION 

Diff: http://git.reviewboard.kde.org/r/104777/diff/


Testing
-------

Built and ran. The test linked from bug works as expected after building (as the test itself generates some of the required .h files). Behavior of non-target includes appears unaffected.


Thanks,

Ben Wagner

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20120501/f446e432/attachment.html>