All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH/next v2 1/2]: pybind11: new package
@ 2021-12-04 12:05 guillaume.bressaix
  2021-12-04 12:05 ` [Buildroot] [PATCH/next v2 2/2]: remove python-pybind guillaume.bressaix
  2021-12-04 13:46 ` [Buildroot] [PATCH/next v2 1/2]: pybind11: new package Yann E. MORIN
  0 siblings, 2 replies; 5+ messages in thread
From: guillaume.bressaix @ 2021-12-04 12:05 UTC (permalink / raw)
  To: buildroot; +Cc: thomas.petazzoni, yann.morin.1998, asafka7, Guillaume W. Bres

From: "Guillaume W. Bres" <guillaume.bressaix@gmail.com>

fixes http://autobuild.buildroot.net/results/b89f1de64b308dffa73675f1f31ccb0b7be5a10d
fixes http://autobuild.buildroot.net/results/d0287b7f64f206b0f074908c5780a3632e0cb799
fixes http://autobuild.buildroot.net/results/27efb545a5a719a5581c8f746d3a3555ff4216ce
fixes http://autobuild.buildroot.net/results/d2f0a0ad8f6c7178517df109e7d885dac9134c3a
fixes http://autobuild.buildroot.net/results/b57e9a3279260dae4a590f9421238fcabb2f7cab
fixes http://autobuild.buildroot.net/results/515e6f2fc6b5780260d98d6bb52b541ce4bf1afe
fixes http://autobuild.buildroot.net/results/d89c4ecc81222d4f80c951da2232d2e393fa1c69
list goes on..

---
v2: Reviewed by gwen@trabucayre.com,
Removed some non needed empty lines,
force -DFINDPYTHON=OFF when using pybind11 without python,
handle legacy package properly in a seperate patch

v1: python-pybind was not the right approach and is in failure since
it's been upgraded to V2.6.1.

Building with setup.py now requires a cmake build first.
With this new approach we can build the package with cmake
for python bindings in C++ AND we also have the
C++ bindings in python as an option (depending & requiring the first one).

I make this a host-only package, in the sense that other packages will
require it at build time, and I don't forsee any reasons to have
such a package as a target package.

Signed-off-by: Guillaume W. Bres <guillaume.bressaix@gmail.com>
---
 DEVELOPERS                     |  1 +
 package/Config.in              |  1 +
 package/pybind11/Config.in     | 25 +++++++++++++++++++++
 package/pybind11/pybind11.hash |  3 +++
 package/pybind11/pybind11.mk   | 41 ++++++++++++++++++++++++++++++++++
 5 files changed, 71 insertions(+)
 create mode 100644 package/pybind11/Config.in
 create mode 100644 package/pybind11/pybind11.hash
 create mode 100644 package/pybind11/pybind11.mk

diff --git a/DEVELOPERS b/DEVELOPERS
index 6f812eb564..8a04efa63f 100644
--- a/DEVELOPERS
+++ b/DEVELOPERS
@@ -1105,6 +1105,7 @@ F:	package/libxcrypt/
 F:	package/liquid-dsp/
 F:	package/pixiewps/
 F:	package/python-pybind/
+F:	package/pybind11/
 F:	package/reaver/
 
 N:	Guo Ren <ren_guo@c-sky.com>
diff --git a/package/Config.in b/package/Config.in
index 311004db2c..5749118ee3 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -2006,6 +2006,7 @@ endif
 	source "package/protobuf/Config.in"
 	source "package/protobuf-c/Config.in"
 	source "package/protozero/Config.in"
+	source "package/pybind11/Config.in"
 	source "package/qhull/Config.in"
 	source "package/qlibc/Config.in"
 	source "package/riemann-c-client/Config.in"
