CMake installation path consistency

[SylvainCorlay]

Problem:

1. In a given installation PREFIX, pip install pybind11 or conda install pybind11 will install pybind11 headers into a new pybind11 subdirectory of the python include directory, while cmake + make install results in pybind11 headers being put into PREFIX/include/pybind11.

   For example, with a conda installation, pip will give

   PREFIX/include/pythonX.Y/pybind11 while cmake yields PREFIX/include/pybind11.

2. It would be nice to have a consistent installation scheme between the two methods, plus the benefit of having the cmake project config files being made available, in the context of conda.

Solution:

This changes the destination of the headers of pybind into PYTHON_INCLUDE_DIRS/pybind11.

The generated project config cmake files are installed in the same location as before, but reflect the new location of the headers.

[SylvainCorlay]

If you are fine with this, I will open another PR which will use the pip.locations method in FindPythonLibsNew.cmake to cover the homebrew distutils scheme.

[dean0x7d]

With this PR, the headers are placed into PYTHON_INCLUDE_DIRS/pybind11 while the CMake config files are going into CMAKE_INSTALL_PREFIX. This isn't going to work because CMake expects that the headers and config files are placed consistently relative to CMAKE_INSTALL_PREFIX. The find_package tests are failing because they're testing expected CMake behavior and I don't think this should be modified.

A better way to do this would be to just give CMake a different CMAKE_INSTALL_PREFIX so that everything is still consistent. But this isn't compatible with the current way pybind11's setup.py installs headers into include/pythonx.x/pybind11. An alternative would be to adopt a numpy-style header location: site-packages/pybind11/include. The nice thing about this is that the CMake config files could also be installed into site-packages/pybind11/share/cmake. On the CMake side, the change then becomes trivial: CMAKE_INSTALL_PREFIX=site-packages/pybind11 just needs to be passed as an argument. This would keep everything nice and consistent. I was thinking about proposing this, but the only downside is that it would complicate setup.py and potentially add cmake as an install dependency.

Note that switching the header install location from include/pythonx.x/pybind11 to site-packages/pybind11/include would also resolve this PyPy issue: #596.

[SylvainCorlay]

With this PR, the headers are placed into PYTHON_INCLUDE_DIRS/pybind11 while the CMake config files are going into CMAKE_INSTALL_PREFIX. This isn't going to work because CMake expects that the headers and config files are placed consistently relative to CMAKE_INSTALL_PREFIX. The find_package tests are failing because they're testing expected CMake behavior and I don't think this should be modified.
A better way to do this would be to just give CMake a different CMAKE_INSTALL_PREFIX so that everything is still consistent. But this isn't compatible with the current way pybind11's setup.py installs headers into include/pythonx.x/pybind11. An alternative would be to adopt a numpy-style header location: site-packages/pybind11/include. The nice thing about this is that the CMake config files could also be installed into site-packages/pybind11/share/cmake. On the CMake side, the change then becomes trivial: CMAKE_INSTALL_PREFIX=site-packages/pybind11 just needs to be passed as an argument. This would keep everything nice and consistent. I was thinking about proposing this, but the only downside is that it would complicate setup.py and potentially add cmake as an install dependency.

Note that switching the header install location from include/pythonx.x/pybind11 to site-packages/pybind11/include would also resolve this PyPy issue: #596.

The correct solution is to fix the cmake.in so as to make the generate Pybind11Config.cmake reflect the correct location. Changing the CMAKE_INSTALL_PREFIX would be very wrong and will have other consequences such as pybind not being being able to find dependencies. Commits coming.

[dean0x7d]

Note that PYTHON_INCLUDE_DIRS/pybind11!= PREFIX/include/pythonX.Y/pybind11.

[SylvainCorlay]

Note that PYTHON_INCLUDE_DIRS/pybind11!= PREFIX/include/pythonX.Y/pybind11.

Yeah only on unix platforms. But my change handles the windows case too. If you are using homebrew python, this is different, but I want to see if there is buy-in for this approach before I work on adjusting the tests for cmake.

[SylvainCorlay]

Eventually, I would like the setup.py to optionally make use cmake install instead of distutils' install_header command, so that we also provide the Pybind11Config.cmake artefacts in the conda package. The location where headers are installed should be unchanged.

(Very much like pyzmq's setup.py which can either bundle zeromq for pypi, or optionally link with an already provided version).

This option will then be used by e.g. the conda recipe so that pybind11 users who rely on the conda package should be able to use it with both distutils and cmake.

[dean0x7d]

This option will then be used by e.g. the conda recipe so that pybind11 users who rely on the conda package should be able to use it with both distutils and cmake.

I would personally love that. Even more if it was also available with pip and not just conda.

But I think there are still some issues with the current implementation:

1. The cmake tests are now intrusive because the install location is an absolute path and the files "escape" the mock_install directory, potentially overwriting existing user files.
2. I'm not sure how the absolute header path would interact with packaging for various platforms.
3. The headers are placed inside a specific Python version, while the config files are global. If multiple versions are installed at once, there would be multiple versions of the headers, while the config files would be overwritten by the last installation of pybind11, thus pointing to their Python version headers and leaving the other headers orphaned.

[SylvainCorlay]

(Item 1 is more an issue with the implementation of the tests IMO.)

[SylvainCorlay]

I'm not sure how the absolute header path would interact with packaging for various platforms.

Quick precision: the header path is not absolute, it is relative to the install prefix unless the found python version is outside of it.

The headers are placed inside a specific Python version, while the config files are global. If multiple versions are installed at once, there would be multiple versions of the headers, while the config files would be overwritten by the last installation of pybind11, thus pointing to their Python version headers and leaving the other headers orphaned.

That is right (also it is the point of the PR). It would probably make sense to provide a cmake option to pass a specific python prefix.

[aldanor]

Yea, it would be nice to have a way of getting headers installed into $PREFIX/include even if using setup.py -- which e.g. is the de facto default location for headers for most conda-installed packages.

[SylvainCorlay]

Yea, it would be nice to have a way of getting headers installed into $PREFIX/include even if using setup.py -- which e.g. is the de facto default location for headers for most conda-installed packages.

That is not what I meant. What I meant is always have the headers in the python include path (even with cmake install like in this PR) but enable the use cmake to produce the Pybind11Config.cmake files.

[aldanor]

What's the suggested way of having the headers installed to prefix/include then?

[SylvainCorlay]

What's the suggested way of having the headers installed to prefix/include then?

We would not enable this. cmake install would place the headers in the python include directory just like distutils's install_headers. find_package still finds it without an issue, so it is not preventing the standard cmake workflow.

EDIT: would you want an option to do so? (I don't think that it would be a good idea for the reasons above).

[jagerman]

If I'm understanding this right, if I install using Python 2.7 as the python version for cmake, then later try to use find_package (but now under Python 3.5), I'm going to end up with both Python 2.7 and Python 3.5 include directories in my search path.

That seems horribly wrong.

[loriab]

I'm all for the conda package putting the headers in a CMake-expected location and actually installing the pybind11Config (See conda-forge/pybind11-feedstock#11). But I don't think CMAKE_INSTALL_INCLUDEDIR should be overwritten (except by user), as it has a recognized meaning from GNUInstallDirs. And even if the headers get moved into site-packages, I would suggest leaving the *.cmake files still at the top-level prefix, as otherwise one would need to pass a extra, python-version-dependent CMAKE_PREFIX_PATH to allow pybind11 to be detected, even within an ordinary conda environment, as site-packages isn't in the usual search paths below.

From CMake, places to search for nameConfig:

<prefix>/                                                       (W)
<prefix>/(cmake|CMake)/                                         (W)
<prefix>/<name>*/                                               (W)
<prefix>/<name>*/(cmake|CMake)/                                 (W)
<prefix>/(lib/<arch>|lib|share)/cmake/<name>*/                  (U)
<prefix>/(lib/<arch>|lib|share)/<name>*/                        (U)
<prefix>/(lib/<arch>|lib|share)/<name>*/(cmake|CMake)/          (U)
<prefix>/<name>*/(lib/<arch>|lib|share)/cmake/<name>*/          (W/U)
<prefix>/<name>*/(lib/<arch>|lib|share)/<name>*/                (W/U)
<prefix>/<name>*/(lib/<arch>|lib|share)/<name>*/(cmake|CMake)/  (W/U)

[SylvainCorlay]

If I'm understanding this right, if I install using Python 2.7 as the python version for cmake, then later try to use find_package (but now under Python 3.5), I'm going to end up with both Python 2.7 and Python 3.5 include directories in my search path.

That seems horribly wrong.

In a given install environment, only one version of python can exist at a time. The python version found by cmake at install time.

I'm all for the conda package putting the headers in a CMake-expected location and actually installing the pybind11Config (See conda-forge/pybind11-feedstock#11). But I don't think CMAKE_INSTALL_INCLUDEDIR should be overwritten (except by user), as it has a recognized meaning from GNUInstallDirs. And even if the headers get moved into site-packages, I would suggest leaving the *.cmake files still at the top-level prefix, as otherwise one would need to pass a extra, python-version-dependent CMAKE_PREFIX_PATH to allow pybind11 to be detected, even within an ordinary conda environment, as site-packages isn't in the usual search paths below.

Side note: Having the headers as package data like numpy causes a lot of pain to users who need to use a numpy's runtime to tell them where its headers are located. Distutils' install_headers directive is a leaner way to do this and pybind11 uses it at the moment.

This PR makes the cmake installation of pybind11 consistent with distutils. Since the opposite is not possible.

[jagerman]

In a given install environment, only one version of python can exist at a time. The python version found by cmake at install time.

Yes. That's exactly the problem.

With this patch, the install environment python version is the only one that can be used by cmake pybind11-using projects, while the python version to build against is supposed to be configurable (e.g. by setting PYTHON_EXECUTABLE in a project's CMakeLists.txt before the find_package(pybind11)). As long as pybind11Config.cmake is being installed in a location that isn't specific to the python version, it's going to be impossible to have two projects that use find_package(pybind11) with one depending on Python 2 and the other on Python 3 on the same system: If my pybind11 install environment used Python 2, my Python 3 project would end up with the Python 2 include path; if it was Python 3, my Python 2 project ends up with the Python 3 include path.

Given that there can only be one pybind11Config.cmake, some ways that could make this work are:

1. move most of the pybind11Config logic into a cmake script that gets installed in a python-version-specific location, then make the main pybind11Config.cmake just detect the python version and dispatch to the version-specific cmake script;
2. install the headers somewhere where the include path will be specific to pybind11;
3. don't install a global pybind11Config.cmake at all; or
4. achieve the desired symmetry by always installing to ${PREFIX}/include (which seems pretty reasonable given that there is absolutely nothing python-version-dependent in the header files).

[aldanor]

@SylvainCorlay EDIT: would you want an option to do so? (I don't think that it would be a good idea for the reasons above).

@jagerman achieve the desired symmetry by always installing to ${PREFIX}/include (which seems pretty reasonable given that there is absolutely nothing python-version-dependent in the header files).

My point too, headers are completely interpreter-version-independent. Also, with conda in mind, all conda packages I've ever seen (on Linux/Mac that is, bar numpy) install non-Python-specific headers to $PREFIX/include and not to the distils include folder which I think was a great decision, because now you can just -isystem $PREFIX/include and be done with it - this works with any interpreter version, and is completely CMake-independent. For this reason we currently have to package pybind11 manually into a conda package so it gets installed to environment's include folder.

[SylvainCorlay]

We could put this new behavior behind an option for cmake. This would allow distributing the Pybind11Config.cmake files in the conda recipe and keep the installation consistent with that of distutils.

[loriab]

I'm not keen on moving the (cmake + make)-built pybind11 headers from <top-level>/include to <top-level>/.../site-.../include, but if that's a distutils constraint then, in the interests of consistency, it's not a problem. Likewise with conda recipes, this just makes "python x.x" a build requirement as well as a run requirement, so there'll be identical (except for install site-pkg dir) py27, py35 packages.

But what is the need to shift pybind11Config into site-pkgs? Distutils, I think, doesn't care about it unless to build the conda pkg, and one can use cmake for that step. Switching conda envs from 2.7 to 3.5 would force switching out the pybind11_py27 package for the 35, so Config would be in the same python-indep place and able to point to the correct py-dep headers location.

EDIT: Sorry, I was a behind on reading the thread..

[SylvainCorlay]

But what is the need to shift pybind11Config into site-pkgs? Distutils, I think, doesn't care about it unless to build the conda pkg, and one can use cmake for that step. Switching conda envs from 2.7 to 3.5 would force switching out the pybind11_py27 package for the 35, so Config would be in the same python-indep place and able to point to the correct py-dep headers location.

The goal of this is for users of the conda recipe who author packages depending on pybind11 to use either cmake or distutils to package it.

[aldanor]

Likewise with conda recipes, this just makes "python x.x" a build requirement as well as a run requirement, so there'll be identical (except for install site-pkg dir) py27, py35 packages.

Btw, just to clarify, latest conda (4.3) now supports Python-type noarch packages, so instead of the almost identical py27, py35, py36 you can now just have python_noarch.

[SylvainCorlay]

Btw latest conda (4.3) now supports Python-type noarch packages, so instead of the almost identical py27, py35, py36 you can now just have python_noarch.

Yeah, the issue with conda noarch is that they have a whitelist of file formats that are ok with it (at least it was like this for it).

[SylvainCorlay]

So I set this behavior behind a flag so that we can use it in the context of the conda-forge recipe.

[jagerman]

Shouldn't the flag also disable installing the cmake script?

[SylvainCorlay]

Shouldn't the flag also disable installing the cmake script?

I don't understand what you mean?

[jagerman]

With this feature enabled, isn't a pybind11Config.cmake still installed?

[loriab]

I think installing pybind11Config.cmake is wanted even with conda-forge. I've had no problems with conda recipes that include Config files; it makes it easier to use the conda packages as dependencies to build something outside the conda realm.

[jagerman]

In that case I don't see what the flag addresses: it still installs something into a non-python-specific path that breaks projects that want to use python major/minor versions other than the one that last installed using the flag. (Except now it also seems like I could have a working pybind11 version-independent installation, then via a dependency, end up breaking my version-independent installation by replacing/overriding its pybind11Tools.cmake with one that is going to inject wrong-version python headers into my search path).

[jagerman]

To back up several steps: Can someone explain to me in a concise way what the specific benefits are of installing pybind11 headers into a python-version-specific include directory?

[SylvainCorlay]

With this feature enabled, isn't a pybind11Config.cmake still installed?

Yes, that is the point of the PR. It is installed in the conda cmake configuration directory. I make a more detailed explanation in the next post.

[dean0x7d]

Just to make sure we're all on the same page here (I think we've been talking past each other a bit), there are three ways to install pybind11, which all do slightly different things.

1. cmake && make install which installs headers in PREFIX/include/pybind11 and config files in PREFIX/share/cmake/pybind11.
2. pip install pybind11 which installs headers in PREFIX/include/pythonX.Y/pybind11 and does not install config files.
3. conda install -c conda-forge pybind11 which currently has the same behavior as pip, but the extended aim of this PR is to also install config files when using conda.

I think we should specify which install method we're talking about to avoid confusion.

[SylvainCorlay]

1. On conda-forge, we like packages that support it to provide projectConfig.cmake files so that other packages built with cmake can find them, and I am diligent about that.

   Examples:

   libzmq: https://github.com/conda-forge/zeromq-feedstock/blob/master/recipe/meta.yaml
   cppzmq: https://github.com/conda-forge/cppzmq-feedstock/blob/master/recipe/meta.yaml#L30
   rapijson: https://github.com/conda-forge/rapidjson-feedstock/blob/master/recipe/meta.yaml#L24
   cryptopp: https://github.com/conda-forge/cryptopp-feedstock/blob/master/recipe/meta.yaml#L38

   and more (clang, xeus, llvm)...

   This allows packages that depend on them to find them at build time when their conda-recipe performs a cmake install.

2. Now, certain packages like pybind11 are distributed on both pypi and conda. The build of the conda recipe only runs setup.py which relies on distutils' install_headers command, which results in headers being put at the distutils location in the conda environement. ($PREFIX/include on unix and %LIBRARY_INCLUDE% on windows typically).

   For people who make packages depending on pybind11 assuming the scheme used on pypi, it is important to not change that (or they will have to special-case conda vs pypi etc...)

3. But now that pybind11 also has a nice cmake recipe, it is able to produce the crucial pybind11Config.cmake files. It would be nice if those were also in the conda package so that clients to pybind11 could use cmake's find_package to find pybind11...

   Therefore, I opened this PR which enables (now behind an opt-in flag) the cmake installation at the same location as with setup.py while producing cmake project config files to find them.

   Since these cmake project files go in a cmake configuration directory under the conda environment prefix, they are local to the conda environment where things are installed and cannot conflict with another python or cmake installation.

[dean0x7d]

So, my main points here are:

• The cmake install method should not be affected -- it should not install headers into Python version-specific directories. That is addressed by the latest commit by making it optional and not default. Great!

• conda install can also invoke cmake and therefore install the config files into PREFIX/share/cmake/pybind11. The only concern raised by others is if the headers should be installed into PREFIX/include/pythonX.Y/pybind11 or PREFIX/include/pybind11. I'd say that this does not matter for conda install because the PREFIX will always be managed by a conda environment which is going to enforce a strict single Python version per environment, so there will be no way for conflicts to happen. I previously objected to this because this affected the pure cmake install method which could have conflicting Python versions. But the latest commit resolves that by making it conda-only.

• There are no changes to the pip install method. I only have one question here: Is there any practical way to also get the config files installed with pip? But this is probably not important now that it's clear that this PR primarily targets the conda install method.

[SylvainCorlay]

There are no changes to the pip install method. I only have one question here: Is there any practical way to also get the config files installed with pip? But this is probably not important now that it's clear that this PR primarily targets the conda install method.

It might be possible to place them under sys.prefix with wheel's data_files but generating them without requiring cmake is another story so I would not try to that :)

[SylvainCorlay]

The only other thing would be to change setup.py to not install the headers is some environment variable PYBIND11_INSTALL_HEADERS is set...

[jagerman]

Thanks @dean0x7d, it seems reasonable to me now. I think the flag should have a big warning comment above it that this will break multi-Python environments, but otherwise seems reasonable.

[wjakob]

The comment could be more explanatory: "Install [..] within the Python include directory instead of the default installation prefix"

[SylvainCorlay]

Makes sense.

[SylvainCorlay]

Done.

[SylvainCorlay]

btw, speaking of conda. There is still a pybind conda channel with an outdated version. It would probably make sense to delete these, especially since 98% of the downloads come from conda-forge:

[SylvainCorlay]

I added a commit which disables the header installation in setup.py if a certain environment variable is set.

[loriab]

I echo @dean0x7d that there should be a warning with USE_PYTHON_INCLUDE_DIR=ON that testing pybind11 "escapes" the out-of-source builddir since that's unexpected behaviour for CMake.

 -DCMAKE_INSTALL_INCLUDEDIR=myincl \

-- Installing: /myfilesystem/pybind11/objdir/mock_install/myincl/pybind11/typeid.h
-- Installing: /myfilesystem/pybind11/objdir/mock_install/share/cmake/pybind11/pybind11Config.cmake

 -DCMAKE_INSTALL_INCLUDEDIR=myincl \
 -DUSE_PYTHON_INCLUDE_DIR=ON \

-- Installing: /myfilesystem/pybind11/objdir/mock_install/../../../miniconda/include/python2.7/pybind11/typeid.h
-- Installing: /myfilesystem/pybind11/objdir/mock_install/share/cmake/pybind11/pybind11Config.cmake

Other than warnings, I've no objection to the current state of the PR and am grateful that @SylvainCorlay has taken up the reconciliation of the distutils/cmake directory structures.

But is there any reason why the same effect couldn't be gotten by installing headers into ${PREFIX}/include/python${PYTHON_MAJOR_VERSION}.${PYTHON_MINOR_VERSION}/pybind11 and subcasing as necessary for Windows, rather than using the relative path to find the actual python headers? Would look the same as current PR in a conda package and would avoid the directory escape-age. If you've already thought through and rejected such a scheme, please ignore this and pardon the noise.

[SylvainCorlay]

But is there any reason why the same effect couldn't be gotten by installing headers into ${PREFIX}/include/python${PYTHON_MAJOR_VERSION}.${PYTHON_MINOR_VERSION}/pybind11 and subcasing as necessary for Windows, rather than using the relative path to find the actual python headers? Would look the same as current PR in a conda package and would avoid the directory escape-age. If you've already thought through and rejected such a scheme, please ignore this and pardon the noise.

So on windows, the change to that would merely be to replace ${PREFIX}/include/ with %LIBRARY_PREFIX%.

I am neutral on this proposal. The advantage of the current approach over this is only in the case where this proves useful in a different context than conda. Homebrew has a different directory structure for example.

[wjakob]

@SylvainCorlay: I deleted the old package.

[SylvainCorlay]

@SylvainCorlay: I deleted the old package.

@wjakob The one in the pybind anaconda org remains.

[SylvainCorlay]

Hey, do you guys need anything else for this to get in?

[wjakob]

Sorry, I'm just swamped with work and haven't had time to sit down and do proper code reviews in the last couple of weeks.

[wjakob]

Hi -- I took a look just now. I don't understand the PYBIND11_USE_CMAKE environment variable listed in setup.py. Who/what sets this, and why is it needed? I could not find any documentation on this.

[SylvainCorlay]

This environment variable can be set externally to disable the installation of the headers by setup.py. In the conda recipe for pybind11, we would set it prior to call setup.py and perform a cmake install for the headers. In doing so, we will have both the pybind11 python module, and the Pybind11Config.cmake files included in the package. Then, users who build upon pybind11 will be able to use either `find_package` or `pip`. Besides, if they simply pip install a package that depends on pybind, it will correctly see it as installed... Sylvain

[wjakob]

Ok, that makes sense! -- I'll merge the PR then.