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

Pybind11 on Windows might need a CMake version newer than 3.10.1? #2357

Closed
patrikhuber opened this issue Aug 1, 2020 · 5 comments · Fixed by #2370
Closed

Pybind11 on Windows might need a CMake version newer than 3.10.1? #2357

patrikhuber opened this issue Aug 1, 2020 · 5 comments · Fixed by #2370
Assignees

Comments

@patrikhuber
Copy link
Contributor

patrikhuber commented Aug 1, 2020

After updating to the latest master commit of pybind11, my AppVeyor build on Windows is hitting an error:

-- pybind11 v2.6.0 dev
-- Found PythonInterp: C:\Python36-x64\python.exe (found version "3.6.8") 
-- Found PythonLibs: C:/Python36-x64/libs/python36.lib
-- Configuring done
CMake Error at 3rdparty/pybind11/tools/pybind11Tools.cmake:152 (target_link_libraries):
  Error evaluating generator expression:
    $<COMPILE_LANGUAGE:CXX>
  $<COMPILE_LANGUAGE:...> may not be used with Visual Studio generators.
Call Stack (most recent call first):
  python/CMakeLists.txt:1 (pybind11_add_module)

(and a few repetitions of that error). Full build log is here, if you can access it: https://ci.appveyor.com/project/patrikhuber/eos/builds/34429669

This is with cmake-3.10.1 from https://cmake.org/files/v3.10/cmake-3.10.1-win64-x64.zip. According to the pybind11 CMake requirements, the minimum is 3.7 so this should work?

Environment: AppVeyor os: Visual Studio 2017, and CMake Generator "Visual Studio 15 Win64".

If I change my AppVeyor build script to cmake-3.18.1 (https://github.com/Kitware/CMake/releases/download/v3.18.1/cmake-3.18.1-win64-x64.zip), then it works, no error.

I could try a few cmake versions in-between, if that helps. The repo/branch for this is https://github.com/patrikhuber/eos/blob/fix-appveyor-pybind11-cmake/appveyor.yml.

I have no issues upgrading my CMake version, no need to fix this for ancient CMake versions. People should be on latest CMake versions anyway :-) But I thought it would be good to report this, so that if appropriate, you might want to update the minimum required version for pybind11. Or perhaps I'm making some mistake!

@patrikhuber
Copy link
Contributor Author

patrikhuber commented Aug 1, 2020

Tried a few versions in-between. cmake-3.10.3 does not work (https://ci.appveyor.com/project/patrikhuber/eos/builds/34430251), cmake-3.11.0 is the first version that works (https://ci.appveyor.com/project/patrikhuber/eos/builds/34430242).

@henryiii
Copy link
Collaborator

henryiii commented Aug 2, 2020

We may end up requiring a higher version of CMake on Windows & macOS, it's rare to see old CMake's there, and the updates are especially important for those two OSs (things often get added and fixed several versions later than on Linux). However, we should be clear in what we support - and I'll try for 3.7 across the board first.

I think this might be fixed by my current WIP to add FindPython support - there are now auxiliary targets that are rebuilt in Config, so we can do a little more "if"-ing when building the targets without generator expressions. I might bug you with a branch to try what that's ready, if you don't mind?

Currently I only test CMake ranges on Linux, looks like we should add macOS and Windows.

I though CMake was supposed to just ignore COMPILE_LANGAUGE on IDEs on older CMake's, but I guess not. It's sad, because it's very important for supporting CUDA correctly, since you need targets protected with COMPILE_LANGAUGE there.

@henryiii henryiii self-assigned this Aug 2, 2020
@henryiii
Copy link
Collaborator

henryiii commented Aug 2, 2020

Fun fact - it doesn't seem to be possible to run old CMake + new Visual Studio version - I can't run 3.7 or 3.11 against Visual Studio 2019, they don't know how to handle the 2019 version. 3.18 works just fine.

From this page, it looks like 3.14+ support 2019, and 3.8+ or 3.7+ support 2017 (3.7 seems to lump 2015 and 2017 together oddly, but probably supports it). Will try with 2017.

@patrikhuber
Copy link
Contributor Author

Hi,

Thanks for the reply!

I might bug you with a branch to try what that's ready, if you don't mind?

Yep, happy to do that, I can just push it to that test branch and AppVeyor will run it.

Fun fact - it doesn't seem to be possible to run old CMake + new Visual Studio version

Yes, I think you are correct there. Actually I also don't have VS 2017 installed locally anymore, I switched to 2019 ages ago (along with the latest CMake). But on my CI I'm trying to run "what's the minimum version that still works", so one of the CI jobs runs VS 2017.

3.7 seems to lump 2015 and 2017 together oddly

I think this was a temporary oddity/potential mistake by the CMake developers, which then got fixed in 3.8. When VS 2017 (15.0) came out with MSVC_VERSION 1910 and binary compatibility to the previous version (which was new for MSVC), it took the community a while to figure out how exactly it all works and how things are named.
From the cmake docs, it looks like support for VS 2015 (14.0) goes back as far as cmake 3.1.3.

I'll update my CI to cmake to 3.11.0 (or 3.11.4), this solves it for me just fine. So feel free to close this at your discretion.

@henryiii
Copy link
Collaborator

henryiii commented Aug 2, 2020

3.7 doesn't seem to actually support VS 2017, I wasn't able to use it - I got the "cl" is not valid error when I tried it.

This probably will be fixed soon - the biggest issue is that I thought that COMPILE_LANGAUGE was ignored by older CMake's on MSVC, and didn't know/remember that it actually causes an error on 3.10 and under. Since it's easier to fix in the new branch, I'll do it there.

@henryiii henryiii changed the title Pybind11 might need a CMake version newer than 3.10.1? Pybind11 on Windows might need a CMake version newer than 3.10.1? Aug 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants