Re: [cmake-developers] Using CMake generated ninja file as a subninja file

2016-05-13 Thread Brad King 
On 05/13/2016 03:25 PM, Brad King wrote:
> so for now please make your logic recognize this case and work around it.

Please split this into two commits to move hunks like this:

> +  this->TargetAll = this->NinjaOutputPath("all");
> +  this->CMakeCacheFile = this->NinjaOutputPath("CMakeCache.txt");

into a preceding commit that performs refactoring with no functional change.

Also, in hunks like this:

> -  std::string convPath = ng->Convert(path, cmOutputConverter::HOME_OUTPUT);
> +  std::string convPath = ng->Convert(path, cmOutputConverter::FULL, format);

please revise the logic so that *nothing changes* from the old logic
when no subninja prefix is set.  If this is going to break anything I'd
like it to be isolated to when this feature is used.  Consolidation of
the two code paths can be done later when we've seen the new feature in
use for a while and gained confidence in it.

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] Using CMake generated ninja file as a subninja file

2016-05-13 Thread Brad King 
On 05/13/2016 12:13 PM, Nicolas Desprès wrote:
> with a value that looks like this: 
> "-Wl,-bundle_loader,/Users/polrop/Documents/cmake/_build 
> ninja/Tests/ExportImport/Export/testExe2"
> 
> lie on the caller site because the "if (li->IsPath)" in OutputLinkLibraries()
> should not be true for such a value.

The problem is that "IsPath" really means "quote this like a path on the 
command line".
The value above *does* need such quoting.  We cannot make the !IsPath code path
do the quoting because it is not expected to do so.  This is kind of a corner 
case
where the link item is a flag that contains a path that needs to be treated as a
path.  Major refactoring would be needed to encode that information structurally
so for now please make your logic recognize this case and work around it.

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

[cmake-developers] [PATCH] FindBoost: Boost 1.61

2016-05-13 Thread Roger Leigh 
Boost 1.61 was released today.  My boost-1.61 branch, merged into next, 
adds the updated dependency information for this release.  The version 
information was already added, so is unchanged.


Regards,
Roger
--

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] FindPkgConfig: optionally create imported target for the found libraries

2016-05-13 Thread Rolf Eike Beer 
Am Freitag, 13. Mai 2016, 13:38:52 schrieb Brad King:
> On 05/13/2016 12:21 PM, Rolf Eike Beer wrote:
> >> Can you just use
> >> 
> >>   set_property(TARGET PkgConfig::${_prefix} PROPERTY ...)
> >> 
> >> in each `if()` block?
> > 
> > I had this before, but I thought I avoid multiple target lookups. You
> > should only ever see that message if none of the branches is taken I
> > think. And that should never happen, or the if before is wrong. Can you
> > add a message() and show what is actually in _props there?
> 
> The problem is that _libs has more than one value, so set_target_properties
> ends up with
> 
>   PROPERTIES PROP1 v1 PROP2 v2a v2b PROP3 v3
> ^^^
> 
> Please use set_property(TARGET).  It is much more robust, and this is not
> exactly a performance-critical loop.

Hopefully all better now.

Eike

signature.asc
Description: This is a digitally signed message part.
-- 

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] FindBoost does not detect absence of header file

2016-05-13 Thread Roger Leigh 

On 13/05/2016 14:36, Brad King wrote:

On 05/12/2016 03:49 PM, Roger Leigh wrote:

I have made the suggested changes above where this was possible, and
merged the boost-component-headers branch into next for testing.


Thanks.  It looks pretty good, but there is one problem:


+find_path(Boost_${UPPERCOMPONENT}_HEADER
+  NAMES ${Boost_${UPPERCOMPONENT}_HEADER_NAME}
+  HINTS ${Boost_INCLUDE_DIR}


This leaves a bunch of Boost_${UPPERCOMPONENT}_HEADER values in
the cache.  The names look public, and they are publicly visible.
We should not expose this implementation detail.

Also, the find_path() approach means it might find the header somewhere
other than Boost_INCLUDE_DIR.  If it is not there, we cannot be confident
that it will match the library found.

Can the check use just if(EXISTS) instead?


Dear Brad,

Thanks for looking over this.  We could definitely use if(EXISTS).  I've 
pushed a change which does this.



Regards,
Roger
--

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] FindPkgConfig: optionally create imported target for the found libraries

