MantisBT - CMake
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0013188CMakeCMakepublic2012-05-04 01:042014-06-02 08:37
Reporterbungeman 
Assigned ToStephen Kelly 
PrioritynormalSeverityminorReproducibilityalways
StatusclosedResolutionfixed 
PlatformUbuntuOSLinuxOS Version3.0.0-17-generic
Product VersionCMake 2.8.8 
Target VersionFixed in VersionCMake 3.0 
Summary0013188: Target and directory include_directory usage.
DescriptionI tried out the newly added target and directory INCLUDE_DIRECTORIES property and ran into some issues. I can't seem to be able to set the directory property at all. The target property appears to treat relative includes as being relative to the build directory, which is confusing since include_directories(...) treats them relative to the source (CMakeLists.txt) directory.
Steps To Reproduce./main.cpp
  #include "target_include.h"
  int main(int, char**) { return foo; }

./include/target_include.h
  #define foo 0

./CMakeLists.txt
  cmake_minimum_required(VERSION 2.8.8)
  project(directory_includes)
  add_executable(target main.cpp)

  #when attempting to 'make' in './build' subdirectory after running 'cmake ..'

  #does not work but should
  #set_property(TARGET target APPEND PROPERTY INCLUDE_DIRECTORIES "includes")
  #set_property(DIRECTORY APPEND PROPERTY INCLUDE_DIRECTORIES "includes")
  #set_directory_properties(PROPERTY INCLUDE_DIRECTORIES "includes")
  #set_property(DIRECTORY APPEND PROPERTY INCLUDE_DIRECTORIES "${CMAKE_CURRENT_SOURCE_DIR}/includes")
  #set_property(DIRECTORY PROPERTY INCLUDE_DIRECTORIES "${CMAKE_CURRENT_SOURCE_DIR}/includes")
  #set_property(DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} PROPERTY INCLUDE_DIRECTORIES "${CMAKE_CURRENT_SOURCE_DIR}/includes")
  #set_directory_properties(PROPERTY INCLUDE_DIRECTORIES "${CMAKE_CURRENT_SOURCE_DIR}/includes")

  #shouldn't work, and doesn't
  #set_property(DIRECTORY APPEND PROPERTY INCLUDE_DIRECTORIES "../includes")
  #set_directory_properties(PROPERTY INCLUDE_DIRECTORIES "../includes")

  #works, and should
  #set_property(TARGET target APPEND PROPERTY INCLUDE_DIRECTORIES "${CMAKE_CURRENT_SOURCE_DIR}/includes")
  include_directories(includes)

  #works, and shouldn't
  #set_property(TARGET target APPEND PROPERTY INCLUDE_DIRECTORIES "../includes")
Additional InformationNote that all of the above which fail parse and generate makefiles fine, but the resulting makefiles fail to build as they do not include 'includes' in the list of include directories. This was tested with current master branch (c196e9ea).

The one test in Tests/IncludeDirectories just tests the one new case which does work and should -- that setting INCLUDE_DIRECTORIES with an absolute path works. There are a few places where the directory version is used or tested, but it only appears to be testing that it parsed ok, not that it actually worked. It seems like there should be at least one test like Tests/IncludeDirectories/TargetIncludeDirectories called DirectoryIncludeDirectories.

