Re: [cmake-developers] Generator expressions for install destination

2015-09-22 Thread Stephen Kelly
Brad King wrote:

> On 09/22/2015 09:58 AM, Robert Goulet wrote:
>> Patch attached for adding makefile to install generators.
>> This refactoring is required for install(FILES) genex support,
>> and most likely other install() signatures in the future.
> 
> Thanks.  I applied that and merged to 'next' for testing:
> 
>  cmInstallGenerator: Add Makefile member
>  http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=d44cb327
> 
> I also extended the topic with some other refactoring it enables:
> 
>  cmInstallFilesGenerator: Drop LocalGenerator member
>  http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=e4b1728c
> 
>  cmInstallTargetGenerator: Simplify using Makefile member
>  http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=3f6b267d

This is going in the wrong direction. See 

 (Merge topic 'generators-use-cmLocalGenerator', 2015-08-24)
 http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=9135e370

Generator expressions are evaluated at generation-time, but with 
configuration-time types (cmMakefile and cmTarget). They should instead use 
generation-time types (cmLocalGenerator and cmGeneratorTarget). cmMakefile 
currently stores cmGeneratorTargets but it should not know them at all. This 
is a layering violation.

See the fix-layering-violation branch in my clone which implements that and 
also contains other orthogonal and wip refactorings for my convenience (I 
have been cherry-picking commits from this into branches to merge to next):

 https://github.com/steveire/CMake/commits/fix-layering-violation

None of that refactoring can proceed until after CMake 3.4 is released:

 
http://thread.gmane.org/gmane.comp.programming.tools.cmake.devel/14286/focus=14323

The patch from Robert should not undo that effort, so the branch should be 
reverted from next and not merged to master.

Thanks,

Steve.



-- 

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


Re: [cmake-developers] Generator expressions for install destination

2015-09-22 Thread Brad King
On 09/22/2015 09:58 AM, Robert Goulet wrote:
> Patch attached for adding makefile to install generators.
> This refactoring is required for install(FILES) genex support,
> and most likely other install() signatures in the future.

Thanks.  I applied that and merged to 'next' for testing:

 cmInstallGenerator: Add Makefile member
 http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=d44cb327

I also extended the topic with some other refactoring it enables:

 cmInstallFilesGenerator: Drop LocalGenerator member
 http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=e4b1728c

 cmInstallTargetGenerator: Simplify using Makefile member
 http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=3f6b267d

Steve, please check whether this runs afoul of your cmState
refactoring design.

Thanks,
-Brad

-- 

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


Re: [cmake-developers] Generator expressions for install destination

2015-09-22 Thread Robert Goulet
Thanks, once this is accepted in master I will send you my updated 
install(FILES) with genex support. I removed the GetDestination() signature 
since I agree it's not needed and might be confusing.

-Original Message-
From: Brad King [mailto:brad.k...@kitware.com] 
Sent: Tuesday, September 22, 2015 1:04 PM
To: Robert Goulet 
Cc: cmake-developers@cmake.org; Stephen Kelly 
Subject: Re: Generator expressions for install destination

On 09/22/2015 09:58 AM, Robert Goulet wrote:
> Patch attached for adding makefile to install generators.
> This refactoring is required for install(FILES) genex support, and 
> most likely other install() signatures in the future.

Thanks.  I applied that and merged to 'next' for testing:

 cmInstallGenerator: Add Makefile member
 http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=d44cb327

I also extended the topic with some other refactoring it enables:

 cmInstallFilesGenerator: Drop LocalGenerator member  
http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=e4b1728c

 cmInstallTargetGenerator: Simplify using Makefile member  
http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=3f6b267d

Steve, please check whether this runs afoul of your cmState refactoring design.

Thanks,
-Brad

-- 

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


Re: [cmake-developers] Regression caused by compute-default-dialect topic

2015-09-22 Thread Stephen Kelly
Brad King wrote:

