| Message ID | 20200608213043.3443-1-guillaume.bressaix@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/1] python-pybind: new package | expand |
Hello Guillaume, I've applied your patch, but after a number of changes. See below. On Mon, 8 Jun 2020 23:30:43 +0200 "Guillaume W. Bres" <guillaume.bressaix@gmail.com> wrote: > PyBind is a light (headers only) package > for C++/Python and Python/C++ bindings. > > host-python-pybind is a dependency > of my future contribution, so I added this feature. This sort of comment should not go in the commit log, but below the "---" sign that follows the Signed-off-by line. And in fact, I have dropped the host variant: it should be added in the patch series together with your "future contribution" that will need the host variant of python-pybind. > diff --git a/package/python-pybind/Config.in b/package/python-pybind/Config.in > new file mode 100644 > index 0000000000..832a9fb971 > --- /dev/null > +++ b/package/python-pybind/Config.in > @@ -0,0 +1,10 @@ > +config BR2_PACKAGE_PYTHON_PYBIND > + bool "python-pybind" > + depends on BR2_PACKAGE_PYTHON3 Its documentation says Python 2.x is supported, so I've dropped this "depends on". > new file mode 100644 > index 0000000000..03cea1c060 > --- /dev/null > +++ b/package/python-pybind/python-pybind.mk > @@ -0,0 +1,15 @@ > +################################################################################ > +# > +# python-pybind > +# > +################################################################################ > + > +PYTHON_PYBIND_VERSION = 2.5.0 > +PYTHON_PYBIND_SOURCE = v$(PYTHON_PYBIND_VERSION).tar.gz > +PYTHON_PYBIND_SITE = https://github.com/pybind/pybind11/archive Here you should have use the "github" macro, because they anyway don't provide their own tarballs, only automatically generated ones. This also allows to have a better tarball name than v2.5.0.tar.gz. > +$(eval $(python-package)) > +$(eval $(host-python-package)) So I said above, I've dropped this last line. Applied with those changes. Thanks! Thomas
Hello, I'm not sure pybind11 is just a "regular" python package. First, it needs C++11 so we have to make sure the toolchain supports that in order to be able to actually compile the generated pybind11 code. Secondly, the installation through setup.py just installs the pybind11 headers and it doesn't give any way for C++ projects to locate the package. For example, this project won't find pybind11 because it uses find_package(pybind11...) in it's CMakeLists.txt: https://github.com/pybind/pybind11_json. The installation of pybind11 using CMake gives a solution to this problem. Those considerations have been taken into account in a previous patch I sent to the mailing list few weeks ago: https://patchwork.ozlabs.org/project/buildroot/patch/20200513183903.8656-1-asafka7@gmail.com/ Regards, Asaf.
Hello, If I understand correctly, this patch only covers the first use case (C++/Python). I only added this one because my following patch requires host-python-pybind, but I agree it certainly lacks a depends on !BR2_INSTALL_LIBSTDCPP. I'll wait for people's point of view before going any further anyway. If you agree on this, I will add the dependency and push a *v2* The other use case that people might need in the future, is not allowed at the moment until we unlock it with $(eval $(cmake-package)) but requires further work (I am also not familiar with cmake at the moment). Maybe you could proceed with adding the cmake support because you are knowledgeable and already got it working? One last question though: "For example, this project won't find pybind11 because it uses find_package(pybind11...)", so we should rename the package to *python-pybind11*? so we don't have any other problems when unlocking the cmake option. If you agree on that point, my v2 will be renamed to *python-pybind11* & and I will also adapt my following work to this new name Guillaume W. Bres Software engineer <guillaume.bressaix@gmail.com> Le jeu. 11 juin 2020 à 20:14, Asaf Kahlon <asafka7@gmail.com> a écrit : > Hello, > > I'm not sure pybind11 is just a "regular" python package. > First, it needs C++11 so we have to make sure the toolchain supports > that in order to be able to actually compile the generated pybind11 > code. > Secondly, the installation through setup.py just installs the pybind11 > headers and it doesn't give any way for C++ projects to locate the > package. For example, this project won't find pybind11 because it uses > find_package(pybind11...) in it's CMakeLists.txt: > https://github.com/pybind/pybind11_json. The installation of pybind11 > using CMake gives a solution to this problem. > > Those considerations have been taken into account in a previous patch > I sent to the mailing list few weeks ago: > > https://patchwork.ozlabs.org/project/buildroot/patch/20200513183903.8656-1-asafka7@gmail.com/ > > Regards, > Asaf. >
Sorry for the double post, but replace 'pushing a v2' by 'pushing a fix', because the package is included in the tree since yesterday, and it contains Thomas's rework of the patch Guillaume W. Bres Software engineer <guillaume.bressaix@gmail.com> Le ven. 12 juin 2020 à 09:42, Guillaume Bres <guillaume.bressaix@gmail.com> a écrit : > Hello, > > If I understand correctly, this patch only covers the first use case > (C++/Python). I only added this one because my following patch requires > host-python-pybind, but I agree it certainly lacks a depends on > !BR2_INSTALL_LIBSTDCPP. I'll wait for people's point of view before going > any further anyway. If you agree on this, I will add the dependency and > push a *v2* > > The other use case that people might need in the future, is not allowed at > the moment until we unlock it with $(eval $(cmake-package)) but requires > further work (I am also not familiar with cmake at the moment). Maybe you > could proceed with adding the cmake support because you are knowledgeable > and already got it working? > > One last question though: "For example, this project won't find pybind11 > because it uses find_package(pybind11...)", so we should rename the package > to *python-pybind11*? so we don't have any other problems when unlocking > the cmake option. If you agree on that point, my v2 will be renamed to > *python-pybind11* & and I will also adapt my following work to this new > name > > Guillaume W. Bres > Software engineer > <guillaume.bressaix@gmail.com> > > > Le jeu. 11 juin 2020 à 20:14, Asaf Kahlon <asafka7@gmail.com> a écrit : > >> Hello, >> >> I'm not sure pybind11 is just a "regular" python package. >> First, it needs C++11 so we have to make sure the toolchain supports >> that in order to be able to actually compile the generated pybind11 >> code. >> Secondly, the installation through setup.py just installs the pybind11 >> headers and it doesn't give any way for C++ projects to locate the >> package. For example, this project won't find pybind11 because it uses >> find_package(pybind11...) in it's CMakeLists.txt: >> https://github.com/pybind/pybind11_json. The installation of pybind11 >> using CMake gives a solution to this problem. >> >> Those considerations have been taken into account in a previous patch >> I sent to the mailing list few weeks ago: >> >> https://patchwork.ozlabs.org/project/buildroot/patch/20200513183903.8656-1-asafka7@gmail.com/ >> >> Regards, >> Asaf. >> >
Hello, On Fri, Jun 12, 2020 at 10:43 AM Guillaume Bres <guillaume.bressaix@gmail.com> wrote: > > Hello, > > If I understand correctly, this patch only covers the first use case (C++/Python). I only added this one because my following patch requires host-python-pybind, but I agree it certainly lacks a depends on !BR2_INSTALL_LIBSTDCPP. I'll wait for people's point of view before going any further anyway. If you agree on this, I will add the dependency and push a v2 > This patch includes only the installation of pybind11 to the target - which isn't worth much since it includes only headers. It should be installed to the staging dir instead. As far as I understand, there's no point in installing this package to the target at all. In addition, pay attention you don't only need C++, but at least C++11. > The other use case that people might need in the future, is not allowed at the moment until we unlock it with $(eval $(cmake-package)) but requires further work (I am also not familiar with cmake at the moment). Maybe you could proceed with adding the cmake support because you are knowledgeable and already got it working? > Actually, the cmake part is already implemented in the patch I sent in the previous mail. Feel free to take any parts you need from it. > One last question though: "For example, this project won't find pybind11 because it uses find_package(pybind11...)", so we should rename the package to python-pybind11? so we don't have any other problems when unlocking the cmake option. If you agree on that point, my v2 will be renamed to python-pybind11 & and I will also adapt my following work to this new name > Well, only when you wrote that I noticed the package is named python-pybind on Buildroot, but the real package name is actually pybind11. So yes, I think it should be changed to python-pybind11. > Guillaume W. Bres > Software engineer > <guillaume.bressaix@gmail.com> > > > Le jeu. 11 juin 2020 à 20:14, Asaf Kahlon <asafka7@gmail.com> a écrit : >> >> Hello, >> >> I'm not sure pybind11 is just a "regular" python package. >> First, it needs C++11 so we have to make sure the toolchain supports >> that in order to be able to actually compile the generated pybind11 >> code. >> Secondly, the installation through setup.py just installs the pybind11 >> headers and it doesn't give any way for C++ projects to locate the >> package. For example, this project won't find pybind11 because it uses >> find_package(pybind11...) in it's CMakeLists.txt: >> https://github.com/pybind/pybind11_json. The installation of pybind11 >> using CMake gives a solution to this problem. >> >> Those considerations have been taken into account in a previous patch >> I sent to the mailing list few weeks ago: >> https://patchwork.ozlabs.org/project/buildroot/patch/20200513183903.8656-1-asafka7@gmail.com/ >> >> Regards, >> Asaf. Best regards, Asaf.
Asaf, Thomas, Well, only when you wrote that I noticed the package is named > python-pybind on Buildroot, but the real package name is actually > pybind11. So yes, I think it should be changed to python-pybind11. I proposed a patch to rename pybind11 so a potential cmake support in the future with $(eval(cmake-package)) does not face problems. The C++11 check is not needed until we make the package a cmake package, I prefered to push only the name changes at the moment, we will look into the cmake thing afterwards Thomas, I will revise the patch I have in standby once this package has been effectively renamed, it needs its dependency to 'host-python-pybind' updated Guillaume W. Bres Software engineer <guillaume.bressaix@gmail.com> Le ven. 12 juin 2020 à 16:55, Asaf Kahlon <asafka7@gmail.com> a écrit : > Hello, > > On Fri, Jun 12, 2020 at 10:43 AM Guillaume Bres > <guillaume.bressaix@gmail.com> wrote: > > > > Hello, > > > > If I understand correctly, this patch only covers the first use case > (C++/Python). I only added this one because my following patch requires > host-python-pybind, but I agree it certainly lacks a depends on > !BR2_INSTALL_LIBSTDCPP. I'll wait for people's point of view before going > any further anyway. If you agree on this, I will add the dependency and > push a v2 > > > This patch includes only the installation of pybind11 to the target - > which isn't worth much since it includes only headers. It should be > installed to the staging dir instead. As far as I understand, there's > no point in installing this package to the target at all. > In addition, pay attention you don't only need C++, but at least C++11. > > > The other use case that people might need in the future, is not allowed > at the moment until we unlock it with $(eval $(cmake-package)) but requires > further work (I am also not familiar with cmake at the moment). Maybe you > could proceed with adding the cmake support because you are knowledgeable > and already got it working? > > > Actually, the cmake part is already implemented in the patch I sent in > the previous mail. Feel free to take any parts you need from it. > > > One last question though: "For example, this project won't find pybind11 > because it uses find_package(pybind11...)", so we should rename the package > to python-pybind11? so we don't have any other problems when unlocking the > cmake option. If you agree on that point, my v2 will be renamed to > python-pybind11 & and I will also adapt my following work to this new name > > > Well, only when you wrote that I noticed the package is named > python-pybind on Buildroot, but the real package name is actually > pybind11. So yes, I think it should be changed to python-pybind11. > > > Guillaume W. Bres > > Software engineer > > <guillaume.bressaix@gmail.com> > > > > > > Le jeu. 11 juin 2020 à 20:14, Asaf Kahlon <asafka7@gmail.com> a écrit : > >> > >> Hello, > >> > >> I'm not sure pybind11 is just a "regular" python package. > >> First, it needs C++11 so we have to make sure the toolchain supports > >> that in order to be able to actually compile the generated pybind11 > >> code. > >> Secondly, the installation through setup.py just installs the pybind11 > >> headers and it doesn't give any way for C++ projects to locate the > >> package. For example, this project won't find pybind11 because it uses > >> find_package(pybind11...) in it's CMakeLists.txt: > >> https://github.com/pybind/pybind11_json. The installation of pybind11 > >> using CMake gives a solution to this problem. > >> > >> Those considerations have been taken into account in a previous patch > >> I sent to the mailing list few weeks ago: > >> > https://patchwork.ozlabs.org/project/buildroot/patch/20200513183903.8656-1-asafka7@gmail.com/ > >> > >> Regards, > >> Asaf. > Best regards, > Asaf. >
diff --git a/DEVELOPERS b/DEVELOPERS index f697c96ce4..9a94307082 100644 --- a/DEVELOPERS +++ b/DEVELOPERS @@ -1052,6 +1052,7 @@ F: package/sdl2/ N: Guillaume William Brs <guillaume.bressaix@gmail.com> F: package/liquid-dsp/ F: package/pixiewps/ +F: package/python-pybind/ F: package/reaver/ N: Guo Ren <ren_guo@c-sky.com> diff --git a/package/Config.in b/package/Config.in index 520e5d5570..b0a7da2f5c 100644 --- a/package/Config.in +++ b/package/Config.in @@ -1070,6 +1070,7 @@ menu "External python modules" source "package/python-pyalsa/Config.in" source "package/python-pyasn1/Config.in" source "package/python-pyasn1-modules/Config.in" + source "package/python-pybind/Config.in" source "package/python-pycairo/Config.in" source "package/python-pycares/Config.in" source "package/python-pycli/Config.in" diff --git a/package/python-pybind/Config.in b/package/python-pybind/Config.in new file mode 100644 index 0000000000..832a9fb971 --- /dev/null +++ b/package/python-pybind/Config.in @@ -0,0 +1,10 @@ +config BR2_PACKAGE_PYTHON_PYBIND + bool "python-pybind" + depends on BR2_PACKAGE_PYTHON3 + help + PyBind is a lightweight header-only library + that exposes C++ types in Python and vice versa, + mainly to create Python bindings of existing C++ + code. + + http://pybind11.readthedocs.org/en/master diff --git a/package/python-pybind/python-pybind.hash b/package/python-pybind/python-pybind.hash new file mode 100644 index 0000000000..6578f60c88 --- /dev/null +++ b/package/python-pybind/python-pybind.hash @@ -0,0 +1,4 @@ +# Locally calculated +sha256 97504db65640570f32d3fdf701c25a340c8643037c3b69aec469c10c93dc8504 v2.5.0.tar.gz +# License files, locally calculated +sha256 9a37ea54aa3cf12c7f3292799f20822ffd4b9b7142b36a7a9997b28c39264dc9 LICENSE diff --git a/package/python-pybind/python-pybind.mk b/package/python-pybind/python-pybind.mk new file mode 100644 index 0000000000..03cea1c060 --- /dev/null +++ b/package/python-pybind/python-pybind.mk @@ -0,0 +1,15 @@ +################################################################################ +# +# python-pybind +# +################################################################################ + +PYTHON_PYBIND_VERSION = 2.5.0 +PYTHON_PYBIND_SOURCE = v$(PYTHON_PYBIND_VERSION).tar.gz +PYTHON_PYBIND_SITE = https://github.com/pybind/pybind11/archive +PYTHON_PYBIND_LICENSE = BSD-3-Clause +PYTHON_PYBIND_LICENSE_FILES = LICENSE +PYTHON_PYBIND_SETUP_TYPE = setuptools + +$(eval $(python-package)) +$(eval $(host-python-package))
PyBind is a light (headers only) package for C++/Python and Python/C++ bindings. host-python-pybind is a dependency of my future contribution, so I added this feature. Signed-off-by: Guillaume W. Bres <guillaume.bressaix@gmail.com> --- DEVELOPERS | 1 + package/Config.in | 1 + package/python-pybind/Config.in | 10 ++++++++++ package/python-pybind/python-pybind.hash | 4 ++++ package/python-pybind/python-pybind.mk | 15 +++++++++++++++ 5 files changed, 31 insertions(+) create mode 100644 package/python-pybind/Config.in create mode 100644 package/python-pybind/python-pybind.hash create mode 100644 package/python-pybind/python-pybind.mk