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.

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");

Source/cmGlobalNinjaGenerator.h

@@ -220,6 +220,19 @@ public:
   cmGeneratedFileStream* GetRulesFileStream() const {
     return this->RulesFileStream; }
 
+  std::string ConvertToNinjaPath(const std::string& path);
+
+  struct MapToNinjaPathImpl {
+    cmGlobalNinjaGenerator* GG;
+    MapToNinjaPathImpl(cmGlobalNinjaGenerator* gg): GG(gg) {}
+    std::string operator()(std::string const& path) {
+      return this->GG->ConvertToNinjaPath(path);
+    }
+  };
+  MapToNinjaPathImpl MapToNinjaPath() {
+    return MapToNinjaPathImpl(this);
+  }
+
   void AddCXXCompileCommand(const std::string &commandLine,
                             const std::string &sourceFile);
 

Source/cmLocalNinjaGenerator.cxx

@@ -284,15 +284,6 @@ void cmLocalNinjaGenerator::WriteProcessedMakefile(std::ostream& os)
   os << std::endl;
 }
 
-std::string cmLocalNinjaGenerator::ConvertToNinjaPath(const std::string& path)
-{
-  std::string convPath = this->Convert(path, cmLocalGenerator::HOME_OUTPUT);
-#ifdef _WIN32
-  cmSystemTools::ReplaceString(convPath, "/", "\\");
-#endif
-  return convPath;
-}
-
 void
 cmLocalNinjaGenerator
 ::AppendTargetOutputs(cmTarget* target, cmNinjaDeps& outputs)
@@ -316,7 +307,8 @@ void cmLocalNinjaGenerator::AppendCustomCommandDeps(
        i != deps.end(); ++i) {
     std::string dep;
     if (this->GetRealDependency(*i, this->GetConfigName(), dep))
-      ninjaDeps.push_back(ConvertToNinjaPath(dep));
+      ninjaDeps.push_back(
+        this->GetGlobalNinjaGenerator()->ConvertToNinjaPath(dep));
   }
 }
 
@@ -413,9 +405,11 @@ cmLocalNinjaGenerator::WriteCustomCommandBuildStatement(
     at us.  How to know which ExternalProject step actually provides it?
 #endif
   std::transform(outputs.begin(), outputs.end(),
-                 ninjaOutputs.begin(), MapToNinjaPath());
+                 ninjaOutputs.begin(),
+                 this->GetGlobalNinjaGenerator()->MapToNinjaPath());
   std::transform(byproducts.begin(), byproducts.end(),
-                 ninjaOutputs.begin() + outputs.size(), MapToNinjaPath());
+                 ninjaOutputs.begin() + outputs.size(),
+                 this->GetGlobalNinjaGenerator()->MapToNinjaPath());
   this->AppendCustomCommandDeps(ccg, ninjaDeps);
 
   for (cmNinjaDeps::iterator i = ninjaOutputs.begin(); i != ninjaOutputs.end();

Source/cmLocalNinjaGenerator.h

@@ -50,21 +50,6 @@ public:
   std::string GetHomeRelativeOutputPath() const
   { return this->HomeRelativeOutputPath; }
 
-  std::string ConvertToNinjaPath(const std::string& path);
-
-  struct map_to_ninja_path {
-    cmLocalNinjaGenerator *LocalGenerator;
-    map_to_ninja_path(cmLocalNinjaGenerator *LocalGen)
-      : LocalGenerator(LocalGen) {}
-    std::string operator()(const std::string &path) {
-      return LocalGenerator->ConvertToNinjaPath(path);
-    }
-  };
-
-  map_to_ninja_path MapToNinjaPath() {
-    return map_to_ninja_path(this);
-  }
-
   void ExpandRuleVariables(std::string& string,
                            const RuleVariables& replaceValues) {
     cmLocalGenerator::ExpandRuleVariables(string, replaceValues);

Source/cmNinjaTargetGenerator.cxx

@@ -771,14 +771,14 @@ cmNinjaTargetGenerator::MacOSXContentGeneratorType::operator()(
   // Get the input file location.
   std::string input = source.GetFullPath();
   input =
-    this->Generator->GetLocalGenerator()->ConvertToNinjaPath(input);
+    this->Generator->GetGlobalGenerator()->ConvertToNinjaPath(input);
 
   // Get the output file location.
   std::string output = macdir;
   output += "/";
   output += cmSystemTools::GetFilenameName(input);
   output =
-    this->Generator->GetLocalGenerator()->ConvertToNinjaPath(output);
+    this->Generator->GetGlobalGenerator()->ConvertToNinjaPath(output);
 
   // Write a build statement to copy the content into the bundle.
   this->Generator->GetGlobalGenerator()->WriteMacOSXContentBuild(input,

Source/cmNinjaTargetGenerator.h

@@ -17,11 +17,11 @@
 
 #include "cmStandardIncludes.h"
 #include "cmNinjaTypes.h"
+#include "cmGlobalNinjaGenerator.h"
 #include "cmLocalNinjaGenerator.h"
 #include "cmOSXBundleGenerator.h"
 
 class cmTarget;
-class cmGlobalNinjaGenerator;
 class cmGeneratedFileStream;
 class cmGeneratorTarget;
 class cmMakefile;
@@ -87,10 +87,10 @@ protected:
                              const std::string& language);
 
   std::string ConvertToNinjaPath(const std::string& path) const {
-    return this->GetLocalGenerator()->ConvertToNinjaPath(path);
+    return this->GetGlobalGenerator()->ConvertToNinjaPath(path);
   }
-  cmLocalNinjaGenerator::map_to_ninja_path MapToNinjaPath() const {
+  cmGlobalNinjaGenerator::MapToNinjaPathImpl MapToNinjaPath() const {
-    return this->GetLocalGenerator()->MapToNinjaPath();
+    return this->GetGlobalGenerator()->MapToNinjaPath();
   }
 
   /// @return the list of link dependency for the given target @a target.

Tests/OutOfBinary/CMakeLists.txt

@@ -1,2 +1,4 @@
 add_library(outlib outlib.c)
 
+add_executable(outexe outexe.c)
+target_link_libraries(outexe subdir)

Tests/OutOfBinary/outexe.c

@@ -0,0 +1,2 @@
+extern int subdir(void);
+int main(void) { return subdir(); }

Tests/OutOfSource/SubDir/CMakeLists.txt

@@ -6,3 +6,5 @@ add_subdirectory(${OutOfSource_SOURCE_DIR}/../OutOfBinary
 
 # subdir to a sibling dir
 add_subdirectory(${OutOfSource_SOURCE_DIR}/${KEN}OutOfSourceSubdir OutOfSourceSubdir )
+
+add_library(subdir subdir.c)

Tests/OutOfSource/SubDir/subdir.c

@@ -0,0 +1 @@
+int subdir(void) { return 0; }