> On 09/16/2015 03:14 PM, Brad King wrote:
>> That eliminates my concern.
> 
> This is now in 'master' as:
> 
>  Project: Determine default language dialect for the compiler.
>  http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=7235334a
> 
> However, I just discovered that it breaks use of toolchain files
> that force a compiler and set the version:
> 
>  $ cat ~/toolchain.cmake
>  include(CMakeForceCompiler)
>  CMAKE_FORCE_C_COMPILER(/usr/bin/gcc GNU)
>  set(CMAKE_C_COMPILER_VERSION 5.2)

I've pushed fix-forced-toolchain-dialect to fix this. It uses an existing 
mechanism already in use to determine whether the compiler was forced.

> This is because the language standard default is computed while
> determining the compiler id, but that is skipped when using a toolchain
> file that forces the compiler.  

Is there any legitimate need to force the compiler? 

For example, why does the android toolchain file force GNU and Clang 
compilers? Is it misunderstanding on the part of the toolchain author? Or 
are there really Clang and GNU releases which can not be executed by CMake 
as usual? In particular, are those GNU and Clang releases part of the 
Android NDK?

Ie: Should CmakeForceCompiler.cmake be deprecated? It seems to date from a 
time when the compiler id and whether it 'basically works' were the only 
things CMake wished to know in order to initialize. Today there is much 
more, including compiler version, ABI and features. 

If the module should remain, then more complex macros should be added and 
maintained, in a design which accommodates future growth of reasons to run 
the compiler during initialization.

> I think we need to:
> 
> * Tolerate the lack of CMAKE__STANDARD_COMPUTED_DEFAULT and fall
>   back to our old lookup table.

As you note below, that would not be useful because no features are recorded 
for forced compilers.

> * Allow toolchain files to set CMAKE__STANDARD_DEFAULT themselves
>   just as they can set CMAKE__COMPILER_{ID,VERSION} now.

As above.

> This may also reveal a larger problem in that compiler features
> functionality does not work well with toolchain files that force
> the compiler.  Doing so skips the test for a working compiler,
> but that also skips ABI and Features detection:
> 
>  
http://www.cmake.org/gitweb?p=cmake.git;a=blob;f=Modules/CMakeTestCCompiler.cmake;hb=v3.3.2#l73
> 
> Asking toolchain files to set the ABI information was not too much,
> but the compiler feature detection produces a lot of information
> that toolchain files should not have to spell out.  

Yes, I think if the module should remain, then more complex macros should be 
added and maintained, in a design which accommodates future growth of 
reasons to run the compiler during initialization.

Thanks,

Steve.


-- 

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


Re: [cmake-developers] CMakeForceCompiler (was: Regression caused by compute-default-dialect topic)

2015-09-22 Thread Brad King
On 09/22/2015 04:29 PM, Stephen Kelly wrote:
> I've pushed fix-forced-toolchain-dialect to fix this. It uses an existing 
> mechanism already in use to determine whether the compiler was forced.

Looks good, thanks.

> Is there any legitimate need to force the compiler?

The CMakeForceCompiler module was added here:

 http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=fda7753f

The commit message mentions:

 ... embedded compilers need a user specific linker file for compiling
 an executable ... thus cannot build the compiler-id program...

Since then the compiler id logic has learned to do things like pass
the --version flag and match vendor-specific output.  That could be
used to identify these embedded compilers.  Then the test for working
compiler will still need a linker file but perhaps we can generate
one once we know the compiler id.

> For example, why does the android toolchain file force GNU and Clang 
> compilers? Is it misunderstanding on the part of the toolchain author?

I don't think the android.toolchain.cmake file needs to force the
compiler.  It can simply set CMAKE_{C,CXX}_COMPILER and let the
normal detection proceed.

> Ie: Should CmakeForceCompiler.cmake be deprecated? It seems to date from a 
> time when the compiler id and whether it 'basically works' were the only 
> things CMake wished to know in order to initialize. Today there is much 
> more, including compiler version, ABI and features. 

Yes, I think it should be deprecated if possible.  First we must
provide alternatives for all its use cases though.

-Brad

-- 

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


[cmake-developers] Help with diagnosing dashboard failure

2015-09-22 Thread Stephen Kelly

Hi, 

