From 85457b63c859f238ef324757f1042cc846cedbc7 Mon Sep 17 00:00:00 2001 From: Deniz Bahadir Date: Mon, 27 Nov 2017 22:02:43 +0100 Subject: target_link_libraries: Return earlier on some error. --- Source/cmTargetLinkLibrariesCommand.cxx | 34 ++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/Source/cmTargetLinkLibrariesCommand.cxx b/Source/cmTargetLinkLibrariesCommand.cxx index 37bcb70..ec060cd 100644 --- a/Source/cmTargetLinkLibrariesCommand.cxx +++ b/Source/cmTargetLinkLibrariesCommand.cxx @@ -388,27 +388,27 @@ bool cmTargetLinkLibrariesCommand::HandleLibrary(const std::string& lib, << this->Target->GetName() << "\" which is not built in this directory."; this->Makefile->IssueMessage(cmake::FATAL_ERROR, e.str()); - } else { + return false; + } - cmTarget* tgt = this->Makefile->GetGlobalGenerator()->FindTarget(lib); - - if (tgt && (tgt->GetType() != cmStateEnums::STATIC_LIBRARY) && - (tgt->GetType() != cmStateEnums::SHARED_LIBRARY) && - (tgt->GetType() != cmStateEnums::UNKNOWN_LIBRARY) && - (tgt->GetType() != cmStateEnums::INTERFACE_LIBRARY) && - !tgt->IsExecutableWithExports()) { - std::ostringstream e; - e << "Target \"" << lib << "\" of type " - << cmState::GetTargetTypeName(tgt->GetType()) - << " may not be linked into another target. " - << "One may link only to STATIC or SHARED libraries, or " - << "to executables with the ENABLE_EXPORTS property set."; - this->Makefile->IssueMessage(cmake::FATAL_ERROR, e.str()); - } + cmTarget* tgt = this->Makefile->GetGlobalGenerator()->FindTarget(lib); - this->Target->AddLinkLibrary(*this->Makefile, lib, llt); + if (tgt && (tgt->GetType() != cmStateEnums::STATIC_LIBRARY) && + (tgt->GetType() != cmStateEnums::SHARED_LIBRARY) && + (tgt->GetType() != cmStateEnums::UNKNOWN_LIBRARY) && + (tgt->GetType() != cmStateEnums::INTERFACE_LIBRARY) && + !tgt->IsExecutableWithExports()) { + std::ostringstream e; + e << "Target \"" << lib << "\" of type " + << cmState::GetTargetTypeName(tgt->GetType()) + << " may not be linked into another target. " + << "One may link only to STATIC or SHARED libraries, or " + << "to executables with the ENABLE_EXPORTS property set."; + this->Makefile->IssueMessage(cmake::FATAL_ERROR, e.str()); } + this->Target->AddLinkLibrary(*this->Makefile, lib, llt); + if (this->CurrentProcessingState == ProcessingLinkLibraries) { this->Target->AppendProperty( "INTERFACE_LINK_LIBRARIES", -- cgit v0.12 From b0e2f1415e680337f4b70b19e319efeba0eb6f12 Mon Sep 17 00:00:00 2001 From: Deniz Bahadir Date: Mon, 27 Nov 2017 22:11:11 +0100 Subject: target_link_libraries: Slightly fix some error-messages. Some error-messages are slightly adjusted to better tell what invocation would be correct instead. Tests are adjusted accordingly. --- Source/cmTargetLinkLibrariesCommand.cxx | 26 +++++++++++----------- .../RunCMake/ObjectLibrary/LinkObjRHS1-stderr.txt | 4 ++-- .../MixedSignature-stderr.txt | 4 ++-- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/Source/cmTargetLinkLibrariesCommand.cxx b/Source/cmTargetLinkLibrariesCommand.cxx index ec060cd..65ab544 100644 --- a/Source/cmTargetLinkLibrariesCommand.cxx +++ b/Source/cmTargetLinkLibrariesCommand.cxx @@ -160,8 +160,9 @@ bool cmTargetLinkLibrariesCommand::InitialPass( this->CurrentProcessingState != ProcessingKeywordPublicInterface && this->CurrentProcessingState != ProcessingKeywordLinkInterface) { this->Makefile->IssueMessage( - cmake::FATAL_ERROR, "The INTERFACE option must appear as the second " - "argument, just after the target name."); + cmake::FATAL_ERROR, + "The INTERFACE, PUBLIC or PRIVATE option must appear as the second " + "argument, just after the target name."); return true; } this->CurrentProcessingState = ProcessingKeywordLinkInterface; @@ -183,7 +184,7 @@ bool cmTargetLinkLibrariesCommand::InitialPass( this->CurrentProcessingState != ProcessingKeywordLinkInterface) { this->Makefile->IssueMessage( cmake::FATAL_ERROR, - "The PUBLIC or PRIVATE option must appear as the second " + "The INTERFACE, PUBLIC or PRIVATE option must appear as the second " "argument, just after the target name."); return true; } @@ -206,7 +207,7 @@ bool cmTargetLinkLibrariesCommand::InitialPass( this->CurrentProcessingState != ProcessingKeywordLinkInterface) { this->Makefile->IssueMessage( cmake::FATAL_ERROR, - "The PUBLIC or PRIVATE option must appear as the second " + "The INTERFACE, PUBLIC or PRIVATE option must appear as the second " "argument, just after the target name."); return true; } @@ -349,12 +350,11 @@ bool cmTargetLinkLibrariesCommand::HandleLibrary(const std::string& lib, // form must be the plain form. const char* existingSig = (sig == cmTarget::KeywordTLLSignature ? "plain" : "keyword"); - e << "The " << existingSig << " signature for target_link_libraries " - "has already been used with the target \"" - << this->Target->GetName() << "\". All uses of " - "target_link_libraries with a target " - << modal << " be either " - "all-keyword or all-plain.\n"; + e << "The " << existingSig << " signature for target_link_libraries has " + "already been used with the target \"" + << this->Target->GetName() + << "\". All uses of target_link_libraries with a target " << modal + << " be either all-keyword or all-plain.\n"; this->Target->GetTllSignatureTraces(e, sig == cmTarget::KeywordTLLSignature ? cmTarget::PlainTLLSignature @@ -401,9 +401,9 @@ bool cmTargetLinkLibrariesCommand::HandleLibrary(const std::string& lib, std::ostringstream e; e << "Target \"" << lib << "\" of type " << cmState::GetTargetTypeName(tgt->GetType()) - << " may not be linked into another target. " - << "One may link only to STATIC or SHARED libraries, or " - << "to executables with the ENABLE_EXPORTS property set."; + << " may not be linked into another target. One may link only to " + "INTERFACE, STATIC or SHARED libraries, or to executables with the " + "ENABLE_EXPORTS property set."; this->Makefile->IssueMessage(cmake::FATAL_ERROR, e.str()); } diff --git a/Tests/RunCMake/ObjectLibrary/LinkObjRHS1-stderr.txt b/Tests/RunCMake/ObjectLibrary/LinkObjRHS1-stderr.txt index 8809f89..d5ee4f9 100644 --- a/Tests/RunCMake/ObjectLibrary/LinkObjRHS1-stderr.txt +++ b/Tests/RunCMake/ObjectLibrary/LinkObjRHS1-stderr.txt @@ -1,6 +1,6 @@ CMake Error at LinkObjRHS1.cmake:3 \(target_link_libraries\): Target "AnObjLib" of type OBJECT_LIBRARY may not be linked into another - target. One may link only to STATIC or SHARED libraries, or to executables - with the ENABLE_EXPORTS property set. + target. One may link only to INTERFACE, STATIC or SHARED libraries, or to + executables with the ENABLE_EXPORTS property set. Call Stack \(most recent call first\): CMakeLists.txt:3 \(include\) diff --git a/Tests/RunCMake/target_link_libraries/MixedSignature-stderr.txt b/Tests/RunCMake/target_link_libraries/MixedSignature-stderr.txt index a0c66db..c6237f4 100644 --- a/Tests/RunCMake/target_link_libraries/MixedSignature-stderr.txt +++ b/Tests/RunCMake/target_link_libraries/MixedSignature-stderr.txt @@ -1,5 +1,5 @@ CMake Error at MixedSignature.cmake:6 \(target_link_libraries\): - The PUBLIC or PRIVATE option must appear as the second argument, just after - the target name. + The INTERFACE, PUBLIC or PRIVATE option must appear as the second argument, + just after the target name. Call Stack \(most recent call first\): CMakeLists.txt:3 \(include\) -- cgit v0.12 From 8112059ee7f38f0eb5cdcaba8185aec659abcbfc Mon Sep 17 00:00:00 2001 From: Deniz Bahadir Date: Mon, 27 Nov 2017 22:30:26 +0100 Subject: target_link_libraries: Simplify implementation and add comments. The implementation of `target_link_libraries` did grow over the years when new features where added. This commit cleans up the implementation and adds comments to better document its intention. The behavior of `target_link_libraries` itself is left untouched. --- Source/cmTargetLinkLibrariesCommand.cxx | 121 ++++++++++++++++++-------------- Source/cmTargetLinkLibrariesCommand.h | 3 + 2 files changed, 70 insertions(+), 54 deletions(-) diff --git a/Source/cmTargetLinkLibrariesCommand.cxx b/Source/cmTargetLinkLibrariesCommand.cxx index 65ab544..97bb0a2 100644 --- a/Source/cmTargetLinkLibrariesCommand.cxx +++ b/Source/cmTargetLinkLibrariesCommand.cxx @@ -25,16 +25,17 @@ const char* cmTargetLinkLibrariesCommand::LinkLibraryTypeNames[3] = { bool cmTargetLinkLibrariesCommand::InitialPass( std::vector const& args, cmExecutionStatus&) { - // must have one argument + // Must have at least one argument. if (args.empty()) { this->SetError("called with incorrect number of arguments"); return false; } - + // Alias targets cannot be on the LHS of this command. if (this->Makefile->IsAlias(args[0])) { this->SetError("can not be used on an ALIAS target."); return false; } + // Lookup the target for which libraries are specified. this->Target = this->Makefile->GetCMakeInstance()->GetGlobalGenerator()->FindTarget( @@ -78,8 +79,7 @@ bool cmTargetLinkLibrariesCommand::InitialPass( break; } } - - // now actually print the message + // Now actually print the message. switch (t) { case cmake::AUTHOR_WARNING: this->Makefile->IssueMessage(cmake::AUTHOR_WARNING, e.str()); @@ -94,6 +94,7 @@ bool cmTargetLinkLibrariesCommand::InitialPass( return true; } + // OBJECT libraries are not allowed on the LHS of the command. if (this->Target->GetType() == cmStateEnums::OBJECT_LIBRARY) { std::ostringstream e; e << "Object library target \"" << args[0] << "\" " @@ -103,6 +104,7 @@ bool cmTargetLinkLibrariesCommand::InitialPass( return true; } + // Having a UTILITY library on the LHS is a bug. if (this->Target->GetType() == cmStateEnums::UTILITY) { std::ostringstream e; const char* modal = nullptr; @@ -129,7 +131,7 @@ bool cmTargetLinkLibrariesCommand::InitialPass( } } - // but we might not have any libs after variable expansion + // But we might not have any libs after variable expansion. if (args.size() < 2) { return true; } @@ -142,8 +144,8 @@ bool cmTargetLinkLibrariesCommand::InitialPass( // specification if the keyword is encountered as the first argument. this->CurrentProcessingState = ProcessingLinkLibraries; - // add libraries, note that there is an optional prefix - // of debug and optimized that can be used + // Add libraries, note that there is an optional prefix + // of debug and optimized that can be used. for (unsigned int i = 1; i < args.size(); ++i) { if (args[i] == "LINK_INTERFACE_LIBRARIES") { this->CurrentProcessingState = ProcessingPlainLinkInterface; @@ -366,10 +368,13 @@ bool cmTargetLinkLibrariesCommand::HandleLibrary(const std::string& lib, } } - // Handle normal case first. + // Handle normal case where the command was called with another keyword than + // INTERFACE / LINK_INTERFACE_LIBRARIES or none at all. (The "LINK_LIBRARIES" + // property of the target on the LHS shall be populated.) if (this->CurrentProcessingState != ProcessingKeywordLinkInterface && this->CurrentProcessingState != ProcessingPlainLinkInterface) { + // Assure that the target on the LHS was created in the current directory. cmTarget* t = this->Makefile->FindLocalNonAliasTarget(this->Target->GetName()); if (!t) { @@ -408,72 +413,80 @@ bool cmTargetLinkLibrariesCommand::HandleLibrary(const std::string& lib, } this->Target->AddLinkLibrary(*this->Makefile, lib, llt); + } - if (this->CurrentProcessingState == ProcessingLinkLibraries) { - this->Target->AppendProperty( - "INTERFACE_LINK_LIBRARIES", - this->Target->GetDebugGeneratorExpressions(lib, llt).c_str()); - return true; - } - if (this->CurrentProcessingState != ProcessingKeywordPublicInterface && - this->CurrentProcessingState != ProcessingPlainPublicInterface) { - if (this->Target->GetType() == cmStateEnums::STATIC_LIBRARY) { - std::string configLib = - this->Target->GetDebugGeneratorExpressions(lib, llt); - if (cmGeneratorExpression::IsValidTargetName(lib) || - cmGeneratorExpression::Find(lib) != std::string::npos) { - configLib = "$"; - } - this->Target->AppendProperty("INTERFACE_LINK_LIBRARIES", - configLib.c_str()); + // Handle (additional) case where the command was called with PRIVATE / + // LINK_PRIVATE and stop its processing. (The "INTERFACE_LINK_LIBRARIES" + // property of the target on the LHS shall only be populated if it is a + // STATIC library.) + if (this->CurrentProcessingState == ProcessingKeywordPrivateInterface || + this->CurrentProcessingState == ProcessingPlainPrivateInterface) { + if (this->Target->GetType() == cmStateEnums::STATIC_LIBRARY) { + std::string configLib = + this->Target->GetDebugGeneratorExpressions(lib, llt); + if (cmGeneratorExpression::IsValidTargetName(lib) || + cmGeneratorExpression::Find(lib) != std::string::npos) { + configLib = "$"; } - // Not a 'public' or 'interface' library. Do not add to interface - // property. - return true; + this->Target->AppendProperty("INTERFACE_LINK_LIBRARIES", + configLib.c_str()); } + return true; } + // Handle general case where the command was called with another keyword than + // PRIVATE / LINK_PRIVATE or none at all. (The "INTERFACE_LINK_LIBRARIES" + // property of the target on the LHS shall be populated.) this->Target->AppendProperty( "INTERFACE_LINK_LIBRARIES", this->Target->GetDebugGeneratorExpressions(lib, llt).c_str()); + // Stop processing if called without any keyword. + if (this->CurrentProcessingState == ProcessingLinkLibraries) { + return true; + } + // Stop processing if policy CMP0022 is set to NEW. const cmPolicies::PolicyStatus policy22Status = this->Target->GetPolicyStatusCMP0022(); - if (policy22Status != cmPolicies::OLD && policy22Status != cmPolicies::WARN) { return true; } - + // Stop processing if called with an INTERFACE library on the LHS. if (this->Target->GetType() == cmStateEnums::INTERFACE_LIBRARY) { return true; } - // Get the list of configurations considered to be DEBUG. - std::vector debugConfigs = - this->Makefile->GetCMakeInstance()->GetDebugConfigs(); - std::string prop; - - // Include this library in the link interface for the target. - if (llt == DEBUG_LibraryType || llt == GENERAL_LibraryType) { - // Put in the DEBUG configuration interfaces. - for (std::string const& dc : debugConfigs) { - prop = "LINK_INTERFACE_LIBRARIES_"; - prop += dc; - this->Target->AppendProperty(prop, lib.c_str()); + // Handle (additional) backward-compatibility case where the command was + // called with PUBLIC / INTERFACE / LINK_PUBLIC / LINK_INTERFACE_LIBRARIES. + // (The policy CMP0022 is not set to NEW.) + { + // Get the list of configurations considered to be DEBUG. + std::vector debugConfigs = + this->Makefile->GetCMakeInstance()->GetDebugConfigs(); + std::string prop; + + // Include this library in the link interface for the target. + if (llt == DEBUG_LibraryType || llt == GENERAL_LibraryType) { + // Put in the DEBUG configuration interfaces. + for (std::string const& dc : debugConfigs) { + prop = "LINK_INTERFACE_LIBRARIES_"; + prop += dc; + this->Target->AppendProperty(prop, lib.c_str()); + } } - } - if (llt == OPTIMIZED_LibraryType || llt == GENERAL_LibraryType) { - // Put in the non-DEBUG configuration interfaces. - this->Target->AppendProperty("LINK_INTERFACE_LIBRARIES", lib.c_str()); - - // Make sure the DEBUG configuration interfaces exist so that the - // general one will not be used as a fall-back. - for (std::string const& dc : debugConfigs) { - prop = "LINK_INTERFACE_LIBRARIES_"; - prop += dc; - if (!this->Target->GetProperty(prop)) { - this->Target->SetProperty(prop, ""); + if (llt == OPTIMIZED_LibraryType || llt == GENERAL_LibraryType) { + // Put in the non-DEBUG configuration interfaces. + this->Target->AppendProperty("LINK_INTERFACE_LIBRARIES", lib.c_str()); + + // Make sure the DEBUG configuration interfaces exist so that the + // general one will not be used as a fall-back. + for (std::string const& dc : debugConfigs) { + prop = "LINK_INTERFACE_LIBRARIES_"; + prop += dc; + if (!this->Target->GetProperty(prop)) { + this->Target->SetProperty(prop, ""); + } } } } diff --git a/Source/cmTargetLinkLibrariesCommand.h b/Source/cmTargetLinkLibrariesCommand.h index f41af49..a2f3ecd 100644 --- a/Source/cmTargetLinkLibrariesCommand.h +++ b/Source/cmTargetLinkLibrariesCommand.h @@ -20,6 +20,9 @@ class cmTarget; * cmTargetLinkLibrariesCommand is used to specify a list of libraries to link * into executable(s) or shared objects. The names of the libraries * should be those defined by the LIBRARY(library) command(s). + * + * Additionally, it allows to propagate usage-requirements (including link + * libraries) from one target into another. */ class cmTargetLinkLibrariesCommand : public cmCommand { -- cgit v0.12