Re: [cmake-developers] [PATCH] Use full path for all source files in ninja build.
On 07/29/2016 05:44 PM, Chaoren Lin via cmake-developers wrote: > From: Chaoren Lin > > Relative paths are difficult for an IDE to parse the output of > a build error. Thanks, applied: Ninja: Use full path for all source files https://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=d867f8a0 This is more consistent with the Makefile generator behavior too. -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] Use full path for all source files in ninja build.
On 08/02/2016 01:08 PM, Brad King wrote: > Ninja: Use full path for all source files > https://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=d867f8a0 I had to revert that because it causes the Qt*Autogen tests to fail. Also it may break builds on Windows. There are multiple problems: * ConvertToNinjaPath does more than just relative path conversions. It also converts slashes on Windows. Furthermore, it deals with CMAKE_NINJA_OUTPUT_PATH_PREFIX, though that won't matter for absolute paths. * Calls to AddAssumedSourceDependencies and HasCustomCommandOutput later in cmNinjaTargetGenerator::WriteObjectBuildStatement expect to be given paths that come out of ConvertToNinjaPath. * Ninja does not seem to be able to reconcile a dependency on an absolute path with a build statement providing that path by relative path. Custom commands that generate source files list their outputs with ConvertToNinjaPath. Basically everything assumes that all paths written as inputs or outputs of Ninja build statements have gone through the ConvertToNinjaPath method. The special case for "RC" in the existing code is likely buggy. Ninja always uses the location of the build.ninja file as the current working directory, so an IDE that sees a relative path in an error message can interpret it relative to that. -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] Use full path for all source files in ninja build.
> Ninja always uses the location of the build.ninja file as > the current working directory, so an IDE that sees a relative > path in an error message can interpret it relative to that. Our IDE has no knowledge of the build.gradle file let alone where it is located, it just sees the build output and tries to located the mentioned files. Similarly, the :make command in vim will have no knowledge of build.gradle, and only tries to parse the build errors directly. Would changing ConvertToNinjaPath to output absolute paths paths work? On Tue, Aug 2, 2016 at 10:38 AM, Brad King wrote: > On 08/02/2016 01:08 PM, Brad King wrote: > > Ninja: Use full path for all source files > > https://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=d867f8a0 > > I had to revert that because it causes the Qt*Autogen tests to > fail. Also it may break builds on Windows. > > There are multiple problems: > > * ConvertToNinjaPath does more than just relative path conversions. > It also converts slashes on Windows. Furthermore, it deals with > CMAKE_NINJA_OUTPUT_PATH_PREFIX, though that won't matter for > absolute paths. > > * Calls to AddAssumedSourceDependencies and HasCustomCommandOutput > later in cmNinjaTargetGenerator::WriteObjectBuildStatement expect > to be given paths that come out of ConvertToNinjaPath. > > * Ninja does not seem to be able to reconcile a dependency on > an absolute path with a build statement providing that path > by relative path. Custom commands that generate source files > list their outputs with ConvertToNinjaPath. > > Basically everything assumes that all paths written as inputs > or outputs of Ninja build statements have gone through the > ConvertToNinjaPath method. The special case for "RC" in the > existing code is likely buggy. > > Ninja always uses the location of the build.ninja file as > the current working directory, so an IDE that sees a relative > path in an error message can interpret it relative to that. > > -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] Use full path for all source files in ninja build.
On 08/02/2016 02:11 PM, Chaoren Lin wrote: > Would changing ConvertToNinjaPath to output absolute paths paths work? No, that will make all paths absolute. Ninja's design is pretty clear in that it prefers canonical relative paths when possible. Note that the conversion to a relative path by ConvertToNinjaPath is not unconditional. In an out-of-source build the paths to the source files in the source tree will be absolute. --- Compilers know the absolute path to all the files in a translation unit since the preprocessor was able to read them. They could be taught to produce absolute paths in their error messages to work better for IDEs regardless of how the build system invokes them. I've never really understood why they don't make error messages and debug info hold absolute paths. For some reason relative paths in these places are always blamed on the build system when in fact the compiler could easily produce them. -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] Use full path for all source files in ninja build.
On Tue, Aug 02, 2016 at 11:11:44 -0700, Chaoren Lin via cmake-developers wrote: > Similarly, the :make command in vim will have no knowledge of build.gradle, > and only tries to parse the build errors directly. FWIW, I haven't encountered any problems with Vim's :make with CMake's generated Ninja files in out-of-source builds for in-source or generated files (I almost never do in-source builds). --Ben -- 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] Use full path for all source files in ninja build.
The relative path is used whenever the build directory is under the source directory (e.g., under src/build/), so it could still be out of source. > FWIW, I haven't encountered any problems with Vim's :make with CMake's > generated Ninja files in out-of-source builds for in-source or generated > files (I almost never do in-source builds). Interesting. It appears that if you do :set makeprg=ninja\ -C\ /path/to/build | make then ninja will output this: > ninja: Entering directory `/path/to/build' and vim interprets that to be the working directory. However, if you use these: :set makeprg=cd\ /path/to/build\ &&\ ninja | make :set makeprg=cmake\ --build\ /path/to/build | make then the "Entering directory" line wouldn't appear, and vim won't know what to do. Our IDE uses "cmake --build /path/to/build" to invoke the build, so that we only have to deal with CMake and not the underlying build system. Compilers know the absolute path to all the files in a translation unit > since the preprocessor was able to read them. They could be taught to > produce absolute paths in their error messages to work better for IDEs > regardless of how the build system invokes them. I would love to see this too, but it's much easier to fix the problem in one place (CMake, or even just Ninja), than to fix that for every compiler. Maybe I can try to fix ninja to resolve paths before sending them to the compiler. Or maybe just add a flag to ConvertToNinjaPath or something. On Tue, Aug 2, 2016 at 11:33 AM, Ben Boeckel wrote: > On Tue, Aug 02, 2016 at 11:11:44 -0700, Chaoren Lin via cmake-developers > wrote: > > Similarly, the :make command in vim will have no knowledge of > build.gradle, > > and only tries to parse the build errors directly. > > FWIW, I haven't encountered any problems with Vim's :make with CMake's > generated Ninja files in out-of-source builds for in-source or generated > files (I almost never do in-source builds). > > --Ben > -- 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] Use full path for all source files in ninja build.
On 08/02/2016 04:42 PM, Chaoren Lin wrote: > Our IDE uses "cmake --build /path/to/build" to invoke the build, > so that we only have to deal with CMake and not the underlying build system. If you know /path/to/build why not just interpret relative paths with respect to it as a heuristic? If the paths exist then use them. > Maybe I can try to fix ninja to resolve paths before sending them to the > compiler. The ninja build tool itself does not have enough information to make such a transformation. CMake does though. See below. > Or maybe just add a flag to ConvertToNinjaPath or something. ConvertToNinjaPath is called from several places that don't all know whether the file they are passing is a source file for compilation or not. Another approach is to use ConvertToNinjaPath for all the build.ninja inputs and outputs on build statements but use something other than $in for the source file on the compilation rule. Using an "IN_ABS = ..." variable could achieve this. -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] Use full path for all source files in ninja build.
> If you know /path/to/build why not just interpret relative paths with > respect to it as a heuristic? If the paths exist then use them. It's... a bit complicated. Our IDE (Android Studio) calls another build system (Gradle) which in turn calls CMake to trigger the build. So the IDE itself doesn't see the build path, only the failure output. Also, in the case of vim, the makeprg is user-supplied, so vim has no idea that it contains a path. > Another approach is to use ConvertToNinjaPath for all the build.ninja > inputs and outputs on build statements but use something other than $in > for the source file on the compilation rule. Using an "IN_ABS = ..." > variable could achieve this. Thanks, I'll try this. On Wed, Aug 3, 2016 at 5:56 AM, Brad King wrote: > On 08/02/2016 04:42 PM, Chaoren Lin wrote: > > Our IDE uses "cmake --build /path/to/build" to invoke the build, > > so that we only have to deal with CMake and not the underlying build > system. > > If you know /path/to/build why not just interpret relative paths with > respect to it as a heuristic? If the paths exist then use them. > > > Maybe I can try to fix ninja to resolve paths before sending them to the > compiler. > > The ninja build tool itself does not have enough information to make > such a transformation. CMake does though. See below. > > > Or maybe just add a flag to ConvertToNinjaPath or something. > > ConvertToNinjaPath is called from several places that don't all know > whether the file they are passing is a source file for compilation > or not. > > Another approach is to use ConvertToNinjaPath for all the build.ninja > inputs and outputs on build statements but use something other than $in > for the source file on the compilation rule. Using an "IN_ABS = ..." > variable could achieve this. > > -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] Use full path for all source files in ninja build.
On Wed, Aug 03, 2016 at 11:11:45 -0700, Chaoren Lin wrote: > Also, in the case of vim, the makeprg is user-supplied, so vim has no idea > that it contains a path. What about instead of "cd path && ninja", using "ninja -C path"? --Ben -- 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] Use full path for all source files in ninja build.
> What about instead of "cd path && ninja", using "ninja -C path"? I suppose we can also change "cmake --build path", to use "ninja -C path" instead of "cd path && ninja", but making ninja always use full paths for source files would have these benefits: 1. consistency with make (as pointed out by Brad) 2. consistency regardless of where the build directory is located (in source, under source, or out of source) 3. all three ways of invoking the build would work for IDEs (cmake --build, cd && ninja, ninja -C), so future IDE writers won't run into the sample problem no matter which way they choose On Wed, Aug 3, 2016 at 1:12 PM, Ben Boeckel wrote: > On Wed, Aug 03, 2016 at 11:11:45 -0700, Chaoren Lin wrote: > > Also, in the case of vim, the makeprg is user-supplied, so vim has no > idea > > that it contains a path. > > What about instead of "cd path && ninja", using "ninja -C path"? > > --Ben > -- 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] Use full path for all source files in ninja build.
From: Chaoren Lin This is consistent with the behavior of the Makefile generators. Relative paths are difficult for an IDE to parse the output of a build error. --- Source/cmNinjaTargetGenerator.cxx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Source/cmNinjaTargetGenerator.cxx b/Source/cmNinjaTargetGenerator.cxx index 855a243..5b4a55d 100644 --- a/Source/cmNinjaTargetGenerator.cxx +++ b/Source/cmNinjaTargetGenerator.cxx @@ -305,7 +305,7 @@ void cmNinjaTargetGenerator::WriteCompileRule(const std::string& lang) vars.RuleLauncher = "RULE_LAUNCH_COMPILE"; vars.CMTarget = this->GetGeneratorTarget(); vars.Language = lang.c_str(); - vars.Source = "$in"; + vars.Source = "$IN_ABS"; vars.Object = "$out"; vars.Defines = "$DEFINES"; vars.Includes = "$INCLUDES"; @@ -529,8 +529,7 @@ void cmNinjaTargetGenerator::WriteObjectBuildStatement( cmSourceFile const* source, bool writeOrderDependsTargetForTarget) { std::string const language = source->GetLanguage(); - std::string const sourceFileName = -language == "RC" ? source->GetFullPath() : this->GetSourceFilePath(source); + std::string const sourceFileName = this->GetSourceFilePath(source); std::string const objectDir = this->ConvertToNinjaPath(this->GeneratorTarget->GetSupportDirectory()); std::string const objectFileName = @@ -539,6 +538,7 @@ void cmNinjaTargetGenerator::WriteObjectBuildStatement( cmSystemTools::GetFilenamePath(objectFileName); cmNinjaVars vars; + vars["IN_ABS"] = source->GetFullPath(); vars["FLAGS"] = this->ComputeFlagsForObject(source, language); vars["DEFINES"] = this->ComputeDefines(source, language); vars["INCLUDES"] = this->GetIncludes(language); -- 2.8.0.rc3.226.g39d4020 -- 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] Use full path for all source files in ninja build.
I'd say that you shouldn't do this unless you have tested it extensively with very long command lines, making sure that there is a response file fallback if it grows too much. There are issues in CMake already on Windows with long command lines and having even longer ones is not going to help. /Orphis On 06/08/2016 01:51, Chaoren Lin via cmake-developers wrote: From: Chaoren Lin This is consistent with the behavior of the Makefile generators. Relative paths are difficult for an IDE to parse the output of a build error. --- Source/cmNinjaTargetGenerator.cxx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Source/cmNinjaTargetGenerator.cxx b/Source/cmNinjaTargetGenerator.cxx index 855a243..5b4a55d 100644 --- a/Source/cmNinjaTargetGenerator.cxx +++ b/Source/cmNinjaTargetGenerator.cxx @@ -305,7 +305,7 @@ void cmNinjaTargetGenerator::WriteCompileRule(const std::string& lang) vars.RuleLauncher = "RULE_LAUNCH_COMPILE"; vars.CMTarget = this->GetGeneratorTarget(); vars.Language = lang.c_str(); - vars.Source = "$in"; + vars.Source = "$IN_ABS"; vars.Object = "$out"; vars.Defines = "$DEFINES"; vars.Includes = "$INCLUDES"; @@ -529,8 +529,7 @@ void cmNinjaTargetGenerator::WriteObjectBuildStatement( cmSourceFile const* source, bool writeOrderDependsTargetForTarget) { std::string const language = source->GetLanguage(); - std::string const sourceFileName = -language == "RC" ? source->GetFullPath() : this->GetSourceFilePath(source); + std::string const sourceFileName = this->GetSourceFilePath(source); std::string const objectDir = this->ConvertToNinjaPath(this->GeneratorTarget->GetSupportDirectory()); std::string const objectFileName = @@ -539,6 +538,7 @@ void cmNinjaTargetGenerator::WriteObjectBuildStatement( cmSystemTools::GetFilenamePath(objectFileName); cmNinjaVars vars; + vars["IN_ABS"] = source->GetFullPath(); vars["FLAGS"] = this->ComputeFlagsForObject(source, language); vars["DEFINES"] = this->ComputeDefines(source, language); vars["INCLUDES"] = this->GetIncludes(language); -- 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] Use full path for all source files in ninja build.
On Sun, Aug 07, 2016 at 04:54:52 +0200, Florent Castelli wrote: > I'd say that you shouldn't do this unless you have tested it extensively > with very long command lines, making sure that there is a response file > fallback if it grows too much. > There are issues in CMake already on Windows with long command lines and > having even longer ones is not going to help. Agreed. RC files compilation already has issues in some projects without absolute paths as is. https://gitlab.kitware.com/cmake/cmake/issues/16171 --Ben -- 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] Use full path for all source files in ninja build.
On 08/05/2016 07:51 PM, Chaoren Lin wrote: > - std::string const sourceFileName = > -language == "RC" ? source->GetFullPath() : > this->GetSourceFilePath(source); > + std::string const sourceFileName = this->GetSourceFilePath(source); I don't think this hunk is needed anymore, correct? 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] Use full path for all source files in ninja build.
> I don't think this hunk is needed anymore, correct? That hunk is the opposite now, or does RC still need that hack for some reason? On Mon, Aug 8, 2016 at 10:41 AM, Brad King wrote: > On 08/05/2016 07:51 PM, Chaoren Lin wrote: > > - std::string const sourceFileName = > > -language == "RC" ? source->GetFullPath() : this->GetSourceFilePath( > source); > > + std::string const sourceFileName = this->GetSourceFilePath(source); > > I don't think this hunk is needed anymore, correct? > > 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] Use full path for all source files in ninja build.
On 08/08/2016 09:19 AM, Ben Boeckel wrote: > On Sun, Aug 07, 2016 at 04:54:52 +0200, Florent Castelli wrote: >> I'd say that you shouldn't do this unless you have tested it extensively >> with very long command lines, making sure that there is a response file >> fallback if it grows too much. >> There are issues in CMake already on Windows with long command lines and >> having even longer ones is not going to help. > > Agreed. RC files compilation already has issues in some projects without > absolute paths as is. > > https://gitlab.kitware.com/cmake/cmake/issues/16171 RC files used absolute paths prior to this patch. The patch is for other languages. Also in out-of-source builds we already use absolute paths to files in the source tree. If there are problems with command line lengths they can be addressed separately. -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] Use full path for all source files in ninja build.
On 08/08/2016 01:42 PM, Chaoren Lin wrote: >> I don't think this hunk is needed anymore, correct? > > That hunk is the opposite now Oops, right. That actually fixes the existing RC bug I mentioned earlier in this thread. With your patch the use of IN_ABS breaks builds with spaces in the path. The reason is that Ninja handles quoting of paths when replacing the `$in` placeholder but does nothing special for `$IN_ABS`. CMake will have to generate the right path in the value. I've applied the patch with the appropriate modification for that: Ninja: Use full path for all source files https://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=955c2a63 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] Use full path for all source files in ninja build.
Thanks. But to address Florent's concern, are there tests that cover the windows command line limit? I guess to test if this breaks we'd need a source file whose compile command is under the limit if using relative path, but over the limit if using absolute path. On Mon, Aug 8, 2016 at 11:04 AM, Brad King wrote: > On 08/08/2016 01:42 PM, Chaoren Lin wrote: > >> I don't think this hunk is needed anymore, correct? > > > > That hunk is the opposite now > > Oops, right. That actually fixes the existing RC bug I mentioned > earlier in this thread. > > With your patch the use of IN_ABS breaks builds with spaces in the > path. The reason is that Ninja handles quoting of paths when > replacing the `$in` placeholder but does nothing special for > `$IN_ABS`. CMake will have to generate the right path in the value. > > I've applied the patch with the appropriate modification for that: > > Ninja: Use full path for all source files > https://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=955c2a63 > > 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] Use full path for all source files in ninja build.
On 08/08/2016 02:04 PM, Brad King wrote: > Ninja: Use full path for all source files > https://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=955c2a63 Unfortunately using an absolute path triggers a ninja limitation and breaks rebuilds in some cases. See the issue [1] and MR to revert the change [2]. Upstream ninja will require changes to allow us to restore this change. -Brad [1] https://gitlab.kitware.com/cmake/cmake/issues/16675 [2] https://gitlab.kitware.com/cmake/cmake/merge_requests/520 -- 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