2016-05-13 Thread Brad King 
On 05/13/2016 12:21 PM, Rolf Eike Beer wrote:
>> Can you just use
>>
>>   set_property(TARGET PkgConfig::${_prefix} PROPERTY ...)
>>
>> in each `if()` block?
> 
> I had this before, but I thought I avoid multiple target lookups. You should
> only ever see that message if none of the branches is taken I think. And that 
> should never happen, or the if before is wrong. Can you add a message() and 
> show what is actually in _props there?

The problem is that _libs has more than one value, so set_target_properties
ends up with

  PROPERTIES PROP1 v1 PROP2 v2a v2b PROP3 v3
^^^

Please use set_property(TARGET).  It is much more robust, and this is not
exactly a performance-critical loop.

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] [Patch] FindPkgConfig: optionally create imported target for the found libraries

2016-05-13 Thread Rolf Eike Beer 
Am Freitag, 13. Mai 2016, 11:16:28 schrieb Brad King:
> On 05/13/2016 11:02 AM, Rolf Eike Beer wrote:
> > Should all be done now.
> 
> Thanks.
> 
> The test fails for me with
> 
> CMake Error at /.../Modules/FindPkgConfig.cmake:239
> (set_target_properties): set_target_properties called with incorrect number
> of arguments.
> At least part of the problem is here:
> > +if(${_prefix}_INCLUDE_DIRS)
> > +  list(APPEND _props INTERFACE_INCLUDE_DIRECTORIES
> > "${${_prefix}_INCLUDE_DIRS}") +endif()
> > +if(_libs)
> > +  list(APPEND _props INTERFACE_LINK_LIBRARIES "${_libs}")
> > +endif()
> > +if(${_prefix}_CFLAGS_OTHER)
> > +  list(APPEND _props INTERFACE_COMPILE_OPTIONS
> > "${${_prefix}_CFLAGS_OTHER}") +endif()
> > +set_target_properties(PkgConfig::${_prefix} PROPERTIES ${_props})
> 
> If any of the property values is empty the `${_props}` will
> remove it from the argument list to set_target_properties.
> Can you just use
> 
>   set_property(TARGET PkgConfig::${_prefix} PROPERTY ...)
> 
> in each `if()` block?

I had this before, but I thought I avoid multiple target lookups. You should 
only ever see that message if none of the branches is taken I think. And that 
should never happen, or the if before is wrong. Can you add a message() and 
show what is actually in _props there?

Greetings,

Eike

signature.asc
Description: This is a digitally signed message part.
-- 

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] Using CMake generated ninja file as a subninja file

2016-05-13 Thread Nicolas Desprès 
On Thu, May 12, 2016 at 10:07 PM, Brad King  wrote:

> On 05/12/2016 02:16 PM, Nicolas Desprès wrote:
> > All done. Thank you for taking a look.
> > https://github.com/nicolasdespres/CMake/commits/ninja-output-path-prefix
>
> The RunCMake.NinjaOutputPathPrefix test fails for me when I have
> the CMake source and build trees in a directory with a space in
> the path.  Also, please move these test cases over to the
> Tests/RunCMake/Ninja directory.  You should be able to just append
> the RunCMakeTest.cmake content.
>
>
I have fixed the bug when a space is in the path and I added a test case
when a space is in the contents of the CMAKE_NINJA_OUTPUT_PATH_PREFIX
variable.

I have 2 failing tests left that are not failing on master: Plugin and
ExportImport.

Both suffers from the same problem which comes from this part of my patch:
https://github.com/nicolasdespres/CMake/commit/1f880c04bcb8cf36cf40be7fa4ef65f9525ea63e#diff-80cd058f986b2b3d5cdafc48a091411eL134

cmLocalNinjaGenerator::ConvertToLinkReference gets called by
cmLocalGenerator::OutputLinkLibraries from here:
https://github.com/nicolasdespres/CMake/blob/1f880c04bcb8cf36cf40be7fa4ef65f9525ea63e/Source/cmLocalGenerator.cxx#L1672
with a value that looks like this:
"-Wl,-bundle_loader,/Users/polrop/Documents/cmake/_build
ninja/Tests/ExportImport/Export/testExe2"

The previous implementation of ConvertToLinkReference returned the string
unchanged because it think it is a relative path, hiding the bug that lie
on the caller site because the "if (li->IsPath)" in OutputLinkLibraries()
should not be true for such a value. I don't know how to fix this.

Cheers,

-- 
Nicolas Desprès
-- 

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] FindPkgConfig: optionally create imported target for the found libraries

2016-05-13 Thread Brad King 
On 05/13/2016 11:02 AM, Rolf Eike Beer wrote:
> Should all be done now.

Thanks.

The test fails for me with

