MantisBT - CMake
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0012477CMakeModulespublic2011-09-30 03:202012-05-09 15:26
ReporterMark Abraham 
Assigned ToAlexey Ozeritsky 
PrioritynormalSeverityminorReproducibilityalways
StatusclosedResolutionfixed 
PlatformunixOSanyOS Versionany
Product VersionCMake 2.8.5 
Target VersionCMake 2.8.7Fixed in VersionCMake 2.8.7 
Summary0012477: FindLAPACK
DescriptionThe linker command line produced for static linking with LAPACK on Unix is broken. The semicolon is not a valid separator.
Steps To ReproduceCall find_package(LAPACK) with UNIX and BLA_STATIC set, then watch linking die later.
Additional InformationRecommended patch:

--- a/cmake/FindLAPACK.cmake
+++ b/cmake/FindLAPACK.cmake
@@ -107,7 +107,7 @@ endforeach(_library ${_list})
 if(_libraries_work)
   # Test this combination of libraries.
   if(UNIX AND BLA_STATIC)
- set(CMAKE_REQUIRED_LIBRARIES ${_flags} "-Wl,--start-group ${${LIBRARIES}} ${_blas};-Wl,--end-group" ${_threads})
+ set(CMAKE_REQUIRED_LIBRARIES ${_flags} "-Wl,--start-group ${${LIBRARIES}} ${_blas} -Wl,--end-group" ${_threads})
   else(UNIX AND BLA_STATIC)
     set(CMAKE_REQUIRED_LIBRARIES ${_flags} ${${LIBRARIES}} ${_blas} ${_threads})
   endif(UNIX AND BLA_STATIC)
TagsNo tags attached.
Relationships
Attached Files
Issue History
Date ModifiedUsernameFieldChange
2011-09-30 03:20Mark AbrahamNew Issue
2011-09-30 03:23Alexey OzeritskyAssigned To => Alexey Ozeritsky
2011-09-30 03:23Alexey OzeritskyStatusnew => assigned
2011-10-22 14:27David ColeTarget Version => CMake 2.8.7
2011-10-22 14:27David ColeNote Added: 0027623
2011-10-24 02:38Mark AbrahamNote Added: 0027634
2011-10-24 11:07Alexey OzeritskyNote Added: 0027637
2011-10-24 20:02Mark AbrahamNote Added: 0027640
2011-10-25 02:32Alexey OzeritskyNote Added: 0027641
2011-11-17 16:20David ColeNote Added: 0027816
2011-11-20 05:26Mark AbrahamNote Added: 0027832
2011-12-02 06:07Alexey OzeritskyNote Added: 0027882
2011-12-02 06:23Alexey OzeritskyNote Added: 0027883
2011-12-02 06:23Alexey OzeritskyNote Deleted: 0027882
2011-12-13 19:19David ColeNote Added: 0027964
2011-12-13 19:19David ColeStatusassigned => resolved
2011-12-13 19:19David ColeFixed in Version => CMake 2.8.7
2011-12-13 19:19David ColeResolutionopen => fixed
2012-05-09 15:26David ColeNote Added: 0029439
2012-05-09 15:26David ColeStatusresolved => closed

Notes
(0027623)
David Cole   
2011-10-22 14:27   
Staged CMake topic branch "findlapack-0012477"
===============================================

Question about your fix for this issue...

You've removed a ";" because the stuff in there is supposed to be space-separated in that context, correct?

But what about the definition and expansion of these variables that still remains:

${${LIBRARIES}} ${_blas}

Are those ever going to have ";" in them? Or are they always going to be correctly expanded?

If you think it's fully correct as-is, then I'll merge this fix in to 'master' this coming Tuesday when we do our next merge session. If it needs more work or investigation, please let me know.
(0027634)
Mark Abraham   
2011-10-24 02:38   
Yes. Space delimitters are required for such linking.

${LIBRARIES} is set in that Check_Lapack_Libraries() macro as a space-delimitted from values from find_library(), so that should be OK. ${_blas} is passed in externally. I can't find any invocations of Check_Lapack_Libraries() to see if there is any problem.

So I think the patch is correct as given.
(0027637)
Alexey Ozeritsky   
2011-10-24 11:07   
You are right. That won't work if ${${LIBRARIES}} contains >= 2 elements.
Dont merge it yet.

The correct patch is
 - set(CMAKE_REQUIRED_LIBRARIES ${_flags} "-Wl,--start-group ${${LIBRARIES}} ${_blas};-Wl,--end-group" ${_threads})
 + set(CMAKE_REQUIRED_LIBRARIES ${_flags} "-Wl,--start-group" ${${LIBRARIES}} ${_blas} "-Wl,--end-group" ${_threads})


But there is another problem: -Wl,--start-group ... -Wl,--end-group won't work in msvc.
(0027640)
Mark Abraham   
2011-10-24 20:02   
Patch looks OK, but I don't understand the difference between ${${LIBRARIES}} and ${LIBRARIES}.

For reference, the correct form of usage from man ld is:
gcc -Wl,--start-group foo.o bar.o -Wl,--end-group

As far as MSVC goes, that doesn't matter directly because the line is only invoked on UNIX because of the if() test.

MSVC and Lapack is worth catering for at some point, because there is a project that aims at making LAPACK-type libraries available to MSVC. http://icl.cs.utk.edu/lapack-for-windows/ [^] Maybe someone there will be useful.

Google tells me that MSVC uses a /LINK command-line option to pass things from the compiler command line to the linker command line, but I do not have the expertise to construct such.
(0027641)
Alexey Ozeritsky   
2011-10-25 02:32   
${LIBRARIES} is LAPACK_LIBRARIES or LAPACK95_LIBRARIES
${${LIBRARIES}} is the value of LAPACK_LIBRARIES or LAPACK95_LIBRARIES

Which blas vendor are you using ?
(0027816)
David Cole   
2011-11-17 16:20   
Is there going to be any activity on this issue in the next 3 weeks in time for the rc1 of CMake 2.8.7? If not, I'm going to back out the change that's currently in 'next' and discard the topic branch from the stage... Let me know.

Thanks.
(0027832)
Mark Abraham   
2011-11-20 05:26   
@Alexey

I have no particular interest in any BLAS package. I need FindLAPACK.cmake to work when linked statically on UNIX. The caller of Check_Lapack_Libraries sets the value of ${_blas}, so correct form of that is their problem.

Alexey's patch in post 0027637 looks good to me.
(0027883)
Alexey Ozeritsky   
2011-12-02 06:23   
Please drop findlapack-0012477 and merge findlapack-0012477-new
(0027964)
David Cole   
2011-12-13 19:19   
Fixed by this commit:

  http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=f44f053a52db8f78288705454a04e001b8f2c74c [^]
(0029439)
David Cole   
2012-05-09 15:26   
Closing resolved issues that have not been updated in more than 4 months.