Spec Header Dirs: Only first include/#13991
Conversation
380738b to
f45e5fc
Compare
|
For the record, $ grep 'spec.*python.*headers' */package.py
boost/package.py: spec['python'].headers.directories[0],
dealii/package.py: python_include = spec['python'].headers.directories[0]
neuron/package.py: py_inc = spec['python'].headers.directories[0]
opencv/package.py: python_include_dir = spec['python'].headers.directories[0]
openspeedshop-utils/package.py: python_include = spec['python'].headers.directories[0]
openspeedshop/package.py: python_include = spec['python'].headers.directories[0]
precice/package.py: python_include = spec['python'].headers.directories[0]
psi4/package.py: os.path.join(spec['python'].headers.directories[0]),
py-gpaw/package.py: python_include = spec['python'].headers.directories[0]I agree that the default should not be recursively searching for headers, we should just use |
|
So it's actually the CPython headers you are searching for in those packages. We then should just overwrite the Side note: As far as I am aware, many C++ projects that build python bindings with Boost.Python or pybind11 can derive the corresponding python public headers by just passing the spec's python executable as an argument to CMake (if they use CMake). Not sure what autotools projects do. |
|
Hm, CI says no. Have to go back to regex school. |
f45e5fc to
d87fe9b
Compare
|
Pew the regex was fine, just forgot to update the group number below :D |
d87fe9b to
c19fd8c
Compare
|
Ha, what am I saying. TIL: non-capture groups. |
|
Just tried this patch on top of f84ad57 and it looks like it is working fine, I was able to build stuff that was previously broken like |
alalazo
left a comment
There was a problem hiding this comment.
Wonder if we can do it changing only one character in the regex
aed496b to
05ed756
Compare
|
Just adding a unit test as well before pushing the fix. |
3cc73b2 to
0ecfff8
Compare
0ecfff8 to
9a5b09f
Compare
Avoid matching recurringly nested include paths that usually refer to internally shipped libraries in packages. Example in CUDA Toolkit, shipping a libc++ fork internally with libcu++ since 10.2.89: `<prefix>/include/cuda/some/more/details/include/` or `<prefix>/include/cuda/std/detail/libcxx/include` regex: non-greedy first match of include Co-Authored-By: Massimiliano Culpo <massimiliano.culpo@gmail.com>
Avoid matching recurringly nested include paths that most certainly refer to internally shipped libraries in packages.
Example in CUDA Toolkit #13819 #13983, shipping a libc++ fork internally with libcu++ since 10.2.89:
<prefix>/include/cuda/std/detail/libcxx/include.Quick fix #13969
A more thorough solution is not to recursively search for headers anymore since this is against the idea of C/C++ facade headers: #13969 (comment) Unfortunately, it's not that easy for us to find which headers are facade and which ones are detail, but I am not sure we have to know anything besides the include dir locations.
Generally, just searching for any
include/in the overall path (instead of just taking the<prefix>/include) is dangerous as well. For example, spack could be installed in/my/large/software/include/here/is/axels/home/src/spack/;-)General policy suggestion: just add
<prefix>/includeto the build environment by default. Packages that break this unix-wide default should provide the@adamjstewart mentioned on Slack that some Python packages might need special treatment, let's see if we can find examples again to double check.