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

[BUG]: pybind11-config --includes does not quote paths #5300

Closed
2 of 3 tasks
QuLogic opened this issue Aug 13, 2024 · 8 comments · Fixed by #5302
Closed
2 of 3 tasks

[BUG]: pybind11-config --includes does not quote paths #5300

QuLogic opened this issue Aug 13, 2024 · 8 comments · Fixed by #5302
Labels
triage New bug, unverified

Comments

@QuLogic
Copy link
Contributor

QuLogic commented Aug 13, 2024

Required prerequisites

What version (or hash if on master) of pybind11 are you using?

2.13.1

Problem description

On a Windows VM, I installed Python (from python.org) globally for external reasons; this resulted in an install to C:\Program Files\Python312. Since pybind11-config --includes adds sysconfig.get_path("include") and sysconfig.get_path("platinclude"), the result includes that system path as well.

Because these paths contain spaces, and the paths are not quoted, they can be interpreted as multiple arguments. I have not tried the CMake integration, but at least with Meson, they are split into separate -IC:\Program and Files\Python312\Include. For example, a build of Matplotlib outputs:

[95/101] "cl" "-Isrc\_ttconv.cp312-win_amd64.pyd.p" "-Isrc" "-I..\..\src" "-Iextern\ttconv" "-I..\..\extern\ttconv" "-IC:\Program" "-Ic:\Users\Elliott\matplotlib\venv\Lib\site-packages\pybind11\include" "-IC:\Program Files\Python312\Include" "-DNDEBUG" "/MT" "/nologo" "/showIncludes" "/utf-8" "/Zc:__cplusplus" "/W2" "/EHsc" "/std:c++17" "/permissive-" "/O2" "/Gw" "Files\Python312\Include" "/d2FH4-" "-DPY_ARRAY_UNIQUE_SYMBOL=MPL__ttconv_ARRAY_API" "/Fdsrc\_ttconv.cp312-win_amd64.pyd.p\_ttconv.cpp.pdb" /Fosrc/_ttconv.cp312-win_amd64.pyd.p/_ttconv.cpp.obj "/c" ../../src/_ttconv.cpp

Magically, this just happens to work because the right include path was added for other reasons, and MSVC ignores the broken one:

cl : Command line warning D9024 : unrecognized source file type 'Files\Python312\Include', object file assumed
cl : Command line warning D9027 : source file 'Files\Python312\Include' ignored

but if it were to become more strict, it might break.

Reproducible example code

No response

Is this a regression? Put the last known working version here if it is.

Not a regression

@QuLogic QuLogic added the triage New bug, unverified label Aug 13, 2024
@henryiii
Copy link
Collaborator

What would the correct way to quote them on Windows? (https://docs.python.org/3/library/shlex.html#shlex.quote has a big warning about Windows)

@henryiii
Copy link
Collaborator

@QuLogic
Copy link
Contributor Author

QuLogic commented Aug 13, 2024

What would the correct way to quote them on Windows? (https://docs.python.org/3/library/shlex.html#shlex.quote has a big warning about Windows)

Meson has an implementation here; I have not compared them, so I can't say what the difference is.

Doesn't python-config have the same issue? https://github.com/python/cpython/blob/db6f5e193315a3bbfa7b0b6644203bae3f76b638/Misc/python-config.in#L43-L48

Probably true yes, but I guess Meson uses the pkgconfig file and doesn't worry about that program.

@henryiii
Copy link
Collaborator

Can you try 2.13.3?

@QuLogic
Copy link
Contributor Author

QuLogic commented Aug 14, 2024

Unfortunately, that didn't work; it ends up as (with spaces between each argument replaced by newlines for readability):

'-IC:\Program Files\Python312\Include'
'-Ic:\Users\Elliott\matplotlib\venv\Lib\site-packages\pybind11\include'

and after passing through Meson, it becomes:

"'-IC:\Program"
"Files\Python312\Include'"
"'-Ic:\Users\Elliott\matplotlib\venv\Lib\site-packages\pybind11\include'"

I believe that's because on Windows, you must use double quotes (which Meson is doing, but shlex.quote is not); single quotes are just normal characters.

So unfortunately, because this also affects the latter path (which didn't need quoting in this case), and single quotes are just normal characters that aren't stripped, cl thinks it got an file named '-Ic:\Users\Elliott\matplotlib\venv\Lib\site-packages\pybind11\include' (which as noted above, it just ignores), so the headers are not found, and 2.13.3 is broken on Windows.

@QuLogic
Copy link
Contributor Author

QuLogic commented Aug 14, 2024

I just noticed #4874 exists, which might cover all the cases?

@WarrenWeckesser
Copy link

WarrenWeckesser commented Aug 14, 2024

FYI: 2.13.3 broke the build of SciPy: scipy/scipy#21385, scipy/scipy#21391

The problem is as @QuLogic explained above: the compiler arguments are wrapped in both double and single quotes, e.g. "'-IC:\hostedtoolcache\windows\Python\3.10.11\x64\Include'" and "'-IC:\hostedtoolcache\windows\Python\3.10.11\x64\lib\site-packages\pybind11\include'".

QuLogic added a commit to QuLogic/matplotlib that referenced this issue Aug 15, 2024
@henryiii
Copy link
Collaborator

Releasing momentarily.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage New bug, unverified
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants