[feature] Download/use standalone python build when chose --python version doesn't exist #1243

alextremblay · 2024-02-04T02:06:55Z

I have added a news fragment under changelog.d/ (if the patch affects the end users)

Summary of changes

As per #1242, this PR updates pipx.interpreter.find_python_interpreter to download the desired python version if pipx is not able to find it in the system, rather than simply failing with an exception.

Test plan

Tested by running

# The following commands were successfully tested on a system which only has python 3.8 and 3.10 installed
$ pipx run --python=python3.9 cowsay -t hello
$ pipx run --python=python3.10 cowsay -t hello
$ pipx run --python=python3.11 cowsay -t hello
$ pipx run --python=python3.12 cowsay -t hello
$ pipx run --python=3.9 cowsay -t hello
$ pipx run --python=3.10 cowsay -t hello
$ pipx run --python=3.11 cowsay -t hello
$ pipx run --python=3.12 cowsay -t hello

The following was also tested:

myscript.py:

#!/usr/bin/env -S pipx run --python=3.11 --path

# /// script
# dependencies = [
#   "requests",  # example of external dependency
# ]
# ///
import requests

print(requests.get("https://example.com").text)

$ chmod +x myscript.py
$ ./myscript.py | head
<!doctype html>
<html>
<head>
    <title>Example Domain</title>

    <meta charset="utf-8" />
    <meta http-equiv="Content-type" content="text/html; charset=utf-8" />
    <meta name="viewport" content="width=device-width, initial-scale=1" />
    <style type="text/css">
    body {

alextremblay · 2024-02-04T02:57:58Z

@dukecat0 @uranusjr I've got a preliminary POC of this feature which I'm quite happy with, but before I sink any more time into this, I'm hoping to get some feedback and make sure I'm on the right track.

Is this something you would like me to keep working on? do you have any notes on my implementation?

dukecat0 · 2024-02-04T05:28:37Z

Thanks for working on this! I personally find this feature very useful. 👍 Probably it will be better if there's an environment variable that allows users to disable this behavior.

chrysle · 2024-02-04T07:46:31Z

Looks like a helpful feature, thanks!

Probably it will be better if there's an environment variable that allows users to disable this behavior.

I agree with that – or even a flag to toggle the behaviour.

Gitznik · 2024-02-04T08:47:56Z

Good stuff!

I would argue that this feature should be opt-in, rather than opt-out. I would be very confused if I accidentally mistype a python version and pipx just starts downloading it unprompted.

chrysle · 2024-02-04T11:45:34Z

opt-in, rather than opt-out

I agree – that's why I suggest a flag (with negative default value).

alextremblay · 2024-02-04T15:53:42Z

Excellent idea! I’m more than happy to implement that. Not sure what to call the flag though… something succinct?

ideally also an environment variable to flip the default

maybe something like —fetch-as-needed? It’s a bit wordy, but I can’t think of anything more succinct that gets the idea across

Gitznik · 2024-02-04T16:31:30Z

ideally also an environment variable to flip the default

+1

maybe something like —fetch-as-needed? It’s a bit wordy, but I can’t think of anything more succinct that gets the idea across

fetch-missing-python? fetch-as-needed could also refer to the package to install/run itself.

src/pipx/constants.py

src/pipx/standalone_python.py

src/pipx/interpreter.py

chrysle · 2024-02-04T19:27:28Z

Please add a news fragment, too. And we'll need tests.

src/pipx/standalone_python.py

changelog.d/1242.feature.md

src/pipx/main.py

src/pipx/standalone_python.py

tests/test_interpreter.py

alextremblay · 2024-02-05T15:13:39Z

Hi All, this feature is now, to the best of my ability, complete.

I've added tests, a news entry, and adopted / implemented just about every change that's been requested.

The tests are all now passing on all platforms and python versions.

I've spent a not insignificant amount of time and energy making this feature implementation as good as it can be in the hopes that it will be accepted and merged. I am running out of time and energy to devote to this now.

I think it's ready to review and/or merge. I'm going to step away from it now. If there are any additional changes you would like to make, feel free to make them before or after merging the feature.

Thank you

tests/test_interpreter.py

Gitznik

I can work on my change requests myself.

src/pipx/standalone_python.py

tests/test_interpreter.py

Gitznik · 2024-02-05T16:06:50Z

Would be nice if there was a command to clean up the python versions, e.g. when a downloaded python version does not have a installed app that uses it anymore. We can treat that as a separate feature request though IMO.

alextremblay · 2024-02-05T16:38:00Z

Would be nice if there was a command to clean up the python versions, e.g. when a downloaded python version does not have a installed app that uses it anymore. We can treat that as a separate feature request though IMO.

Also a command to update one or all standalone versions (ie someone uses --python=3.9 --fetch-missing-python and wind up with v3.9.10 installed to PIPX_HOME/py/3.9. several months later, they come back and install another package with --python=3.9 --fetch-missing-python, and that package gets installed with python 3.9.10, even though v3.9.18 is now available)

That's a hard problem to solve automatically, so i think a manual process / command would be the way to go

alextremblay · 2024-02-05T16:52:43Z

This latest error in the windows unit test is... interesting, to say the least.

is PIPX_HOME cached in CI between test runs?

I'm not quite sure how else to explain why PIPX_HOME/py/3.11 already exists on the windows CI server...

chrysle · 2024-02-05T21:24:41Z

Probably we should use pytest's tmp_path fixture?

alextremblay · 2024-02-05T21:49:45Z

Probably we should use pytest's tmp_path fixture?

to override the PIPX_STANDALONE_PYTHON_CACHEDIR constant?

I suppose so, but that somehow feels wrong. the unit test should as closely as possible reflect what will actually happen on a user's machine

ordinarily, this error we're seeing should be impossible. if standalone python 3.11's already been downloaded, the function should return early without trying to redownload it. this error seems to indicate that a previous run failed and produced an empy PIPX_HOME/py/3.11 folder...

alextremblay · 2024-02-06T01:37:53Z

Well, apart from that rate limit error on the windows test, I think we're now good to go.

Are there any additional changes you need, or can this be merged?

src/pipx/interpreter.py

tests/test_interpreter.py

chrysle · 2024-02-06T14:43:27Z

I'd like to raise awareness for #1243 (comment) that should still be implemented IMHO, as it tests basic functionality.

As for the rate limit, I see that as a possible issue, as these tests will be run many times. I propose skipping this test if standalone_python.py isn't modified (don't know how to achieve that, yet), or scheduling it.

