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

Consider presence on the PATH an opt-in to Python pre-releases #7469

Closed
zanieb opened this issue Sep 17, 2024 · 2 comments · Fixed by #7470
Closed

Consider presence on the PATH an opt-in to Python pre-releases #7469

zanieb opened this issue Sep 17, 2024 · 2 comments · Fixed by #7470
Labels
needs-decision Undecided if this should be done

Comments

@zanieb
Copy link
Member

zanieb commented Sep 17, 2024

From @henryiii in #7300 (comment)

Yes, the issue wasn't VIRTUAL_ENV, but rather that we were making the venv assuming it would use the Python on the top of the PATH. cibuildwheel 2.21.1 fixes this.

It's really weird, though, that it no longer takes the Python at the top of the PATH when --system is used. If someone goes to the trouble to put Python in the PATH, intentionally skipping over it is really surprising; and if you type python, you don't get the one you just installed to. For example:

- uses: actions/setup-python@v5
  with:
    python-version: ${{ matrix.python-version }}
    allow-prereleases: true
- uses: astral-sh/setup-uv@v3
- run: uv pip install --system some_lib
- run: python -c "import some_lib"

now fails only on 3.13 because uv installed to the 3.12 instead of the 3.13 that's been prepared by setup-python. The python above is 3.13, while uv is skipping it and installing into 3.12. (I've seen this failure somewhere but have forgotten which repos it affected).

I understand this makes sense with managed Python (where you have a bunch of Pythons and uv's just picking out the best one), but it is very surprising to have it do this to the Python on PATH.

We may want to consider allowing --system to opt-in here, but I'm a little hesitant to have different discovery behaviors.

cc @carljm / @AlexWaygood who suggested to me that we should not select Python pre-releases by default.

@zanieb zanieb added the needs-decision Undecided if this should be done label Sep 17, 2024
@henryiii
Copy link
Contributor

henryiii commented Sep 17, 2024

I don't like handling pre-releases differently from full releases in general - if someone is helping test pre-releases, making it harder for them isn't very helpful. We want to encourage pre-release to be easy to test and use like regular releases (especially RC's). The only exception I know of is the original motivation: if you have installed several managed Pythons, defaulting to the latest non-prerelease is a good default. But these aren't on the PATH. What about only having special handling for commands that pick up a managed release automatically, and letting PATH work normally? Is that possible?

- uses: actions/setup-python@v5
  with:
    python-version: "3.13"
    allow-prereleases: true
- run: python --version  # 3.13
- run: uv venv # I would kind of expect Python 3.13 here
- run: uv python install 3.11
- run: uv python install 3.12
- run: uv python install 3.13
- run: python --version # This is whatever's already present, or nothing
- run: uv venv  # I would not expect Python 3.13 here, either system or 3.12

@zanieb
Copy link
Member Author

zanieb commented Sep 17, 2024

It's definitely possible — we just need to align on the behavior.

I believe #7470 would achieve the behavior here.

@zanieb zanieb changed the title Consider --system an opt-in to Python pre-releases Consider PATH an opt-in to Python pre-releases Sep 17, 2024
@zanieb zanieb changed the title Consider PATH an opt-in to Python pre-releases Consider presence on the PATH an opt-in to Python pre-releases Sep 17, 2024
@zanieb zanieb closed this as completed in 4611412 Sep 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-decision Undecided if this should be done
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants