From a56edad6d61268204af8228b8d58fa26d8f72269 Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 30 Oct 2018 09:37:07 -0400 Subject: CSharp: Fix regression in VS project type selection for custom target A target created by `add_custom_target` should always be a `.vcxproj` file even if it has `.cs` sources involved in custom commands and such. The latter case was broken by refactoring in commit v3.12.0-rc1~160^2~7 (remove TargetIsCSharpOnly() and use methods from cmGeneratorTarget, 2018-03-19). The reason is that the `HasLanguage` method added by commit v3.12.0-rc1~239^2~6 (cmGeneratorTarget: add HasLanguage() as wrapper for GetLanguages(), 2018-03-19) does not check the target type and so is not a suitable check for deciding the project file extension. The `HasLanguage` method was an attempt at an abstraction that turns out not to work very well. Replace it with a dedicated `IsCSharpOnly` method that considers the target type, sources, and non-transitive `LINKER_LANGUAGE`. Fixes: #18515 --- Source/cmExportFileGenerator.cxx | 2 +- Source/cmGeneratorTarget.cxx | 28 ++++++++++++---------- Source/cmGeneratorTarget.h | 6 +---- Source/cmGlobalVisualStudio71Generator.cxx | 2 +- Source/cmVisualStudio10TargetGenerator.cxx | 16 ++++++------- Tests/CSharpOnly/CMakeLists.txt | 4 ++-- .../CSharpCustomCommand/RunCMakeTest.cmake | 8 +++++++ .../TargetWithCommand-build-stdout.txt | 1 + .../CSharpCustomCommand/TargetWithCommand.cmake | 4 ++++ 9 files changed, 40 insertions(+), 31 deletions(-) create mode 100644 Tests/RunCMake/CSharpCustomCommand/TargetWithCommand-build-stdout.txt create mode 100644 Tests/RunCMake/CSharpCustomCommand/TargetWithCommand.cmake diff --git a/Source/cmExportFileGenerator.cxx b/Source/cmExportFileGenerator.cxx index 4cf9dd7..bddc3c4 100644 --- a/Source/cmExportFileGenerator.cxx +++ b/Source/cmExportFileGenerator.cxx @@ -855,7 +855,7 @@ void cmExportFileGenerator::SetImportDetailProperties( std::string propval; if (auto* p = target->GetProperty("COMMON_LANGUAGE_RUNTIME")) { propval = p; - } else if (target->HasLanguage("CSharp", config)) { + } else if (target->IsCSharpOnly()) { // C# projects do not have the /clr flag, so we set the property // here to mark the target as (only) managed (i.e. no .lib file // to link to). Otherwise the COMMON_LANGUAGE_RUNTIME target diff --git a/Source/cmGeneratorTarget.cxx b/Source/cmGeneratorTarget.cxx index 80fb621..5c294f8 100644 --- a/Source/cmGeneratorTarget.cxx +++ b/Source/cmGeneratorTarget.cxx @@ -5594,20 +5594,23 @@ void cmGeneratorTarget::GetLanguages(std::set& languages, } } -bool cmGeneratorTarget::HasLanguage(std::string const& language, - std::string const& config, - bool exclusive) const +bool cmGeneratorTarget::IsCSharpOnly() const { - std::set languages; - this->GetLanguages(languages, config); - // The "exclusive" check applies only to source files and not - // the linker language which may be affected by dependencies. - if (exclusive && languages.size() > 1) { + // Only certain target types may compile CSharp. + if (this->GetType() != cmStateEnums::SHARED_LIBRARY && + this->GetType() != cmStateEnums::STATIC_LIBRARY && + this->GetType() != cmStateEnums::EXECUTABLE) { return false; } - // add linker language (if it is different from compiler languages) - languages.insert(this->GetLinkerLanguage(config)); - return languages.count(language) > 0; + std::set languages; + this->GetLanguages(languages, ""); + // Consider an explicit linker language property, but *not* the + // computed linker language that may depend on linked targets. + const char* linkLang = this->GetProperty("LINKER_LANGUAGE"); + if (linkLang && *linkLang) { + languages.insert(linkLang); + } + return languages.size() == 1 && languages.count("CSharp") > 0; } void cmGeneratorTarget::ComputeLinkImplementationLanguages( @@ -5971,6 +5974,5 @@ cmGeneratorTarget::ManagedType cmGeneratorTarget::GetManagedType( // C# targets are always managed. This language specific check // is added to avoid that the COMMON_LANGUAGE_RUNTIME target property // has to be set manually for C# targets. - return this->HasLanguage("CSharp", config) ? ManagedType::Managed - : ManagedType::Native; + return this->IsCSharpOnly() ? ManagedType::Managed : ManagedType::Native; } diff --git a/Source/cmGeneratorTarget.h b/Source/cmGeneratorTarget.h index b1daa53..5ed8e5a 100644 --- a/Source/cmGeneratorTarget.h +++ b/Source/cmGeneratorTarget.h @@ -372,11 +372,7 @@ public: void GetLanguages(std::set& languages, std::string const& config) const; - // Evaluate if the target uses the given language for compilation - // and/or linking. If 'exclusive' is true, 'language' is expected - // to be the only language used in source files for the target. - bool HasLanguage(std::string const& language, std::string const& config, - bool exclusive = true) const; + bool IsCSharpOnly() const; void GetObjectLibrariesCMP0026( std::vector& objlibs) const; diff --git a/Source/cmGlobalVisualStudio71Generator.cxx b/Source/cmGlobalVisualStudio71Generator.cxx index 0b086b0..ba12fac 100644 --- a/Source/cmGlobalVisualStudio71Generator.cxx +++ b/Source/cmGlobalVisualStudio71Generator.cxx @@ -98,7 +98,7 @@ void cmGlobalVisualStudio71Generator::WriteProject(std::ostream& fout, ext = ".vfproj"; project = "Project(\"{6989167D-11E4-40FE-8C1A-2192A86A7E90}\") = \""; } - if (t->HasLanguage("CSharp", "")) { + if (t->IsCSharpOnly()) { ext = ".csproj"; project = "Project(\"{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}\") = \""; } diff --git a/Source/cmVisualStudio10TargetGenerator.cxx b/Source/cmVisualStudio10TargetGenerator.cxx index 09c08d3..334c15b 100644 --- a/Source/cmVisualStudio10TargetGenerator.cxx +++ b/Source/cmVisualStudio10TargetGenerator.cxx @@ -215,12 +215,11 @@ static bool cmVS10IsTargetsFile(std::string const& path) return cmSystemTools::Strucmp(ext.c_str(), ".targets") == 0; } -static std::string computeProjectFileExtension(cmGeneratorTarget const* t, - const std::string& config) +static std::string computeProjectFileExtension(cmGeneratorTarget const* t) { std::string res; res = ".vcxproj"; - if (t->HasLanguage("CSharp", config)) { + if (t->IsCSharpOnly()) { res = ".csproj"; } return res; @@ -315,8 +314,8 @@ void cmVisualStudio10TargetGenerator::Generate() this->GeneratorTarget->GetProperty("EXTERNAL_MSPROJECT")) { return; } - const std::string ProjectFileExtension = computeProjectFileExtension( - this->GeneratorTarget, *this->Configurations.begin()); + const std::string ProjectFileExtension = + computeProjectFileExtension(this->GeneratorTarget); if (ProjectFileExtension == ".vcxproj") { this->ProjectType = vcxproj; this->Managed = false; @@ -1409,8 +1408,7 @@ void cmVisualStudio10TargetGenerator::WriteGroups() std::string path = this->LocalGenerator->GetCurrentBinaryDirectory(); path += "/"; path += this->Name; - path += computeProjectFileExtension(this->GeneratorTarget, - *this->Configurations.begin()); + path += computeProjectFileExtension(this->GeneratorTarget); path += ".filters"; cmGeneratedFileStream fout(path); fout.SetCopyIfDifferent(true); @@ -3812,7 +3810,7 @@ void cmVisualStudio10TargetGenerator::WriteProjectReferences(Elem& e0) path = lg->GetCurrentBinaryDirectory(); path += "/"; path += dt->GetName(); - path += computeProjectFileExtension(dt, *this->Configurations.begin()); + path += computeProjectFileExtension(dt); } ConvertToWindowsSlash(path); Elem e2(e1, "ProjectReference"); @@ -3838,7 +3836,7 @@ void cmVisualStudio10TargetGenerator::WriteProjectReferences(Elem& e0) } // Workaround for static library C# targets if (referenceNotManaged && dt->GetType() == cmStateEnums::STATIC_LIBRARY) { - referenceNotManaged = !dt->HasLanguage("CSharp", ""); + referenceNotManaged = !dt->IsCSharpOnly(); } if (referenceNotManaged) { e2.Element("ReferenceOutputAssembly", "false"); diff --git a/Tests/CSharpOnly/CMakeLists.txt b/Tests/CSharpOnly/CMakeLists.txt index 84b58ca..82049c7 100644 --- a/Tests/CSharpOnly/CMakeLists.txt +++ b/Tests/CSharpOnly/CMakeLists.txt @@ -9,5 +9,5 @@ add_executable(CSharpOnly csharponly.cs) target_link_libraries(CSharpOnly lib1 lib2) -add_custom_target(CSharpCustom SOURCES empty.cs) -add_custom_target(custom.cs DEPENDS empty.txt) +add_custom_target(CSharpCustom ALL SOURCES empty.cs) +add_custom_target(custom.cs ALL DEPENDS empty.txt) diff --git a/Tests/RunCMake/CSharpCustomCommand/RunCMakeTest.cmake b/Tests/RunCMake/CSharpCustomCommand/RunCMakeTest.cmake index fa5618a..ab3e51b 100644 --- a/Tests/RunCMake/CSharpCustomCommand/RunCMakeTest.cmake +++ b/Tests/RunCMake/CSharpCustomCommand/RunCMakeTest.cmake @@ -1,5 +1,13 @@ include(RunCMake) +function(run_TargetWithCommand) + set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/TargetWithCommand-build) + run_cmake(TargetWithCommand) + set(RunCMake_TEST_NO_CLEAN 1) + run_cmake_command(TargetWithCommand-build ${CMAKE_COMMAND} --build . --config Debug) +endfunction() +run_TargetWithCommand() + # Use a single build tree for a few tests without cleaning. set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/CommandWithOutput-build) set(RunCMake_TEST_NO_CLEAN 1) diff --git a/Tests/RunCMake/CSharpCustomCommand/TargetWithCommand-build-stdout.txt b/Tests/RunCMake/CSharpCustomCommand/TargetWithCommand-build-stdout.txt new file mode 100644 index 0000000..c212a8f --- /dev/null +++ b/Tests/RunCMake/CSharpCustomCommand/TargetWithCommand-build-stdout.txt @@ -0,0 +1 @@ +Custom target with CSharp source diff --git a/Tests/RunCMake/CSharpCustomCommand/TargetWithCommand.cmake b/Tests/RunCMake/CSharpCustomCommand/TargetWithCommand.cmake new file mode 100644 index 0000000..fdaea5c --- /dev/null +++ b/Tests/RunCMake/CSharpCustomCommand/TargetWithCommand.cmake @@ -0,0 +1,4 @@ +enable_language(CSharp) + +add_custom_target(drive ALL SOURCES dummy.cs + COMMAND ${CMAKE_COMMAND} -E echo "Custom target with CSharp source") -- cgit v0.12 From 1acd1c2b50f461ee2cf90fb7166240f180147f19 Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 30 Oct 2018 09:37:07 -0400 Subject: CSharp: Fix regression in VS project type selection for custom target A target created by `add_custom_target` should always be a `.vcxproj` file even if it has `.cs` sources involved in custom commands and such. The latter case was broken by refactoring in commit v3.12.0-rc1~160^2~7 (remove TargetIsCSharpOnly() and use methods from cmGeneratorTarget, 2018-03-19). The reason is that the `HasLanguage` method added by commit v3.12.0-rc1~239^2~6 (cmGeneratorTarget: add HasLanguage() as wrapper for GetLanguages(), 2018-03-19) does not check the target type and so is not a suitable check for deciding the project file extension. The `HasLanguage` method was an attempt at an abstraction that turns out not to work very well. Replace it with a dedicated `IsCSharpOnly` method that considers the target type, sources, and non-transitive `LINKER_LANGUAGE`. Fixes: #18515 --- Source/cmExportFileGenerator.cxx | 2 +- Source/cmGeneratorTarget.cxx | 28 ++++++++++++---------- Source/cmGeneratorTarget.h | 6 +---- Source/cmGlobalVisualStudio71Generator.cxx | 2 +- Source/cmVisualStudio10TargetGenerator.cxx | 16 ++++++------- .../CSharpCustomCommand/RunCMakeTest.cmake | 8 +++++++ .../TargetWithCommand-build-stdout.txt | 1 + .../CSharpCustomCommand/TargetWithCommand.cmake | 4 ++++ 8 files changed, 38 insertions(+), 29 deletions(-) create mode 100644 Tests/RunCMake/CSharpCustomCommand/TargetWithCommand-build-stdout.txt create mode 100644 Tests/RunCMake/CSharpCustomCommand/TargetWithCommand.cmake diff --git a/Source/cmExportFileGenerator.cxx b/Source/cmExportFileGenerator.cxx index 9e6560f..ce9bc87 100644 --- a/Source/cmExportFileGenerator.cxx +++ b/Source/cmExportFileGenerator.cxx @@ -788,7 +788,7 @@ void cmExportFileGenerator::SetImportDetailProperties( std::string propval; if (auto* p = target->GetProperty("COMMON_LANGUAGE_RUNTIME")) { propval = p; - } else if (target->HasLanguage("CSharp", config)) { + } else if (target->IsCSharpOnly()) { // C# projects do not have the /clr flag, so we set the property // here to mark the target as (only) managed (i.e. no .lib file // to link to). Otherwise the COMMON_LANGUAGE_RUNTIME target diff --git a/Source/cmGeneratorTarget.cxx b/Source/cmGeneratorTarget.cxx index 8aab1be..9918b87 100644 --- a/Source/cmGeneratorTarget.cxx +++ b/Source/cmGeneratorTarget.cxx @@ -5221,20 +5221,23 @@ void cmGeneratorTarget::GetLanguages(std::set& languages, } } -bool cmGeneratorTarget::HasLanguage(std::string const& language, - std::string const& config, - bool exclusive) const +bool cmGeneratorTarget::IsCSharpOnly() const { - std::set languages; - this->GetLanguages(languages, config); - // The "exclusive" check applies only to source files and not - // the linker language which may be affected by dependencies. - if (exclusive && languages.size() > 1) { + // Only certain target types may compile CSharp. + if (this->GetType() != cmStateEnums::SHARED_LIBRARY && + this->GetType() != cmStateEnums::STATIC_LIBRARY && + this->GetType() != cmStateEnums::EXECUTABLE) { return false; } - // add linker language (if it is different from compiler languages) - languages.insert(this->GetLinkerLanguage(config)); - return languages.count(language) > 0; + std::set languages; + this->GetLanguages(languages, ""); + // Consider an explicit linker language property, but *not* the + // computed linker language that may depend on linked targets. + const char* linkLang = this->GetProperty("LINKER_LANGUAGE"); + if (linkLang && *linkLang) { + languages.insert(linkLang); + } + return languages.size() == 1 && languages.count("CSharp") > 0; } void cmGeneratorTarget::ComputeLinkImplementationLanguages( @@ -5555,6 +5558,5 @@ cmGeneratorTarget::ManagedType cmGeneratorTarget::GetManagedType( // C# targets are always managed. This language specific check // is added to avoid that the COMMON_LANGUAGE_RUNTIME target property // has to be set manually for C# targets. - return this->HasLanguage("CSharp", config) ? ManagedType::Managed - : ManagedType::Native; + return this->IsCSharpOnly() ? ManagedType::Managed : ManagedType::Native; } diff --git a/Source/cmGeneratorTarget.h b/Source/cmGeneratorTarget.h index 2810887..14197f8 100644 --- a/Source/cmGeneratorTarget.h +++ b/Source/cmGeneratorTarget.h @@ -364,11 +364,7 @@ public: void GetLanguages(std::set& languages, std::string const& config) const; - // Evaluate if the target uses the given language for compilation - // and/or linking. If 'exclusive' is true, 'language' is expected - // to be the only language used in source files for the target. - bool HasLanguage(std::string const& language, std::string const& config, - bool exclusive = true) const; + bool IsCSharpOnly() const; void GetObjectLibrariesCMP0026( std::vector& objlibs) const; diff --git a/Source/cmGlobalVisualStudio71Generator.cxx b/Source/cmGlobalVisualStudio71Generator.cxx index 0b086b0..ba12fac 100644 --- a/Source/cmGlobalVisualStudio71Generator.cxx +++ b/Source/cmGlobalVisualStudio71Generator.cxx @@ -98,7 +98,7 @@ void cmGlobalVisualStudio71Generator::WriteProject(std::ostream& fout, ext = ".vfproj"; project = "Project(\"{6989167D-11E4-40FE-8C1A-2192A86A7E90}\") = \""; } - if (t->HasLanguage("CSharp", "")) { + if (t->IsCSharpOnly()) { ext = ".csproj"; project = "Project(\"{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}\") = \""; } diff --git a/Source/cmVisualStudio10TargetGenerator.cxx b/Source/cmVisualStudio10TargetGenerator.cxx index b8b04ae..c455fa2 100644 --- a/Source/cmVisualStudio10TargetGenerator.cxx +++ b/Source/cmVisualStudio10TargetGenerator.cxx @@ -215,12 +215,11 @@ static bool cmVS10IsTargetsFile(std::string const& path) return cmSystemTools::Strucmp(ext.c_str(), ".targets") == 0; } -static std::string computeProjectFileExtension(cmGeneratorTarget const* t, - const std::string& config) +static std::string computeProjectFileExtension(cmGeneratorTarget const* t) { std::string res; res = ".vcxproj"; - if (t->HasLanguage("CSharp", config)) { + if (t->IsCSharpOnly()) { res = ".csproj"; } return res; @@ -316,8 +315,8 @@ void cmVisualStudio10TargetGenerator::Generate() this->GeneratorTarget->GetProperty("EXTERNAL_MSPROJECT")) { return; } - const std::string ProjectFileExtension = computeProjectFileExtension( - this->GeneratorTarget, *this->Configurations.begin()); + const std::string ProjectFileExtension = + computeProjectFileExtension(this->GeneratorTarget); if (ProjectFileExtension == ".vcxproj") { this->ProjectType = vcxproj; this->Managed = false; @@ -1399,8 +1398,7 @@ void cmVisualStudio10TargetGenerator::WriteGroups() std::string path = this->LocalGenerator->GetCurrentBinaryDirectory(); path += "/"; path += this->Name; - path += computeProjectFileExtension(this->GeneratorTarget, - *this->Configurations.begin()); + path += computeProjectFileExtension(this->GeneratorTarget); path += ".filters"; cmGeneratedFileStream fout(path.c_str()); fout.SetCopyIfDifferent(true); @@ -3734,7 +3732,7 @@ void cmVisualStudio10TargetGenerator::WriteProjectReferences(Elem& e0) path = lg->GetCurrentBinaryDirectory(); path += "/"; path += dt->GetName(); - path += computeProjectFileExtension(dt, *this->Configurations.begin()); + path += computeProjectFileExtension(dt); } ConvertToWindowsSlash(path); Elem e2(e1, "ProjectReference"); @@ -3761,7 +3759,7 @@ void cmVisualStudio10TargetGenerator::WriteProjectReferences(Elem& e0) // Workaround for static library C# targets if (referenceNotManaged && dt->GetType() == cmStateEnums::STATIC_LIBRARY) { - referenceNotManaged = !dt->HasLanguage("CSharp", ""); + referenceNotManaged = !dt->IsCSharpOnly(); } if (referenceNotManaged) { e2.Element("ReferenceOutputAssembly", "false"); diff --git a/Tests/RunCMake/CSharpCustomCommand/RunCMakeTest.cmake b/Tests/RunCMake/CSharpCustomCommand/RunCMakeTest.cmake index fa5618a..ab3e51b 100644 --- a/Tests/RunCMake/CSharpCustomCommand/RunCMakeTest.cmake +++ b/Tests/RunCMake/CSharpCustomCommand/RunCMakeTest.cmake @@ -1,5 +1,13 @@ include(RunCMake) +function(run_TargetWithCommand) + set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/TargetWithCommand-build) + run_cmake(TargetWithCommand) + set(RunCMake_TEST_NO_CLEAN 1) + run_cmake_command(TargetWithCommand-build ${CMAKE_COMMAND} --build . --config Debug) +endfunction() +run_TargetWithCommand() + # Use a single build tree for a few tests without cleaning. set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/CommandWithOutput-build) set(RunCMake_TEST_NO_CLEAN 1) diff --git a/Tests/RunCMake/CSharpCustomCommand/TargetWithCommand-build-stdout.txt b/Tests/RunCMake/CSharpCustomCommand/TargetWithCommand-build-stdout.txt new file mode 100644 index 0000000..c212a8f --- /dev/null +++ b/Tests/RunCMake/CSharpCustomCommand/TargetWithCommand-build-stdout.txt @@ -0,0 +1 @@ +Custom target with CSharp source diff --git a/Tests/RunCMake/CSharpCustomCommand/TargetWithCommand.cmake b/Tests/RunCMake/CSharpCustomCommand/TargetWithCommand.cmake new file mode 100644 index 0000000..fdaea5c --- /dev/null +++ b/Tests/RunCMake/CSharpCustomCommand/TargetWithCommand.cmake @@ -0,0 +1,4 @@ +enable_language(CSharp) + +add_custom_target(drive ALL SOURCES dummy.cs + COMMAND ${CMAKE_COMMAND} -E echo "Custom target with CSharp source") -- cgit v0.12