A few days ago I merged a commit which moves the construction of 
cmLocalGenerator objects.

 http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=a3aa333d

It fails on the RunCMake.try_compile test on several dashboards in the 
CMP0056 test at the end where the policy is set to NEW. 

I didn't see an obvious pattern of which dashboards fail. I am unable to 
reproduce the problem. I can not see any reason for the problem.

If you can reproduce the problem, please assist in debugging that. Simply 
cherry-pick the a3aa333d commit, build and run the RunCMake.try_compile 
CMP0056 test. If it fails, please investigate why it fails. For example:

1) Is the policy correctly determined to be NEW in cmCoreTryCompile.cxx?

2) Is the CMAKE_EXE_LINKER_FLAGS correctly generated in the CMakeLists 
generated for the try_compile?

3) etc

Thanks,

Steve.


-- 

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


Re: [cmake-developers] Generator expressions for install destination

2015-09-22 Thread Brad King
On 09/22/2015 04:35 PM, Robert Goulet wrote:
> Ok so that brings the question, how is 3.4 schedule looking?

The feature freeze will be Oct 1 shortly after which post-3.4 development
will open.  Steve will then start his post-3.4 refactoring topic merges
and then we can come back to this feature.

> We really wanted to get install(FILES) destination genex in CMake 3.4
> release version... this is a bit disappointing because we'll have to
> stick with our custom branch another round. Oh well.

Sorry about that but the timing just didn't work out.  At least your
genex support for OUTPUT_NAME and OUTPUT_DIRECTORY will be in 3.4.

Thanks for your patience,
-Brad

-- 

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


Re: [cmake-developers] Generator expressions for install destination

2015-09-22 Thread Brad King
On 09/22/2015 04:00 PM, Stephen Kelly wrote:
> This is going in the wrong direction.
>  (Merge topic 'generators-use-cmLocalGenerator', 2015-08-24)
>  http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=9135e370
[snip]
> The patch from Robert should not undo that effort, so the branch
> should be reverted from next and not merged to master.

Thanks, Steve.  I suspected this which is why I called your attention
to this review.  Reverted.

Robert, this feature is going to have to wait until post-3.4 development
opens so it can be based on Steve's refactoring work.  I suspect it will
be easier to implement after that refactoring anyway.  Sorry this trouble
was not identified earlier in review.

-Brad

-- 

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


Re: [cmake-developers] Generator expressions for install destination

2015-09-22 Thread Robert Goulet
Ok then perhaps the refactoring can wait but we'd still like to get 
install(FILES) destination genex support in 3.4 if possible, and genex 
evaluation requires the cmMakefile. How should we proceed then?

-Original Message-
From: cmake-developers [mailto:cmake-developers-boun...@cmake.org] On Behalf Of 
Stephen Kelly
Sent: Tuesday, September 22, 2015 4:00 PM
To: cmake-developers@cmake.org
Subject: Re: [cmake-developers] Generator expressions for install destination

Brad King wrote:

> On 09/22/2015 09:58 AM, Robert Goulet wrote:
>> Patch attached for adding makefile to install generators.
>> This refactoring is required for install(FILES) genex support, and 
>> most likely other install() signatures in the future.
> 
> Thanks.  I applied that and merged to 'next' for testing:
> 
>  cmInstallGenerator: Add Makefile member
>  http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=d44cb327
> 
> I also extended the topic with some other refactoring it enables:
> 
>  cmInstallFilesGenerator: Drop LocalGenerator member  
> http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=e4b1728c
> 
>  cmInstallTargetGenerator: Simplify using Makefile member  
> http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=3f6b267d

This is going in the wrong direction. See 

 (Merge topic 'generators-use-cmLocalGenerator', 2015-08-24)
 http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=9135e370

Generator expressions are evaluated at generation-time, but with 
configuration-time types (cmMakefile and cmTarget). They should instead use 
generation-time types (cmLocalGenerator and cmGeneratorTarget). cmMakefile 
currently stores cmGeneratorTargets but it should not know them at all. This is 
a layering violation.

