Ninja: Centralize path conversion in global generator (#15757)

In the Ninja generator we run all build rules from the top of the build
tree rather than changing into each subdirectory.  Therefore we convert
all paths relative to the HOME_OUTPUT directory.  However, the Convert
method on cmLocalGenerator restricts relative path conversions to avoid
leaving the build tree with a "../" sequence.  Therefore conversions
performed for "subdirectories" that are outside the top of the build
tree always use full paths while conversions performed for
subdirectories that are inside the top of the build tree may use
relative paths to refer to the same files.

Since Ninja always runs rules from the top of the build tree we should
convert them using only the top-level cmLocalGenerator in order to
remain consistent.  Also extend the test suite with a case that fails
without this fix.

1 files changed, 19 insertions, 14 deletions

diff --git a/Source/cmGlobalNinjaGenerator.cxx b/Source/cmGlobalNinjaGenerator.cxx
index 120bb03..9d8193b 100644
--- a/Source/cmGlobalNinjaGenerator.cxx
+++ b/Source/cmGlobalNinjaGenerator.cxx
@@ -818,6 +818,17 @@ void cmGlobalNinjaGenerator::CloseRulesFileStream()
    }
 }
 
+std::string cmGlobalNinjaGenerator::ConvertToNinjaPath(const std::string& path)
+{
+  cmLocalNinjaGenerator *ng =
+    static_cast<cmLocalNinjaGenerator *>(this->LocalGenerators[0]);
+  std::string convPath = ng->Convert(path, cmOutputConverter::HOME_OUTPUT);
+#ifdef _WIN32
+  cmSystemTools::ReplaceString(convPath, "/", "\\");
+#endif
+  return convPath;
+}
+
 void cmGlobalNinjaGenerator::AddCXXCompileCommand(
                                       const std::string &commandLine,
                                       const std::string &sourceFile)
@@ -907,8 +918,6 @@ cmGlobalNinjaGenerator
 {
   std::string configName =
     target->GetMakefile()->GetSafeDefinition("CMAKE_BUILD_TYPE");
-  cmLocalNinjaGenerator *ng =
-    static_cast<cmLocalNinjaGenerator *>(this->LocalGenerators[0]);
 
   // for frameworks, we want the real name, not smple name
   // frameworks always appear versioned, and the build.ninja
@@ -923,13 +932,13 @@ cmGlobalNinjaGenerator
   case cmTarget::MODULE_LIBRARY:
     {
     cmGeneratorTarget *gtgt = this->GetGeneratorTarget(target);
-    outputs.push_back(ng->ConvertToNinjaPath(
+    outputs.push_back(this->ConvertToNinjaPath(
       gtgt->GetFullPath(configName, false, realname)));
     break;
     }
   case cmTarget::OBJECT_LIBRARY:
   case cmTarget::UTILITY: {
-    std::string path = ng->ConvertToNinjaPath(
+    std::string path = this->ConvertToNinjaPath(
       target->GetMakefile()->GetCurrentBinaryDirectory());
     if (path.empty() || path == ".")
       outputs.push_back(target->GetName());
@@ -1041,8 +1050,6 @@ void cmGlobalNinjaGenerator::WriteUnknownExplicitDependencies(std::ostream& os)
 
   //get the list of files that cmake itself has generated as a
   //product of configuration.
-  cmLocalNinjaGenerator *ng =
-    static_cast<cmLocalNinjaGenerator *>(this->LocalGenerators[0]);
 
   for (std::vector<cmLocalGenerator *>::const_iterator i =
        this->LocalGenerators.begin(); i != this->LocalGenerators.end(); ++i)
@@ -1054,7 +1061,7 @@ void cmGlobalNinjaGenerator::WriteUnknownExplicitDependencies(std::ostream& os)
     typedef std::vector<std::string>::const_iterator vect_it;
     for(vect_it j = files.begin(); j != files.end(); ++j)
       {
-      knownDependencies.insert( ng->ConvertToNinjaPath( *j ) );
+      knownDependencies.insert( this->ConvertToNinjaPath( *j ) );
       }
     //get list files which are implicit dependencies as well and will be phony
     //for rebuild manifest
@@ -1062,7 +1069,7 @@ void cmGlobalNinjaGenerator::WriteUnknownExplicitDependencies(std::ostream& os)
     typedef std::vector<std::string>::const_iterator vect_it;
     for(vect_it j = lf.begin(); j != lf.end(); ++j)
       {
-      knownDependencies.insert( ng->ConvertToNinjaPath( *j ) );
+      knownDependencies.insert( this->ConvertToNinjaPath( *j ) );
       }
     std::vector<cmGeneratorExpressionEvaluationFile*> const& ef =
         (*i)->GetMakefile()->GetEvaluationFiles();
@@ -1074,7 +1081,7 @@ void cmGlobalNinjaGenerator::WriteUnknownExplicitDependencies(std::ostream& os)
       std::vector<std::string> evaluationFiles = (*li)->GetFiles();
       for(vect_it j = evaluationFiles.begin(); j != evaluationFiles.end(); ++j)
         {
-        knownDependencies.insert( ng->ConvertToNinjaPath( *j ) );
+        knownDependencies.insert( this->ConvertToNinjaPath( *j ) );
         }
       }
     }
@@ -1084,7 +1091,7 @@ void cmGlobalNinjaGenerator::WriteUnknownExplicitDependencies(std::ostream& os)
       i != this->TargetAliases.end();
       ++i)
     {
-    knownDependencies.insert( ng->ConvertToNinjaPath(i->first) );
+    knownDependencies.insert( this->ConvertToNinjaPath(i->first) );
     }
 
   //remove all source files we know will exist.
@@ -1093,7 +1100,7 @@ void cmGlobalNinjaGenerator::WriteUnknownExplicitDependencies(std::ostream& os)
       i != this->AssumedSourceDependencies.end();
       ++i)
     {
-    knownDependencies.insert( ng->ConvertToNinjaPath(i->first) );
+    knownDependencies.insert( this->ConvertToNinjaPath(i->first) );
     }
 
   //now we difference with CombinedCustomCommandExplicitDependencies to find
@@ -1214,8 +1221,6 @@ void cmGlobalNinjaGenerator::WriteTargetRebuildManifest(std::ostream& os)
             /*restat=*/ "",
             /*generator=*/ true);
 
-  cmLocalNinjaGenerator *ng = static_cast<cmLocalNinjaGenerator *>(lg);
-
   cmNinjaDeps implicitDeps;
   for(std::vector<cmLocalGenerator*>::const_iterator i =
         this->LocalGenerators.begin(); i != this->LocalGenerators.end(); ++i)
@@ -1224,7 +1229,7 @@ void cmGlobalNinjaGenerator::WriteTargetRebuildManifest(std::ostream& os)
     for(std::vector<std::string>::const_iterator fi = lf.begin();
         fi != lf.end(); ++fi)
       {
-      implicitDeps.push_back(ng->ConvertToNinjaPath(*fi));
+      implicitDeps.push_back(this->ConvertToNinjaPath(*fi));
       }
     }
   implicitDeps.push_back("CMakeCache.txt");