From f59c33a763ba1483129f0e721bc2394bb7442876 Mon Sep 17 00:00:00 2001 From: Fujii Hironori Date: Fri, 23 Mar 2018 15:44:42 +0900 Subject: VS: Generate a custom command only in the least dependent target If a custom command is assigned to multiple targets, generate the build rule only in the least-dependent `.vcxproj` file. Otherwise MSBuild will run the command on the first build of a dependent target even if its dependencies already brought the command up to date (in order to populates its build log). Generate targets in least-to-most-dependent order, and assign a custom command to the least dependent target. Added cmLocalVisualStudio10Generator::GenerateTargetsDepthFirst to call cmVisualStudio10TargetGenerator::Generate in least-dependent order. Moved SourcesVisited from cmVisualStudio10TargetGenerator to cmLocalVisualStudio10Generator to avoid attaching a custom command to multiple targets among the local generator. Fixes: #16767 --- Source/cmLocalVisualStudio10Generator.cxx | 52 ++++++++++++++++------ Source/cmLocalVisualStudio10Generator.h | 9 ++++ Source/cmVisualStudio10TargetGenerator.cxx | 5 ++- Source/cmVisualStudio10TargetGenerator.h | 1 - .../AssigningMultipleTargets.cmake | 13 ++++++ .../RunCMake/add_custom_command/RunCMakeTest.cmake | 7 +++ Tests/RunCMake/add_custom_command/a.c | 3 ++ .../add_custom_command/generate-once.cmake | 8 ++++ 8 files changed, 82 insertions(+), 16 deletions(-) create mode 100644 Tests/RunCMake/add_custom_command/AssigningMultipleTargets.cmake create mode 100644 Tests/RunCMake/add_custom_command/a.c create mode 100644 Tests/RunCMake/add_custom_command/generate-once.cmake diff --git a/Source/cmLocalVisualStudio10Generator.cxx b/Source/cmLocalVisualStudio10Generator.cxx index 2803d4a..5b6e781 100644 --- a/Source/cmLocalVisualStudio10Generator.cxx +++ b/Source/cmLocalVisualStudio10Generator.cxx @@ -62,21 +62,47 @@ cmLocalVisualStudio10Generator::~cmLocalVisualStudio10Generator() { } +void cmLocalVisualStudio10Generator::GenerateTargetsDepthFirst( + cmGeneratorTarget* target, std::vector& remaining) +{ + if (target->GetType() == cmStateEnums::INTERFACE_LIBRARY) { + return; + } + // Find this target in the list of remaining targets. + auto it = std::find(remaining.begin(), remaining.end(), target); + if (it == remaining.end()) { + // This target was already handled. + return; + } + // Remove this target from the list of remaining targets because + // we are handling it now. + *it = nullptr; + auto& deps = this->GlobalGenerator->GetTargetDirectDepends(target); + for (auto& d : deps) { + // FIXME: Revise CreateSingleVCProj so we do not have to drop `const` here. + auto dependee = const_cast(&*d); + GenerateTargetsDepthFirst(dependee, remaining); + // Take the union of visited source files of custom commands + auto visited = GetSourcesVisited(dependee); + GetSourcesVisited(target).insert(visited.begin(), visited.end()); + } + if (static_cast(this->GlobalGenerator) + ->TargetIsFortranOnly(target)) { + this->CreateSingleVCProj(target->GetName(), target); + } else { + cmVisualStudio10TargetGenerator tg( + target, static_cast( + this->GetGlobalGenerator())); + tg.Generate(); + } +} + void cmLocalVisualStudio10Generator::Generate() { - const std::vector& tgts = this->GetGeneratorTargets(); - for (cmGeneratorTarget* l : tgts) { - if (l->GetType() == cmStateEnums::INTERFACE_LIBRARY) { - continue; - } - if (static_cast(this->GlobalGenerator) - ->TargetIsFortranOnly(l)) { - this->CreateSingleVCProj(l->GetName(), l); - } else { - cmVisualStudio10TargetGenerator tg( - l, static_cast( - this->GetGlobalGenerator())); - tg.Generate(); + std::vector remaining = this->GetGeneratorTargets(); + for (auto& t : remaining) { + if (t) { + GenerateTargetsDepthFirst(t, remaining); } } this->WriteStampFiles(); diff --git a/Source/cmLocalVisualStudio10Generator.h b/Source/cmLocalVisualStudio10Generator.h index bcdc307..a4150b9 100644 --- a/Source/cmLocalVisualStudio10Generator.h +++ b/Source/cmLocalVisualStudio10Generator.h @@ -33,10 +33,19 @@ public: void ReadAndStoreExternalGUID(const std::string& name, const char* path) override; + std::set& GetSourcesVisited(cmGeneratorTarget* target) + { + return SourcesVisited[target]; + }; + protected: const char* ReportErrorLabel() const override; bool CustomCommandUseLocal() const override { return true; } private: + void GenerateTargetsDepthFirst(cmGeneratorTarget* target, + std::vector& remaining); + + std::map> SourcesVisited; }; #endif diff --git a/Source/cmVisualStudio10TargetGenerator.cxx b/Source/cmVisualStudio10TargetGenerator.cxx index 625cb83..d27bec9 100644 --- a/Source/cmVisualStudio10TargetGenerator.cxx +++ b/Source/cmVisualStudio10TargetGenerator.cxx @@ -1178,7 +1178,6 @@ void cmVisualStudio10TargetGenerator::WriteNsightTegraConfigurationValues( void cmVisualStudio10TargetGenerator::WriteCustomCommands() { - this->SourcesVisited.clear(); this->CSharpCustomCommandNames.clear(); std::vector customCommands; this->GeneratorTarget->GetCustomCommands(customCommands, ""); @@ -1199,7 +1198,9 @@ void cmVisualStudio10TargetGenerator::WriteCustomCommands() void cmVisualStudio10TargetGenerator::WriteCustomCommand( cmSourceFile const* sf) { - if (this->SourcesVisited.insert(sf).second) { + if (this->LocalGenerator->GetSourcesVisited(this->GeneratorTarget) + .insert(sf) + .second) { if (std::vector const* depends = this->GeneratorTarget->GetSourceDepends(sf)) { for (cmSourceFile const* di : *depends) { diff --git a/Source/cmVisualStudio10TargetGenerator.h b/Source/cmVisualStudio10TargetGenerator.h index 53334bb..bbe3f7f 100644 --- a/Source/cmVisualStudio10TargetGenerator.h +++ b/Source/cmVisualStudio10TargetGenerator.h @@ -209,7 +209,6 @@ private: cmGlobalVisualStudio10Generator* const GlobalGenerator; cmGeneratedFileStream* BuildFileStream; cmLocalVisualStudio10Generator* const LocalGenerator; - std::set SourcesVisited; std::set CSharpCustomCommandNames; bool IsMissingFiles; std::vector AddedFiles; diff --git a/Tests/RunCMake/add_custom_command/AssigningMultipleTargets.cmake b/Tests/RunCMake/add_custom_command/AssigningMultipleTargets.cmake new file mode 100644 index 0000000..fe1cceb --- /dev/null +++ b/Tests/RunCMake/add_custom_command/AssigningMultipleTargets.cmake @@ -0,0 +1,13 @@ +enable_language(CXX) + +add_custom_command(OUTPUT generated.cpp + MAIN_DEPENDENCY a.c + COMMAND ${CMAKE_COMMAND} -P ${CMAKE_CURRENT_SOURCE_DIR}/generate-once.cmake ${CMAKE_CURRENT_BINARY_DIR}/generated.cpp + VERBATIM) + +add_executable(exe1 ${CMAKE_CURRENT_BINARY_DIR}/generated.cpp) +add_executable(exe2 ${CMAKE_CURRENT_BINARY_DIR}/generated.cpp) +add_executable(exe3 ${CMAKE_CURRENT_BINARY_DIR}/generated.cpp) + +add_dependencies(exe1 exe2) +add_dependencies(exe3 exe1) diff --git a/Tests/RunCMake/add_custom_command/RunCMakeTest.cmake b/Tests/RunCMake/add_custom_command/RunCMakeTest.cmake index c12e5aa..0387dbb 100644 --- a/Tests/RunCMake/add_custom_command/RunCMakeTest.cmake +++ b/Tests/RunCMake/add_custom_command/RunCMakeTest.cmake @@ -14,3 +14,10 @@ run_cmake(TargetNotInDir) if(${RunCMake_GENERATOR} MATCHES "Visual Studio ([^89]|[89][0-9])") run_cmake(RemoveEmptyCommands) endif() + +run_cmake(AssigningMultipleTargets) +set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/AssigningMultipleTargets-build) +set(RunCMake_TEST_NO_CLEAN 1) +run_cmake_command(AssigningMultipleTargets-build ${CMAKE_COMMAND} --build .) +unset(RunCMake_TEST_BINARY_DIR) +unset(RunCMake_TEST_NO_CLEAN) diff --git a/Tests/RunCMake/add_custom_command/a.c b/Tests/RunCMake/add_custom_command/a.c new file mode 100644 index 0000000..707c1c3 --- /dev/null +++ b/Tests/RunCMake/add_custom_command/a.c @@ -0,0 +1,3 @@ +void a() +{ +} diff --git a/Tests/RunCMake/add_custom_command/generate-once.cmake b/Tests/RunCMake/add_custom_command/generate-once.cmake new file mode 100644 index 0000000..2a8e843 --- /dev/null +++ b/Tests/RunCMake/add_custom_command/generate-once.cmake @@ -0,0 +1,8 @@ +if (${CMAKE_ARGC} LESS 4) + message(FATAL_ERROR "Too few arguments") +endif() +set(output "${CMAKE_ARGV3}") +if(EXISTS ${output}) + message(FATAL_ERROR "${output} already exists") +endif() +file(WRITE ${output} "int main() { return 0; }\n") -- cgit v0.12