See the fix-layering-violation branch in my clone which implements that and 
also contains other orthogonal and wip refactorings for my convenience (I have 
been cherry-picking commits from this into branches to merge to next):

 https://github.com/steveire/CMake/commits/fix-layering-violation

None of that refactoring can proceed until after CMake 3.4 is released:

 
http://thread.gmane.org/gmane.comp.programming.tools.cmake.devel/14286/focus=14323

The patch from Robert should not undo that effort, so the branch should be 
reverted from next and not merged to master.

Thanks,

Steve.



-- 

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

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


Re: [cmake-developers] Generator expressions for install destination

2015-09-22 Thread Robert Goulet
Yeah I guess it could look something like this (attachment). However I just 
realized that if I don't also get install(DIRECTORY) in 3.4 then this is not 
worth rushing it, since as you mention I would also need to write proper test 
cases.

Just let me know when I can start looking at this again (after your refactoring 
is done) and I'll try to do the proper work to make it work with all install 
types destinations (files, directory and export).

-Original Message-
From: cmake-developers [mailto:cmake-developers-boun...@cmake.org] On Behalf Of 
Stephen Kelly
Sent: Tuesday, September 22, 2015 4:53 PM
To: cmake-developers@cmake.org
Subject: Re: [cmake-developers] Generator expressions for install destination

Robert Goulet wrote:

> Ok then perhaps the refactoring can wait but we'd still like to get
> install(FILES) destination genex support in 3.4 if possible, and genex 
> evaluation requires the cmMakefile. How should we proceed then?

cmLocalGenerator has a GetMakefile method (currently). Don't you just need to 
use

 this->LocalGenerator->GetMakefile() 

in cmInstallFilesGenerator?

Whether Brad would still accept a patch like that for 3.4 I don't know, but I 
think it should be a relatively simple implementation? However, there may need 
to be more tests for the new functionality and error cases etc.

Thanks,

Steve.


-- 

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


install-files-dest-genex.patch
Description: install-files-dest-genex.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

Re: [cmake-developers] Generator expressions for install destination

2015-09-22 Thread Stephen Kelly
Brad King wrote:

> On 09/22/2015 04:00 PM, Stephen Kelly wrote:
>> This is going in the wrong direction.
>>  (Merge topic 'generators-use-cmLocalGenerator', 2015-08-24)
>>  http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=9135e370
> [snip]
>> The patch from Robert should not undo that effort, so the branch
>> should be reverted from next and not merged to master.
> 
> Thanks, Steve.  I suspected this which is why I called your attention
> to this review.  Reverted.
> 
> Robert, this feature is going to have to wait until post-3.4 development
> opens so it can be based on Steve's refactoring work.  I suspect it will
> be easier to implement after that refactoring anyway.  Sorry this trouble
> was not identified earlier in review.

Yes, sorry from me too! I haven't been following this thread until now.

Thanks,

Steve.


-- 

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


Re: [cmake-developers] Generator expressions for install destination

2015-09-22 Thread Robert Goulet
Ok so that brings the question, how is 3.4 schedule looking?

We really wanted to get install(FILES) destination genex in CMake 3.4 release 
version... this is a bit disappointing because we'll have to stick with our 
custom branch another round. Oh well.

Thanks.

-Original Message-
From: Brad King [mailto:brad.k...@kitware.com] 
Sent: Tuesday, September 22, 2015 4:27 PM
To: Robert Goulet 
Cc: cmake-developers@cmake.org
Subject: Re: [cmake-developers] Generator expressions for install destination

On 09/22/2015 04:00 PM, Stephen Kelly wrote:
> This is going in the wrong direction.
>  (Merge topic 'generators-use-cmLocalGenerator', 2015-08-24)
>  http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=9135e370
[snip]
> The patch from Robert should not undo that effort, so the branch 
> should be reverted from next and not merged to master.

Thanks, Steve.  I suspected this which is why I called your attention to this 
review.  Reverted.

Robert, this feature is going to have to wait until post-3.4 development opens 
so it can be based on Steve's refactoring work.  I suspect it will be easier to 
implement after that refactoring anyway.  Sorry this trouble was not identified 
earlier in review.

