Re: [cmake-developers] [PATCH] Use full path for all source files in ninja build.

[Brad King]

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

Re: [cmake-developers] [PATCH] Use full path for all source files in ninja build.

[Chaoren Lin via cmake-developers]

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.

[Brad King]

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.

[Brad King]

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.

[Chaoren Lin via cmake-developers]

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

[Brad King]

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.

[Ben Boeckel]

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.

[Florent Castelli]

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.

[Chaoren Lin via cmake-developers]

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.

[Chaoren Lin via cmake-developers]

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

[Ben Boeckel]

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.

[Chaoren Lin via cmake-developers]

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

[Brad King]

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.

[Chaoren Lin via cmake-developers]

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.

[Ben Boeckel]

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.

[Brad King]

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.

[Chaoren Lin via cmake-developers]

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

[Brad King]

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.

[Brad King]

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

[cmake-developers] [PATCH] Use full path for all source files in ninja build.

[Chaoren Lin via cmake-developers]

From: Chaoren Lin 

Relative paths are difficult for an IDE to parse the output of
a build error.
---
 Source/cmNinjaTargetGenerator.cxx | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/Source/cmNinjaTargetGenerator.cxx 
b/Source/cmNinjaTargetGenerator.cxx
index 855a243..7849c5e 100644
--- a/Source/cmNinjaTargetGenerator.cxx
+++ b/Source/cmNinjaTargetGenerator.cxx
@@ -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 = source->GetFullPath();
   std::string const objectDir =
 this->ConvertToNinjaPath(this->GeneratorTarget->GetSupportDirectory());
   std::string const objectFileName =
-- 
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