View Issue Details Jump to Notes ] Print ]
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.
Attached Files

 Relationships

  Notes
(0027623)
David Cole (manager)
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 (reporter)
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 (reporter)
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 (reporter)
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 (reporter)
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 (manager)
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 (reporter)
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 (reporter)
2011-12-02 06:23

Please drop findlapack-0012477 and merge findlapack-0012477-new
(0027964)
David Cole (manager)
2011-12-13 19:19

Fixed by this commit:

  http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=f44f053a52db8f78288705454a04e001b8f2c74c [^]
(0029439)
David Cole (manager)
2012-05-09 15:26

Closing resolved issues that have not been updated in more than 4 months.

 Issue History
Date Modified Username Field Change
2011-09-30 03:20 Mark Abraham New Issue
2011-09-30 03:23 Alexey Ozeritsky Assigned To => Alexey Ozeritsky
2011-09-30 03:23 Alexey Ozeritsky Status new => assigned
2011-10-22 14:27 David Cole Target Version => CMake 2.8.7
2011-10-22 14:27 David Cole Note Added: 0027623
2011-10-24 02:38 Mark Abraham Note Added: 0027634
2011-10-24 11:07 Alexey Ozeritsky Note Added: 0027637
2011-10-24 20:02 Mark Abraham Note Added: 0027640
2011-10-25 02:32 Alexey Ozeritsky Note Added: 0027641
2011-11-17 16:20 David Cole Note Added: 0027816
2011-11-20 05:26 Mark Abraham Note Added: 0027832
2011-12-02 06:07 Alexey Ozeritsky Note Added: 0027882
2011-12-02 06:23 Alexey Ozeritsky Note Added: 0027883
2011-12-02 06:23 Alexey Ozeritsky Note Deleted: 0027882
2011-12-13 19:19 David Cole Note Added: 0027964
2011-12-13 19:19 David Cole Status assigned => resolved
2011-12-13 19:19 David Cole Fixed in Version => CMake 2.8.7
2011-12-13 19:19 David Cole Resolution open => fixed
2012-05-09 15:26 David Cole Note Added: 0029439
2012-05-09 15:26 David Cole Status resolved => closed


Copyright © 2000 - 2018 MantisBT Team