-Brad

-- 

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


Re: [cmake-developers] [CMake] Visual Studio - Ninja Generator

2015-09-22 Thread Stephen Kelly
James Johnston wrote:

>> > it would be useful to have Visual Studio available as an "Extra" CMake
>> > generator.  For example, specification of "Visual Studio 2015 - Ninja"
>> 
>> This functionality sounds reasonable but the name of the extra/generator
>> pair looks funny when spelled out that way.  We should consider having
>> another way to specify the extra generator.
> 
> This name is consistent with the other extra generators:

... however the design is not.

The 'extra generators' do their generation after the 'real' makefile or 
ninja generator.

Your proposal seems to be something different design-wise. From what I 
understand, you are proposing something which would need to do different 
things during the 'real' generate.

In terms of name though, it might not matter that IDE and non-IDE 'extra' 
generators are implemented in completely different ways.

> But I agree it is just confusing at this point.  Maybe this format could
> be deprecated in favor of a new cmake.exe switch to specify an extra
> generator.

Perhaps. It seems Xcode+Ninja could also be a valid pairing.

Thanks,

Steve.



-- 

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


Re: [cmake-developers] Generator expressions for install destination

2015-09-22 Thread Stephen Kelly
Robert Goulet wrote:

> Ok then perhaps the refactoring can wait but we'd still like to get
> install(FILES) destination genex support in 3.4 if possible, and genex
> evaluation requires the cmMakefile. How should we proceed then?

cmLocalGenerator has a GetMakefile method (currently). Don't you just need 
to use

 this->LocalGenerator->GetMakefile() 

in cmInstallFilesGenerator?

Whether Brad would still accept a patch like that for 3.4 I don't know, but 
I think it should be a relatively simple implementation? However, there may 
need to be more tests for the new functionality and error cases etc.

Thanks,

Steve.


-- 

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


Re: [cmake-developers] Generator expressions for install destination

2015-09-22 Thread Stephen Kelly
Robert Goulet wrote:

> Yeah I guess it could look something like this (attachment). 

Yes, something like that (modulo the long line).

> However I
> just realized that if I don't also get install(DIRECTORY) in 3.4 then this
> is not worth rushing it, since as you mention I would also need to write
> proper test cases.
> 
> Just let me know when I can start looking at this again (after your
> refactoring is done) and I'll try to do the proper work to make it work
> with all install types destinations (files, directory and export).

My refactoring won't affect the files and directory much, so you can proceed 
similar to that patch without conflicting. An appropriate branch like that 
could be committed before my refactoring, depending on when it is ready. 

My refactoring will only remove the `->GetMakefile()` part from affected 
lines, but it will take a while to get to the point where that can actually 
be done, so you don't need to wait for it.

Thanks,

Steve.


-- 

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


Re: [cmake-developers] [Patch] Adding Windows 10 support

2015-09-22 Thread Gilles Khouzam
Thanks for the feedback Brad,

I have rebased the changes and I think that I have the proper default 
functionality properly implemented. I've extracted the 
WINDOWS_TARGET_PLATFORM_VERSION changes into a separate patch.

WINDOWS_TARGET_PLATFORM_VERSION is a target property, for that will specify the 
WindowsTargetPlatformVersion tag in VS.
WINDOWS_TARGET_PLATFORM_MIN_VERSION is a target property that will specify the 
WindowsTargetPlatformMinVersion tag in VS.

Now, for the default behavior, if CMAKE_WINDOWS_TARGET_PLATFORM_VERSION is set 
through a toolchain file or the project, then that will be the default which 
will initialize the WINDOWS_TARGET_PLATFORM_VERSION for each target through the 
SetPropertyDefault initialization call. On the other hand, if 
CMAKE_WINDOWS_TARGET_PLATFORM_VERSION is not set, nothing should happen since 
this is not a required property other than for Windows 10 Universal (store) 
apps, the default behavior in that case should be to not have the property.

