| Message ID | 20220309150224.2661618-1-ross.burton@arm.com |
|---|---|
| State | Accepted, archived |
| Commit | e6e4c63bbdd09d91428e55cb5a852170511f05cc |
| Headers | show |
| Series | pip_install_wheel: install wheel with a glob | expand |
On 09.03.22 16:02, Ross Burton wrote: > Now that the build systems that use pip_install_wheel are all building > their wheel into a directory that we knew was empty before, we can just > install *.whl and not need to know the precise names. > > By design a pyproject.toml will always build a single wheel, so there > shouldn't be any way for this to end up installing more than expected. > > This obsoletes PIP_INSTALL_PACKAGE and PYPA_WHEEL, neither of which are > needed anymore. > > Signed-off-by: Ross Burton <ross.burton@arm.com> > --- > meta/classes/pip_install_wheel.bbclass | 14 +++----------- > 1 file changed, 3 insertions(+), 11 deletions(-) > > diff --git a/meta/classes/pip_install_wheel.bbclass b/meta/classes/pip_install_wheel.bbclass > index 954a6b750f..5d09e795d0 100644 > --- a/meta/classes/pip_install_wheel.bbclass > +++ b/meta/classes/pip_install_wheel.bbclass > @@ -1,20 +1,10 @@ > DEPENDS:append = " python3-pip-native" > > -def guess_pip_install_package_name(d): > - import re > - '''https://www.python.org/dev/peps/pep-0491/#escaping-and-unicode''' > - name = d.getVar('PYPI_PACKAGE') or re.sub(r"^python3-", "", d.getVar('BPN')) > - return name.replace('-', '_') > - > -PIP_INSTALL_PACKAGE ?= "${@guess_pip_install_package_name(d)}" > - > # The directory where wheels should be written too. Build classes > # will ideally [cleandirs] this but we don't do that here in case > # a recipe wants to install prebuilt wheels. > PIP_INSTALL_DIST_PATH ?= "${WORKDIR}/dist" > > -PYPA_WHEEL ??= "${PIP_INSTALL_DIST_PATH}/${PIP_INSTALL_PACKAGE}-*-*.whl" > - > PIP_INSTALL_ARGS = "\ > -vvvv \ > --ignore-installed \ > @@ -29,7 +19,9 @@ PIP_INSTALL_PYTHON = "python3" > PIP_INSTALL_PYTHON:class-native = "nativepython3" > > pip_install_wheel_do_install () { > - nativepython3 -m pip install ${PIP_INSTALL_ARGS} ${PYPA_WHEEL} || > + test -f ${PIP_INSTALL_DIST_PATH}/*.whl || bbfatal No wheels in ${PIP_INSTALL_DIST_PATH} Can we, for the very unlikely case that do have more than one wheel in this dir, bbfatal out here as well - otherwise I'm worried that this will never get noticed > + > + nativepython3 -m pip install ${PIP_INSTALL_ARGS} ${PIP_INSTALL_DIST_PATH}/*.whl || > bbfatal_log "Failed to pip install wheel. Check the logs." > > cd ${D} > > > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#162964): https://lists.openembedded.org/g/openembedded-core/message/162964 > Mute This Topic: https://lists.openembedded.org/mt/89664344/3647476 > Group Owner: openembedded-core+owner@lists.openembedded.org > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [kweihmann@outlook.com] > -=-=-=-=-=-=-=-=-=-=-=- >
On Wed, 9 Mar 2022 at 15:47, Konrad Weihmann <kweihmann@outlook.com> wrote: > Can we, for the very unlikely case that do have more than one wheel in > this dir, bbfatal out here as well - otherwise I'm worried that this > will never get noticed I'm undecided on this. In the unlikely case (impossible, by design) that a number of wheels are built for a single package, wouldn't we want to install all of them as we're building the package? Ross
On 09.03.22 16:53, Ross Burton wrote: > On Wed, 9 Mar 2022 at 15:47, Konrad Weihmann <kweihmann@outlook.com> wrote: >> Can we, for the very unlikely case that do have more than one wheel in >> this dir, bbfatal out here as well - otherwise I'm worried that this >> will never get noticed > > I'm undecided on this. In the unlikely case (impossible, by design) > that a number of wheels are built for a single package, wouldn't we > want to install all of them as we're building the package? Classic assertion case IMO - as you said there shouldn't more than one, but in the event of having more than one, I would just terminate the build, as likely something is very wrong (like people playing around if task appends or custom tasks in between compile and install). I'm just concerned that this here would create a blind spot, where there none needed > > Ross
On Wed, Mar 9, 2022 at 7:57 AM Konrad Weihmann <kweihmann@outlook.com> wrote: > > > > On 09.03.22 16:53, Ross Burton wrote: > > On Wed, 9 Mar 2022 at 15:47, Konrad Weihmann <kweihmann@outlook.com> wrote: > >> Can we, for the very unlikely case that do have more than one wheel in > >> this dir, bbfatal out here as well - otherwise I'm worried that this > >> will never get noticed > > > > I'm undecided on this. In the unlikely case (impossible, by design) > > that a number of wheels are built for a single package, wouldn't we > > want to install all of them as we're building the package? > > Classic assertion case IMO - as you said there shouldn't more than one, > but in the event of having more than one, I would just terminate the > build, as likely something is very wrong (like people playing around if > task appends or custom tasks in between compile and install). > > I'm just concerned that this here would create a blind spot, where there > none needed will install work if there are more than one .whl file expected ? I think we will get install failures, in that case it maybe just an over optimization > > > > > Ross > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#162968): https://lists.openembedded.org/g/openembedded-core/message/162968 > Mute This Topic: https://lists.openembedded.org/mt/89664344/1997914 > Group Owner: openembedded-core+owner@lists.openembedded.org > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [raj.khem@gmail.com] > -=-=-=-=-=-=-=-=-=-=-=- >
On Wed, 9 Mar 2022 at 17:20, Khem Raj <raj.khem@gmail.com> wrote: > will install work if there are more than one .whl file expected ? > I think we will get install failures, in that case it maybe just an > over optimization I confirmed with Python developers that a single project (ie a pyproject.toml) will build a single wheel. If a single recipe builds multiple wheels then it's already doing funky things, and we can revisit this then. Ross
diff --git a/meta/classes/pip_install_wheel.bbclass b/meta/classes/pip_install_wheel.bbclass index 954a6b750f..5d09e795d0 100644 --- a/meta/classes/pip_install_wheel.bbclass +++ b/meta/classes/pip_install_wheel.bbclass @@ -1,20 +1,10 @@ DEPENDS:append = " python3-pip-native" -def guess_pip_install_package_name(d): - import re - '''https://www.python.org/dev/peps/pep-0491/#escaping-and-unicode''' - name = d.getVar('PYPI_PACKAGE') or re.sub(r"^python3-", "", d.getVar('BPN')) - return name.replace('-', '_') - -PIP_INSTALL_PACKAGE ?= "${@guess_pip_install_package_name(d)}" - # The directory where wheels should be written too. Build classes # will ideally [cleandirs] this but we don't do that here in case # a recipe wants to install prebuilt wheels. PIP_INSTALL_DIST_PATH ?= "${WORKDIR}/dist" -PYPA_WHEEL ??= "${PIP_INSTALL_DIST_PATH}/${PIP_INSTALL_PACKAGE}-*-*.whl" - PIP_INSTALL_ARGS = "\ -vvvv \ --ignore-installed \ @@ -29,7 +19,9 @@ PIP_INSTALL_PYTHON = "python3" PIP_INSTALL_PYTHON:class-native = "nativepython3" pip_install_wheel_do_install () { - nativepython3 -m pip install ${PIP_INSTALL_ARGS} ${PYPA_WHEEL} || + test -f ${PIP_INSTALL_DIST_PATH}/*.whl || bbfatal No wheels in ${PIP_INSTALL_DIST_PATH} + + nativepython3 -m pip install ${PIP_INSTALL_ARGS} ${PIP_INSTALL_DIST_PATH}/*.whl || bbfatal_log "Failed to pip install wheel. Check the logs." cd ${D}
Now that the build systems that use pip_install_wheel are all building their wheel into a directory that we knew was empty before, we can just install *.whl and not need to know the precise names. By design a pyproject.toml will always build a single wheel, so there shouldn't be any way for this to end up installing more than expected. This obsoletes PIP_INSTALL_PACKAGE and PYPA_WHEEL, neither of which are needed anymore. Signed-off-by: Ross Burton <ross.burton@arm.com> --- meta/classes/pip_install_wheel.bbclass | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-)