[Buildroot] [PATCH/next v2 1/2]: pybind11: new package
Arnout Vandecappelle
arnout at mind.be
Sun Dec 5 14:07:53 UTC 2021
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 at gmail.com <mailto:guillaume.bressaix at gmail.com>>
>
>
> Le sam. 4 déc. 2021 à 14:46, Yann E. MORIN <yann.morin.1998 at free.fr
> <mailto:yann.morin.1998 at free.fr>> a écrit :
>
> Guillaume, All,
>
> On 2021-12-04 13:05 +0100, guillaume.bressaix at gmail.com
> <mailto:guillaume.bressaix at gmail.com> spake thusly:
> > From: "Guillaume W. Bres" <guillaume.bressaix at gmail.com
> <mailto:guillaume.bressaix at 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 at trabucayre.com <mailto:gwen at 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 at gmail.com
> <mailto:guillaume.bressaix at 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 at buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot
>
More information about the buildroot
mailing list