[cmake-developers] target_include_directories branch in stage

David Cole david.cole at kitware.com
Fri Jan 27 11:39:50 EST 2012


On Mon, Jan 23, 2012 at 3:09 PM, Stephen Kelly <steveire at gmail.com> wrote:
> David Cole wrote:
>
>> On Fri, Jan 20, 2012 at 3:20 PM, Stephen Kelly
>> <steveire at 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.



More information about the cmake-developers mailing list