diff --git a/package/pybind11/Config.in b/package/pybind11/Config.in
new file mode 100644
index 0000000000..4fc6c5eebc
--- /dev/null
+++ b/package/pybind11/Config.in
@@ -0,0 +1,25 @@
+comment "pybind11 needs a toolchain w/ C++, wchar"
+	depends on !BR2_INSTALL_LIBSTDCPP || !BR2_USE_WCHAR
+
+config BR2_PACKAGE_PYBIND11
+	bool "pybind11"
+	depends on BR2_USE_WCHAR # boost
+	depends on BR2_INSTALL_LIBSTDCPP # boost
+	depends on !BR2_TOOLCHAIN_HAS_GCC_BUG_64735 # boost-thread
+	select BR2_PACKAGE_BOOST
+	help
+	  Pybind11 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
+
+if BR2_PACKAGE_PYBIND11
+
+config BR2_PACKAGE_PYBIND11_WITH_PYTHON
+	bool "pybind11-python"
+	depends on BR2_PACKAGE_PYTHON3
+	help
+	  Activate support for python
+
+endif
diff --git a/package/pybind11/pybind11.hash b/package/pybind11/pybind11.hash
new file mode 100644
index 0000000000..ab8825bf04
--- /dev/null
+++ b/package/pybind11/pybind11.hash
@@ -0,0 +1,3 @@
+# Locally calculated
+sha256 f1bcc07caa568eb312411dde5308b1e250bd0e1bc020fae855bf9f43209940cc  pybind11-2.8.1.tar.gz
+sha256 83965b843b98f670d3a85bd041ed4b372c8ec50d7b4a5995a83ac697ba675dcb  LICENSE
diff --git a/package/pybind11/pybind11.mk b/package/pybind11/pybind11.mk
new file mode 100644
index 0000000000..a67ce237ea
--- /dev/null
+++ b/package/pybind11/pybind11.mk
@@ -0,0 +1,41 @@
+################################################################################
+#
+# pybind11
+#
+################################################################################
+
+PYBIND11_VERSION = 2.8.1
+PYBIND11_SITE = $(call github,pybind,pybind11,v$(PYBIND11_VERSION))
+PYBIND11_LICENSE = BSD-3-Clause
+PYBIND11_LICENSE_FILES = LICENSE
+PYBIND11_INSTALL_STAGING = YES
+PYBIND11_SUPPORTS_IN_SOURCE_BUILD = YES
+
+HOST_PYBIND11_CONF_OPTS = \
+	-DBUILD_DOCS=OFF \
+	-DDOWNLOAD_EIGEN=OFF
+
+# pybind11 python support activation
+ifeq ($(BR2_PACKAGE_PYBIND11_WITH_PYTHON),y)
+HOST_PYBIND11_DEPENDENCIES += host-python3
+
+# pybind11 with python requires cmake install in $(@D)
+HOST_PYBIND11_CONF_OPTS += \
+	-DCMAKE_INSTALL_PREFIX=$(@D)/pybind11 \
+	-DPYTHON=$(HOST_DIR)/bin/python3 \
+	-DPYTHON_PREFIX=$(STAGING_DIR)/usr \
+	-DPYBIND_FINDPYTHON=ON \
+	-DPYBIND11_NOPYTHON=OFF
+
+define PYBIND11_PYTHON_BUILD
+	cd $(@D) && $(HOST_DIR)/bin/python setup.py install
+endef
+
+HOST_PYBIND11_POST_INSTALL_HOOKS += PYBIND11_PYTHON_BUILD
+else
+HOST_PYBIND11_CONF_OPTS += \
+	-DPYBIND_FINDPYTHON=OFF \
+	-DPYBIND11_NOPYTHON=ON
+endif
+
+$(eval $(host-cmake-package))
-- 
2.20.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [Buildroot] [PATCH/next v2 2/2]: remove python-pybind
  2021-12-04 12:05 [Buildroot] [PATCH/next v2 1/2]: pybind11: new package guillaume.bressaix
@ 2021-12-04 12:05 ` guillaume.bressaix
  2021-12-04 13:46 ` [Buildroot] [PATCH/next v2 1/2]: pybind11: new package Yann E. MORIN
  1 sibling, 0 replies; 5+ messages in thread
From: guillaume.bressaix @ 2021-12-04 12:05 UTC (permalink / raw)
  To: buildroot; +Cc: thomas.petazzoni, yann.morin.1998, asafka7, Guillaume W. Bres

From: "Guillaume W. Bres" <guillaume.bressaix@gmail.com>

python-pybind was not the right approach and build fails
since v2.6.1

Signed-off-by: Guillaume W. Bres <guillaume.bressaix@gmail.com>
---
 Config.in.legacy                         |  7 +++++++
 DEVELOPERS                               |  1 -
 package/Config.in                        |  1 -
 package/python-pybind/Config.in          |  8 --------
 package/python-pybind/python-pybind.hash |  4 ----
 package/python-pybind/python-pybind.mk   | 13 -------------
 6 files changed, 7 insertions(+), 27 deletions(-)
 delete mode 100644 package/python-pybind/Config.in
 delete mode 100644 package/python-pybind/python-pybind.hash
 delete mode 100644 package/python-pybind/python-pybind.mk

