cinder / Cinder

Cinder is a community-developed, free and open source library for professional-quality creative coding in C++.
http://libcinder.org
Other
5.34k stars 943 forks source link

Build error with CMake on Windows, MSVC 14 #1910

Open heisters opened 7 years ago

heisters commented 7 years ago

I generated CMake configurations like this:

mkdir build
cd build
"C:\Program Files\CMake\bin\cmake.exe" -G"Visual Studio 14 2015 Win64" ..

Then built Debug and Release versions of the cinder.sln using Visual Studio 2015. I then tried to build my Cinder block using the following Cinder-FrameGraphConfig.cmake:

if( NOT TARGET Cinder-FrameGraph )
    set(CMAKE_CXX_STANDARD 14)
    set(CMAKE_CXX_STANDARD_REQUIRED ON)
    set(CMAKE_CXX_EXTENSIONS OFF)

    get_filename_component( FrameGraph_SOURCE_PATH "${CMAKE_CURRENT_LIST_DIR}/../../src" ABSOLUTE )
    get_filename_component( FrameGraph_INCLUDE_PATH "${CMAKE_CURRENT_LIST_DIR}/../../include" ABSOLUTE )
    get_filename_component( FrameGraph_LIB_PATH "${CMAKE_CURRENT_LIST_DIR}/../../lib" ABSOLUTE )
    get_filename_component( CINDER_PATH "${CMAKE_CURRENT_LIST_DIR}/../../../.." ABSOLUTE )

        list( APPEND SOURCES
                ${FrameGraph_INCLUDE_PATH}/Cinder-FrameGraph.hpp
                ${FrameGraph_INCLUDE_PATH}/Cinder-FrameGraph/Types.hpp
                ${FrameGraph_INCLUDE_PATH}/Cinder-FrameGraph/ColorGradeNode.hpp
                ${FrameGraph_INCLUDE_PATH}/Cinder-FrameGraph/LUTNode.hpp
                ${FrameGraph_INCLUDE_PATH}/Cinder-FrameGraph/FullScreenQuadRenderer.hpp
                ${FrameGraph_INCLUDE_PATH}/Cinder-FrameGraph/VecNode.hpp
                ${FrameGraph_SOURCE_PATH}/Cinder-FrameGraph.cpp
                ${FrameGraph_SOURCE_PATH}/Cinder-FrameGraph/LUTNode.cpp
                ${FrameGraph_SOURCE_PATH}/Cinder-FrameGraph/ColorGradeNode.cpp
                ${FrameGraph_LIB_PATH}/libnodes/src/libnodes/Node.cpp
                )

    add_library( Cinder-FrameGraph ${SOURCES} )

    target_include_directories( Cinder-FrameGraph PUBLIC "${FrameGraph_INCLUDE_PATH}" )
    target_include_directories( Cinder-FrameGraph PUBLIC "${FrameGraph_LIB_PATH}/libnodes/include" )
    target_include_directories( Cinder-FrameGraph SYSTEM BEFORE PUBLIC "${CINDER_PATH}/include" )

    if( NOT TARGET cinder )
            include( "${CINDER_PATH}/proj/cmake/configure.cmake" )
            find_package( cinder REQUIRED PATHS
                "${CINDER_PATH}/${CINDER_LIB_DIRECTORY}"
                "$ENV{CINDER_PATH}/${CINDER_LIB_DIRECTORY}" )
    endif()
    target_link_libraries( Cinder-FrameGraph PRIVATE cinder )

endif()

This fails with the following error:

NMAKE : fatal error U1073: don't know how to make 'C:\Users\ian\code\Cinder\lib\msw\x64\Debug\\cinder.lib'
Stop.

I traced this to build/lib/msw/x64/Debug/cinderTargets.cmake, where it appears the value PlatformToolset is blank. I tried setting -Tv140 in CMake's arguments, but NMake does not allow specifying a toolset like that. I finally fixed it by hardcoding v140 in build/lib/msw/x64/Debug/cinderTargets.cmake in place of \$(PlatformToolset). I'm not sure what the right way to address this is.

Good news is: the build works now in CLion 2017.2.3!

meshula commented 7 years ago

