(0038027)
Brad King (manager)
2015-02-23 14:41

Right, the $<LINK_ONLY:...> expression says to link without the rest of the usage requirements. The intersection of that functionality with INTERFACE libraries is the empty set.

(0038028)
Ben Boeckel (developer)
2015-02-23 14:48

So it should just skip INTERFACE libraries (whether public or private) rather than even bothering to error when LINK_ONLY is being considered?

(0038029)
Brad King (manager)
2015-02-23 14:53

Re 0015415:0038028: Yes, but it is not immediately obvious how. The LINK_ONLY expression is generated by target_link_libraries here:

http://www.cmake.org/gitweb?p=cmake.git;a=blob;f=Source/cmTargetLinkLibrariesCommand.cxx;hb=v3.2.0-rc2#l431 [^]

when asked to populate a PRIVATE link dependency of a static library. However, at that point it does not know the type of the dependency, which may not even exist yet. Later during generation of the project itself we encounter and evaluate the LINK_ONLY expression, lookup the target, see that it is an INTERFACE library, and discard it.

These LINK_ONLY expressions may have to be filtered during export.

(0038030)
Ben Boeckel (developer)
2015-02-23 14:57

OK, this is what I thought; it didn't look like something I could figure out from right there. Suitable for 3.2?

(0038031)
Brad King (manager)
2015-02-23 15:00

Re 0015415:0038030: Not suitable for 3.2. We're already on the second release candidate. This is not a regression.

(0038032)
Brad King (manager)
2015-02-23 15:06

Re 0015415:0038029: Actually I take this back. The value inside $<LINK_ONLY:...> can be an arbitrary generator expression. Its final value may not be known until it is processed in the consuming project long after export. Furthermore, the INTERFACE library may be a private implementation dependency of the static library because it specifies INTERFACE_LINK_LIBRARIES on real libraries on which the static library depends at link time. We have to preserve this in the exported target.

(0038033)
Ben Boeckel (developer)
2015-02-23 15:34

Actually, in trying to make a test for this, I was able to get it to error out for not exporting a dependent INTERFACE library…now why is my original instance *not* triggering it?

(0038034)
Stephen Kelly (developer)
2015-02-23 15:36

I just noticed the same thing. The difference between my 'error as expected' and 'no error' testcase was the minimum version of CMake 2.8.11/2.8.12. It must be policy behavior (CMP0022).

(0038036)
Ben Boeckel (developer)
2015-02-23 15:54

I have a minimum of 3.1 in this project. No policy settings otherwise.