There is one open issue though. How should we have the value for 
CMAKE_WINDOWS_TARGET_PLATFORM_VERSION be the latest installed SDK when this is 
not a Windows Store project? For Windows Store projects this would get set if 
the property is not defined through the InitializeSystem procedure. How would 
we handle this for the non Windows Store case? Do this based on the version and 
no CMAKE_SYSTEM_NAME defined? Or should we force there to be a 
CMAKE_SYSTEM_NAME to be defined as Windows for example?


-Original Message-
From: Brad King [mailto:brad.k...@kitware.com] 
Sent: Monday, September 21, 2015 08:09
To: Gilles Khouzam 
Cc: cmake-developers@cmake.org
Subject: Re: [cmake-developers] [Patch] Adding Windows 10 support

On 09/10/2015 09:31 PM, Gilles Khouzam wrote:
> Here is the patch for Windows 10 Support which would be issue 15686.
[snip]
> This change requires the change for issue 0015674 for determining the 
> version of the OS.

That change is now done as posted here:

 [PATCH] [CMake 0015674]: Windows: Correctly determine Windows version
 
http://thread.gmane.org/gmane.comp.programming.tools.cmake.devel/14327/focus=14478

Please rebase the rest of your Win 10 patch on that and revise according to the 
comments below.

> New Properties Added:
> VS_TARGET_PLATFORM_VERSION: Target property. Specifies that the SDK 
> being used to compile the project. For Windows 10 RTM, that will be 
> 10.0.10240.0.
> For Store apps, this property is required by Visual Studio. If the 
> property is not specified, the system will be queried for the 
> available Windows 10 SDKs installed and the most recent but less than 
> or equal version than the host system version will be set as a default 
> (in
> CMAKE_VS_TARGET_PLATFORM_VERSION) and used.

Please split this part out into its own (preceding) patch, much like was done 
for the windows version detection.  Also, currently the patch does not 
implement [CMAKE_]VS_TARGET_PLATFORM_VERSION as previously discussed:

 
http://thread.gmane.org/gmane.comp.programming.tools.cmake.devel/14201/focus=14246

For example:

> +  if (this->SystemVersion == "10.0")
> +{
> +// Find the default version of the Windows 10 SDK and set
> +// a default CMAKE_VS_TARGET_PLATFORM_VERSION
> +std::string sdkVersion = GetWindows10SDKVersion();
> +if(sdkVersion.empty())
> +  {
> +  e << "Could not find an appropriate version of the Windows 10 SDK"
> +<< "installed on this machine";
> +  mf->IssueMessage(cmake::FATAL_ERROR, e.str());
> +  return false;
> +  }
> +mf->AddDefinition("CMAKE_VS_TARGET_PLATFORM_VERSION",
> +  sdkVersion.c_str());
> +}

Logic like this should be used to select a default SDK when neither the 
property nor the variable is set.  I think we've had some confusion between the 
user-settable target property/variable and the generator- reported selection 
result used by the compiler id.  The latter is used here in your patch:

> +set(id_WindowsTargetPlatformVersion 
> + "${CMAKE_VS_TARGET_PLATFORM_VERSION} + WindowsTargetPlatformVersion>")

Currently the CMAKE_VS_ convention is used by the VS generators to report 
information about what the've decided.  I think this makes sense so a variable 
called CMAKE_VS_WINDOWS_TARGET_PLATFORM_VERSION
could serve this purpose (reporting to project code what VS decided to use).

What we need separately is an interface for users and/or project code to select 
a specific WindowsTargetPlatformVersion.  The target property currently called 
VS_TARGET_PLATFORM_VERSION will work well for project code to set the value for 
specific targets.  We still need another setting for users or toolchain files 
to set to make it a project-wide default.  Perhaps we should have a separate 
variable called

  CMAKE_WINDOWS_TARGET_PLATFORM_VERSION

and rename the target property to just

  WINDOWS_TARGET_PLATFORM_VERSION

The property should be initialized via 

Re: [cmake-developers] [CPackDeb] empty directories in packages

2015-09-22 Thread Domen Vrankar
> I was looking at this issue
> http://public.kitware.com/Bug/view.php?id=13009
>
> and apparently it is not possible to install empty directories (I just
> tested).
>
> I believe that it should be possible to do that (even if there are better
> ways like postinst).
> What is your opinion?

