search

robotology / ycm-cmake-modules

YCM (YCM CMake modules) is a collection of various useful CMake modules.
http://robotology.github.io/ycm-cmake-modules
Other
50 stars 22 forks source link

Update FindLibOVR.cmake to support the latest version LibOVRKernel by VC2017 #134

Open mebbaid opened 6 years ago

mebbaid commented 6 years ago

As documented here, the following Part of the FindLibOVR.cmake:

if(MSVC10)
      set(_arch "VS2010")
    elseif(MSVC11)
      set(_arch "VS2012")
    elseif(MSVC12)
      set(_arch "VS2013")
    elseif(MSVC14)
      set(_arch "VS2013")
    endif()

should be appended to look like this:

   if(MSVC10)
      set(_arch "VS2010")
    elseif(MSVC11)
      set(_arch "VS2012")
    elseif(MSVC12)
      set(_arch "VS2013")
    elseif(MSVC14)
      set(_arch "VS2013")
    endif()
  if(MSVC)
   set(_arch "VS2015")
endif

Notice: The latest version for the LibOVRKernel has some issues when built for the VS2017 architecture, so even though we are checking for VS2017 using if(MSVC) we build the 2015 version of the library and so it generates the following directory Lib/windows/x64/Release/VS2015

traversaro commented 6 years ago

Notice: The latest version for the LibOVRKernel has some issues when built for the VS2017 architecture, so even though we are checking for VS2017 using if(MSVC) we build the 2015 version of the library and so it generates the following directory Lib/windows/x64/Release/VS2015

I think this is because, while historically every Visual Studio release had a different ABI, Visual Studio 2015 (MSVC14) and Visual Studio 2017 (MSVC_VERSION in 1910-1919) are the first two major version of Visual Studio that share a compatible ABI. For this reason, I think the check should be modified in the following way, from: 

    elseif(MSVC14)
      set(_arch "VS2013")
    endif()
endif()

to 

    elseif((MSVC_VERSION GREATER 1890) AND (MSVC_VERSION LESS 1990) )
      set(_arch "VS2015")
    endif()
endif()

I guess the if(MSVC14) ==> "VS2013" logic that is present now was a typo, perhaps @drdanz can provide more insight.

drdanz commented 6 years ago

Seriously?

$ cmake --help-variable MSVC14
MSVC14
------

Discouraged.  Use the ``MSVC_VERSION`` variable instead.

``True`` when using the Microsoft Visual Studio ``v140`` or ``v141``
toolset (``cl`` version 19) or another compiler that simulates it.
$ cmake --help-variable MSVC_VERSION
MSVC_VERSION
------------

The version of Microsoft Visual C/C++ being used if any.
If a compiler simulating Visual C++ is being used, this variable is set
to the toolset version simulated as given by the ``_MSC_VER``
preprocessor definition.

Known version numbers are::

 1200      = VS  6.0
 1300      = VS  7.0
 1310      = VS  7.1
 1400      = VS  8.0 (v80 toolset)
 1500      = VS  9.0 (v90 toolset)
 1600      = VS 10.0 (v100 toolset)
 1700      = VS 11.0 (v110 toolset)
 1800      = VS 12.0 (v120 toolset)
 1900      = VS 14.0 (v140 toolset)
 1910-1919 = VS 15.0 (v141 toolset)

See also the  ``CMAKE_<LANG>_COMPILER_VERSION`` variable.

This is getting more complicated day by day...

To be honest I don't remember if the VS2013 was a typo, or a temporary hack to get it working on VS 2015... Anyway, I'm ok with changing it to use MSVC_VERSION, but I'm not so sure about the upper limit...