Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RDY] CMake deps handling - continuation of pr #1261 #1274

Merged
merged 4 commits into from Nov 11, 2017

Conversation

TheCycoONE
Copy link
Member

@TheCycoONE TheCycoONE commented Nov 5, 2017

Compared to #1261 this PR:

  • Handles SDL2 on multiple configuration targets when using built in vcpkg, by using the provided sdl2 cmake config.
  • Doesn't modify FindSDL2.cmake (those modifications didn't work)
  • Copies lua files from the vcpkg/installed/**/share/lua/* directory
  • Checks out the correct commit when cloning vcpkg the first time
  • Checks out a newer vcpkg commit, containing luasockets and newer sdl packages
  • Fixes whitespace
  • Modifies appveyor to take advantage of these changes

@TheCycoONE TheCycoONE force-pushed the build_vcpkg branch 4 times, most recently from 18b4b26 to 8ee98c5 Compare November 5, 2017 01:47
@@ -0,0 +1,19 @@
# Add an extra step to copy built DLLs on MSVC

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should do this automatically in Vcpkg!

Could you let me know what specifically isn't working?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind, I see below that you're not using the toolchain file.

@ras0219-msft
Copy link

ras0219-msft commented Nov 6, 2017

I'd like to help you use the toolchain file if possible!

Note that CMAKE_TOOLCHAIN_FILE only appears to take effect during the first call to project(), so if you want to download the prebuilt libraries, you could do that first:

if(NOT CMAKE_TOOLCHAIN_FILE)
  # download stuff here
  set(CMAKE_TOOLCHAIN_FILE /downloaded/path/to/vcpkg.cmake CACHE STRING "")
endif()

cmake_minimum_required(VERSION 3.8)
project(foo CXX)

message(STATUS "VCPKG_TARGET_TRIPLET=${VCPKG_TARGET_TRIPLET}")

Also, the architecture of the Visual Studio generator can be controlled with -A x64.

@TheCycoONE TheCycoONE changed the title CMake deps handling - continuation of pr #1261 [RFC] CMake deps handling - continuation of pr #1261 Nov 7, 2017
@TheCycoONE
Copy link
Member Author

@ras0219-msft I played for awhile tonight trying to use the CMAKE_TOOLCHAIN_FILE like you suggest, but so far haven't been able to turn it into a good experience.

I'll try again tomorrow, and document what I'm seeing then if I still can't get it to work.

@TheCycoONE
Copy link
Member Author

@ras0219-msft The first problem I run into with that approach is that MSVC isn't set before PROJECT, so I don't know if we are in an appropriate environment to use VCPKG.

@TheCycoONE
Copy link
Member Author

@ras0219-msft Disregard the above - I've gotten considerably closer.

Now the issue I run into when I use the vcpkg toolchain is:

Building CorsixTH
CMake Error at vcpkg/scripts/buildsystems/vcpkg.cmake:159 (_find_package):
  Could not find a package configuration file provided by "SDL2" with any of
  the following names:

    SDL2Config.cmake
    sdl2-config.cmake

  Add the installation prefix of "SDL2" to CMAKE_PREFIX_PATH or set
  "SDL2_DIR" to a directory containing one of the above files.  If "SDL2"
  provides a separate development package or SDK, be sure it has been
  installed.
Call Stack (most recent call first):
  CorsixTH/CMakeLists.txt:97 (FIND_PACKAGE)

Compared to the code I just committed, this was my changes:

diff --git a/CMake/VcpkgDeps.cmake b/CMake/VcpkgDeps.cmake
index 08933a19..1d778194 100644
--- a/CMake/VcpkgDeps.cmake
+++ b/CMake/VcpkgDeps.cmake
@@ -56,23 +56,4 @@ if(err_val)
       "\nIf this error persists try deleting the 'vcpkg' folder.\n")
 endif()

-# We cannot use a toolchain file at this point despite it being recommended by MS.
-# The arch is determined by the generator in use on Windows. For example
-# Visual Studio xx Win64 <= implies 64 bit build. If we use a toolchain this
-# always defaults to a 32 bit build.
-
-# If the user specified their own generator it is too late at this point.
-# We would need to restart CMake at this point, delete the
-# cache and invoke CMake again with the toolchain set.
-
-# For both of these reasons we will use CMAKE_PREFIX_PATH
-
-set(VCPKG_INSTALLED_PATH ${VCPKG_PARENT_DIR}/vcpkg/installed/${VCPKG_TARGET_TRIPLET})
-
-if(CMAKE_BUILD_TYPE MATCHES "^Debug$" OR NOT DEFINED CMAKE_BUILD_TYPE)
-  list(APPEND CMAKE_PREFIX_PATH ${VCPKG_INSTALLED_PATH}/debug)
-  list(APPEND CMAKE_LIBRARY_PATH ${VCPKG_INSTALLED_PATH}/debug/lib/manual-link)
-endif()
-
-list(APPEND CMAKE_PREFIX_PATH ${VCPKG_INSTALLED_PATH})
-list(APPEND CMAKE_LIBRARY_PATH ${_VCPKG_INSTALLED_DIR}/lib/manual-link)
+set(CMAKE_TOOLCHAIN_FILE ${VCPKG_PARENT_DIR}/vcpkg/scripts/buildsystems/vcpkg.cmake)

@ras0219-msft
Copy link

ras0219-msft commented Nov 8, 2017

It looks like SDL2 doesn't provide a -config.cmake, so you'll probably need to write a FindSDL2.cmake script manually (which you probably needed anyway?).

There are a few floating around that might be interesting: https://github.com/search?utf8=%E2%9C%93&q=filename%3Afindsdl2.cmake&type=Code

@TheCycoONE
Copy link
Member Author

It turned out that the VCPKG_TRIPLET was ending up as just -windows.

in vcpkg.cmake, if VCPKG_TARGET_TRIPLET is specified it skips setting _VCPKG_TARGET_TRIPLET_ARCH, then later in the file it sets VCPKG_TARGET_TRIPLET again. I don't know how this is normally handled when you use -DVCPKG_TARGET_TRIPLET, but when I was setting it in VcpkgDeps.cmake the ARCH was never set and then removed.

By avoiding setting VCPKG_TARGET_TRIPLET, it lets the toolchain detect it and set it properly.

@TheCycoONE
Copy link
Member Author

AppVeyor is broken because I haven't put lfs.dll in the lua cpath for lua.exe

@TheCycoONE
Copy link
Member Author

At @Alberth289346's request I've made the indentation for the two new CMake files internally consistent.

If no one else has anything to add, I'll squash these up and move this PR back to RDY tomorrow.

TheCycoONE and others added 4 commits November 10, 2017 21:33
Our TravisCI environment is using 3.2, so we are not actively testing on any
older version.
This commit adds a new module which clones and automatically sets
the include and library paths for the precompiled dependencies.

Linux users can opt in however they must choose the final arch
(i.e. x86 or x64) they intend to compile against.
This commit adds a new script which can be executed on
a Windows machine to build and package the libraries
for CorsixTH. A new folder called vcpkg will be created
in the same folder as the script. The script is
integrated into CMake and run by default when
targetting MSVC.
@TheCycoONE TheCycoONE changed the title [RFC] CMake deps handling - continuation of pr #1261 [RDY] CMake deps handling - continuation of pr #1261 Nov 11, 2017
@TheCycoONE
Copy link
Member Author

All squished up and ready to go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants