On Fri, May 13, 2016 at 9:38 PM, Brad King <brad.k...@kitware.com> wrote:

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

Done


>
> 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.
>
>
Ok. See the new implementation of
cmGlobalNinjaGenerator::ConvertToNinjaPath() and
cmLocalNinjaGenerator::ConvertToLinkReference().

Now all the tests pass because the old logic is preserved when
CMAKE_NINJA_OUTPUT_PATH_PREFIX is empty. But the ExportImport and Plugin
tests are likely to fail when used with this new feature. I can't figure
out right now how to add a test for this without duplicating the Plugin and
ExportImport test into RunCMake/Ninja/.

https://github.com/nicolasdespres/CMake/commits/ninja-output-path-prefix

Thanks for your help,
-Nico
-- 

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