diff --git a/Config.in.legacy b/Config.in.legacy
index f3ecf981c0..37f56e4b9d 100644
--- a/Config.in.legacy
+++ b/Config.in.legacy
@@ -146,6 +146,13 @@ endif
 
 comment "Legacy options removed in 2022.02"
 
+config BR2_PACKAGE_PYTHON_PYBIND
+	bool "python-pyind removed"
+	select BR2_PACKAGE_LEGACY
+	help
+	  'python-pybind' is not the right approach since v2.6.1,
+	  replaced by host-pybind11
+
 config BR2_PACKAGE_LIBMEDIAART_BACKEND_NONE
 	bool "libmediaart 'none' backend removed"
 	select BR2_LEGACY
diff --git a/DEVELOPERS b/DEVELOPERS
index 8a04efa63f..e5e6d0f538 100644
--- a/DEVELOPERS
+++ b/DEVELOPERS
@@ -1104,7 +1104,6 @@ F:	package/libnids/
 F:	package/libxcrypt/
 F:	package/liquid-dsp/
 F:	package/pixiewps/
-F:	package/python-pybind/
 F:	package/pybind11/
 F:	package/reaver/
 
diff --git a/package/Config.in b/package/Config.in
index 5749118ee3..b3bf61a13e 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -1132,7 +1132,6 @@ 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
deleted file mode 100644
index 604cb9ee67..0000000000
--- a/package/python-pybind/Config.in
+++ /dev/null
@@ -1,8 +0,0 @@
-config BR2_PACKAGE_PYTHON_PYBIND
-	bool "python-pybind"
-	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
deleted file mode 100644
index a68ac846e2..0000000000
--- a/package/python-pybind/python-pybind.hash
+++ /dev/null
@@ -1,4 +0,0 @@
-# Locally calculated
-sha256  cdbe326d357f18b83d10322ba202d69f11b2f49e2d87ade0dc2be0c5c34f8e2a  python-pybind-2.6.1.tar.gz
-# License files, locally calculated
-sha256  83965b843b98f670d3a85bd041ed4b372c8ec50d7b4a5995a83ac697ba675dcb  LICENSE
diff --git a/package/python-pybind/python-pybind.mk b/package/python-pybind/python-pybind.mk
deleted file mode 100644
index a6a1bdb976..0000000000
--- a/package/python-pybind/python-pybind.mk
+++ /dev/null
@@ -1,13 +0,0 @@
-################################################################################
-#
-# python-pybind
-#
-################################################################################
-
-PYTHON_PYBIND_VERSION = 2.6.1
-PYTHON_PYBIND_SITE = $(call github,pybind,pybind11,v$(PYTHON_PYBIND_VERSION))
-PYTHON_PYBIND_LICENSE = BSD-3-Clause
-PYTHON_PYBIND_LICENSE_FILES = LICENSE
-PYTHON_PYBIND_SETUP_TYPE = setuptools
-
-$(eval $(python-package))
-- 
2.20.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [Buildroot] [PATCH/next v2 1/2]: pybind11: new package
  2021-12-04 12:05 [Buildroot] [PATCH/next v2 1/2]: pybind11: new package guillaume.bressaix
  2021-12-04 12:05 ` [Buildroot] [PATCH/next v2 2/2]: remove python-pybind guillaume.bressaix
@ 2021-12-04 13:46 ` Yann E. MORIN
  2021-12-05 11:35   ` Guillaume Bres
  1 sibling, 1 reply; 5+ messages in thread
From: Yann E. MORIN @ 2021-12-04 13:46 UTC (permalink / raw)
  To: guillaume.bressaix; +Cc: buildroot, asafka7, thomas.petazzoni

Guillaume, All,

On 2021-12-04 13:05 +0100, guillaume.bressaix@gmail.com spake thusly:
> From: "Guillaume W. Bres" <guillaume.bressaix@gmail.com>
> 
> fixes http://autobuild.buildroot.net/results/b89f1de64b308dffa73675f1f31ccb0b7be5a10d
> fixes http://autobuild.buildroot.net/results/d0287b7f64f206b0f074908c5780a3632e0cb799
> fixes http://autobuild.buildroot.net/results/27efb545a5a719a5581c8f746d3a3555ff4216ce
> fixes http://autobuild.buildroot.net/results/d2f0a0ad8f6c7178517df109e7d885dac9134c3a
> fixes http://autobuild.buildroot.net/results/b57e9a3279260dae4a590f9421238fcabb2f7cab
> fixes http://autobuild.buildroot.net/results/515e6f2fc6b5780260d98d6bb52b541ce4bf1afe
> fixes http://autobuild.buildroot.net/results/d89c4ecc81222d4f80c951da2232d2e393fa1c69
> list goes on..
> 
> ---
> v2: Reviewed by gwen@trabucayre.com,
> Removed some non needed empty lines,
> force -DFINDPYTHON=OFF when using pybind11 without python,
> handle legacy package properly in a seperate patch
> 
> v1: python-pybind was not the right approach and is in failure since
> it's been upgraded to V2.6.1.

This form here:

> Building with setup.py now requires a cmake build first.
> With this new approach we can build the package with cmake
> for python bindings in C++ AND we also have the
> C++ bindings in python as an option (depending & requiring the first one).
> 
> I make this a host-only package, in the sense that other packages will
> require it at build time, and I don't forsee any reasons to have
> such a package as a target package.
> 
> Signed-off-by: Guillaume W. Bres <guillaume.bressaix@gmail.com>

... to here, should be part of the commit log, i.e. before the first
'---' line, above. Otherwise, it is dropped by git when the patch is
applied.

However, I am still not sure what is going on here...

First, you are removing pybind to then introduce pybind11, although they
are the exact same package upstream:
    http://pybind11.readthedocs.org/en/master (home)
    https://github.com/pybind/pybind11 (repo)

But since this is the same upstream, you should just fix the existing
pybind package in-place; there is no reason to intriduce a new pacjage
to fix an existing one.

Then, I did not find the explanations in the commit log very convincing.
Why state that "python-pybind was not the right approach"? As far as I
understand, it was working as expected until the bump to 2.6.1, and thus
was not a "failure".

If the bump to 2.6.1 broke the package, then that means the bump was not
careful, not that the package is a failure.

Fionally, why is a host-only package? If it installs C++ headers, then
we can expect packages built for the target to include those ehaders,
and so we need them in staging too. And this is made obvious by your
post-install hook, that uses STAGING_DIR. This is ultimately wrong,
because then that means that host packages won't be able to find/use
those headers.

Minor nit: the commit log should not be in the first singular person,
ie.e do not use "I", but in the `neutral' first person plural, i.e. use
"we".

