View Issue Details [ Jump to Notes ]

[ Print ]

ID

Project

Category

View Status

Date Submitted

Last Update

0011292

CMake

Modules

public

2010-10-06 08:49

2010-11-18 23:51

Reporter

Aaron C. Meadows

Assigned To

Philip Lowman

Priority

normal

Severity

feature

Reproducibility

N/A

Status

closed

Resolution

not fixable

Platform

OS

OS Version

Product Version

CMake-2-8

Target Version

Fixed in Version

Summary

0011292: CMakeDetermineVSServicePack.cmake uses try_compile to get the compiler version. cl.exe /? is much faster

Description

Using try_compile(), there is a lot more work going on than is necessary just to get the version from the compiler. It is present at the top of the help output in the same format. I've modified the script to use that command instead and it is quite a bit faster at picking out the version of the compiler.

Patch attached.

Additional Information

I tested this with VS2005 sp1, vs2008 sp1, and vs2010.

Tags

No tags attached.

Attached Files

CMakeDetermineVSServicePack.Fast.patch [^] (1,345 bytes) 2010-10-06 08:49 [Show Content] [Hide Content]

--- CMakeDetermineVSServicePack.cmake	2010-09-29 20:26:20.530212900 -0500
+++ CMakeDetermineVSServicePackFast.cmake	2010-10-06 07:47:01.231012900 -0500
@@ -7,6 +7,7 @@
 #    vc80sp1
 #    vc90
 #    vc90sp1
+#    vc100
 #
 # Usage:
 # ===========================
@@ -60,16 +61,18 @@ endfunction()
 # in use.  See documentation above.
 function(DetermineVSServicePack _pack)
     if(NOT DETERMINED_VS_SERVICE_PACK OR NOT ${_pack})
-        file(WRITE "${CMAKE_BINARY_DIR}/return0.cc"
-            "int main() { return 0; }\n")
 
-        try_compile(DETERMINED_VS_SERVICE_PACK
-            "${CMAKE_BINARY_DIR}"
-            "${CMAKE_BINARY_DIR}/return0.cc"
-            OUTPUT_VARIABLE _output
-            COPY_FILE "${CMAKE_BINARY_DIR}/return0.cc")
-        
-        file(REMOVE "${CMAKE_BINARY_DIR}/return0.cc")
+        if(EXISTS ${CMAKE_CXX_COMPILER})
+          execute_process(
+              COMMAND ${CMAKE_CXX_COMPILER} /?
+              ERROR_VARIABLE _output
+              OUTPUT_QUIET
+            )
+
+          # cl.exe returns 0 for /?, setting this true to preserve behavior of
+          # the original script.
+          set(DETERMINED_VS_SERVICE_PACK true)
+        endif()
                 
         if(DETERMINED_VS_SERVICE_PACK AND _output)
             string(REGEX MATCH "Compiler Version [0-9]+.[0-9]+.[0-9]+.[0-9]+"

Relationships

Notes
(0023117) Bill Hoffman (manager) 2010-11-09 22:27	But this will only work if Cl is in the path, and setup correctly. So, we can't use this patch. It will break for most folks doing an IDE build.

(0023119) Aaron C. Meadows (reporter) 2010-11-10 01:49	Cl.exe doesn't need to be in the path, it just needs to be found by CMake and put in the CMAKE_CXX_COMPILER variable. I don't think I agree with your assessment that it will be broken for IDE builds. Is try_compile using some magic beyond the mentioned variable? I did not see these issues you bring up when I tested, but I will test specifically for that tomorrow.

(0023130) Bill Hoffman (manager) 2010-11-10 07:55	But if cl is not in the PATH, and the PATH does not have the dll's it needs to run. The IDE try-compile is doing a magic of sorts. CMake finds the IDE executable, say devenv. When CMake runs devenv, devenv sets up all the environment information to be able to run the compiler. You have to run vcvars.bat in order to be able to invoke cl directly. That is why the IDE try-compiles have to use solution files to do anything with the compiler. Just google the CMake list and look at how many times someone has trouble getting the nmake generator to work because they did not run vcvars.bat before running CMake. And seriously, this is only run once per configuration, the savings will be totally minimal.

(0023171) Philip Lowman (developer) 2010-11-11 10:01	Aaron, It sounds like for this patch to be merged we would have to be reasonably sure that "cl.exe" is either statically linked or capable of being run (with the /? flags) without any special environment variables set. Can you do some testing to confirm this? If we find that all of the versions of MSVC that CMakeDetermineVSServicePack supports work properly with /? with no special environment variables, we can still leave the try_compile() support in as a fallback if there are any errors running cl or parsing the version number (for future safety).

(0023421) Philip Lowman (developer) 2010-11-18 23:50	Aaron, I tried running "C:\Program Files (x86)\Microsoft Visual Studio 9.0\VC\bin\cl" tonight from a command prompt (Visual Studio Express C++ 2008). I was greeted with a popup dialog Window. The program can't start because mspdb80.dll is missing from your computer. Try reinstalling the program to fix this problem. I get the same error running "cl" after changing directories to the directory it is in. I think this means unfortunately that the patch can't be accepted. Thanks for suggesting it though, it was a good idea. If you have any other ideas for improving CMake or many of it's modules please bring them up on the mailing list or create tickets.

Issue History
Date Modified	Username	Field	Change
2010-10-06 08:49	Aaron C. Meadows	New Issue
2010-10-06 08:49	Aaron C. Meadows	File Added: CMakeDetermineVSServicePack.Fast.patch
2010-11-09 21:57	Philip Lowman	Status	new => assigned
2010-11-09 21:57	Philip Lowman	Assigned To	=> Philip Lowman
2010-11-09 22:27	Bill Hoffman	Note Added: 0023117
2010-11-10 01:49	Aaron C. Meadows	Note Added: 0023119
2010-11-10 07:55	Bill Hoffman	Note Added: 0023130
2010-11-11 10:01	Philip Lowman	Note Added: 0023171
2010-11-18 23:42	Philip Lowman	Category	CMake => Modules
2010-11-18 23:50	Philip Lowman	Note Added: 0023421
2010-11-18 23:51	Philip Lowman	Status	assigned => closed
2010-11-18 23:51	Philip Lowman	Resolution	open => not fixable

Copyright © 2000 - 2018 MantisBT Team