fix: never use .. in a header include

[henryiii]

Description

You are not allowed to use .. in a current-directory "" include. This style of include is really designed to allow local files in the same directory (like .inl files) to be injected into the current location. <> includes are designed for headers. It's currently working in most cases due to implementation details.

This is currently broken, however, if you use a long path prefix on Windows (\\?\). For example, if you pass the include as /I:system \\?\C:\pybind11\include, then the compiler constructs the path as \\?\C:\pybind11\include\pybind11\detail\../thing.h - but that's invalid, as .. is not processed as a "up one" in a Windows long path, but as a directory named ... Long paths are being used in specific situations (pypa/cibuildwheel#1975), breaking pybind11 usage.

This is the minimum to fix that. Ideally, files should never include something from a parent folder, and (at least) public includes should be included via the public include path, but this is the minimum to fix pybind11 for MSVC.

Suggested changelog entry:

* Fix includes when using Windows long paths (``\\?\`` prefix)

[rwgk]

LGTM.

I'll try to run global testing overnight.

On slack you wrote:

I’m not fine with steps in the middle that are not correct and just might happen to work due to unspecified implementation behaviors.

I didn't look up the rules, but this argument makes a lot of sense to me.

[rwgk]

I didn't see any build failures in the global testing (just some flakes, as usual).

[daeden]

So this is why my builds are broken..

[daeden]

Please suggest how I should fix:

set (PYBIND_INC "/usr/local/lib/${PYTHON_NAME}/site-packages/pybind11/include/pybind11/")

This used to work just fine. Now I get:

In file included from /usr/local/lib/python3.11/site-packages/pybind11/include/pybind11/pybind11.h:13,
                 from /usr/local/lib/python3.11/site-packages/pybind11/include/pybind11/embed.h:12,
                 from /home/gperez/git/cts/core/qra/qra/qra/components/ticks.h:68,
                 from /home/gperez/git/cts/core/qra/qra/qra/components/src/ticks.cpp:62:
/usr/local/lib/python3.11/site-packages/pybind11/include/pybind11/detail/class.h:12:10: fatal error: pybind11/attr.h: No such file or directory
   12 | #include <pybind11/attr.h>
      |          ^~~~~~~~~~~~~~~~~

Just remove the pybind you say

set (PYBIND_INC "/usr/local/lib/${PYTHON_NAME}/site-packages/pybind11/include/")

That fails with:

In file included from /home/gperez/git/cts/crabel-shared/crabel/series/framework/Factory.h:12,                                                                                                                                                      
                 from /home/gperez/git/cts/crabel-shared/crabel/series/core/Bar.h:8,
                 from /home/gperez/git/cts/crabel-shared/crabel/series/core/FilteredBar.h:3,
                 from /home/gperez/git/cts/crabel-shared/crabel/series/core/src/RangeRatio.cpp:2:
/home/gperez/git/cts/crabel-shared/crabel/series/framework/NameResolver.h:12:10: fatal error: pybind11.h: No such file or directory 
   12 | #include <pybind11.h>                                                                                                                                                                                                                       
      |          ^~~~~~~~~~~~  

[henryiii]

You are including pybind11 incorrectly. It should be #include <pybind11/pybind11.h>. If you want to avoid changes, you could temporarily include both paths, or add a “pybind11.h” file that includes <pybind11/pybind11.h>.

[tgaldes]

also broke our builds...
fix was to have TWO include paths 💩

in cmake
set (PYBIND_INC "/usr/local/lib/${PYTHON_NAME}/site-packages/pybind11/include/pybind11/")
set (PYBIND_INC "/usr/local/lib/${PYTHON_NAME}/site-packages/pybind11/include/")

@henryiii is there an anticipated date when we will only need one include path?

[henryiii]

You don't need two includes. You never include pybind11/include/pybind11, only pybind11/include. The same is true for any other library, you should never include boost/include/boost, you should never include fmtlib/include/fmt, you should never include eigen/include/Eigen, etc. If you change all if your includes to include the <pybind11/ prefix, then you won't need the pybind11/include/pybind11 path.

In short, this is correct:

#include <pybind11/pybind11.h>
#include <pybind11/stl.h>
#include <pybind11/literals.h>
#include <boost/histogram.hpp>
#include <fmt/core.h>
#include <Eigen/Dense>

This is not:

#include <pybind11.h>
#include <stl.h>
#include <literals.h>
#include <histogram.hpp>
#include <core.h>
#include <Dense>

Also the above CMake snippet, the second set overrides the previous one, so that's actually only one include path.