From 6d1b197b4669c70da94c8efb516374e1935d3ea3 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Thu, 8 Jul 2021 09:02:48 -0700 Subject: [PATCH] Splitting out pybind11/stl/filesystem.h. (#3077) * Splitting out pybind11/stl/filesystem.h. To solve breakages like: https://github.com/deepmind/open_spiel/runs/2999582108 Mostly following the suggestion here: https://github.com/pybind/pybind11/pull/2730#issuecomment-750507575 Except using pybind11/stl/filesystem.h instead of pybind11/stlfs.h, as decided via chat. stl.h restored to the exact state before merging PR #2730 via: ``` git checkout 733f8de24feed964f96b639a0a44247f46bed868 stl.h ``` * Properly including new stl subdirectory in pip wheel config. This now passes interactively: ``` pytest tests/extra_python_package/ ``` * iwyu cleanup. iwyuh.py -c -std=c++17 -DPYBIND11_TEST_BOOST -Ipybind11/include -I/usr/include/python3.9 -I/usr/include/eigen3 include/pybind11/stl/filesystem.h * Adding PYBIND11_HAS_FILESYSTEM_IS_OPTIONAL. * Eliminating else after return. --- CMakeLists.txt | 3 +- include/pybind11/stl.h | 81 ------------------ include/pybind11/stl/filesystem.h | 103 +++++++++++++++++++++++ tests/extra_python_package/test_files.py | 7 +- tests/test_stl.cpp | 5 ++ tools/setup_global.py.in | 4 +- tools/setup_main.py.in | 2 + 7 files changed, 121 insertions(+), 84 deletions(-) create mode 100644 include/pybind11/stl/filesystem.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 152763eb4..c988ea0b5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -125,7 +125,8 @@ set(PYBIND11_HEADERS include/pybind11/pybind11.h include/pybind11/pytypes.h include/pybind11/stl.h - include/pybind11/stl_bind.h) + include/pybind11/stl_bind.h + include/pybind11/stl/filesystem.h) # Compare with grep and warn if mismatched if(PYBIND11_MASTER_PROJECT AND NOT CMAKE_VERSION VERSION_LESS 3.12) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index 2350a5247..ca20b7483 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -41,21 +41,11 @@ # include # define PYBIND11_HAS_VARIANT 1 # endif -// std::filesystem::path -# if defined(PYBIND11_CPP17) && __has_include() && \ - PY_VERSION_HEX >= 0x03060000 -# include -# define PYBIND11_HAS_FILESYSTEM 1 -# endif #elif defined(_MSC_VER) && defined(PYBIND11_CPP17) # include # include # define PYBIND11_HAS_OPTIONAL 1 # define PYBIND11_HAS_VARIANT 1 -# if PY_VERSION_HEX >= 0x03060000 -# include -# define PYBIND11_HAS_FILESYSTEM 1 -# endif #endif PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) @@ -387,77 +377,6 @@ template struct type_caster> : variant_caster> { }; #endif -#if defined(PYBIND11_HAS_FILESYSTEM) -template struct path_caster { - -private: - static PyObject* unicode_from_fs_native(const std::string& w) { -#if !defined(PYPY_VERSION) - return PyUnicode_DecodeFSDefaultAndSize(w.c_str(), ssize_t(w.size())); -#else - // PyPy mistakenly declares the first parameter as non-const. - return PyUnicode_DecodeFSDefaultAndSize( - const_cast(w.c_str()), ssize_t(w.size())); -#endif - } - - static PyObject* unicode_from_fs_native(const std::wstring& w) { - return PyUnicode_FromWideChar(w.c_str(), ssize_t(w.size())); - } - -public: - static handle cast(const T& path, return_value_policy, handle) { - if (auto py_str = unicode_from_fs_native(path.native())) { - return module::import("pathlib").attr("Path")(reinterpret_steal(py_str)) - .release(); - } - return nullptr; - } - - bool load(handle handle, bool) { - // PyUnicode_FSConverter and PyUnicode_FSDecoder normally take care of - // calling PyOS_FSPath themselves, but that's broken on PyPy (PyPy - // issue #3168) so we do it ourselves instead. - PyObject* buf = PyOS_FSPath(handle.ptr()); - if (!buf) { - PyErr_Clear(); - return false; - } - PyObject* native = nullptr; - if constexpr (std::is_same_v) { - if (PyUnicode_FSConverter(buf, &native)) { - if (auto c_str = PyBytes_AsString(native)) { - // AsString returns a pointer to the internal buffer, which - // must not be free'd. - value = c_str; - } - } - } else if constexpr (std::is_same_v) { - if (PyUnicode_FSDecoder(buf, &native)) { - if (auto c_str = PyUnicode_AsWideCharString(native, nullptr)) { - // AsWideCharString returns a new string that must be free'd. - value = c_str; // Copies the string. - PyMem_Free(c_str); - } - } - } - Py_XDECREF(native); - Py_DECREF(buf); - if (PyErr_Occurred()) { - PyErr_Clear(); - return false; - } else { - return true; - } - } - - PYBIND11_TYPE_CASTER(T, _("os.PathLike")); -}; - -template<> struct type_caster - : public path_caster {}; -#endif - PYBIND11_NAMESPACE_END(detail) inline std::ostream &operator<<(std::ostream &os, const handle &obj) { diff --git a/include/pybind11/stl/filesystem.h b/include/pybind11/stl/filesystem.h new file mode 100644 index 000000000..7a8acdb60 --- /dev/null +++ b/include/pybind11/stl/filesystem.h @@ -0,0 +1,103 @@ +// Copyright (c) 2021 The Pybind Development Team. +// All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +#pragma once + +#include "../cast.h" +#include "../pybind11.h" +#include "../pytypes.h" + +#include "../detail/common.h" +#include "../detail/descr.h" + +#include + +#ifdef __has_include +# if defined(PYBIND11_CPP17) && __has_include() && \ + PY_VERSION_HEX >= 0x03060000 +# include +# define PYBIND11_HAS_FILESYSTEM 1 +# endif +#endif + +#if !defined(PYBIND11_HAS_FILESYSTEM) && !defined(PYBIND11_HAS_FILESYSTEM_IS_OPTIONAL) +# error \ + "#include is not available. (Use -DPYBIND11_HAS_FILESYSTEM_IS_OPTIONAL to ignore.)" +#endif + +PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) +PYBIND11_NAMESPACE_BEGIN(detail) + +#if defined(PYBIND11_HAS_FILESYSTEM) +template struct path_caster { + +private: + static PyObject* unicode_from_fs_native(const std::string& w) { +#if !defined(PYPY_VERSION) + return PyUnicode_DecodeFSDefaultAndSize(w.c_str(), ssize_t(w.size())); +#else + // PyPy mistakenly declares the first parameter as non-const. + return PyUnicode_DecodeFSDefaultAndSize( + const_cast(w.c_str()), ssize_t(w.size())); +#endif + } + + static PyObject* unicode_from_fs_native(const std::wstring& w) { + return PyUnicode_FromWideChar(w.c_str(), ssize_t(w.size())); + } + +public: + static handle cast(const T& path, return_value_policy, handle) { + if (auto py_str = unicode_from_fs_native(path.native())) { + return module_::import("pathlib").attr("Path")(reinterpret_steal(py_str)) + .release(); + } + return nullptr; + } + + bool load(handle handle, bool) { + // PyUnicode_FSConverter and PyUnicode_FSDecoder normally take care of + // calling PyOS_FSPath themselves, but that's broken on PyPy (PyPy + // issue #3168) so we do it ourselves instead. + PyObject* buf = PyOS_FSPath(handle.ptr()); + if (!buf) { + PyErr_Clear(); + return false; + } + PyObject* native = nullptr; + if constexpr (std::is_same_v) { + if (PyUnicode_FSConverter(buf, &native)) { + if (auto c_str = PyBytes_AsString(native)) { + // AsString returns a pointer to the internal buffer, which + // must not be free'd. + value = c_str; + } + } + } else if constexpr (std::is_same_v) { + if (PyUnicode_FSDecoder(buf, &native)) { + if (auto c_str = PyUnicode_AsWideCharString(native, nullptr)) { + // AsWideCharString returns a new string that must be free'd. + value = c_str; // Copies the string. + PyMem_Free(c_str); + } + } + } + Py_XDECREF(native); + Py_DECREF(buf); + if (PyErr_Occurred()) { + PyErr_Clear(); + return false; + } + return true; + } + + PYBIND11_TYPE_CASTER(T, _("os.PathLike")); +}; + +template<> struct type_caster + : public path_caster {}; +#endif // PYBIND11_HAS_FILESYSTEM + +PYBIND11_NAMESPACE_END(detail) +PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE) diff --git a/tests/extra_python_package/test_files.py b/tests/extra_python_package/test_files.py index 064a3e12f..c1d12ed76 100644 --- a/tests/extra_python_package/test_files.py +++ b/tests/extra_python_package/test_files.py @@ -46,6 +46,10 @@ detail_headers = { "include/pybind11/detail/typeid.h", } +stl_headers = { + "include/pybind11/stl/filesystem.h", +} + cmake_files = { "share/cmake/pybind11/FindPythonLibsNew.cmake", "share/cmake/pybind11/pybind11Common.cmake", @@ -67,7 +71,7 @@ py_files = { "setup_helpers.pyi", } -headers = main_headers | detail_headers +headers = main_headers | detail_headers | stl_headers src_files = headers | cmake_files all_files = src_files | py_files @@ -77,6 +81,7 @@ sdist_files = { "pybind11/include", "pybind11/include/pybind11", "pybind11/include/pybind11/detail", + "pybind11/include/pybind11/stl", "pybind11/share", "pybind11/share/cmake", "pybind11/share/cmake/pybind11", diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp index 7183c56b7..dc75762e8 100644 --- a/tests/test_stl.cpp +++ b/tests/test_stl.cpp @@ -11,6 +11,11 @@ #include "constructor_stats.h" #include +#ifndef PYBIND11_HAS_FILESYSTEM_IS_OPTIONAL +#define PYBIND11_HAS_FILESYSTEM_IS_OPTIONAL +#endif +#include + #include #include diff --git a/tools/setup_global.py.in b/tools/setup_global.py.in index 4cf040b2d..8b7e53871 100644 --- a/tools/setup_global.py.in +++ b/tools/setup_global.py.in @@ -33,8 +33,9 @@ class InstallHeadersNested(install_headers): main_headers = glob.glob("pybind11/include/pybind11/*.h") detail_headers = glob.glob("pybind11/include/pybind11/detail/*.h") +stl_headers = glob.glob("pybind11/include/pybind11/stl/*.h") cmake_files = glob.glob("pybind11/share/cmake/pybind11/*.cmake") -headers = main_headers + detail_headers +headers = main_headers + detail_headers + stl_headers cmdclass = {"install_headers": InstallHeadersNested} $extra_cmd @@ -58,6 +59,7 @@ setup( (base + "share/cmake/pybind11", cmake_files), (base + "include/pybind11", main_headers), (base + "include/pybind11/detail", detail_headers), + (base + "include/pybind11/stl", stl_headers), ], cmdclass=cmdclass, ) diff --git a/tools/setup_main.py.in b/tools/setup_main.py.in index 2231a08fd..bcd4ef4ae 100644 --- a/tools/setup_main.py.in +++ b/tools/setup_main.py.in @@ -16,12 +16,14 @@ setup( "pybind11", "pybind11.include.pybind11", "pybind11.include.pybind11.detail", + "pybind11.include.pybind11.stl", "pybind11.share.cmake.pybind11", ], package_data={ "pybind11": ["py.typed", "*.pyi"], "pybind11.include.pybind11": ["*.h"], "pybind11.include.pybind11.detail": ["*.h"], + "pybind11.include.pybind11.stl": ["*.h"], "pybind11.share.cmake.pybind11": ["*.cmake"], }, extras_require={