I wonder if a toolchain file can be used to explicitly set up a 64 bit build without having to specify a generator at the command line.

https://cmake.org/Wiki/CMake_Cross_Compiling#The_toolchain_file

PetrosKataras commented 7 years ago

Hey, I haven't used CMake on Windows so I m kind of shooting in the dark here but if I understand correctly you tried setting the PlatformToolset option when building your block ? If that is the case try to pass the toolset option when generating the CMake configs for Cinder only ( remove your CMakeCache.txt file before doing that if you have already configured at least once ). This should set the PlatformToolset variable in your cinderTargets.cmake and if that is all you need you should ( hopefully ) be good to go.

heisters commented 7 years ago

@PetrosKataras using -T v140 or -DCMAKE_VS_PLATFORM_TOOLSET=v140 when generating the cinderTargets.cmake didn't change anything. Is $(PlatformToolset) the right variable name in cinderTargets.cmake?

PetrosKataras commented 7 years ago

Looking at this https://github.com/cinder/Cinder/blob/master/proj/cmake/libcinder_target.cmake#L32 seems that you need to add -DPLATFORM_TOOLSET=v140 in your configure step of Cinder, when generating the project files, in order for the library to be located properly ( the toolset version is used for the build directory as you can see slightly further down from the linked line ). Remember to delete your CMakeCache.txt file again before trying this and hope it helps.

heisters commented 7 years ago

PR #1913 allowed me to pass the toolset when running CMake to generate cinderTargets.cmake. Without it, cinderTargets.cmake still used $(PlatformToolset), which was empty.

richardeakin commented 7 years ago

I'm trying to catch up here and from what I see, the issue seems to be that libcinder_target.cmake is using the string $(PlatformToolset) for it's paths, which is expected to be a macro evaluated by MSVC and not cmake. cinderTargets.cmake ends up seeing that as an empty variable (PlatformToolset). Perhaps instead we should pick the actual platform toolset depending on the generator and assign that to PLATFORM_TOOLSET? I think making that a cache var would be fine but it seems problematic to add it as a var when it doesn't really change the Platform Toolset that MSVC will use, ex. if we merge #1913 the following:

cmake.exe -G"Visual Studio 14" -DPLATFORM_TOOLSET=v120 ..

would still use v140 toolset but paths would look for dependencies in a v120 folder.

Related: @heisters are you saying that the following doesn't work for you? Or is that your block won't get configured right afterwards:

cmake -G"Visual Studio 14 2015 Win64" -T v120 ..

From what I'm reading, that's the way you should adjust the platform toolset to be something non-default (I guess this doesn't actually get useful until you're using something like the clang or xp toolsets). Also, does anyone know if we can get the result of -T or the default from within cmake? Haven't found much info on that one.

@meshula I'd love to see if toolchains would make cmake + MSVC more natural, right now it does seem like we're doing a bit of manual plumbing in there to make it all work.

Also great to hear that things are building with latest CLion on Windows. How's the debug support there now? That'll be a great cross platform option for some folks.

heisters commented 7 years ago

@richardeakin I think your understanding of the issue and your conclusions are correct. #1913 probably isn't the right solution to the issue.

Passing -Tv140 to cmake did not address the issue, but it could be that I didn't have the space in there? However, I would think that if that fixed it, it wouldn't have been an issue in the first place: MSVC isn't setting the macro at all, and changing the toolset shouldn't effect that. I can verify that, though.

CLion has no debugging support right now. So there's that. (As a side note, CLion + clang would really be where it's at IMO, so you could have the same IDE and compiler on all platforms. This is how I've been doing MacOS/Linux cross platform, and I'd love to add Windows to that workflow.) I am successfully building the project itself (built libcinder on command line) in VS2017 with its native CMake support. I just got that working, so I'll go back and check how VS2017 deals with the $(PlatformToolset) macro in the libcinder build.

heisters commented 7 years ago

So VS2017 builds fine, so it must be setting $(PlatformToolset). Using -T "v140" on the commandline should set CMAKE_GENERATOR_TOOLSET according to this bug. I can't find any documentation on what the recommended way is of defaulting $(PlatformToolset) to the value of CMAKE_GENERATOR_TOOLSET if it is unset.