Ben, Did you have a chance to review my patches?
Cheers, Nicolas On Sunday, February 14, 2016, Nicolas Desprès <nicolas.desp...@gmail.com> wrote: > 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 > <javascript:_e(%7B%7D,'cvml','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 > -- 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