So what's the plan then? Bring back the error? Trim out known-INTERFACE libraries with no transitive link interface from $<LINK_ONLY> during tll()/export? Make a list of INTERFACE libraries to trim out at use-time? The first doesn't help me (not that I'm not already doomed to either requiring 3.3 or rearchitecting this repo) and the last ones are not pretty. I'm also fine with some other solution.

(0038037)
Stephen Kelly (developer)
2015-02-23 16:09

cmExportFileGenerator::ResolveTargetsInGeneratorExpression could be taught to pierce the veil of LINK_ONLY:

http://www.cmake.org/gitweb?p=cmake.git;a=blob;f=Source/cmExportFileGenerator.cxx;h=71728be2;hb=HEAD#l686 [^]

Even then it's only a best-effort attempt at issuing an error, because LINK_ONLY does not require literal content.

This is also not-an-error:

  add_library(a SHARED bar.cpp)
  add_library(b SHARED foo.cpp)
  target_link_libraries(b PUBLIC $<$<CONFIG:Debug>:a>)
  install(TARGETS b EXPORT foo
    DESTINATION lib
  )
  install(EXPORT foo
    DESTINATION lib/cmake
  )

but the user can write

target_link_libraries(b PUBLIC $<$<CONFIG:Debug>:$<TARGET_NAME:a>>)

So, actually, rather than piercing the veil of LINK_ONLY, maybe this is the basis of a solution:

  diff --git a/Source/cmTargetLinkLibrariesCommand.cxx b/Source/cmTargetLinkLibrariesCommand.cxx
  index 75c94c5..1c303aa 100644
  --- a/Source/cmTargetLinkLibrariesCommand.cxx
  +++ b/Source/cmTargetLinkLibrariesCommand.cxx
  @@ -438,7 +438,7 @@ cmTargetLinkLibrariesCommand::HandleLibrary(const std::string& lib,
          if (cmGeneratorExpression::IsValidTargetName(lib)
              || cmGeneratorExpression::Find(lib) != std::string::npos)
            {
  - configLib = "$<LINK_ONLY:" + configLib + ">";
  + configLib = "$<LINK_ONLY:$<TARGET_NAME:" + configLib + ">>";
            }
          this->Target->AppendProperty("INTERFACE_LINK_LIBRARIES",
                                        configLib.c_str());

(0038038)
Brad King (manager)
2015-02-23 16:22

Re 0015415:0038036: It looks like this error case is suppressed by CMP0022 NEW behavior. In the OLD behavior there is no need for the LINK_ONLY expression and the export set checker successfully identifies the missing export of the dependency of the static library. In the NEW behavior the dependency is hidden behind LINK_ONLY and the export set checker can't evaluate generator expressions to check what might be missing after evaluation. I don't think it is always possible to bring back the error. Trimming or any other processing on LINK_ONLY is not really clean either for the same reason: we can't evaluate generator expressions during export.

Re 0015415:0038037: We can't hard-code $<TARGET_NAME:...> there because that genex requires a literal target name inside it, so

target_link_libraries(b PRIVATE $<1:a>)

would break (of course in practice the genex may be more complex).

Changing Ben's example to:

target_link_libraries(b PRIVATE $<TARGET_NAME:a>)

does in fact restore the error during export even with CMP0022 set to NEW.

(0038039)
Brad King (manager)
2015-02-23 16:24

Re 0015415:0038038: Perhaps target_link_libraries can add $<TARGET_NAME:...> if and only if the configLib already matches a target name. That will restore the error for the basic use case. We shouldn't even need a policy for it because we're just moving the error to export time instead of delaying it until a consuming application tries to use the bad target info.

(0038040)
Brad King (manager)
2015-02-23 16:26

Re 0015415:0038039: Then during export we could pierce $<LINK_ONLY:$<TARGET_NAME:tgt>> specifically and filter it out if "tgt" names an INTERFACE library with no INTERFACE_LINK_LIBRARIES.

(0038042)
Stephen Kelly (developer)
2015-02-23 16:48

There's no need to pierce $<LINK_ONLY:$<TARGET_NAME:tgt>> specifically. The code I linked to already finds $<TARGET_NAME:tgt> because the LHS must be literal in that genex (that's the only purpose of that genex).

(0038043)
Ben Boeckel (developer)
2015-02-23 16:50

That patch has the test case I'm working with. Even with the $<TARGET_NAME:> addition, there's errors about exporting with dependencies on notExportedInterface.

(0038046)
Brad King (manager)
2015-02-24 08:34

Re 0015415:0038042: Yes, just adding TARGET_NAME is sufficient to restore the error message when tll is given a normal target name. I'm saying that on export we can filter out $<LINK_ONLY:$<TARGET_NAME:tgt>> specifically when tgt is known to not be needed. We cannot filter out any more general LINK_ONLY expressions because their evaluation result is not known.

Re 0015415:0038043: Yes, the proposed TARGET_NAME addition is meant to catch such errors. Additional work will be needed to filter them out on export.

(0042718)
Kitware Robot (administrator)
2016-06-10 14:29

Resolving issue as `moved`.

This issue tracker is no longer used. Further discussion of this issue may take place in the current CMake Issues page linked in the banner at the top of this page.

Notes
(0038027) Brad King (manager) 2015-02-23 14:41	Right, the $<LINK_ONLY:...> expression says to link without the rest of the usage requirements. The intersection of that functionality with INTERFACE libraries is the empty set.

(0038028) Ben Boeckel (developer) 2015-02-23 14:48	So it should just skip INTERFACE libraries (whether public or private) rather than even bothering to error when LINK_ONLY is being considered?

(0038029) Brad King (manager) 2015-02-23 14:53	Re 0015415:0038028: Yes, but it is not immediately obvious how. The LINK_ONLY expression is generated by target_link_libraries here: http://www.cmake.org/gitweb?p=cmake.git;a=blob;f=Source/cmTargetLinkLibrariesCommand.cxx;hb=v3.2.0-rc2#l431 [^] when asked to populate a PRIVATE link dependency of a static library. However, at that point it does not know the type of the dependency, which may not even exist yet. Later during generation of the project itself we encounter and evaluate the LINK_ONLY expression, lookup the target, see that it is an INTERFACE library, and discard it. These LINK_ONLY expressions may have to be filtered during export.

(0038030) Ben Boeckel (developer) 2015-02-23 14:57	OK, this is what I thought; it didn't look like something I could figure out from right there. Suitable for 3.2?

(0038031) Brad King (manager) 2015-02-23 15:00	Re 0015415:0038030: Not suitable for 3.2. We're already on the second release candidate. This is not a regression.

(0038032) Brad King (manager) 2015-02-23 15:06	Re 0015415:0038029: Actually I take this back. The value inside $<LINK_ONLY:...> can be an arbitrary generator expression. Its final value may not be known until it is processed in the consuming project long after export. Furthermore, the INTERFACE library may be a private implementation dependency of the static library because it specifies INTERFACE_LINK_LIBRARIES on real libraries on which the static library depends at link time. We have to preserve this in the exported target.

(0038033) Ben Boeckel (developer) 2015-02-23 15:34	Actually, in trying to make a test for this, I was able to get it to error out for not exporting a dependent INTERFACE library…now why is my original instance not triggering it?

(0038034) Stephen Kelly (developer) 2015-02-23 15:36	I just noticed the same thing. The difference between my 'error as expected' and 'no error' testcase was the minimum version of CMake 2.8.11/2.8.12. It must be policy behavior (CMP0022).

(0038036) Ben Boeckel (developer) 2015-02-23 15:54	I have a minimum of 3.1 in this project. No policy settings otherwise. So what's the plan then? Bring back the error? Trim out known-INTERFACE libraries with no transitive link interface from $<LINK_ONLY> during tll()/export? Make a list of INTERFACE libraries to trim out at use-time? The first doesn't help me (not that I'm not already doomed to either requiring 3.3 or rearchitecting this repo) and the last ones are not pretty. I'm also fine with some other solution.

(0038037) Stephen Kelly (developer) 2015-02-23 16:09	cmExportFileGenerator::ResolveTargetsInGeneratorExpression could be taught to pierce the veil of LINK_ONLY: http://www.cmake.org/gitweb?p=cmake.git;a=blob;f=Source/cmExportFileGenerator.cxx;h=71728be2;hb=HEAD#l686 [^] Even then it's only a best-effort attempt at issuing an error, because LINK_ONLY does not require literal content. This is also not-an-error: add_library(a SHARED bar.cpp) add_library(b SHARED foo.cpp) target_link_libraries(b PUBLIC $<$<CONFIG:Debug>:a>) install(TARGETS b EXPORT foo DESTINATION lib ) install(EXPORT foo DESTINATION lib/cmake ) but the user can write target_link_libraries(b PUBLIC $<$<CONFIG:Debug>:$<TARGET_NAME:a>>) So, actually, rather than piercing the veil of LINK_ONLY, maybe this is the basis of a solution: diff --git a/Source/cmTargetLinkLibrariesCommand.cxx b/Source/cmTargetLinkLibrariesCommand.cxx index 75c94c5..1c303aa 100644 --- a/Source/cmTargetLinkLibrariesCommand.cxx +++ b/Source/cmTargetLinkLibrariesCommand.cxx @@ -438,7 +438,7 @@ cmTargetLinkLibrariesCommand::HandleLibrary(const std::string& lib, if (cmGeneratorExpression::IsValidTargetName(lib) \|\| cmGeneratorExpression::Find(lib) != std::string::npos) { - configLib = "$<LINK_ONLY:" + configLib + ">"; + configLib = "$<LINK_ONLY:$<TARGET_NAME:" + configLib + ">>"; } this->Target->AppendProperty("INTERFACE_LINK_LIBRARIES", configLib.c_str());

(0038038) Brad King (manager) 2015-02-23 16:22	Re 0015415:0038036: It looks like this error case is suppressed by CMP0022 NEW behavior. In the OLD behavior there is no need for the LINK_ONLY expression and the export set checker successfully identifies the missing export of the dependency of the static library. In the NEW behavior the dependency is hidden behind LINK_ONLY and the export set checker can't evaluate generator expressions to check what might be missing after evaluation. I don't think it is always possible to bring back the error. Trimming or any other processing on LINK_ONLY is not really clean either for the same reason: we can't evaluate generator expressions during export. Re 0015415:0038037: We can't hard-code $<TARGET_NAME:...> there because that genex requires a literal target name inside it, so target_link_libraries(b PRIVATE $<1:a>) would break (of course in practice the genex may be more complex). Changing Ben's example to: target_link_libraries(b PRIVATE $<TARGET_NAME:a>) does in fact restore the error during export even with CMP0022 set to NEW.

(0038039) Brad King (manager) 2015-02-23 16:24	Re 0015415:0038038: Perhaps target_link_libraries can add $<TARGET_NAME:...> if and only if the configLib already matches a target name. That will restore the error for the basic use case. We shouldn't even need a policy for it because we're just moving the error to export time instead of delaying it until a consuming application tries to use the bad target info.

(0038040) Brad King (manager) 2015-02-23 16:26	Re 0015415:0038039: Then during export we could pierce $<LINK_ONLY:$<TARGET_NAME:tgt>> specifically and filter it out if "tgt" names an INTERFACE library with no INTERFACE_LINK_LIBRARIES.

(0038042) Stephen Kelly (developer) 2015-02-23 16:48	There's no need to pierce $<LINK_ONLY:$<TARGET_NAME:tgt>> specifically. The code I linked to already finds $<TARGET_NAME:tgt> because the LHS must be literal in that genex (that's the only purpose of that genex).

(0038043) Ben Boeckel (developer) 2015-02-23 16:50	That patch has the test case I'm working with. Even with the $<TARGET_NAME:> addition, there's errors about exporting with dependencies on notExportedInterface.

(0038046) Brad King (manager) 2015-02-24 08:34	Re 0015415:0038042: Yes, just adding TARGET_NAME is sufficient to restore the error message when tll is given a normal target name. I'm saying that on export we can filter out $<LINK_ONLY:$<TARGET_NAME:tgt>> specifically when tgt is known to not be needed. We cannot filter out any more general LINK_ONLY expressions because their evaluation result is not known. Re 0015415:0038043: Yes, the proposed TARGET_NAME addition is meant to catch such errors. Additional work will be needed to filter them out on export.

(0042718) Kitware Robot (administrator) 2016-06-10 14:29	Resolving issue as `moved`. This issue tracker is no longer used. Further discussion of this issue may take place in the current CMake Issues page linked in the banner at the top of this page.

Issue History
Date Modified	Username	Field	Change
2015-02-23 14:35	Ben Boeckel	New Issue
2015-02-23 14:35	Ben Boeckel	Status	new => assigned
2015-02-23 14:35	Ben Boeckel	Assigned To	=> Stephen Kelly
2015-02-23 14:41	Brad King	Note Added: 0038027
2015-02-23 14:48	Ben Boeckel	Note Added: 0038028
2015-02-23 14:53	Brad King	Note Added: 0038029
2015-02-23 14:57	Ben Boeckel	Assigned To	Stephen Kelly => Ben Boeckel
2015-02-23 14:57	Ben Boeckel	Note Added: 0038030
2015-02-23 15:00	Brad King	Note Added: 0038031
2015-02-23 15:06	Brad King	Note Added: 0038032
2015-02-23 15:34	Ben Boeckel	Note Added: 0038033
2015-02-23 15:36	Stephen Kelly	Note Added: 0038034
2015-02-23 15:54	Ben Boeckel	Note Added: 0038036
2015-02-23 16:09	Stephen Kelly	Note Added: 0038037
2015-02-23 16:22	Brad King	Note Added: 0038038
2015-02-23 16:24	Brad King	Note Added: 0038039
2015-02-23 16:26	Brad King	Note Added: 0038040
2015-02-23 16:48	Stephen Kelly	Note Added: 0038042
2015-02-23 16:48	Ben Boeckel	File Added: export-with-interface-libs.patch
2015-02-23 16:50	Ben Boeckel	Note Added: 0038043
2015-02-24 08:34	Brad King	Note Added: 0038046
2015-04-22 09:55	Brad King	Target Version	CMake 3.3 =>
2016-06-10 14:29	Kitware Robot	Note Added: 0042718
2016-06-10 14:29	Kitware Robot	Status	assigned => resolved
2016-06-10 14:29	Kitware Robot	Resolution	open => moved
2016-06-10 14:31	Kitware Robot	Status	resolved => closed

Relationships