From 6e2a4087f2ff93fcea8391963a4101fce52ee94d Mon Sep 17 00:00:00 2001 From: Brad King Date: Fri, 25 Sep 2015 13:26:44 -0400 Subject: [PATCH] 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 | 33 ++++++++++++++----------- Source/cmGlobalNinjaGenerator.h | 13 ++++++++++ Source/cmLocalNinjaGenerator.cxx | 18 +++++--------- Source/cmLocalNinjaGenerator.h | 15 ----------- Source/cmNinjaTargetGenerator.cxx | 4 +-- Source/cmNinjaTargetGenerator.h | 8 +++--- Tests/OutOfBinary/CMakeLists.txt | 2 ++ Tests/OutOfBinary/outexe.c | 2 ++ Tests/OutOfSource/SubDir/CMakeLists.txt | 2 ++ Tests/OutOfSource/SubDir/subdir.c | 1 + 10 files changed, 51 insertions(+), 47 deletions(-) create mode 100644 Tests/OutOfBinary/outexe.c create mode 100644 Tests/OutOfSource/SubDir/subdir.c diff --git a/Source/cmGlobalNinjaGenerator.cxx b/Source/cmGlobalNinjaGenerator.cxx index 120bb036a..9d8193b7c 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(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(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(this->LocalGenerators[0]); for (std::vector::const_iterator i = this->LocalGenerators.begin(); i != this->LocalGenerators.end(); ++i) @@ -1054,7 +1061,7 @@ void cmGlobalNinjaGenerator::WriteUnknownExplicitDependencies(std::ostream& os) typedef std::vector::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::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 const& ef = (*i)->GetMakefile()->GetEvaluationFiles(); @@ -1074,7 +1081,7 @@ void cmGlobalNinjaGenerator::WriteUnknownExplicitDependencies(std::ostream& os) std::vector 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(lg); - cmNinjaDeps implicitDeps; for(std::vector::const_iterator i = this->LocalGenerators.begin(); i != this->LocalGenerators.end(); ++i) @@ -1224,7 +1229,7 @@ void cmGlobalNinjaGenerator::WriteTargetRebuildManifest(std::ostream& os) for(std::vector::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 d204a50ca..292f7c7e7 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 7525bf2ed..c46adc15d 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 8d3d49cee..1645a8d65 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 f46c5b90b..6e6dc60ef 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 a10ceba7b..0267f63e3 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 e32754143..f50536e70 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 000000000..6f1404358 --- /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 c5df36e1b..e18dbb96c 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 000000000..0d0d8279e --- /dev/null +++ b/Tests/OutOfSource/SubDir/subdir.c @@ -0,0 +1 @@ +int subdir(void) { return 0; }