I agree. This is also an issue on other packaging generators (e.g. tar gz)

> The attached patch addresses this (and adds the corresponding test). It is
> based on my previous patch
> "0001-CPackDeb-preventing-md5sum-on-symlinks.patch".

Thanks, applied to next for testing:
http://www.cmake.org/gitweb?p=cmake.git;a=commit;h=47b060

Your patch only fixed the issue for component packaging so I extended
it to non component packaging as well. This also fixes the issue for
other generators:
http://www.cmake.org/gitweb?p=cmake.git;a=commit;h=b58de9f

Other fixed bug reports:
http://public.kitware.com/Bug/view.php?id=8767 and
http://public.kitware.com/Bug/view.php?id=14978


Thanks,
Domen
-- 

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


Re: [cmake-developers] [CPackDeb] empty directories in packages

2015-09-22 Thread Rolf Eike Beer

Domen Vrankar wrote:

I was looking at this issue
http://public.kitware.com/Bug/view.php?id=13009

and apparently it is not possible to install empty directories (I just
tested).

I believe that it should be possible to do that (even if there are 
better

ways like postinst).
What is your opinion?


I agree. This is also an issue on other packaging generators (e.g. tar 
gz)


The attached patch addresses this (and adds the corresponding test). 
It is

based on my previous patch
"0001-CPackDeb-preventing-md5sum-on-symlinks.patch".


Thanks, applied to next for testing:
http://www.cmake.org/gitweb?p=cmake.git;a=commit;h=47b060

Your patch only fixed the issue for component packaging so I extended
it to non component packaging as well. This also fixes the issue for
other generators:
http://www.cmake.org/gitweb?p=cmake.git;a=commit;h=b58de9f


You left a commented out line in Source/CPack/cmCPackGenerator.cxx.

Greetings,

Eike
--

--

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


Re: [cmake-developers] [CPackDeb] empty directories in packages

2015-09-22 Thread Domen Vrankar
> You left a commented out line in Source/CPack/cmCPackGenerator.cxx.

Hm that should not be there. I'll delete it when I get to my PC.

Thanks,
Domen
-- 

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


[cmake-developers] Regression caused by compute-default-dialect topic

2015-09-22 Thread Brad King
On 09/16/2015 03:14 PM, Brad King wrote:
> That eliminates my concern.

This is now in 'master' as:

 Project: Determine default language dialect for the compiler.
 http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=7235334a

However, I just discovered that it breaks use of toolchain files
that force a compiler and set the version:

 $ cat ~/toolchain.cmake
 include(CMakeForceCompiler)
 CMAKE_FORCE_C_COMPILER(/usr/bin/gcc GNU)
 set(CMAKE_C_COMPILER_VERSION 5.2)

 $ cmake -DCMAKE_TOOLCHAIN_FILE=~/toolchain.cmake ../CMake/Tests/COnly
 CMake Error at /.../share/cmake-3.3/Modules/Compiler/GNU-C.cmake:27 (message):
   CMAKE_C_STANDARD_COMPUTED_DEFAULT should be set for GNU (/usr/bin/gcc)
   version 5.2
 Call Stack (most recent call first):
   /.../share/cmake-3.3/Modules/CMakeCInformation.cmake:33 (include)
   CMakeLists.txt:3 (project)

This is because the language standard default is computed while
determining the compiler id, but that is skipped when using a toolchain
file that forces the compiler.  I think we need to:

* Tolerate the lack of CMAKE__STANDARD_COMPUTED_DEFAULT and fall
  back to our old lookup table.

* Allow toolchain files to set CMAKE__STANDARD_DEFAULT themselves
  just as they can set CMAKE__COMPILER_{ID,VERSION} now.

* Add a RunCMake test that uses incrementally more complex toolchain
  files like the above (ID only, ID+version, ID+version+standard).

---

This may also reveal a larger problem in that compiler features
functionality does not work well with toolchain files that force
the compiler.  Doing so skips the test for a working compiler,
but that also skips ABI and Features detection:

 
http://www.cmake.org/gitweb?p=cmake.git;a=blob;f=Modules/CMakeTestCCompiler.cmake;hb=v3.3.2#l73