However, I admit that the whole situation evades my understanding, so I
may have miss more tricky details... I will gladly accept being
corrected on those. ;-)

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Buildroot] [PATCH/next v2 1/2]: pybind11: new package
  2021-12-04 13:46 ` [Buildroot] [PATCH/next v2 1/2]: pybind11: new package Yann E. MORIN
@ 2021-12-05 11:35   ` Guillaume Bres
  2021-12-05 14:07     ` Arnout Vandecappelle
  0 siblings, 1 reply; 5+ messages in thread
From: Guillaume Bres @ 2021-12-05 11:35 UTC (permalink / raw)
  To: Yann E. MORIN, buildroot, Thomas Petazzoni


[-- Attachment #1.1: Type: text/plain, Size: 5555 bytes --]

Hello Yann,
thanks for the feedback.

I integrated all your recommendations for my next submission, and I am
currently testing all combinations.
I will follow your suggestion and integrate this as a patch serie, so there
won't be an actual v3 follow up,
but this way you guys can see the whole picture.

First, here's the answer to your question that is still pending:

>But since this is the same upstream, you should just fix the existing
>pybind package in-place; there is no reason to intriduce a new pacjage
>to fix an existing one.

This is not doable, because of the python-pkg naming convention.
The previous approach was partially fine, that is true.
It delivered C++/Py bindings (and only those) and was built as a python-pkg.
So basically, the cmake side (Py/C++ bindings) was left out.
Now the package requires cmake whatever happens, therefore it can no longer
be a python-only package.

>why is a host-only package? If it installs C++ headers, then
>we can expect packages built for the target to include those headers,
>and so we need them in staging too. And this is made obvious by your
>post-install hook, that uses STAGING_DIR
I am in the process of testing all the combinations, but I managed to have
a target + staging install to also work.
This being a pure library, would be handy to other people that might have a
different use case than mine

Guillaume W. Bres
Software engineer
<guillaume.bressaix@gmail.com>


Le sam. 4 déc. 2021 à 14:46, Yann E. MORIN <yann.morin.1998@free.fr> a
écrit :

> Guillaume, All,
>
> On 2021-12-04 13:05 +0100, guillaume.bressaix@gmail.com spake thusly:
> > From: "Guillaume W. Bres" <guillaume.bressaix@gmail.com>
> >
> > fixes
> http://autobuild.buildroot.net/results/b89f1de64b308dffa73675f1f31ccb0b7be5a10d
> > fixes
> http://autobuild.buildroot.net/results/d0287b7f64f206b0f074908c5780a3632e0cb799
> > fixes
> http://autobuild.buildroot.net/results/27efb545a5a719a5581c8f746d3a3555ff4216ce
> > fixes
> http://autobuild.buildroot.net/results/d2f0a0ad8f6c7178517df109e7d885dac9134c3a
> > fixes
> http://autobuild.buildroot.net/results/b57e9a3279260dae4a590f9421238fcabb2f7cab
> > fixes
> http://autobuild.buildroot.net/results/515e6f2fc6b5780260d98d6bb52b541ce4bf1afe
> > fixes
> http://autobuild.buildroot.net/results/d89c4ecc81222d4f80c951da2232d2e393fa1c69
> > list goes on..
> >
> > ---
> > v2: Reviewed by gwen@trabucayre.com,
> > Removed some non needed empty lines,
> > force -DFINDPYTHON=OFF when using pybind11 without python,
> > handle legacy package properly in a seperate patch
> >
> > v1: python-pybind was not the right approach and is in failure since
> > it's been upgraded to V2.6.1.
>
> This form here:
>
> > Building with setup.py now requires a cmake build first.
> > With this new approach we can build the package with cmake
> > for python bindings in C++ AND we also have the
> > C++ bindings in python as an option (depending & requiring the first
> one).
> >
> > I make this a host-only package, in the sense that other packages will
> > require it at build time, and I don't forsee any reasons to have
> > such a package as a target package.
> >
> > Signed-off-by: Guillaume W. Bres <guillaume.bressaix@gmail.com>
>
> ... to here, should be part of the commit log, i.e. before the first
> '---' line, above. Otherwise, it is dropped by git when the patch is
> applied.
>
> However, I am still not sure what is going on here...
>
> First, you are removing pybind to then introduce pybind11, although they
> are the exact same package upstream:
>     http://pybind11.readthedocs.org/en/master (home)
>     https://github.com/pybind/pybind11 (repo)
>
> But since this is the same upstream, you should just fix the existing
> pybind package in-place; there is no reason to intriduce a new pacjage
> to fix an existing one.
>
> Then, I did not find the explanations in the commit log very convincing.
> Why state that "python-pybind was not the right approach"? As far as I
> understand, it was working as expected until the bump to 2.6.1, and thus
> was not a "failure".
>
> If the bump to 2.6.1 broke the package, then that means the bump was not
> careful, not that the package is a failure.
>
> Fionally, why is a host-only package? If it installs C++ headers, then
> we can expect packages built for the target to include those ehaders,
> and so we need them in staging too. And this is made obvious by your
> post-install hook, that uses STAGING_DIR. This is ultimately wrong,
> because then that means that host packages won't be able to find/use
> those headers.
>
> Minor nit: the commit log should not be in the first singular person,
> ie.e do not use "I", but in the `neutral' first person plural, i.e. use
> "we".
>
> However, I admit that the whole situation evades my understanding, so I
> may have miss more tricky details... I will gladly accept being
> corrected on those. ;-)
>
> Regards,
> Yann E. MORIN.
>
> --
>
> .-----------------.--------------------.------------------.--------------------.
> |  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics'
> conspiracy: |
> | +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___
>      |
> | +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is
> no  |
> | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v
>  conspiracy.  |
>
> '------------------------------^-------^------------------^--------------------'
>