If these are supposed to be relative to the build directory, then it would be useful to document that, though it certainly seems like they should be relative to the source directory. I suppose there should be tests for this as well.
TagsNo tags attached.
Relationships
Attached Files
Issue History
Date ModifiedUsernameFieldChange
2012-05-04 01:04bungemanNew Issue
2012-05-05 16:58Stephen KellyNote Added: 0029403
2012-05-07 09:11Brad KingAssigned To => Stephen Kelly
2012-05-07 09:11Brad KingStatusnew => assigned
2012-05-07 09:17Brad KingNote Added: 0029414
2012-05-17 14:03David ColeNote Added: 0029502
2012-05-21 13:09Stephen KellyNote Added: 0029530
2012-05-21 13:09Stephen KellyStatusassigned => resolved
2012-05-21 13:09Stephen KellyResolutionopen => fixed
2012-05-24 13:14David ColeNote Added: 0029559
2012-05-24 13:14David ColeAssigned ToStephen Kelly => David Cole
2012-05-24 13:14David ColeStatusresolved => assigned
2012-11-21 14:57David ColeNote Added: 0031660
2012-11-21 14:59David ColeAssigned ToDavid Cole =>
2012-11-21 15:11David ColeStatusassigned => new
2012-11-21 15:11David ColeNote Added: 0031669
2013-11-02 09:40Stephen KellyNote Added: 0034317
2013-11-02 09:40Stephen KellyStatusnew => resolved
2013-11-02 09:40Stephen KellyFixed in Version => CMake 3.0
2013-11-02 09:40Stephen KellyAssigned To => Stephen Kelly
2014-06-02 08:37Robert MaynardNote Added: 0035983
2014-06-02 08:37Robert MaynardStatusresolved => closed

Notes
(0029403)
Stephen Kelly   
2012-05-05 16:58   
I staged a fix for this issue. Thanks for trying it and reporting.

Note that my fix only affects the calls to change the target property, but not the directory property calls in your example.

I'm not sure if treating relative directories in target properties as being relative to the source dir is a break from convention. I couldn't find any comparable example, and if your example is correct regarding the directory properties (I didn't try it), that would be a counter-example.
(0029414)
Brad King   
2012-05-07 09:17   
The include_directories command has treated relative paths with respect to the source tree since at least CMake 2.6.0:

  http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=21089bf9 [^]

However, the command is able to do the translation because it knows where it is called. If one adds a relative path to the directory property and it is later copied into a subdirectory and used to initialize a target there then the base of the relative path will be different.
(0029502)
David Cole   
2012-05-17 14:03   
The problem with "relative" directory specifications is always the same: "relative" to what? In the case of an include_directories call, there is a clearly defined CMAKE_CURRENT_SOURCE_DIR for a relative base directory.

In the case of the INCLUDE_DIRECTORIES property, there is no such clearly defined variable that is constant across all accesses of the property. So... if somebody appends a relative directory in one CMAKE_CURRENT_SOURCE_DIR, and then later on, somebody else appends to the same target property with a different CMAKE_CURRENT_SOURCE_DIR, then it becomes ambiguous.

For this reason, I would say that we ought to insist on using fully specified absolute paths for the INCLUDE_DIRECTORIES properly, document it as such, and warn or perhaps even error on using a relative path as one of its entries.

I think the presently staged change should be reverted.

If you want the behavior of include_directories, use it. If you want to use the INCLUDE_DIRECTORIES property, use full absolute paths.
(0029530)
Stephen Kelly   
2012-05-21 13:09   
Ok. I think it's better to use full paths too really.

I've reverted and removed the branch.
(0029559)
David Cole   
2012-05-24 13:14   
to resolve this we should: add some documentation, possibly add a warning/error if non-full paths are detected
(0031660)
David Cole   
2012-11-21 14:57   
Un-assigning bugs that are not on the active roadmap, which no developers are actively working on for the CMake 2.8.11 release.

If one gets put back on the roadmap, re-assign it appropriately at that time.
(0031669)
David Cole   
2012-11-21 15:11   
Re-setting status back to "new" for bugs that are "assigned" but not assigned to a specific developer. When/if these bugs go back on the roadmap for a specific version, assignment to an appropriate developer should take place then...
(0034317)
Stephen Kelly   
2013-11-02 09:40   
8c6363a62f8ea61c6a572e4ffd08ce3e970f47b5
(0035983)
Robert Maynard   
2014-06-02 08:37   
Closing resolved issues that have not been updated in more than 4 months.