feat: CMake update, part 1

[henryiii]

This is a complete CMake update, part 1 of approximately 2. The key themes are better use of CMake built-in features, better consistency between subdirectory and config mode, and a more deterministic config file. This will enable new features in subsequent PRs.

CMake 3.7 or better is now required

This enables many simplifications, and a more unified approach to targets. Many if statements dropped, though a few more added for enhanced support for later CMake versions.

Reworked C++ support to properly handle C++ version in CMake instead manually

CMake 2.8, which came out three years before 2011, does not have C++11 support (hopefully not a surprise, though given the number of people that seem to want to use a new compiler with a CMake version that significantly predates it, I'm not sure). Now it does, so PyBind11 plays nice with C++ support! FINALLY! Key points:

• PYBIND11_CPP_STANDARD is deprecated, and issues a warning. It does still work, though, just grabbing the last two digits and setting CMAKE_CXX_STANDARD with it.
• If a compiler has a reasonable default (like many do now), we get to just use that, instead of injecting a flag.
• CMAKE_CXX_STANDARD is not required - CMake knows that pybind requires at least C++11 support, and will automatically add at least the C++11 flag if needed to any target that links to it (via compile features) if the compiler has a default less than C++11.
• If a target requires a higher level, that will be used instead (not duplicated, like before), either through compile features (per target) or via CMAKE_CXX_STANDARD (global) or CXX_STANDARD (per target, not inherited)
• Because it's built into CMake, it correctly supports all compilers, and correctly mixes languages (that is, it is escaped correctly for CUDA!)
• We get to remove the old caveats about setting the C++ standard before PyBind11 is used - it can be placed anywhere now.

Closes #1097

If-less config files

The config file generated will not depend on the host system - this is a requirement for the CMake config files to be distributed in a pure python wheel - which can't know what the host compiler / python version is beforehand. Generator expressions are used, which are evaluated during the build, rather than before.

Note: there are two ifs that affect the configs: If you generate the file with CMake 3.13+, it cannot be read by 3.7-3.12. However, this is much better than before, were you couldn't change much of anything after generating a Config file, and crucially, the 3.13+ one has a lower potential for bugs (it enforces that the dynamic lookup does not get de-duplicated accidentally). The main planned user of this feature is PyPI package, and there should be no problem getting CMake 3.13+ using pip.

Key first step for #2266!

PS: Normal if's in the Config.in are fine, just not if's making the targets themselves in the main file.

SYSTEM changes

The current handling of SYSTEM is not consistent with CMake standards. CMake treats all IMPORTED targets as SYSTEM, so this was/is always on when using configuration mode. This now treats Python always as SYSTEM (may fix the warning we struggled with for a while), and treats PyBind11 as SYSTEM when being used as a subdirectory, consistent with the config mode. The SYSTEM argument to pybind11_add_module is no longer needed and is inconsistent with target_link_libraries. A warning is printed if given. You can always set NO_SYSTEM_FROM_IMPORTED if you want to make imported targets non-system.

Smaller features

• The library prints the CMake version when used as a master project. It also no longer unconditionally prints the PyBind11 version when included as a submodule.
• CMake polices are set to the newest version tested, reducing warnings and hack-arounds
  • I'm supporting the built-in, modified CMake 3.11 that shipped in some versions of MSVC instead of using the native syntax for supporting a version range.
  • Closes link time optimization disabled due to OLD behaviour of CMake policy 0069 #1903
• The version number is now a standard CMake VERSION - this has the downside that it must be made up of valid numbers
  • Adding a new dev flag
  • Available as pybind11_VERSION* and PROJECT_VERSION*(, picked up correctly by CMake wherever it's needed (for example, by CPack)
• The standard BUILD_TESTING flag is now supported, along with PYBIND11_TEST
  • Both have distinct uses unless you remove building tests when this is not the master project, which is not a bad idea IMO
  • I didn't use include(CTest) here, which sets BUILD_TESTING up for you, but I could.
• The visibility=hidden flag attempt has been removed from the pybind11::module target; it caused issues (closes pybind11 incorrectly adds "-fvisibility" flag to nvcc #1710, combined with flto change closes CXX flags passed to nvcc using CMake make compilation fail #1991), it went against the description (simplest possible flags), it clashes with the correct way to do this in the pybind11_add_module function, and is better handled by a nice example (added to the docs).
• flto now uses CMake instead of custom flag checking for CMake 3.9+, closes CXX flags passed to nvcc using CMake make compilation fail #1991, partially CMake 3.12+ Python Find Routines, IPO/LTO #2201 - added PYPBIND11_CLASSIC_LTO to force the old search method and manual flags
• Python 2.7 warning suppression added to targets (I could be talked out of that one, but it nicely fixes the issues we were having at the target level - the new SYSTEM handling helps, but likely still breaks in C++17 mode without this fix)
• Moved the version macros up to the top of the detail/config file to make them easier to find and update
• Don't override additional Python versions if variable already set, closes Pybind fails to build with MSVC 19.16 in C++17 mode #2089
• Building against a debug Python less than 3.8 should work now, closes Debug build does not set -DPy_DEBUG with cmake #2018
• Targets are always installed when everything else is installed, closes Missing file pybind11Targets.cmake? #1733
• Spacing changed to 4-space indent, 2-space keyword that I've seen used a bit and like, but maybe 2-space is more standard? I usually use 4 space everywhere, but the CMake docs does use 2-space (just checked)

This has gotten large enough, so I'll be working on new FindPython support in the next PR. Also, we currently might be a bit sloppy about putting build products in the source tree - I'll investigate that in the near future, too. This is mostly focused on correctness, which should make alternate compilers, CUDA, and such easier to support (since we are relying on CMake to support them, rather than trying to hardcode everything ourselves), but we do get 5 or so issues closed with this (not counting 1-2 of mine). This also should enable CMake configs in the python wheel, which when mixed with PEP 518 builds and scikit-build, could make something truly beautiful. :)

As a further test, I dropped this into boost-histogram (CMake mode) and awkward1 (both submodule systems) and verified that no changes were needed, it is backward compatible and still builds correctly locally.

---

Upgrade guide:

Several details of the CMake support have been deprecated; warnings will be shown if you need to change something. The changes are:

• PYBIND11_CPP_STANDARD=<platform-flag> is deprecated, please use CMAKE_CXX_STANDARD=<number> instead, or any other valid CMake CXX or CUDA standard selection method, like target_compile_features.
• If you do not request a standard, PyBind11 targets will compile with the compiler default, but not less than C++11, instead of forcing C++14.
• Direct pybind11::module usage should always be accompanied by at least set(CMAKE_CXX_VISIBILITY_PRESET hidden) or similar - it used to try to manually force this compiler flag (but not correctly on all compilers or with CUDA).
• pybind11_add_module's SYSTEM argument is deprecated - this now behaves like other imported libraries in both config and submodule mode, and behaves like a SYSTEM library by default.
• CMake 3.9+ will use CMake's built-in INTERPROCEDURAL_OPTIMIZATION instead of a custom flag discovery algorithm - set PYPBIND11_CLASSIC_LTO to force the old system on all CMake versions. THIN_LTO has no effect when the new system is being used, it will select the thin_lto as applicable.

[YannickJadoul]

* `PYBIND11_CPP_STANDARD` is deprecated, and issues a warning. It _does_ still work, though, just grabbing the last two digits and setting `CMAKE_CXX_STANDARD` with it.

Beware of PYBIND11_CPP_STANDARD=/std:c++latest, though. This has been the way to get C++17 before C++17 was fully implemented in MSVC, I believe. Now it should give you sóme C++20 features, I believe.

Were there now talks to preserve some legacy support for old CMake? I'd happily drop that, actually. CMake 3.7 is +3.5 years old (https://cmake.org/pipermail/cmake/2016-November/064540.html), so we can't keep up support forever.

[henryiii]

Were there now talks to preserve some legacy support for old CMake? I'd happily drop that, actually.

@wjakob made this the minimum for now, based on Debian oldstable (see https://cliutils.gitlab.io/modern-cmake/chapters/intro/installing.html#full-list ) .

Personally, I think it's easier to tell users to update CMake than to maintain old code and the bugs in older versions which cause issues (I see quite a few in our issue tracker that would have been fixed by updates to CMake), and your CMake should always be newer than your compiler, and CMake is not required if you just want to use the library, you can pip install it, it's extremely backward compatible, etc... But I do understand wanting to keep older CMake support, and I support 3.4+ currently with CLI11, so I guess I'm not one to talk. ;)

[YannickJadoul]

Personally, I think it's easier to tell users to update CMake than to maintain old code and the bugs in older versions which cause issues (I see quite a few in our issue tracker that would have been fixed by updates to CMake), and your CMake should always be newer than your compiler, and CMake is not required if you just want to use the library, you can pip install it, it's extremely backward compatible, etc... But I do understand wanting to keep older CMake support, and I support 3.4+ currently with CLI11, so I guess I'm not one to talk. ;)

No, I completely agree. If it's been 3.5 years, we should be able to drop some of the burden!
Just curious, because I had interpreter some discussions as "let's keep some fallback", but I should probably read better ;-)

[henryiii]

On: docs/compiling.rst

Before, we hacked in the compile flags for visibility into the target, and it showed - we were breaking on mixed language builds and compilers where we didn't know the correct flags to add. The point of pybind11_add_module is to add these sorts of extra flags, and there's no easy way to remove them from the pybind11::module target; even if they are breaking the build. There is a simple CMake idiom to add this, so we should use that, rather than trying to hack it in ourselves. And once a user learns the idiom, they can use it on their own projects that don't use PyBind11 too!

If CMake added an INTERFACE_CXX_VISIBILITY_PRESET and INTERFACE_CUDA_VISIBILITY_PRESET, this could be done via a target, but until then, we should probably not try to inject manual flags here ourselves, and use the CMake feature instead. (That might be something to ask for in CMake, by the way).

[YannickJadoul]

On: docs/compiling.rst

Sorry, because of GitHub's clipping of the changelog, I completely missed that this is for the pybind11::module (I guess what you link against for embedding?), and not for pybind11_add_module. This is fine, as this is arguably advanced use. Sorry for the confusion!

EDIT: Same discussion happened here: #995 (comment)

[henryiii]

pybind11::embed is for embedding. pybind11::module is you want to build things yourself, and not rely on a magic function. It is meant to be for advanced users, and it even says clearly in the docs it does not add extra flags - it was just trying to add this one anyway before and now it doesn't.

We link to pybind11::module in pybind11_add_module, so trying to add flags not only could break that target if we do it incorrectly (such as not protecting for CUDA, not knowing what some compiler requires), but it also breaks the "good" usage too - pybind11_add_module gets those flags too besides the ones from the CXX_VISIBILITY_PRESET property!

[henryiii]

I addressed the PYBIND11_CPP_STANDARD=/std:c++latest with an explicit error now, telling a user to update to CMAKE_CXX_STANDARD. It will only issue a warning and continue if it detects a valid C++ version.

[henryiii]

I found a nice CMake format tool, this is more like the old format, everything is now consistent, and pre-commit checked too! :)

[henryiii]

Added more protections for the Py2.7 flag, and added a flag for windows C++17 + Py 2.7. I didn't protect the Win flag with CXX; if you use it with CUDA, this might break it (and to enable Windows 2.7 support only, that would be a shame). I don't use CUDA + MSVC, though, so not sure. Fixed a small typo in master that was not making warnings errors.

[wjakob]

Hi @henryiii -- it looks great! A few comments/questions from my end.

[wjakob]

First time I heard about .cmake-format, that's neat! One related question: we're beginning to "clog up" the root directory with many dot-files related to CI infrastructure. Can we move them to '.github' as well?

[henryiii]

Putting them in .github is probably a bad idea, as the files in there have special meanings for GitHub - like .github/CONTRIBUTING.md, issue templates, pull request templates, actions, workflows, etc. But this one could go anywhere, such as in tools. The problem is that if you try to run cmake-format from the command line, it will pick this up only if it's in the main folder. However, I would expect this and all the other formatters to only be run by pre-commit, so that's not a big deal. The .pre-commit-config.yaml file needs to be in the main folder. I rather think the .clang-format file will need to be in the main folder (IIRC, -style=file is the parameter you run, which doesn't include a path).

These are all just dot files, so they only make the main GitHub page longer (which is still a little irritating, but a common issue, especially for Python).

[henryiii]

(My general rule is that I always use the default name/location if something expects it, so tools "just work" if someone tries to run them; but if there's not special meaning to a name/path, I put it in a folder to keep the page clean)

[henryiii]

Dropping the changlog here - will fill out and finish in part 2:

v2.6.0 (IN PROGRESS)
-----------------------------------------------------

* CMake 3.7 or newer is now required, with newer versions strongly preferred.
  `#2338 <https://github.com/pybind/pybind11/pull/2338>`_.

Several details of the CMake support have been deprecated; warnings will be
shown if you need to change something. The changes are:

* ``PYBIND11_CPP_STANDARD=<platform-flag>`` is deprecated, please use
  ``CMAKE_CXX_STANDARD=<number>`` instead, or any other valid CMake CXX or CUDA
  standard selection method, like ``target_compile_features``.

* If you do not request a standard, PyBind11 targets will compile with the
  compiler default, but not less than C++11, instead of forcing C++14.

* Direct ``pybind11::module`` usage should always be accompanied by at least
  ``set(CMAKE_CXX_VISIBILITY_PRESET hidden)`` or similar - it used to try to
  manually force this compiler flag (but not correctly on all compilers or with
  CUDA).

* ``pybind11_add_module``'s ``SYSTEM`` argument is deprecated - this now behaves
  like other imported libraries in both config and submodule mode, and behaves
  like a ``SYSTEM`` library by default.

* CMake 3.9+ will use CMake's built-in ``INTERPROCEDURAL_OPTIMIZATION`` instead
  of a custom flag discovery algorithm - set ``PYPBIND11_CLASSIC_LTO`` to force
  the old system on all CMake versions. ``THIN_LTO`` has no effect when the new
  system is being used, it will select the ``thin_lto`` as applicable.

[henryiii]

Timed out error on appveyor, can be ignored. I don't have permission to restart the appveyor builds.

[wjakob]

I restarted it