Hi Mattijs,

On 4/16/25 4:21 PM, Mattijs Korpershoek wrote:
Hi Quentin,

Thank you for the review.

On mer., avril 16, 2025 at 14:47, Quentin Schulz <quentin.sch...@cherry.de> 
wrote:

Hi Mattijs,

On 4/16/25 2:36 PM, Mattijs Korpershoek wrote:
Recent Ubuntu versions (24.04+) disallow pip by default when
installing packages. The recommended approach is to use a virtual
environment (venv) instead.
Because of this, "make pip" is failing on such versions.

To prepare CI container migration to Ubuntu 24.04, use a venv in the
make_pip script.

Note: This has been reported on [1]

[1] https://source.denx.de/u-boot/custodians/u-boot-dm/-/issues/37

Signed-off-by: Mattijs Korpershoek <mkorpersh...@kernel.org>
---
This has been tested in docker on ubuntu:24.04 after running:
$ apt install python3 python3-venv

with:
$ ./scripts/make_pip.sh u_boot_pylib "-n"

And shows:
Successfully built u_boot_pylib-0.0.6.tar.gz and 
u_boot_pylib-0.0.6-py3-none-any.whl

Also tested with "$ make pip".
---
Changes in v2:
- Use venv instead of virtualenv (Tom)
- Link to v1: 
https://lore.kernel.org/r/20250409-ubuntu-24-04-v1-1-056728207...@kernel.org
---
   scripts/make_pip.sh | 6 ++++++
   1 file changed, 6 insertions(+)

diff --git a/scripts/make_pip.sh b/scripts/make_pip.sh
index d2639ffd6e43..33ad51ada703 100755
--- a/scripts/make_pip.sh
+++ b/scripts/make_pip.sh
@@ -106,6 +106,10 @@ fi
   mkdir ${dir}/tests
   cd ${dir}
+# Use virtual environment
+python3 -m venv .venv

Do we want to make sure there are no leftovers from previous runs? If
so, maybe add --clear here?

I've thought about this, and looking at the script itself it already
suffers from those issues. For example, the $dir created with mktemp is
also not removed.


+source .venv/bin/activate
+
   # Make sure the tools are up to date
   python3 -m pip install --upgrade build
   python3 -m pip install --upgrade twine
@@ -122,6 +126,8 @@ if [ -n "${upload}" ]; then
        echo "Completed upload of ${tool}"
   fi
+# Finish using virtual environment
+deactivate

Maybe you want this in a trap so that if we exit early (which may happen
due to the script doing `set -e`), we're out of the venv?

And I tested otherwise, with:

#!/usr/bin/env bash
python3 -m venv .venv
source .venv/bin/activate

and it seems that once I exit the script it's deactivated anyways?

Ah, interesting to know. I just added it to have a "symetric cleanup".


Yup, absolutely makes sense.


So, I guess no need for the trap or the deactivate, but we can play it
safe and still have it :)

I think using a trap makes sense, but this should also be done for the
${dir} directory.


Indeed.

I'd also argue that this would more be the candidate for another patch
than to be part of this one.


Fair enough. I believe the --clear addition to python3 -m venv .venv would still make sense though? What do you think?

Cheers,
Quentin

Reply via email to