[-- Attachment #1.2: Type: text/html, Size: 8238 bytes --]

[-- Attachment #2: Type: text/plain, Size: 150 bytes --]

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Buildroot] [PATCH/next v2 1/2]: pybind11: new package
  2021-12-05 11:35   ` Guillaume Bres
@ 2021-12-05 14:07     ` Arnout Vandecappelle
  0 siblings, 0 replies; 5+ messages in thread
From: Arnout Vandecappelle @ 2021-12-05 14:07 UTC (permalink / raw)
  To: Guillaume Bres, Yann E. MORIN, buildroot, Thomas Petazzoni



On 05/12/2021 12:35, Guillaume Bres wrote:
> Hello Yann,
> thanks for the feedback.
> 
> I integrated all your recommendations for my next submission, and I am currently 
> testing all combinations.
> I will follow your suggestion and integrate this as a patch serie, so there 
> won't be an actual v3 follow up,
> but this way you guys can see the whole picture.
> 
> First, here's the answer to your question that is still pending:
> 
>  >But since this is the same upstream, you should just fix the existing
>  >pybind package in-place; there is no reason to intriduce a new pacjage
>  >to fix an existing one.
> 
> This is not doable, because of the python-pkg naming convention.
> The previous approach was partially fine, that is true.
> It delivered C++/Py bindings (and only those) and was built as a python-pkg.

  I think you're conflating naming convention with package infrastructure here.

  The python-foo naming convention is just a name that indicates that it's some 
infrastructure that is strictly related to the Python language. pybind is a bit 
of a border case because it's about bindings between python and other languages, 
but Python is still pretty much central in it. It's true that it's theoretically 
possible to use the package without any python on the target, but is that really 
a use case? So maybe there are arguments to remove the python- prefix from it, 
but I would say the overhead of legacy handling makes it that those arguments 
would have to be really strong. (With overhead I mean the pain it causes people 
who upgrade buildroot, not so much the maintenance overhead.)

  The objection you're raising, however, is that the python infrastructure is 
not appropriate. However, it's perfectly acceptable for a python-foo package to 
have some other infrastructure. For example, python-gobject uses meson, 
python-sip uses generic (not surprisingly, both of those are also bindings 
packages).

  Bottom line: keep the name python-pybind, but change it from python-package to 
cmake-package.


  Regards,
  Arnout


> So basically, the cmake side (Py/C++ bindings) was left out.
> Now the package requires cmake whatever happens, therefore it can no longer be a 
> python-only package.
> 
>  >why is a host-only package? If it installs C++ headers, then
>  >we can expect packages built for the target to include those headers,
>  >and so we need them in staging too. And this is made obvious by your
>  >post-install hook, that uses STAGING_DIR
> I am in the process of testing all the combinations, but I managed to have a 
> target + staging install to also work.
> This being a pure library, would be handy to other people that might have a 
> different use case than mine
> 
> Guillaume W. Bres
> Software engineer
> <guillaume.bressaix@gmail.com <mailto:guillaume.bressaix@gmail.com>>
> 
> 
> Le sam. 4 déc. 2021 à 14:46, Yann E. MORIN <yann.morin.1998@free.fr 
> <mailto:yann.morin.1998@free.fr>> a écrit :
> 
>     Guillaume, All,
> 
>     On 2021-12-04 13:05 +0100, guillaume.bressaix@gmail.com
>     <mailto:guillaume.bressaix@gmail.com> spake thusly:
>      > From: "Guillaume W. Bres" <guillaume.bressaix@gmail.com
>     <mailto:guillaume.bressaix@gmail.com>>
>      >
>      > fixes
>     http://autobuild.buildroot.net/results/b89f1de64b308dffa73675f1f31ccb0b7be5a10d
>     <http://autobuild.buildroot.net/results/b89f1de64b308dffa73675f1f31ccb0b7be5a10d>
>      > fixes
>     http://autobuild.buildroot.net/results/d0287b7f64f206b0f074908c5780a3632e0cb799
>     <http://autobuild.buildroot.net/results/d0287b7f64f206b0f074908c5780a3632e0cb799>
>      > fixes
>     http://autobuild.buildroot.net/results/27efb545a5a719a5581c8f746d3a3555ff4216ce
>     <http://autobuild.buildroot.net/results/27efb545a5a719a5581c8f746d3a3555ff4216ce>
>      > fixes
>     http://autobuild.buildroot.net/results/d2f0a0ad8f6c7178517df109e7d885dac9134c3a
>     <http://autobuild.buildroot.net/results/d2f0a0ad8f6c7178517df109e7d885dac9134c3a>
>      > fixes
>     http://autobuild.buildroot.net/results/b57e9a3279260dae4a590f9421238fcabb2f7cab
>     <http://autobuild.buildroot.net/results/b57e9a3279260dae4a590f9421238fcabb2f7cab>
>      > fixes
>     http://autobuild.buildroot.net/results/515e6f2fc6b5780260d98d6bb52b541ce4bf1afe
>     <http://autobuild.buildroot.net/results/515e6f2fc6b5780260d98d6bb52b541ce4bf1afe>
>      > fixes
>     http://autobuild.buildroot.net/results/d89c4ecc81222d4f80c951da2232d2e393fa1c69
>     <http://autobuild.buildroot.net/results/d89c4ecc81222d4f80c951da2232d2e393fa1c69>
>      > list goes on..
>      >
>      > ---
>      > v2: Reviewed by gwen@trabucayre.com <mailto:gwen@trabucayre.com>,
>      > Removed some non needed empty lines,
>      > force -DFINDPYTHON=OFF when using pybind11 without python,
>      > handle legacy package properly in a seperate patch
>      >
>      > v1: python-pybind was not the right approach and is in failure since
>      > it's been upgraded to V2.6.1.
> 
>     This form here:
> 
>      > Building with setup.py now requires a cmake build first.
>      > With this new approach we can build the package with cmake
>      > for python bindings in C++ AND we also have the
>      > C++ bindings in python as an option (depending & requiring the first one).
>      >
>      > I make this a host-only package, in the sense that other packages will
>      > require it at build time, and I don't forsee any reasons to have
>      > such a package as a target package.
>      >
>      > Signed-off-by: Guillaume W. Bres <guillaume.bressaix@gmail.com
>     <mailto:guillaume.bressaix@gmail.com>>
> 
>     ... to here, should be part of the commit log, i.e. before the first
>     '---' line, above. Otherwise, it is dropped by git when the patch is
>     applied.
> 
>     However, I am still not sure what is going on here...
> 
>     First, you are removing pybind to then introduce pybind11, although they
>     are the exact same package upstream:
>     http://pybind11.readthedocs.org/en/master
>     <http://pybind11.readthedocs.org/en/master> (home)
>     https://github.com/pybind/pybind11 <https://github.com/pybind/pybind11> (repo)
> 
>     But since this is the same upstream, you should just fix the existing
>     pybind package in-place; there is no reason to intriduce a new pacjage
>     to fix an existing one.
> 
>     Then, I did not find the explanations in the commit log very convincing.
>     Why state that "python-pybind was not the right approach"? As far as I
>     understand, it was working as expected until the bump to 2.6.1, and thus
>     was not a "failure".
> 
>     If the bump to 2.6.1 broke the package, then that means the bump was not
>     careful, not that the package is a failure.
> 
>     Fionally, why is a host-only package? If it installs C++ headers, then
>     we can expect packages built for the target to include those ehaders,
>     and so we need them in staging too. And this is made obvious by your
>     post-install hook, that uses STAGING_DIR. This is ultimately wrong,
>     because then that means that host packages won't be able to find/use
>     those headers.
> 
>     Minor nit: the commit log should not be in the first singular person,
>     ie.e do not use "I", but in the `neutral' first person plural, i.e. use
>     "we".
> 
>     However, I admit that the whole situation evades my understanding, so I
>     may have miss more tricky details... I will gladly accept being
>     corrected on those. ;-)
> 
>     Regards,
>     Yann E. MORIN.
> 
>     -- 
>     .-----------------.--------------------.------------------.--------------------.
>     |  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
>     | +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
>     | +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
>     | http://ymorin.is-a-geek.org/ <http://ymorin.is-a-geek.org/> | _/*\_ | / \
>     HTML MAIL    |   v   conspiracy.  |
>     '------------------------------^-------^------------------^--------------------'
> 
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot
> 
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-12-05 14:08 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-04 12:05 [Buildroot] [PATCH/next v2 1/2]: pybind11: new package guillaume.bressaix
2021-12-04 12:05 ` [Buildroot] [PATCH/next v2 2/2]: remove python-pybind guillaume.bressaix
2021-12-04 13:46 ` [Buildroot] [PATCH/next v2 1/2]: pybind11: new package Yann E. MORIN
2021-12-05 11:35   ` Guillaume Bres
2021-12-05 14:07     ` Arnout Vandecappelle

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.