[cmake-developers] target_include_directories branch in stage

[Stephen Kelly]

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.

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

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

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

>   - 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,

Steve.