Ninja: Always compile sources using absolute paths

The Ninja generator traditionally referenced source files and include
directories using paths relative to the build directory if they could be
expressed without a `../` sequence that leaves the build and source
directories.  For example, when using a `build/` directory inside the
source tree, sources would be compiled as `-c ../src.c` and include
directories would be referenced as `-I ../include`.  This approach
matches the traditional Ninja convention of using relative paths
whenever possible, but has undesirable side effects such as:

* Compiler diagnostic messages may not use absolute paths, making it
  harder for IDEs/editors to find the referenced sources or headers.

* Debug symbols may not use absolute paths, making it harder for
  debuggers to find the referenced sources or headers.

* Different results depending on the path to the build tree relative
  to the source tree.

* Inconsistent with the Makefile generators, which use absolute paths.

Switch to always using absolute paths to reference source files and
include directories on compiler command lines.  While alternative
solutions for diagnostic messages and debug symbols may exist with
specific tooling, this is the simplest and most consistent approach.

Note that a previous attempt to do this in commit 955c2a630a (Ninja: Use
full path for all source files, 2016-08-05, v3.7.0-rc1~275^2) was
reverted by commit 666ad1df2d (Revert "Ninja: Use full path for all
source files", 2017-02-24, v3.8.0-rc2~9^2) due to problems hooking up
depfile dependencies on generated files.  This time, the changes in
commit 2725ecff38 (Ninja: Handle depfiles with absolute paths to
generated files, 2021-05-19) should avoid those problems.

Fixes: #13894, #17450

5 files changed, 15 insertions, 11 deletions

diff --git a/Help/release/dev/ninja-absolute-paths.rst b/Help/release/dev/ninja-absolute-paths.rst
new file mode 100644
index 0000000000..2dfc1b76fe
--- /dev/null
+++ b/Help/release/dev/ninja-absolute-paths.rst
@@ -0,0 +1,6 @@
+ninja-absolute-paths
+--------------------
+
+* The :ref:`Ninja Generators` now pass source files and include directories
+  to the compiler using absolute paths.  This makes diagnostic messages and
+  debug symbols more consistent, and matches the :ref:`Makefile Generators`.
diff --git a/Source/cmGlobalNinjaGenerator.cxx b/Source/cmGlobalNinjaGenerator.cxx
index a8a49a0372..cbe1bc8501 100644
--- a/Source/cmGlobalNinjaGenerator.cxx
+++ b/Source/cmGlobalNinjaGenerator.cxx
@@ -335,6 +335,7 @@ void cmGlobalNinjaGenerator::CCOutputs::Add(
       // This output is expressed as a relative path.  Repeat it,
       // but expressed as an absolute path for Ninja Issue 1251.
       this->WorkDirOuts.emplace_back(out);
+      this->GG->SeenCustomCommandOutput(this->GG->ConvertToNinjaAbsPath(path));
     }
     this->GG->SeenCustomCommandOutput(out);
     this->ExplicitOuts.emplace_back(std::move(out));
diff --git a/Source/cmLocalNinjaGenerator.cxx b/Source/cmLocalNinjaGenerator.cxx
index 22a1a1a66a..81425991c4 100644
--- a/Source/cmLocalNinjaGenerator.cxx
+++ b/Source/cmLocalNinjaGenerator.cxx
@@ -208,11 +208,9 @@ std::string cmLocalNinjaGenerator::ConvertToIncludeReference(
   std::string const& path, IncludePathStyle pathStyle,
   cmOutputConverter::OutputFormat format)
 {
-  if (pathStyle == IncludePathStyle::Absolute) {
-    return this->ConvertToOutputFormat(path, format);
-  }
-  return this->ConvertToOutputFormat(this->MaybeRelativeToTopBinDir(path),
-                                     format);
+  // FIXME: Remove IncludePathStyle infrastructure.  It is no longer used.
+  static_cast<void>(pathStyle);
+  return this->ConvertToOutputFormat(path, format);
 }
 
 // Private methods.
diff --git a/Source/cmNinjaTargetGenerator.cxx b/Source/cmNinjaTargetGenerator.cxx
index 5499ff7c5b..4feb0bb228 100644
--- a/Source/cmNinjaTargetGenerator.cxx
+++ b/Source/cmNinjaTargetGenerator.cxx
@@ -382,7 +382,8 @@ cmNinjaDeps cmNinjaTargetGenerator::ComputeLinkDeps(
 std::string cmNinjaTargetGenerator::GetCompiledSourceNinjaPath(
   cmSourceFile const* source) const
 {
-  return this->ConvertToNinjaPath(source->GetFullPath());
+  // Pass source files to the compiler by absolute path.
+  return this->ConvertToNinjaAbsPath(source->GetFullPath());
 }
 
 std::string cmNinjaTargetGenerator::GetObjectFilePath(
@@ -1199,9 +1200,7 @@ void cmNinjaTargetGenerator::WriteObjectBuildStatement(
   const std::string& fileConfig, bool firstForConfig)
 {
   std::string const language = source->GetLanguage();
-  std::string const sourceFilePath = language == "RC"
-    ? source->GetFullPath()
-    : this->GetCompiledSourceNinjaPath(source);
+  std::string const sourceFilePath = this->GetCompiledSourceNinjaPath(source);
   std::string const objectDir = this->ConvertToNinjaPath(
     cmStrCat(this->GeneratorTarget->GetSupportDirectory(),
              this->GetGlobalGenerator()->ConfigDirectory(config)));
diff --git a/Tests/RunCMake/Ninja/RunCMakeTest.cmake b/Tests/RunCMake/Ninja/RunCMakeTest.cmake
index 1350326164..8c91b34d9d 100644
--- a/Tests/RunCMake/Ninja/RunCMakeTest.cmake
+++ b/Tests/RunCMake/Ninja/RunCMakeTest.cmake
@@ -163,12 +163,12 @@ run_LooseObjectDepends()
 function (run_AssumedSources)
   set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/AssumedSources-build)
   run_cmake(AssumedSources)
-  run_ninja("${RunCMake_TEST_BINARY_DIR}" "target.c")
+  run_ninja("${RunCMake_TEST_BINARY_DIR}" "${RunCMake_TEST_BINARY_DIR}/target.c")
   if (NOT EXISTS "${RunCMake_TEST_BINARY_DIR}/target.c")
     message(FATAL_ERROR
       "Dependencies for an assumed source did not hook up properly for 'target.c'.")
   endif ()
-  run_ninja("${RunCMake_TEST_BINARY_DIR}" "target-no-depends.c")
+  run_ninja("${RunCMake_TEST_BINARY_DIR}" "${RunCMake_TEST_BINARY_DIR}/target-no-depends.c")
   if (EXISTS "${RunCMake_TEST_BINARY_DIR}/target-no-depends.c")
     message(FATAL_ERROR
       "Dependencies for an assumed source were magically hooked up for 'target-no-depends.c'.")