alextremblay · 2024-02-06T15:24:24Z

I'd like to raise awareness for #1243 (comment) that should still be implemented IMHO, as it tests basic functionality.

As for the rate limit, I see that as a possible issue, as these tests will be run many times. I propose skipping this test if standalone_python.py isn't modified (don't know how to achieve that, yet), or scheduling it.

The first suggestion from that comment (ensuring the target python isn't found on dev's machines) has already been implemented (by mocking out which in both the windows test and the unix test.

The second comment, adding a python_version variable to these two unit tests, doesn't make sense to me. none of the other tests in that test suite define such a variable, and i don't understand why we would want to add such an inconsistency or why the lack of this variable is a blocker towards getting this change merged?

alextremblay · 2024-02-06T15:25:40Z

I'd like to raise awareness for #1243 (comment) that should still be implemented IMHO, as it tests basic functionality.

As for the rate limit, I see that as a possible issue, as these tests will be run many times. I propose skipping this test if standalone_python.py isn't modified (don't know how to achieve that, yet), or scheduling it.

regarding the rate limit, that is definitely a concern. I'm going to try and write a fixture to mock out the API call specifically, that should take care of that problem

stand by

Co-authored-by: chrysle <[email protected]>

chrysle · 2024-02-06T16:13:50Z

The first suggestion from that comment (ensuring the target python isn't found on dev's machines) has already been implemented (by mocking out which in both the windows test and the unix test.

However, implementing it would ensure the exception is raised correctly, which isn't entirely useless.

for more information, see https://pre-commit.ci

alextremblay · 2024-02-06T16:18:29Z

However, implementing it would ensure the exception is raised correctly, which isn't entirely useless.

oh, i see, you're suggesting i write a test for the failure mode. gotcha, will do

alextremblay · 2024-02-06T17:08:33Z

I'd like to raise awareness for #1243 (comment) that should still be implemented IMHO, as it tests basic functionality.

hi @chrysle, I've implemented test cases for versions of python which aren't available in the python-build-standalone project

in the future, we might want to expand the logic of the standalone_python module to collect data from multiple releases of the python-build-standalone project and merge it all together so that we can support fetching more python versions than are available in the current release (ie. 20231002 was the last release to include a version of python 3.8), but that's a massive increase in complexity for very little gain, and far outside the scope of this PR

chrysle

Two suggestions, looks good to me other than that!

tests/test_interpreter.py

Co-authored-by: chrysle <[email protected]>

chrysle · 2024-02-07T14:31:13Z

Thank you for all the work you have invested into this contribution!

Would be nice if there was a command to clean up the python versions, e.g. when a downloaded python version does not have a installed app that uses it anymore. We can treat that as a separate feature request though IMO.

Also a command to update one or all standalone versions

Additional concerns, as a pipx clean (?) command, shall be addressed in follow-ups.

Gitznik · 2024-02-07T14:32:21Z

Thank you for bearing with us through the review process @alextremblay. Great contribution!

We also moved logging the exceptions from the interpreter resolution outside of the scope of this issue, so that also needs a follow-up.

alextremblay force-pushed the feature-1242 branch from 1ead70b to 97cadfc Compare February 4, 2024 02:26

chrysle reviewed Feb 4, 2024

View reviewed changes

Gitznik requested changes Feb 4, 2024

View reviewed changes

src/pipx/standalone_python.py Outdated Show resolved Hide resolved

src/pipx/standalone_python.py Outdated Show resolved Hide resolved

chrysle reviewed Feb 5, 2024

View reviewed changes

alextremblay marked this pull request as ready for review February 5, 2024 15:13

alextremblay requested a review from Gitznik February 5, 2024 15:13

chrysle reviewed Feb 5, 2024

View reviewed changes

tests/test_interpreter.py Outdated Show resolved Hide resolved

Gitznik requested changes Feb 5, 2024

View reviewed changes

src/pipx/standalone_python.py Outdated Show resolved Hide resolved

tests/test_interpreter.py Outdated Show resolved Hide resolved

alextremblay force-pushed the feature-1242 branch from 4cbba39 to 7380938 Compare February 6, 2024 01:11

Gitznik requested changes Feb 6, 2024

View reviewed changes

src/pipx/interpreter.py Show resolved Hide resolved

tests/test_interpreter.py Outdated Show resolved Hide resolved

alextremblay closed this Feb 6, 2024

alextremblay reopened this Feb 6, 2024

alextremblay and others added 4 commits February 6, 2024 10:31

Apply suggestions from code review, fixup unit tests

325bbfa

Co-authored-by: chrysle <[email protected]>

implement changes requested by chrysle

26a292f

implement changes requested by gitznik, fixup unit tests

6d14d32

group and de-duplicate python cli args

4b22db9

refactor unit tests, mock out github API response

c3f92ee

alextremblay force-pushed the feature-1242 branch from 7380938 to c3f92ee Compare February 6, 2024 16:16

[pre-commit.ci] auto fixes from pre-commit.com hooks

839ae35

for more information, see https://pre-commit.ci

alextremblay added 2 commits February 6, 2024 11:27

add unit test for python_standalone failure mode

f3c694b

refactor unit tests, remove need to derive target python version

00cf6a7

alextremblay force-pushed the feature-1242 branch from 9fb8ea0 to 00cf6a7 Compare February 6, 2024 17:03

Gitznik previously approved these changes Feb 6, 2024

View reviewed changes

Gitznik linked an issue Feb 6, 2024 that may be closed by this pull request

[feature] Download/use standalone python build when chose --python version doesn't exist #1242

Closed

chrysle reviewed Feb 7, 2024

View reviewed changes

tests/test_interpreter.py Show resolved Hide resolved

tests/test_interpreter.py Outdated Show resolved Hide resolved

de-duplicate unit test

78db220

Co-authored-by: chrysle <[email protected]>

alextremblay dismissed Gitznik’s stale review via 78db220 February 7, 2024 13:51

chrysle approved these changes Feb 7, 2024

View reviewed changes

Merge branch 'main' into feature-1242

c939234

chrysle enabled auto-merge (squash) February 7, 2024 14:11

chrysle merged commit 407b797 into pypa:main Feb 7, 2024
12 checks passed

alextremblay deleted the feature-1242 branch February 7, 2024 14:37

This was referenced Feb 7, 2024

Command for cleanup of installed standalone python versions #1248

Closed

Command to update local standalone python versions #1249

Closed

Add source interpreter to pipx metadata #1251

Merged

branchvincent mentioned this pull request Feb 14, 2024

docs: improve Python docs, add section about PEP 668 Homebrew/brew#16643

Merged

7 tasks

huxuan mentioned this pull request Jun 6, 2024

automatically handle the need for reinstall-all #1442

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[feature] Download/use standalone python build when chose --python version doesn't exist #1243

[feature] Download/use standalone python build when chose --python version doesn't exist #1243

alextremblay commented Feb 4, 2024 •

edited by chrysle

Loading

alextremblay commented Feb 4, 2024

dukecat0 commented Feb 4, 2024

chrysle commented Feb 4, 2024

Gitznik commented Feb 4, 2024

chrysle commented Feb 4, 2024

alextremblay commented Feb 4, 2024

Gitznik commented Feb 4, 2024 •

edited

Loading

chrysle commented Feb 4, 2024

alextremblay commented Feb 5, 2024

Gitznik left a comment

Gitznik commented Feb 5, 2024

alextremblay commented Feb 5, 2024

alextremblay commented Feb 5, 2024

chrysle commented Feb 5, 2024

alextremblay commented Feb 5, 2024

alextremblay commented Feb 6, 2024

chrysle commented Feb 6, 2024

alextremblay commented Feb 6, 2024

alextremblay commented Feb 6, 2024

chrysle commented Feb 6, 2024

alextremblay commented Feb 6, 2024

alextremblay commented Feb 6, 2024

chrysle left a comment

chrysle commented Feb 7, 2024

Gitznik commented Feb 7, 2024 •

edited

Loading

[feature] Download/use standalone python build when chose --python version doesn't exist #1243

[feature] Download/use standalone python build when chose --python version doesn't exist #1243

Conversation

alextremblay commented Feb 4, 2024 • edited by chrysle Loading

Summary of changes

Test plan

alextremblay commented Feb 4, 2024

dukecat0 commented Feb 4, 2024

chrysle commented Feb 4, 2024

Gitznik commented Feb 4, 2024

chrysle commented Feb 4, 2024

alextremblay commented Feb 4, 2024

Gitznik commented Feb 4, 2024 • edited Loading

chrysle commented Feb 4, 2024

alextremblay commented Feb 5, 2024

Gitznik left a comment

Choose a reason for hiding this comment

Gitznik commented Feb 5, 2024

alextremblay commented Feb 5, 2024

alextremblay commented Feb 5, 2024

chrysle commented Feb 5, 2024

alextremblay commented Feb 5, 2024

alextremblay commented Feb 6, 2024

chrysle commented Feb 6, 2024

alextremblay commented Feb 6, 2024

alextremblay commented Feb 6, 2024

chrysle commented Feb 6, 2024

alextremblay commented Feb 6, 2024

alextremblay commented Feb 6, 2024

chrysle left a comment

Choose a reason for hiding this comment

chrysle commented Feb 7, 2024

Gitznik commented Feb 7, 2024 • edited Loading

alextremblay commented Feb 4, 2024 •

edited by chrysle

Loading

Gitznik commented Feb 4, 2024 •

edited

Loading

Gitznik commented Feb 7, 2024 •

edited

Loading