View Issue Details Jump to Notes ] Print ]
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.
Attached Files

 Relationships

  Notes
(0029403)
Stephen Kelly (developer)
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 (manager)
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 (manager)
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 (developer)
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 (manager)
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 (manager)
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 (manager)
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 (developer)
2013-11-02 09:40

8c6363a62f8ea61c6a572e4ffd08ce3e970f47b5
(0035983)
Robert Maynard (manager)
2014-06-02 08:37

Closing resolved issues that have not been updated in more than 4 months.

 Issue History
Date Modified Username Field Change
2012-05-04 01:04 bungeman New Issue
2012-05-05 16:58 Stephen Kelly Note Added: 0029403
2012-05-07 09:11 Brad King Assigned To => Stephen Kelly
2012-05-07 09:11 Brad King Status new => assigned
2012-05-07 09:17 Brad King Note Added: 0029414
2012-05-17 14:03 David Cole Note Added: 0029502
2012-05-21 13:09 Stephen Kelly Note Added: 0029530
2012-05-21 13:09 Stephen Kelly Status assigned => resolved
2012-05-21 13:09 Stephen Kelly Resolution open => fixed
2012-05-24 13:14 David Cole Note Added: 0029559
2012-05-24 13:14 David Cole Assigned To Stephen Kelly => David Cole
2012-05-24 13:14 David Cole Status resolved => assigned
2012-11-21 14:57 David Cole Note Added: 0031660
2012-11-21 14:59 David Cole Assigned To David Cole =>
2012-11-21 15:11 David Cole Status assigned => new
2012-11-21 15:11 David Cole Note Added: 0031669
2013-11-02 09:40 Stephen Kelly Note Added: 0034317
2013-11-02 09:40 Stephen Kelly Status new => resolved
2013-11-02 09:40 Stephen Kelly Fixed in Version => CMake 3.0
2013-11-02 09:40 Stephen Kelly Assigned To => Stephen Kelly
2014-06-02 08:37 Robert Maynard Note Added: 0035983
2014-06-02 08:37 Robert Maynard Status resolved => closed


Copyright © 2000 - 2018 MantisBT Team