Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

After creating venv, unconditionally find new binary. #83

Merged
merged 6 commits into from
May 29, 2024

Conversation

thatch
Copy link
Contributor

@thatch thatch commented Jan 29, 2024

Description

On archlinux with no .thx cache, things are pretty broken, at least when running with an explciit version like thx -v -p 3.9 test. It will create the .thx/venv/3.9.18/ venv but not actually use it in the subsequent attempt to update pip since it only adjusts context.python_path in the live case. Arch is special in that installing python doesn't imply installing pip automatically; venvs get it via ensurepip. I don't know if this occurs on other distros.

I think this is close to the right change, but find_runtime when passed a venv dir can still return a non-venv python and that should probably be more explicitly handled.

I think what this was doing before was either doing a --user upgrade or operating on a writable pyenv install, and not actually upgrading the one in the venv. That's my only explanation for how this wasn't erroring out.

@thatch
Copy link
Contributor Author

thatch commented Jan 29, 2024

Here's the repro on archlinux (looks like the python_version constraint on typing-extensions isn't quite right either, this is 3.11)

docker run -ti archlinux:latest /bin/bash
pacman -Sy
pacman -Su python git
git clone https://github.com/omnilib/thx
cd thx
python -m venv /foo
/foo/bin/pip install .
/foo/bin/pip install typing-extensions
/foo/bin/thx test

and on the official python image, debian (note that the system python 3.11 is the one that fails; 3.9 is in /usr/local)

docker run -ti python:3.9 /bin/bash
apt update
apt install python3.11-venv 
git clone https://github.com/omnilib/thx
cd thx
python -m venv /foo
/foo/bin/pip install .
/foo/bin/thx test

thatch and others added 2 commits May 28, 2024 21:44
This isn't perfect, it can still find a non-venv binary, but it should
be closer to right.
@amyreese amyreese force-pushed the find-venv-binary branch from 88b351a to 93a304b Compare May 29, 2024 04:58
@amyreese amyreese merged commit c985a75 into omnilib:main May 29, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants