Skip to content

Conversation

@airportyh
Copy link
Collaborator

@airportyh airportyh commented Feb 4, 2025

Why

Currently if a repl is based on uv + virtualenv, pip install doesn't work because we have a global PIP_CONFIG_FILE setting which forces --user option for pip installs, but that fails in virtualenvs. This breaks some packages which use python -m pip install internally like https://spacy.io/ but also breaks python -m pip install in general.

What changed

  1. Added a check in sitecustomize.py to see if the python executable is coming from the virtualenv .pythonlibs. If so: remove the PIP_CONFIG_FILE entry from the environment. This allows python -m pip install ... to work in both the virturalenv and the non-virtualenv cases.

Test plan

  1. Test spacy e2e without virtualenv:
    a. Create a python repl
    b. rm -fr .pythonlibs
    c. pip install -U spacy
    d. python -m spacy download en_core_web_sm
  2. Test spacy e2e with virtualenv:
    a. Create a new python repl
    c. uv sync
    d. pip install -U spacy
    e. python -m spacy download en_core_web_sm
  3. Test python -m pip install colorama with the same above 2 cases.

Rollout

  • [ x] This is fully backward and forward compatible

@airportyh airportyh requested a review from a team as a code owner February 4, 2025 21:20
@airportyh airportyh requested review from blast-hardcheese and cbrewster and removed request for a team February 4, 2025 21:20
Copy link
Contributor

@blast-hardcheese blast-hardcheese left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hard to maintain the matrix of different environments we support. This looks OK. We should definitely template-tester as well as verify agent uv flow

@airportyh
Copy link
Collaborator Author

@blast-hardcheese After finding an edge case that didn't work with the logic in main.go because the resulting binary getting symlinked by uv into .pythonlibs/bin, I ended up with a different approach based on just a few lines in sitecustomize.py. Please re-review.

Copy link
Contributor

@blast-hardcheese blast-hardcheese left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems good

@airportyh airportyh merged commit 1113fae into main Feb 5, 2025
3 checks passed
@airportyh airportyh deleted the th-fix-pip-shell-install branch February 5, 2025 14:15
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.

3 participants