CMake Error at /.../Modules/FindPkgConfig.cmake:239 (set_target_properties):
  set_target_properties called with incorrect number of arguments.

At least part of the problem is here:

> +if(${_prefix}_INCLUDE_DIRS)
> +  list(APPEND _props INTERFACE_INCLUDE_DIRECTORIES 
> "${${_prefix}_INCLUDE_DIRS}")
> +endif()
> +if(_libs)
> +  list(APPEND _props INTERFACE_LINK_LIBRARIES "${_libs}")
> +endif()
> +if(${_prefix}_CFLAGS_OTHER)
> +  list(APPEND _props INTERFACE_COMPILE_OPTIONS 
> "${${_prefix}_CFLAGS_OTHER}")
> +endif()
> +set_target_properties(PkgConfig::${_prefix} PROPERTIES ${_props})

If any of the property values is empty the `${_props}` will
remove it from the argument list to set_target_properties.
Can you just use

  set_property(TARGET PkgConfig::${_prefix} PROPERTY ...)

in each `if()` block?

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] [Patch] FindPkgConfig: optionally create imported target for the found libraries

2016-05-13 Thread Rolf Eike Beer 
Am Donnerstag, 12. Mai 2016, 13:17:42 schrieb Brad King:
> On 05/12/2016 12:48 PM, Rolf Eike Beer wrote:
> > Good point. I have changed this accordingly and pushed it to next. There
> > are no tests yet, so maybe not immediately merge it to next but wait for
> > the weekend or so.
> 
> Okay, thanks.  Please also add a Help/release/dev/FindPkgConfig-targets.rst
> file with a release note for the feature.

Should all be done now.

Greetings,

Eike

signature.asc
Description: This is a digitally signed message part.
-- 

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] FindBoost does not detect absence of header file

2016-05-13 Thread Brad King 
On 05/12/2016 03:49 PM, Roger Leigh wrote:
> I have made the suggested changes above where this was possible, and 
> merged the boost-component-headers branch into next for testing.

Thanks.  It looks pretty good, but there is one problem:

> +find_path(Boost_${UPPERCOMPONENT}_HEADER
> +  NAMES ${Boost_${UPPERCOMPONENT}_HEADER_NAME}
> +  HINTS ${Boost_INCLUDE_DIR}

This leaves a bunch of Boost_${UPPERCOMPONENT}_HEADER values in
the cache.  The names look public, and they are publicly visible.
We should not expose this implementation detail.

Also, the find_path() approach means it might find the header somewhere
other than Boost_INCLUDE_DIR.  If it is not there, we cannot be confident
that it will match the library found.

Can the check use just if(EXISTS) instead?

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

[cmake-developers] [CMake 0016103]: AUTORCC will not regenerate qrc when a resource changes

2016-05-13 Thread Mantis Bug Tracker 

The following issue has been SUBMITTED. 
== 
https://cmake.org/Bug/view.php?id=16103 
== 
Reported By:OregonGhost
Assigned To:
== 
Project:CMake
Issue ID:   16103
Category:   CMake
Reproducibility:always
Severity:   minor
Priority:   normal
Status: new
== 
Date Submitted: 2016-05-13 06:36 EDT
Last Modified:  2016-05-13 06:36 EDT
== 
Summary:AUTORCC will not regenerate qrc when a resource
changes
Description: 
I am using CMake with Visual C++ (2010 & 2013) and Qt 5.4. My primary target has
a qrc file listed as source. AUTOMOC, AUTORCC and AUTOUIC are ON.

When the qrc file itself is changed, rcc is called.
When a resource the qrc file links to is changed, rcc is not called.

As a result, CMake happily builds an executable with old resources, unless the
qrc has been changed.

According to https://cmake.org/Bug/view.php?id=15074, this should work for some
time now. It did not for me with CMake 3.2, which I was using until recently,
nor with 3.5.2, which I have currently installed.

Steps to Reproduce: 
Relevant portions of CMakeLists.txt:

set(CMAKE_AUTORCC ON)
...
find_package(Qt5Core REQUIRED)
...
set( my_sources ... my.qrc )
...
add_executable( my WIN32 ... ${my_sources} )

Example from my.qrc:



Resources/Tags/BulletBlue.png



=> Change BulletBlue.png and qrc_my.cpp is NOT regenerated, nor recompiled.

Additional Information: 
Running on Windows 10, currently with Visual C++ 2013, but the problem also
happens on Windows 7 and with Visual C++ 2010.
== 

Issue History 
Date ModifiedUsername   FieldChange   
== 
2016-05-13 06:36 OregonGhostNew Issue
==

-- 

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