[PATCH v2] tools/make_pip: Use venv when invoking pip

[Quentin Schulz]

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.schulz at 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 <mkorpershoek at 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-056728207b6c@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