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

get_include() returns wrong directory when temporary installation by pyproject.toml #2115

Closed
ymd-h opened this issue Feb 19, 2020 · 2 comments

Comments

@ymd-h
Copy link

ymd-h commented Feb 19, 2020

Issue description

pybind11.get_include() returns wrong directory when I set pybind11 as build-system:requires in pyproject.toml

pip installs build-system:requires to temporary directory (e.g. /tmp/pip-XXX) only during setup.

(Since setup_requires option uses easy_install and does not install pybind11 header correctly,
pyproject.toml is only solution (as far as I know) for using pybind11 in setup.py without pre-installation.)

This issue probably has same origin with #1425

Reproducible example code

pyproject.toml

[build-system]
requires = ["setuptools","wheel","pybind11"]

setup.py

import os
import platform

from setuptools import setup, Extension
import pybind11

if platform.system() == 'Windows':
    extra_compile_args = ["/std:c++17"]
    extra_link_args = ["/std:c++17"]
else:
    extra_compile_args = ["-std=c++17"]
    extra_link_args = ["-std=c++17"]

setup(name="test",
      ext_modules = [Extension("test",
                               sources=["test/test.cc"],
                               extra_compile_args=extra_compile_args,
                               extra_link_args=extra_link_args)],
      include_dirs = [pybind11.get_include()],
      packages = ["test"])

test/test.cc

#include <pybind11/pybind11.h>

PYBIND11_MODULE(test,m){
  m.def("add",[](int a,int b){ return a+b; });
}

Then, execute following command

docker run -it --rm -v $(pwd):/test python bash
pip install /test

Possible Fix

The installation directories are defined at distutils.command.install (and copied to install_heders etc.)

The following code convert from install module directory to header directory.
It should be fine, as long as installation tool uses standard distutils.

import os
import sys

from distutils.command.install import INSTALL_SCHEMES
from site import USER_BASE, USER_SITE
import pybind11

def pybind11_get_include():
    expand_list = [("$py_version_nodot","%d%d" % sys.version_info[:2]),
                   ("$py_version_short","%d.%d" % sys.version_info[:2]),
                   ("$abiflags",getattr(sys,"abiflags","")),
                   ("$usersite",USER_SITE),
                   ("$userbase",USER_BASE),
                   ("$dist_name","pybind11")]

    def expand_path(_path):
        for _from, _to in expand_list:
            _path = _path.replace(_from,_to)
        return _path

    purelib = os.path.dirname(os.path.dirname(pybind11.__file__))

    for v in INSTALL_SCHEMES.values():
        relpath = os.path.relpath(expand_path(v["headers"]),
                                  start=expand_path(v["purelib"]))
        headers = os.path.realpath(os.path.join(purelib,relpath))
        if os.path.exists(headers):
            return os.path.dirname(headers)

    raise ValueError("pybind11 is not installed by standard `distutils`")
ymd-h pushed a commit to ymd-h/pybind11 that referenced this issue Feb 19, 2020
…utils`

Install directories are defined at `distutils.command.install`
https://github.com/python/cpython/blob/master/Lib/distutils/command/install.py

Depending on OS type, user site etc., one of the install schemes is selected.

This patch
  1. expand parameters (python version etc.) in `INSTALL_SCHEMES`
  2. calculates relative path from "purelib" to "headers"
  3. checks existence of "header" directory for all schemes one by one
  4. returns the include directory if it exists
  5  falls back to the original non-installated case when no schemes match

This implementation should work as long as installation
tool (e.g. pip) uses standard `distutils`.

Notice:
  step 1 (expansion parameters) cannot be taken out from `distutils`
  source and copied by manually, which means `get_include()` will
  break when `distutils` changes its implementation.
@dvirtz
Copy link

dvirtz commented Apr 23, 2020

According to #2116 (comment) this should be fixed in 2.5.0. Is this the case?

@ymd-h
Copy link
Author

ymd-h commented Apr 25, 2020

@dvirtz

Yes, I have just confirmed with pybind11 2.5.0.

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

No branches or pull requests

2 participants