Thanks for the feedback. I made some changes and rebased on the trunk.

> The hunk in Modules/CMakeSystemSpecificInformation.cmake is now just removing 
> a blank line so it should not be needed.
Removed

>> +message("GHS-DetermineCompiler.cmake")
>
>This looks like a leftover debugging message.
Removed

>> there are a few changes to find boost libraries with this generator.
>[snip]
>> -if ( WIN32 AND Boost_USE_STATIC_LIBS AND NOT CYGWIN)
>> +if ("Windows" STREQUAL  ${CMAKE_HOST_SYSTEM_NAME} AND
>> +    Boost_USE_STATIC_LIBS AND NOT CYGWIN)
>
>Why does the host affect the library prefix for binaries meant for the target 
>platform?
I'm still unsure if the WIN32 cmake variables is the target, platform or both. 
I originally thought this check should be based on the host, but it should be 
based on the target platform. I added a special case for the GHS Multi platform.

>> Some of the code is C++11. I'm not sure if that is an issue.
>
> It is.  We build on compilers that barely support C++98.
I removed usages of nullptr and std::anyof with a lamda expression.

>> Also, it skips the step where it determines the compiler by using the
>> force compiler macro. I don't see any other cmake module using this
>> functionality.
>[snip]
>> +set(CMAKE_SYSTEM_NAME "GHS-MULTI")
>> +set(CMAKE_SYSTEM_PROCESSOR "ARM")
>...
>> +include(CMakeForceCompiler)
>> +CMAKE_FORCE_CXX_COMPILER("cxarm" "GhsMultiArmCXX")
>
>Those APIs are meant for use in a CMAKE_TOOLCHAIN_FILE.
>
>The cmGlobalGhsMultiGenerator::EnableLanguage method should be able to take 
>care of those.  This is the first time we've had a generator that locks to a 
>specific target >platform (and implicitly cross-compiles).
>
>Is CMAKE_SYSTEM_VERSION always hard-coded on a given system or should the user 
>be able to choose it in some way?
Platform/GHS-MULTI-C.cmake and Platform/GHS-MULTI-CXX.cmake have been moved 
into the enable language method.

The cmake system version can be changed with the GHS_COMP_ROOT cache variable. 
The version variable is not currently being used internally, since I've only 
tested it on what I think is the latest version. There was a bug in the 
previous use of find_program for CMAKE_MAKE_PROGRAM, where I thought the paths 
were interpreted as a regular expression.


Geoffrey Viola
SOFTWARE ENGINEER
T

+1.435.755.2980

ext 1077
M

+1.215.896.6521


asirobots.com




-----Original Message-----
From: Brad King [mailto:[email protected]]
Sent: Wednesday, November 05, 2014 12:37 PM
To: Geoffrey Viola
Cc: [email protected]
Subject: Re: [cmake-developers] Initial Attempt at Green Hill MULTI IDE 
Generator Support

On 11/05/2014 01:11 PM, Geoffrey Viola wrote:
> I rebased and squashed the previous commits and made some new changes.

Thanks.  Here are more comments.

The hunk in Modules/CMakeSystemSpecificInformation.cmake is now just removing a 
blank line so it should not be needed.

> +message("GHS-DetermineCompiler.cmake")

This looks like a leftover debugging message.

> there are a few changes to find boost libraries with this generator.
[snip]
> -if ( WIN32 AND Boost_USE_STATIC_LIBS AND NOT CYGWIN)
> +if ("Windows" STREQUAL  ${CMAKE_HOST_SYSTEM_NAME} AND
> +    Boost_USE_STATIC_LIBS AND NOT CYGWIN)

Why does the host affect the library prefix for binaries meant for the target 
platform?

> Some of the code is C++11. I'm not sure if that is an issue.

It is.  We build on compilers that barely support C++98.

> Also, it skips the step where it determines the compiler by using the
> force compiler macro. I don't see any other cmake module using this
> functionality.
[snip]
> +set(CMAKE_SYSTEM_NAME "GHS-MULTI")
> +set(CMAKE_SYSTEM_PROCESSOR "ARM")
...
> +include(CMakeForceCompiler)
> +CMAKE_FORCE_CXX_COMPILER("cxarm" "GhsMultiArmCXX")

Those APIs are meant for use in a CMAKE_TOOLCHAIN_FILE.

The cmGlobalGhsMultiGenerator::EnableLanguage method should be able to take 
care of those.  This is the first time we've had a generator that locks to a 
specific target platform (and implicitly cross-compiles).

Is CMAKE_SYSTEM_VERSION always hard-coded on a given system or should the user 
be able to choose it in some way?

Thanks,
-Brad

This message contains confidential information and is intended only for the 
recipient. If you are not the named addressee you should not disseminate, 
distribute or copy this e-mail. Please notify the sender immediately if you 
have received this e-mail by mistake and delete this e-mail from your system. 
Finally, the recipient should check this email and any attachments for the 
presence of viruses. The company accepts no liability for any damage caused by 
any virus transmitted by this email.

Attachment: 0001-Added-some-support-for-a-Green-Hill-MULTI.patch
Description: 0001-Added-some-support-for-a-Green-Hill-MULTI.patch

-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

Reply via email to