[Buildroot] package/pkg-python.mk: refactor setup-type variables
James Hilliard
james.hilliard1 at gmail.com
Sun Nov 26 18:22:28 UTC 2023
On Sun, Nov 26, 2023 at 4:21 AM Yann E. MORIN <yann.morin.1998 at free.fr> wrote:
>
> James, All,
>
> On 2023-11-25 18:40 -0700, James Hilliard spake thusly:
> > On Sat, Nov 25, 2023 at 2:22 PM Yann E. MORIN <yann.morin.1998 at free.fr> wrote:
> > > I think we could even go a bit further and drop the conditional block
> > > that defines the dependencies...
> > Which conditional dependencies block are you referring to?
>
> This one:
>
> https://gitlab.com/buildroot.org/buildroot/-/blob/next/package/pkg-python.mk#L333
>
> > I rebased my setuptools pep517 migration, once that's merged and we've
> > removed distutils support there's a good bit of refactoring we can do to
> > simplify things as everything will then use pep517 build/install commands.
> >
> > https://patchwork.ozlabs.org/project/buildroot/patch/20231126011932.1380631-5-james.hilliard1@gmail.com/
>
> As far as I can see, that patch does not refactor the dependencies in a
> way similar to how the other variables have been refactored, i.e.
> something like:
>
> PKG_PYTHON_SETUPTOOLS_DEPENDENCIES = host-python-setuptools
>
> PKG_PYTHON_SETUPTOOLS_RUST_DEPENDENCIES = host-python-setuptools host-python-setuptools-rust
>
> and so on (and for the HOST_ variants as well), and then:
Actually HOST_ variants are not needed here since all setup type specific
dependencies are the same for host and target package builds.
>
> $(2)_DEPENDENCIES += $$(PKG_PYTHON_$$($$(PKG)_SETUP_TYPE_UPPER)_DEPENDENCIES)
I had to tweak this a little to get it to work since it seems we can't
use $$(PKG) here:
$(2)_DEPENDENCIES += $$(PKG_PYTHON_$$($(2)_SETUP_TYPE_UPPER)_DEPENDENCIES)
>
> which, as I suggested, would do with the dependency comnditional
> block basically the same as the previous patches have done for the build
> commands.
Ok, I refactored my v6 to use this style:
https://patchwork.ozlabs.org/project/buildroot/patch/20231126180840.2081945-5-james.hilliard1@gmail.com/
>
> And if we're smart, we can go even further and handle all the
> setup-type-based variables similarly:
>
> PKG_PYTHON_SETUPTOOLS_RUST_DL_ENV = $$(PKG_CARGO_ENV)
>
> PKG_PYTHON_MATURIN_RUST_DL_ENV = $$(PKG_CARGO_ENV)
>
> and then:
>
> $(2)_DL_ENV += $$(PKG_PYTHON_$$($$(PKG)_SETUP_TYPE_UPPER)_DL_ENV)
>
> It is not that the remaining conditional block are as unwieldy as the
> previos one about build sommands, but it would be goos to have some
> consistency in the way the various setup-types are handled.
I'm not sure this makes sense since all the cargo DL_ENV stuff shouldn't
change based on a package being setuptools-rust vs maturin.
>
> Regards,
> Yann E. MORIN.
>
> > >
> > > Regards,
> > > Yann E. MORIN.
> > >
> > > > The rist two patches are simple cleanups, removing variables that are
> > > > not really needed.
> > > >
> > > > The third patch is a preparatory one that splits the build commands for
> > > > target and host builds. This is necessary because we need to use
> > > > different variables for the two, i.e. HOST_PKG_PYTHON_* for host build.
> > > >
> > > > The fourth and fifth patch each replace one variable from the
> > > > conditional tree with an indirectly addressed one.
> > > >
> > > > The following three patches are needed because the ninth patch removes
> > > > the PKG_PYTHON_*_OPTS variables, which were used in 3 packages. It is in
> > > > fact not strictly needed to remove those variables, but IMHO they don't
> > > > add sufficient value to keep them. I think it's better to handle the few
> > > > special cases explicitly.
> > > >
> > > > The ninth patch replaces the remaining variables from the conditional
> > > > tree with indirectly addressed ones. The conditional tree is now empty,
> > > > and the error handling that was in there is made more explicit.
> > > >
> > > > The tenth patch edits the documentation to remove the references to the
> > > > PKG_PYTHON_*_OPTS variables that were removed in the previous patch.
> > > >
> > > > This series adds lines rather than removing lines. That is because
> > > > things are made more explicit, introducing per-setup-type variables
> > > > where previously several setup types were (partially) reusing the pep517
> > > > variables. There are also some additional lines because of more line
> > > > splitting.
> > > >
> > > > To test this series, I built all the python packages with one specific
> > > > toolchain configuration, and I ran all runtime tests for python packages.
> > > > One package failed to build: host-python-sip. It also fails on master.
> > > > For the runtime tests, 14 of them failed, all of them also fail on
> > > > master.
> > > >
> > > >
> > > > The following changes since commit 7906272c39744e26ed73028725787aa3a4441c54:
> > > >
> > > > package/python-rtoml: migrate to setuptools-rust infrastructure (2023-09-29 22:02:31 +0200)
> > > >
> > > > are available in the Git repository at:
> > > >
> > > > git at gitlab.com:arnout/buildroot.git pkg-python-refactor-variables
> > > >
> > > > for you to fetch changes up to 89b33e004fc13e22a9c0bdcafcee245a97d2dd51:
> > > >
> > > > docs/manual: remove references to PKG_PYTHON_*_OPTS (2023-09-30 15:18:48 +0200)
> > > >
> > > > ----------------------------------------------------------------
> > > > Arnout Vandecappelle (Essensium/Mind) (10):
> > > > package/pkg-python.mk: remove $(2)_PYTHON_INTERPRETER variable
> > > > package/pkg-python.mk: remove _BASE_BUILD_OPTS variable
> > > > package/pkg-python.mk: split the commands in a target and host section
> > > > package/pkg-python.mk: replace $(_BASE_ENV) with $($(SETUP_TYPE)_ENV)
> > > > package/pkg-python.mk: replace $(_BASE_BUILD_CMD) with $($(SETUP_TYPE)_BUILD_CMD)
> > > > package/python-flit-core: instantiate _INSTALL_CMDS
> > > > package/jailhouse: expand PKG_PYTHON_SETUPTOOLS_INSTALL_TARGET_OPTS
> > > > package/i2c-tools: expand PKG_PYTHON_SETUPTOOLS_INSTALL_TARGET_OPTS
> > > > package/pkg-python.mk: replace $(_BASE_INSTALL*_CMD) with $($(SETUP_TYPE)_INSTALL*_CMD)
> > > > docs/manual: remove references to PKG_PYTHON_*_OPTS
> > > >
> > > > docs/manual/adding-packages-python.txt | 14 +-------
> > > > package/i2c-tools/i2c-tools.mk | 6 ++--
> > > > package/jailhouse/jailhouse.mk | 7 +++-
> > > > package/pkg-python.mk | 256 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------------------------------------------
> > > > package/python-flit-core/python-flit-core.mk | 9 +++--
> > > > 5 files changed, 173 insertions(+), 119 deletions(-)
> > > >
> > > >
> > > >
> > > > _______________________________________________
> > > > buildroot mailing list
> > > > buildroot at buildroot.org
> > > > https://lists.buildroot.org/mailman/listinfo/buildroot
> > >
> > > --
> > > .-----------------.--------------------.------------------.--------------------.
> > > | 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. |
> > > '------------------------------^-------^------------------^--------------------'
>
> --
> .-----------------.--------------------.------------------.--------------------.
> | 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. |
> '------------------------------^-------^------------------^--------------------'
More information about the buildroot
mailing list