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

Fix pyenv FAQ env var commands #144

Merged
merged 6 commits into from
Sep 10, 2021

Conversation

cas--
Copy link
Contributor

@cas-- cas-- commented Aug 19, 2021

When using pyenv and global not set (defaulting to system) or global set to system the Launcher was unable to determine correct version to use.

Fixed by using pyenv exec to correctly determine current Python version instead of parsing pyenv config files.

Fixes #141

When using pyenv and global not set (defaulting to system) or global set to `system` the Launcher was unable to determine correct version to use.

Fixed by using `pyenv exec` to correctly determine current Python version instead of parsing pyenv config files.

Fixes brettcannon#141
Copy link
Owner

@brettcannon brettcannon left a comment

Choose a reason for hiding this comment

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

One tricky thing with this compared to the old solution is PY_PYTHON is the default version to use when no specific major version is requested, e.g. it's not the same as PY_PYTHON2. That means this isn't technically right as you have to make an assumption as to what major version of Python people want to depend on compared to the solution of reading (pyenv root)/version).

Maybe it's better to list both solutions?

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@cas--
Copy link
Contributor Author

cas-- commented Aug 29, 2021

I merged your suggestions but then changed my mind and reverted as I think there has been a misunderstanding.

One tricky thing with this compared to the old solution is PY_PYTHON is the default version to use when no specific major version is requested e.g. it's not the same as PY_PYTHON2

The use of python is not meant to refer to python2. Using $(pyenv exec python) solution mirrors the old solution since pyenv python is what the user set via pyenv global (the (pyenv root)/version file) or fallback to system.

I hope this make sense?

@brettcannon
Copy link
Owner

Then please drop the PY_PYTHON3 example as I don't think many people will care enough to warrant making the example more complex.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Co-authored-by: Brett Cannon <brett@python.org>
@cas--
Copy link
Contributor Author

cas-- commented Sep 1, 2021

I agree, was including it for completeness for -3 option

@brettcannon brettcannon self-requested a review September 1, 2021 18:52
@brettcannon brettcannon self-assigned this Sep 8, 2021
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Copy link
Owner

@brettcannon brettcannon left a comment

Choose a reason for hiding this comment

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

I just realized the changes were made against the main copy of the README and not the template which the README is generated from. Would you mind applying the changes to https://github.com/brettcannon/python-launcher/blob/main/docs/readme/template.md instead?

@brettcannon brettcannon self-requested a review September 10, 2021 22:25
@brettcannon brettcannon merged commit e25e75c into brettcannon:main Sep 10, 2021
@brettcannon
Copy link
Owner

Thanks for the help with this!

@brettcannon brettcannon added the impact-maintenance Maintenance of the code/project label Mar 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact-maintenance Maintenance of the code/project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pyenv 'command not found' with system python set
2 participants