Ben, I have pushed a complete version of the patch here: https://github.com/nicolasdespres/CMake/commits/ninja-output-path-prefix
Since the last push, I have addressed the comments you made in your previous review (i.e. move some code to cmAlgorithm.h) and added a RunCMake.NinjaOutputPathPrefix tests. The 'default' statement is no longer generated when CMAKE_NINJA_OUTPUT_PATH_PREFIX is set because, unlike rules, "default" statement are not scoped. Thus, if the user does not include one in the top build.ninja only the default target of the subninja file was built. This was a surprising behaviour for me, since I expected to build all "leaf" output targets of the graph as usual when running ninja without argument. Let me know what you think about this change. Last note: some of the tests were failing on master (dec7d5c4de7870ec) on my machines (macbookpro and linux box), so I don't know if they are broken or not by my patch: 194 - CTestCoverageCollectGCOV (Failed) 393 - RunCMake.CommandLine (Failed) 401 - RunCMake.IfacePaths_INCLUDE_DIRECTORIES (Failed) Cheers, Nicolas On Fri, Jan 22, 2016 at 5:50 PM, Ben Boeckel <ben.boec...@kitware.com> wrote: > On Fri, Jan 22, 2016 at 17:34:03 +0100, Nicolas Desprès wrote: > > I have pushed a first draft of the patch here: > > > > > https://github.com/nicolasdespres/CMake/commit/67a4faad47378c07c97a29dd229d6f9fa500763b > > > > I have tested with a small project and it looks good. > > Looks like a good start to me as well. I added some comments on the > commit. > > > I would like to add some regression tests but I don't know from where to > > start. > > > > The principle would be to duplicate and modify existing tests so that > > CMAKE_NINJA_OUTPUT_PATH_PREFIX is set to "sub/" and the build directory > is > > changed from _build to _build/sub. A dummy build.ninja file containing > > "subninja sub/build.ninja" should be generated in _build/build.ninja and > > ninja should be called from _build instead of _build/sub. > > > > It would be great if you could point me some existing tests that could > > serve as a base to do the above. > > The RunCMake test infrastructure would probably be the best place to > start. It could be modified to do some of this work. > > Thanks, > > --Ben > -- 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
