From 22d3eb5d5e4ce9c6371ab709655928552453fbda Mon Sep 17 00:00:00 2001 From: Brad King Date: Fri, 12 Jul 2019 11:22:36 -0400 Subject: Refactor checks for whether a target has an import library Use `HasImportLibrary` for such checks. --- Source/cmComputeLinkInformation.cxx | 21 ++++++--------------- Source/cmComputeLinkInformation.h | 1 - Source/cmExportBuildFileGenerator.cxx | 9 +++++---- Source/cmFileAPICodemodel.cxx | 17 ++++++++--------- Source/cmGeneratorTarget.cxx | 33 +++++++++++++++++++++------------ Source/cmGeneratorTarget.h | 8 ++++++-- Source/cmJsonObjects.cxx | 4 +++- Source/cmLinkLineComputer.cxx | 3 +-- 8 files changed, 50 insertions(+), 46 deletions(-) diff --git a/Source/cmComputeLinkInformation.cxx b/Source/cmComputeLinkInformation.cxx index b366ebb..5a03670 100644 --- a/Source/cmComputeLinkInformation.cxx +++ b/Source/cmComputeLinkInformation.cxx @@ -268,10 +268,6 @@ cmComputeLinkInformation::cmComputeLinkInformation( return; } - // Check whether we should use an import library for linking a target. - this->UseImportLibrary = - this->Makefile->IsDefinitionSet("CMAKE_IMPORT_LIBRARY_SUFFIX"); - // Check whether we should skip dependencies on shared library files. this->LinkDependsNoShared = this->Target->GetPropertyAsBool("LINK_DEPENDS_NO_SHARED"); @@ -280,7 +276,7 @@ cmComputeLinkInformation::cmComputeLinkInformation( // to use when creating a plugin (module) that obtains symbols from // the program that will load it. this->LoaderFlag = nullptr; - if (!this->UseImportLibrary && + if (!this->Target->IsDLLPlatform() && this->Target->GetType() == cmStateEnums::MODULE_LIBRARY) { std::string loader_flag_var = "CMAKE_SHARED_MODULE_LOADER_"; loader_flag_var += this->LinkLanguage; @@ -493,9 +489,7 @@ bool cmComputeLinkInformation::Compute() std::set const& wrongItems = cld.GetOldWrongConfigItems(); for (cmGeneratorTarget const* tgt : wrongItems) { - bool implib = (this->UseImportLibrary && - (tgt->GetType() == cmStateEnums::SHARED_LIBRARY)); - cmStateEnums::ArtifactType artifact = implib + cmStateEnums::ArtifactType artifact = tgt->HasImportLibrary(this->Config) ? cmStateEnums::ImportLibraryArtifact : cmStateEnums::RuntimeBinaryArtifact; this->OldLinkDirItems.push_back( @@ -578,7 +572,7 @@ void cmComputeLinkInformation::AddItem(std::string const& item, // Compute the proper name to use to link this library. const std::string& config = this->Config; bool impexe = (tgt && tgt->IsExecutableWithExports()); - if (impexe && !this->UseImportLibrary && !this->LoaderFlag) { + if (impexe && !tgt->HasImportLibrary(config) && !this->LoaderFlag) { // Skip linking to executables on platforms with no import // libraries or loader flags. return; @@ -592,7 +586,7 @@ void cmComputeLinkInformation::AddItem(std::string const& item, // platform. Add it now. std::string linkItem; linkItem = this->LoaderFlag; - cmStateEnums::ArtifactType artifact = this->UseImportLibrary + cmStateEnums::ArtifactType artifact = tgt->HasImportLibrary(config) ? cmStateEnums::ImportLibraryArtifact : cmStateEnums::RuntimeBinaryArtifact; @@ -616,10 +610,7 @@ void cmComputeLinkInformation::AddItem(std::string const& item, // Its object-files should already have been extracted for linking. } else { // Decide whether to use an import library. - bool implib = - (this->UseImportLibrary && - (impexe || tgt->GetType() == cmStateEnums::SHARED_LIBRARY)); - cmStateEnums::ArtifactType artifact = implib + cmStateEnums::ArtifactType artifact = tgt->HasImportLibrary(config) ? cmStateEnums::ImportLibraryArtifact : cmStateEnums::RuntimeBinaryArtifact; @@ -694,7 +685,7 @@ void cmComputeLinkInformation::AddSharedDepItem(std::string const& item, // linked will be able to find it. std::string lib; if (tgt) { - cmStateEnums::ArtifactType artifact = this->UseImportLibrary + cmStateEnums::ArtifactType artifact = tgt->HasImportLibrary(this->Config) ? cmStateEnums::ImportLibraryArtifact : cmStateEnums::RuntimeBinaryArtifact; lib = tgt->GetFullPath(this->Config, artifact); diff --git a/Source/cmComputeLinkInformation.h b/Source/cmComputeLinkInformation.h index 863639c..3be2c7f 100644 --- a/Source/cmComputeLinkInformation.h +++ b/Source/cmComputeLinkInformation.h @@ -179,7 +179,6 @@ private: bool OldLinkDirMode; bool OpenBSD; bool LinkDependsNoShared; - bool UseImportLibrary; bool RuntimeUseChrpath; bool NoSONameUsesPath; bool LinkWithRuntimePath; diff --git a/Source/cmExportBuildFileGenerator.cxx b/Source/cmExportBuildFileGenerator.cxx index 5800629..de3e0e2 100644 --- a/Source/cmExportBuildFileGenerator.cxx +++ b/Source/cmExportBuildFileGenerator.cxx @@ -236,14 +236,15 @@ void cmExportBuildFileGenerator::SetImportLocationProperty( } // Add the import library for windows DLLs. - if (target->HasImportLibrary(config) && - mf->GetDefinition("CMAKE_IMPORT_LIBRARY_SUFFIX")) { + if (target->HasImportLibrary(config)) { std::string prop = "IMPORTED_IMPLIB"; prop += suffix; std::string value = target->GetFullPath(config, cmStateEnums::ImportLibraryArtifact); - target->GetImplibGNUtoMS(config, value, value, - "${CMAKE_IMPORT_LIBRARY_SUFFIX}"); + if (mf->GetDefinition("CMAKE_IMPORT_LIBRARY_SUFFIX")) { + target->GetImplibGNUtoMS(config, value, value, + "${CMAKE_IMPORT_LIBRARY_SUFFIX}"); + } properties[prop] = value; } } diff --git a/Source/cmFileAPICodemodel.cxx b/Source/cmFileAPICodemodel.cxx index 73b4123..7b916cd 100644 --- a/Source/cmFileAPICodemodel.cxx +++ b/Source/cmFileAPICodemodel.cxx @@ -1076,17 +1076,16 @@ Json::Value Target::DumpArtifacts() } // Add Windows-specific artifacts produced by the linker. + if (this->GT->HasImportLibrary(this->Config)) { + Json::Value artifact = Json::objectValue; + artifact["path"] = + RelativeIfUnder(this->TopBuild, + this->GT->GetFullPath( + this->Config, cmStateEnums::ImportLibraryArtifact)); + artifacts.append(std::move(artifact)); // NOLINT(*) + } if (this->GT->IsDLLPlatform() && this->GT->GetType() != cmStateEnums::STATIC_LIBRARY) { - if (this->GT->GetType() == cmStateEnums::SHARED_LIBRARY || - this->GT->IsExecutableWithExports()) { - Json::Value artifact = Json::objectValue; - artifact["path"] = - RelativeIfUnder(this->TopBuild, - this->GT->GetFullPath( - this->Config, cmStateEnums::ImportLibraryArtifact)); - artifacts.append(std::move(artifact)); // NOLINT(*) - } cmGeneratorTarget::OutputInfo const* output = this->GT->GetOutputInfo(this->Config); if (output && !output->PdbDir.empty()) { diff --git a/Source/cmGeneratorTarget.cxx b/Source/cmGeneratorTarget.cxx index 845937a..54f6e80 100644 --- a/Source/cmGeneratorTarget.cxx +++ b/Source/cmGeneratorTarget.cxx @@ -468,7 +468,7 @@ std::string cmGeneratorTarget::GetFilePrefix( const std::string& config, cmStateEnums::ArtifactType artifact) const { if (this->IsImported()) { - const char* prefix = this->GetFilePrefixInternal(artifact); + const char* prefix = this->GetFilePrefixInternal(config, artifact); return prefix ? prefix : std::string(); } @@ -481,7 +481,7 @@ std::string cmGeneratorTarget::GetFileSuffix( const std::string& config, cmStateEnums::ArtifactType artifact) const { if (this->IsImported()) { - const char* suffix = this->GetFileSuffixInternal(artifact); + const char* suffix = this->GetFileSuffixInternal(config, artifact); return suffix ? suffix : std::string(); } @@ -508,7 +508,8 @@ std::string cmGeneratorTarget::GetFilePostfix(const std::string& config) const } const char* cmGeneratorTarget::GetFilePrefixInternal( - cmStateEnums::ArtifactType artifact, const std::string& language) const + std::string const& config, cmStateEnums::ArtifactType artifact, + const std::string& language) const { // no prefix for non-main target types. if (this->GetType() != cmStateEnums::STATIC_LIBRARY && @@ -523,8 +524,7 @@ const char* cmGeneratorTarget::GetFilePrefixInternal( // Return an empty prefix for the import library if this platform // does not support import libraries. - if (isImportedLibraryArtifact && - !this->Makefile->GetDefinition("CMAKE_IMPORT_LIBRARY_SUFFIX")) { + if (isImportedLibraryArtifact && !this->NeedImportLibraryName(config)) { return nullptr; } @@ -558,7 +558,8 @@ const char* cmGeneratorTarget::GetFilePrefixInternal( return targetPrefix; } const char* cmGeneratorTarget::GetFileSuffixInternal( - cmStateEnums::ArtifactType artifact, const std::string& language) const + std::string const& config, cmStateEnums::ArtifactType artifact, + const std::string& language) const { // no suffix for non-main target types. if (this->GetType() != cmStateEnums::STATIC_LIBRARY && @@ -573,8 +574,7 @@ const char* cmGeneratorTarget::GetFileSuffixInternal( // Return an empty suffix for the import library if this platform // does not support import libraries. - if (isImportedLibraryArtifact && - !this->Makefile->GetDefinition("CMAKE_IMPORT_LIBRARY_SUFFIX")) { + if (isImportedLibraryArtifact && !this->NeedImportLibraryName(config)) { return nullptr; } @@ -3924,8 +3924,7 @@ void cmGeneratorTarget::GetFullNameInternal( // Return an empty name for the import library if this platform // does not support import libraries. - if (isImportedLibraryArtifact && - !this->Makefile->GetDefinition("CMAKE_IMPORT_LIBRARY_SUFFIX")) { + if (isImportedLibraryArtifact && !this->NeedImportLibraryName(config)) { outPrefix.clear(); outBase.clear(); outSuffix.clear(); @@ -3934,8 +3933,8 @@ void cmGeneratorTarget::GetFullNameInternal( // retrieve prefix and suffix std::string ll = this->GetLinkerLanguage(config); - const char* targetPrefix = this->GetFilePrefixInternal(artifact, ll); - const char* targetSuffix = this->GetFileSuffixInternal(artifact, ll); + const char* targetPrefix = this->GetFilePrefixInternal(config, artifact, ll); + const char* targetSuffix = this->GetFileSuffixInternal(config, artifact, ll); // The implib option is only allowed for shared libraries, module // libraries, and executables. @@ -6363,6 +6362,16 @@ bool cmGeneratorTarget::HasImportLibrary(std::string const& config) const this->GetManagedType(config) != ManagedType::Managed); } +bool cmGeneratorTarget::NeedImportLibraryName(std::string const& config) const +{ + return this->HasImportLibrary(config) || + // On DLL platforms we always generate the import library name + // just in case the sources have export markup. + (this->IsDLLPlatform() && + (this->GetType() == cmStateEnums::EXECUTABLE || + this->GetType() == cmStateEnums::MODULE_LIBRARY)); +} + std::string cmGeneratorTarget::GetSupportDirectory() const { std::string dir = this->LocalGenerator->GetCurrentBinaryDirectory(); diff --git a/Source/cmGeneratorTarget.h b/Source/cmGeneratorTarget.h index 627a055..852f8fd 100644 --- a/Source/cmGeneratorTarget.h +++ b/Source/cmGeneratorTarget.h @@ -743,9 +743,13 @@ private: mutable std::map DebugCompatiblePropertiesDone; - const char* GetFilePrefixInternal(cmStateEnums::ArtifactType artifact, + bool NeedImportLibraryName(std::string const& config) const; + + const char* GetFilePrefixInternal(std::string const& config, + cmStateEnums::ArtifactType artifact, const std::string& language = "") const; - const char* GetFileSuffixInternal(cmStateEnums::ArtifactType artifact, + const char* GetFileSuffixInternal(std::string const& config, + cmStateEnums::ArtifactType artifact, const std::string& language = "") const; std::string GetFullNameInternal(const std::string& config, diff --git a/Source/cmJsonObjects.cxx b/Source/cmJsonObjects.cxx index 505188f..2423faf 100644 --- a/Source/cmJsonObjects.cxx +++ b/Source/cmJsonObjects.cxx @@ -516,9 +516,11 @@ static Json::Value DumpTarget(cmGeneratorTarget* target, Json::Value artifacts = Json::arrayValue; artifacts.append( target->GetFullPath(config, cmStateEnums::RuntimeBinaryArtifact)); - if (target->IsDLLPlatform()) { + if (target->HasImportLibrary(config)) { artifacts.append( target->GetFullPath(config, cmStateEnums::ImportLibraryArtifact)); + } + if (target->IsDLLPlatform()) { const cmGeneratorTarget::OutputInfo* output = target->GetOutputInfo(config); if (output && !output->PdbDir.empty()) { diff --git a/Source/cmLinkLineComputer.cxx b/Source/cmLinkLineComputer.cxx index 469faca..8746b35 100644 --- a/Source/cmLinkLineComputer.cxx +++ b/Source/cmLinkLineComputer.cxx @@ -110,8 +110,7 @@ std::string cmLinkLineComputer::ComputeLinkPath( if (target->GetType() == cmStateEnums::STATIC_LIBRARY || target->GetType() == cmStateEnums::SHARED_LIBRARY) { cmStateEnums::ArtifactType type = cmStateEnums::RuntimeBinaryArtifact; - if (target->GetType() == cmStateEnums::SHARED_LIBRARY && - target->IsDLLPlatform()) { + if (target->HasImportLibrary(cli.GetConfig())) { type = cmStateEnums::ImportLibraryArtifact; } -- cgit v0.12 From 79f5ef19fe418efe2d9bfc55fcd16dfd75f18d49 Mon Sep 17 00:00:00 2001 From: Brad King Date: Fri, 12 Jul 2019 13:36:15 -0400 Subject: De-duplicate checks for whether a platform uses Windows DLLs --- Source/cmGeneratorTarget.cxx | 5 +---- Source/cmGeneratorTarget.h | 1 - Source/cmInstallCommand.cxx | 8 ++------ Source/cmTarget.cxx | 13 +++++++++---- Source/cmTarget.h | 3 +++ 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/Source/cmGeneratorTarget.cxx b/Source/cmGeneratorTarget.cxx index 54f6e80..c807f6d 100644 --- a/Source/cmGeneratorTarget.cxx +++ b/Source/cmGeneratorTarget.cxx @@ -259,9 +259,6 @@ cmGeneratorTarget::cmGeneratorTarget(cmTarget* t, cmLocalGenerator* lg) t->GetSourceBacktraces(), this->SourceEntries, true); - this->DLLPlatform = - !this->Makefile->GetSafeDefinition("CMAKE_IMPORT_LIBRARY_SUFFIX").empty(); - this->PolicyMap = t->GetPolicyMap(); } @@ -2357,7 +2354,7 @@ void cmGeneratorTarget::ComputeModuleDefinitionInfo( bool cmGeneratorTarget::IsDLLPlatform() const { - return this->DLLPlatform; + return this->Target->IsDLLPlatform(); } void cmGeneratorTarget::GetAutoUicOptions(std::vector& result, diff --git a/Source/cmGeneratorTarget.h b/Source/cmGeneratorTarget.h index 852f8fd..e86535d 100644 --- a/Source/cmGeneratorTarget.h +++ b/Source/cmGeneratorTarget.h @@ -913,7 +913,6 @@ private: mutable bool DebugSourcesDone; mutable bool LinkImplementationLanguageIsContextDependent; mutable bool UtilityItemsDone; - bool DLLPlatform; bool ComputePDBOutputDir(const std::string& kind, const std::string& config, std::string& out) const; diff --git a/Source/cmInstallCommand.cxx b/Source/cmInstallCommand.cxx index dba4bbb..750ed8c 100644 --- a/Source/cmInstallCommand.cxx +++ b/Source/cmInstallCommand.cxx @@ -389,10 +389,6 @@ bool cmInstallCommand::HandleTargetsMode(std::vector const& args) return true; } - // Check whether this is a DLL platform. - bool dll_platform = - !this->Makefile->GetSafeDefinition("CMAKE_IMPORT_LIBRARY_SUFFIX").empty(); - for (std::string const& tgt : targetList) { if (this->Makefile->IsAlias(tgt)) { @@ -472,7 +468,7 @@ bool cmInstallCommand::HandleTargetsMode(std::vector const& args) // Shared libraries are handled differently on DLL and non-DLL // platforms. All windows platforms are DLL platforms including // cygwin. Currently no other platform is a DLL platform. - if (dll_platform) { + if (target.IsDLLPlatform()) { // When in namelink only mode skip all libraries on Windows. if (namelinkMode == cmInstallTargetGenerator::NamelinkModeOnly) { continue; @@ -641,7 +637,7 @@ bool cmInstallCommand::HandleTargetsMode(std::vector const& args) // On DLL platforms an executable may also have an import // library. Install it to the archive destination if it // exists. - if (dll_platform && !archiveArgs.GetDestination().empty() && + if (target.IsDLLPlatform() && !archiveArgs.GetDestination().empty() && target.IsExecutableWithExports()) { // The import library uses the ARCHIVE properties. archiveGenerator = CreateInstallTargetGenerator( diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx index b10b30f..2237d9b 100644 --- a/Source/cmTarget.cxx +++ b/Source/cmTarget.cxx @@ -168,7 +168,7 @@ public: cmPropertyMap Properties; bool IsGeneratorProvided; bool HaveInstallRule; - bool DLLPlatform; + bool IsDLLPlatform; bool IsAndroid; bool IsImportedTarget; bool ImportedGloballyVisible; @@ -216,7 +216,7 @@ cmTarget::cmTarget(std::string const& name, cmStateEnums::TargetType type, impl->Name = name; impl->IsGeneratorProvided = false; impl->HaveInstallRule = false; - impl->DLLPlatform = false; + impl->IsDLLPlatform = false; impl->IsAndroid = false; impl->IsImportedTarget = (vis == VisibilityImported || vis == VisibilityImportedGlobally); @@ -224,7 +224,7 @@ cmTarget::cmTarget(std::string const& name, cmStateEnums::TargetType type, impl->BuildInterfaceIncludesAppended = false; // Check whether this is a DLL platform. - impl->DLLPlatform = + impl->IsDLLPlatform = !impl->Makefile->GetSafeDefinition("CMAKE_IMPORT_LIBRARY_SUFFIX").empty(); // Check whether we are targeting an Android platform. @@ -1657,6 +1657,11 @@ cmPropertyMap const& cmTarget::GetProperties() const return impl->Properties; } +bool cmTarget::IsDLLPlatform() const +{ + return impl->IsDLLPlatform; +} + bool cmTarget::IsImported() const { return impl->IsImportedTarget; @@ -1872,7 +1877,7 @@ bool cmTarget::GetMappedConfig(std::string const& desired_config, // If we needed to find one of the mapped configurations but did not // On a DLL platform there may be only IMPORTED_IMPLIB for a shared // library or an executable with exports. - bool allowImp = (impl->DLLPlatform && + bool allowImp = (this->IsDLLPlatform() && (this->GetType() == cmStateEnums::SHARED_LIBRARY || this->IsExecutableWithExports())); diff --git a/Source/cmTarget.h b/Source/cmTarget.h index f47cc14..d9e87b8 100644 --- a/Source/cmTarget.h +++ b/Source/cmTarget.h @@ -177,6 +177,9 @@ public: //! Get all properties cmPropertyMap const& GetProperties() const; + //! Return whether or not the target is for a DLL platform. + bool IsDLLPlatform() const; + bool IsImported() const; bool IsImportedGloballyVisible() const; -- cgit v0.12 From 7ff9ab3b101b131bd4a91a459f66ce27f97f4757 Mon Sep 17 00:00:00 2001 From: Brad King Date: Fri, 12 Jul 2019 12:37:28 -0400 Subject: Makefile: De-duplicate executable link rule lookup Use `GetCreateRuleVariable` instead of duplicating the variable lookup. --- Source/cmMakefileExecutableTargetGenerator.cxx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Source/cmMakefileExecutableTargetGenerator.cxx b/Source/cmMakefileExecutableTargetGenerator.cxx index 2001c1f..2950bfb 100644 --- a/Source/cmMakefileExecutableTargetGenerator.cxx +++ b/Source/cmMakefileExecutableTargetGenerator.cxx @@ -482,9 +482,8 @@ void cmMakefileExecutableTargetGenerator::WriteExecutableRule(bool relink) // Construct the main link rule. std::vector real_link_commands; - std::string linkRuleVar = "CMAKE_"; - linkRuleVar += linkLanguage; - linkRuleVar += "_LINK_EXECUTABLE"; + std::string linkRuleVar = this->GeneratorTarget->GetCreateRuleVariable( + linkLanguage, this->ConfigName); std::string linkRule = this->GetLinkRule(linkRuleVar); std::vector commands1; cmSystemTools::ExpandListArgument(linkRule, real_link_commands); -- cgit v0.12