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.

10 files changed, 51 insertions, 47 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");
diff --git a/Source/cmGlobalNinjaGenerator.h b/Source/cmGlobalNinjaGenerator.h
index d204a50..292f7c7 100644
--- a/Source/cmGlobalNinjaGenerator.h
+++ b/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);
 
diff --git a/Source/cmLocalNinjaGenerator.cxx b/Source/cmLocalNinjaGenerator.cxx
index 7525bf2..c46adc1 100644
--- a/Source/cmLocalNinjaGenerator.cxx
+++ b/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();
diff --git a/Source/cmLocalNinjaGenerator.h b/Source/cmLocalNinjaGenerator.h
index 8d3d49c..1645a8d 100644
--- a/Source/cmLocalNinjaGenerator.h
+++ b/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);
diff --git a/Source/cmNinjaTargetGenerator.cxx b/Source/cmNinjaTargetGenerator.cxx
index f46c5b9..6e6dc60 100644
--- a/Source/cmNinjaTargetGenerator.cxx
+++ b/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,
diff --git a/Source/cmNinjaTargetGenerator.h b/Source/cmNinjaTargetGenerator.h
index a10ceba..0267f63 100644
--- a/Source/cmNinjaTargetGenerator.h
+++ b/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 {
-    return this->GetLocalGenerator()->MapToNinjaPath();
+  cmGlobalNinjaGenerator::MapToNinjaPathImpl MapToNinjaPath() const {
+    return this->GetGlobalGenerator()->MapToNinjaPath();
   }
 
   /// @return the list of link dependency for the given target @a target.
diff --git a/Tests/OutOfBinary/CMakeLists.txt b/Tests/OutOfBinary/CMakeLists.txt
index e327541..f50536e 100644
--- a/Tests/OutOfBinary/CMakeLists.txt
+++ b/Tests/OutOfBinary/CMakeLists.txt
@@ -1,2 +1,4 @@
 add_library(outlib outlib.c)
 
+add_executable(outexe outexe.c)
+target_link_libraries(outexe subdir)
diff --git a/Tests/OutOfBinary/outexe.c b/Tests/OutOfBinary/outexe.c
new file mode 100644
index 0000000..6f14043
--- /dev/null
+++ b/Tests/OutOfBinary/outexe.c
@@ -0,0 +1,2 @@
+extern int subdir(void);
+int main(void) { return subdir(); }
diff --git a/Tests/OutOfSource/SubDir/CMakeLists.txt b/Tests/OutOfSource/SubDir/CMakeLists.txt
index c5df36e..e18dbb9 100644
--- a/Tests/OutOfSource/SubDir/CMakeLists.txt
+++ b/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)
diff --git a/Tests/OutOfSource/SubDir/subdir.c b/Tests/OutOfSource/SubDir/subdir.c
new file mode 100644
index 0000000..0d0d827
--- /dev/null
+++ b/Tests/OutOfSource/SubDir/subdir.c
@@ -0,0 +1 @@
+int subdir(void) { return 0; }