Asking toolchain files to set the ABI information was not too much,
but the compiler feature detection produces a lot of information
that toolchain files should not have to spell out.  We may need to
consider moving the ABI and Feature checks to somewhere that occurs
even when forcing the compiler with a toolchain file.  That is beyond
the scope of the immediate need for a fix to the above error though.

Thanks,
-Brad

-- 

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


Re: [cmake-developers] Add command line options for deprecation message control

2015-09-22 Thread Brad King
On 09/21/2015 05:51 PM, Michael Scott wrote:
> Yes the -Werr-dev, -Wno-err-dev, -Werr-deprectated and 
> -Wno-err-deprecated may be trickier than expected to get behaving as 
> intended. I'll try and get a better idea of the users of IssueMessage 
> and see if some ideas come to mind. Removing them would be fair enough 
> at this point, better to not include them than to include broken 
> functionality I'm sure. Do new features, such as -Werr-dev, have to be 
> sent by October 1st is it?

Oct 1 is the feature freeze in 'master' for CMake 3.4, though the
scale/scope of new features should be going down as we approach
that date.  In this case we have a bug in a new feature that was
introduced in post-3.3 development so we need to either fix it or
remove the offending parts of the new features before Oct 1 for 3.4.

Thanks,
-Brad

-- 

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


Re: [cmake-developers] Generator expressions for install destination

2015-09-22 Thread Robert Goulet
> This and its related changes are also refactoring that should go in its own 
> commit.

Ok let's begin with this. Patch attached for adding makefile to install 
generators. This refactoring is required for install(FILES) genex support, and 
most likely other install() signatures in the future.

Thanks.

-Original Message-
From: Brad King [mailto:brad.k...@kitware.com] 
Sent: Monday, September 21, 2015 1:55 PM
To: Robert Goulet 
Cc: cmake-developers@cmake.org
Subject: Re: Generator expressions for install destination

On 09/18/2015 03:49 PM, Robert Goulet wrote:
> Here's a version that is more conservative. It doesn't change the 
> install(EXPORT) behavior.

> install(TARGET) already supported genex

Right.  I'd forgotten about these changes:

 install: Allow generator expressions in TARGETS DESTINATION (#14317)  
http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=f30022eb
 http://www.cmake.org/Bug/view.php?id=14317#c37959

>, so basically this patch adds install(FILES) destination genex.

Okay.  I see many hunks of this form:

> -   this->Destination,
> +   this->GetDestination(),

Making the Destination member private and moving GetDestination() back into 
cmInstallGenerator is refactoring that should be split out into its own commit. 
 Note that GetDestination() was removed from the base class here:

 cmInstallGenerator: Move GetDestination to subclasses that need it  
http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=f1db

Restoring it will need a justification in the commit message.
I'm concerned about having both overloads avilable:

> +  std::string GetDestination() const;  std::string 
> + GetDestination(std::string const& config) const;

when only one is safe to call from each subclass.  This is why the above-linked 
commit removed the method from cmInstallGenerator in favor of having the needed 
overload in each subclass.

> -  cmInstallGenerator(const char* destination,
> +  cmInstallGenerator(cmMakefile* mf,
> + const char* destination,

This and its related changes are also refactoring that should go in its own 
commit.

> +  // We need per-config actions if destination have generator expressions.
> +  if(cmGeneratorExpression::Find(Destination) != std::string::npos)
> +{
> +this->ActionsPerConfig = true;
> +}

Was this the solution to the availability of a configuration for calls to 
GetDestination()?

> Perhaps we should update the Help to only mention install(FILES) 
> destination instead of all variations of install?

Yes, the actual availability of the behavior should be documented.

Also please look at making the test suite actually verify that the installation 
works as expected.  The above-linked commit f30022eb adds a test that fails if 
generator expressions are not evaluated to the correct values.

Thanks,
-Brad



add-makefile-to-install-generators.patch
Description: add-makefile-to-install-generators.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