From 42db2ebc756a48ecdb15841c18747cb69e9df11f Mon Sep 17 00:00:00 2001 From: Alexis Murzeau Date: Sun, 23 Oct 2016 18:58:28 +0200 Subject: [PATCH] Ninja: Use binary dir for `$subdir/all` targets The targets added by commit v3.6.0-rc1~240^2~2 (Ninja: Add `$subdir/all` targets, 2016-03-11) use as `$subdir` the relative path from the top of the source tree to the current source directory. This is not correct when using `add_subdirectory(test test_bin)`. Instead we need to use the relative path from the top of the binary tree to the current binary directory as was done for related targets by commit v3.7.0-rc1~268^2 (Ninja: Add `$subdir/{test,install,package}` targets, 2016-08-05). --- Source/cmGlobalNinjaGenerator.cxx | 44 +++++++------------ Source/cmGlobalNinjaGenerator.h | 1 - Tests/RunCMake/Ninja/RunCMakeTest.cmake | 9 ++++ Tests/RunCMake/Ninja/SubDir.cmake | 1 + .../Ninja/SubDirBinary-build-stdout.txt | 1 + .../Ninja/SubDirBinary-install-stdout.txt | 1 + .../Ninja/SubDirBinary-test-stdout.txt | 1 + .../Ninja/SubDirSource/CMakeLists.txt | 6 +++ 8 files changed, 35 insertions(+), 29 deletions(-) create mode 100644 Tests/RunCMake/Ninja/SubDirBinary-build-stdout.txt create mode 100644 Tests/RunCMake/Ninja/SubDirBinary-install-stdout.txt create mode 100644 Tests/RunCMake/Ninja/SubDirBinary-test-stdout.txt create mode 100644 Tests/RunCMake/Ninja/SubDirSource/CMakeLists.txt diff --git a/Source/cmGlobalNinjaGenerator.cxx b/Source/cmGlobalNinjaGenerator.cxx index f5a0e68af..5e6036df8 100644 --- a/Source/cmGlobalNinjaGenerator.cxx +++ b/Source/cmGlobalNinjaGenerator.cxx @@ -843,20 +843,6 @@ std::string cmGlobalNinjaGenerator::ConvertToNinjaPath(const std::string& path) return convPath; } -std::string cmGlobalNinjaGenerator::ConvertToNinjaFolderRule( - const std::string& path) -{ - cmLocalNinjaGenerator* ng = - static_cast(this->LocalGenerators[0]); - std::string convPath = ng->ConvertToRelativePath( - this->LocalGenerators[0]->GetState()->GetSourceDirectory(), path + "/all"); - convPath = this->NinjaOutputPath(convPath); -#ifdef _WIN32 - std::replace(convPath.begin(), convPath.end(), '/', '\\'); -#endif - return convPath; -} - void cmGlobalNinjaGenerator::AddCXXCompileCommand( const std::string& commandLine, const std::string& sourceFile) { @@ -1083,11 +1069,11 @@ void cmGlobalNinjaGenerator::WriteFolderTargets(std::ostream& os) this->LocalGenerators.begin(); lgi != this->LocalGenerators.end(); ++lgi) { cmLocalGenerator const* lg = *lgi; - const std::string currentSourceFolder( - lg->GetStateSnapshot().GetDirectory().GetCurrentSource()); + const std::string currentBinaryFolder( + lg->GetStateSnapshot().GetDirectory().GetCurrentBinary()); // The directory-level rule should depend on the target-level rules // for all targets in the directory. - targetsPerFolder[currentSourceFolder] = cmNinjaDeps(); + targetsPerFolder[currentBinaryFolder] = cmNinjaDeps(); for (std::vector::const_iterator ti = lg->GetGeneratorTargets().begin(); ti != lg->GetGeneratorTargets().end(); ++ti) { @@ -1098,7 +1084,7 @@ void cmGlobalNinjaGenerator::WriteFolderTargets(std::ostream& os) type == cmState::MODULE_LIBRARY || type == cmState::OBJECT_LIBRARY || type == cmState::UTILITY) && !gt->GetPropertyAsBool("EXCLUDE_FROM_ALL")) { - targetsPerFolder[currentSourceFolder].push_back(gt->GetName()); + targetsPerFolder[currentBinaryFolder].push_back(gt->GetName()); } } @@ -1109,28 +1095,30 @@ void cmGlobalNinjaGenerator::WriteFolderTargets(std::ostream& os) for (std::vector::const_iterator stateIt = children.begin(); stateIt != children.end(); ++stateIt) { - targetsPerFolder[currentSourceFolder].push_back( - this->ConvertToNinjaFolderRule( - stateIt->GetDirectory().GetCurrentSource())); + std::string const currentBinaryDir = + stateIt->GetDirectory().GetCurrentBinary(); + + targetsPerFolder[currentBinaryFolder].push_back( + this->ConvertToNinjaPath(currentBinaryDir + "/all")); } } - std::string const rootSourceDir = - this->LocalGenerators[0]->GetSourceDirectory(); + std::string const rootBinaryDir = + this->LocalGenerators[0]->GetBinaryDirectory(); for (std::map::const_iterator it = targetsPerFolder.begin(); it != targetsPerFolder.end(); ++it) { cmGlobalNinjaGenerator::WriteDivider(os); - std::string const& currentSourceDir = it->first; + std::string const& currentBinaryDir = it->first; - // Do not generate a rule for the root source dir. - if (rootSourceDir.length() >= currentSourceDir.length()) { + // Do not generate a rule for the root binary dir. + if (rootBinaryDir.length() >= currentBinaryDir.length()) { continue; } - std::string const comment = "Folder: " + currentSourceDir; + std::string const comment = "Folder: " + currentBinaryDir; cmNinjaDeps output(1); - output.push_back(this->ConvertToNinjaFolderRule(currentSourceDir)); + output.push_back(this->ConvertToNinjaPath(currentBinaryDir + "/all")); this->WritePhonyBuild(os, comment, output, it->second); } diff --git a/Source/cmGlobalNinjaGenerator.h b/Source/cmGlobalNinjaGenerator.h index dcf7406c8..2ce3c808b 100644 --- a/Source/cmGlobalNinjaGenerator.h +++ b/Source/cmGlobalNinjaGenerator.h @@ -234,7 +234,6 @@ public: } std::string ConvertToNinjaPath(const std::string& path); - std::string ConvertToNinjaFolderRule(const std::string& path); struct MapToNinjaPathImpl { diff --git a/Tests/RunCMake/Ninja/RunCMakeTest.cmake b/Tests/RunCMake/Ninja/RunCMakeTest.cmake index 778f2c111..1d3639d29 100644 --- a/Tests/RunCMake/Ninja/RunCMakeTest.cmake +++ b/Tests/RunCMake/Ninja/RunCMakeTest.cmake @@ -45,14 +45,23 @@ function(run_SubDir) set(SubDir_all [[SubDir\all]]) set(SubDir_test [[SubDir\test]]) set(SubDir_install [[SubDir\install]]) + set(SubDirBinary_test [[SubDirBinary\test]]) + set(SubDirBinary_all [[SubDirBinary\all]]) + set(SubDirBinary_install [[SubDirBinary\install]]) else() set(SubDir_all [[SubDir/all]]) set(SubDir_test [[SubDir/test]]) set(SubDir_install [[SubDir/install]]) + set(SubDirBinary_all [[SubDirBinary/all]]) + set(SubDirBinary_test [[SubDirBinary/test]]) + set(SubDirBinary_install [[SubDirBinary/install]]) endif() run_cmake_command(SubDir-build ${CMAKE_COMMAND} --build . --target ${SubDir_all}) run_cmake_command(SubDir-test ${CMAKE_COMMAND} --build . --target ${SubDir_test}) run_cmake_command(SubDir-install ${CMAKE_COMMAND} --build . --target ${SubDir_install}) + run_cmake_command(SubDirBinary-build ${CMAKE_COMMAND} --build . --target ${SubDirBinary_all}) + run_cmake_command(SubDirBinary-test ${CMAKE_COMMAND} --build . --target ${SubDirBinary_test}) + run_cmake_command(SubDirBinary-install ${CMAKE_COMMAND} --build . --target ${SubDirBinary_install}) endfunction() run_SubDir() diff --git a/Tests/RunCMake/Ninja/SubDir.cmake b/Tests/RunCMake/Ninja/SubDir.cmake index d227753b2..11f467a4b 100644 --- a/Tests/RunCMake/Ninja/SubDir.cmake +++ b/Tests/RunCMake/Ninja/SubDir.cmake @@ -1,5 +1,6 @@ include(CTest) add_subdirectory(SubDir) +add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/SubDirSource SubDirBinary) add_custom_target(TopFail ALL COMMAND does_not_exist) add_test(NAME TopTest COMMAND ${CMAKE_COMMAND} -E echo "Running TopTest") install(CODE [[ diff --git a/Tests/RunCMake/Ninja/SubDirBinary-build-stdout.txt b/Tests/RunCMake/Ninja/SubDirBinary-build-stdout.txt new file mode 100644 index 000000000..244eaa071 --- /dev/null +++ b/Tests/RunCMake/Ninja/SubDirBinary-build-stdout.txt @@ -0,0 +1 @@ +Building SubDirSourceInAll diff --git a/Tests/RunCMake/Ninja/SubDirBinary-install-stdout.txt b/Tests/RunCMake/Ninja/SubDirBinary-install-stdout.txt new file mode 100644 index 000000000..6b6c6ddbe --- /dev/null +++ b/Tests/RunCMake/Ninja/SubDirBinary-install-stdout.txt @@ -0,0 +1 @@ +-- Installing SubDirSource diff --git a/Tests/RunCMake/Ninja/SubDirBinary-test-stdout.txt b/Tests/RunCMake/Ninja/SubDirBinary-test-stdout.txt new file mode 100644 index 000000000..d6d6605ae --- /dev/null +++ b/Tests/RunCMake/Ninja/SubDirBinary-test-stdout.txt @@ -0,0 +1 @@ +1/1 Test #1: SubDirSourceTest diff --git a/Tests/RunCMake/Ninja/SubDirSource/CMakeLists.txt b/Tests/RunCMake/Ninja/SubDirSource/CMakeLists.txt new file mode 100644 index 000000000..26642610e --- /dev/null +++ b/Tests/RunCMake/Ninja/SubDirSource/CMakeLists.txt @@ -0,0 +1,6 @@ +add_custom_target(SubDirSourceFail COMMAND does_not_exist) +add_custom_target(SubDirSourceInAll ALL COMMAND ${CMAKE_COMMAND} -E echo "Building SubDirSourceInAll") +add_test(NAME SubDirSourceTest COMMAND ${CMAKE_COMMAND} -E echo "Running SubDirSourceTest") +install(CODE [[ + message(STATUS "Installing SubDirSource") +]])