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

pyenv-win compatibility - another approach #16287

Merged
merged 1 commit into from
Oct 29, 2024

Conversation

viking1304
Copy link
Contributor

Description

Another (hopefully better) approach that might replace #16265
Based on the pseudocode that @w-e-w provided there, since no one has proposed a better approach yet.

Should fix #16205 and #16261

Screenshots/videos:

Checklist:

@viking1304 viking1304 changed the title pyenv-win compatibility pyenv-win compatibility - another approach Jul 28, 2024
@w-e-w
Copy link
Collaborator

w-e-w commented Jul 29, 2024

yeah sorry I forgot to reply

I remember I saw your message and then went testing then forgot about it

your method seems to work

@viking1304
Copy link
Contributor Author

viking1304 commented Jul 29, 2024

I also tested both in a virtual machine, and everything worked as intended.

EDIT:

Thanks to @w-e-w I now see that this part is completely wrong, so it should be ignored.

However, even though it works, I still dislike mixing venv with pyenv. I think venv should be disabled, and pyenv local should be used instead.

If we agree that venv should be disabled, I can switch back to my original implementation with the python_is_exe variable. Then, I can test the value of this variable in combination with the tests for PYTHON and VENV_DIR to make a cleaner version of the code.

I would not mess with pyenv local, I would just disable venv by serting it to -.

What do you think?

@w-e-w
Copy link
Collaborator

w-e-w commented Jul 29, 2024

I still dislike mixing venv with pyenv

what??
I suspect you don't know what pyenv and venv do

pyenv and pyenv-win are Python version management
similer to py similer to py launcher
it's main job is to allow you to switch / choose between different python versions

venv is an isolated python envirment
it's isolated packages dependencies from the global or other environments

you are meant to mix them because they serve different purpose

pyenv is purely optional for experience users it can simplify installation of different python versions

is very necessary to provide stability to python environment in case if a user uses Python for anything else other then system

@viking1304
Copy link
Contributor Author

viking1304 commented Jul 29, 2024

EDIT:

My issue with mixing those two is the fact that pyenv local does practically the same thing as venv:
https://github.com/pyenv/pyenv/blob/master/COMMANDS.md#pyenv-local

Or am I missing something?

@w-e-w
Copy link
Collaborator

w-e-w commented Jul 29, 2024

Or I am missing something?

yes you are
https://github.com/pyenv/pyenv?tab=readme-ov-file#understanding-python-version-selection
pyenv local is just a version selector
pyenv is is just a version selector

it doese not isolate the python environment

@w-e-w
Copy link
Collaborator

w-e-w commented Jul 29, 2024

https://github.com/pyenv/pyenv?tab=readme-ov-file#in-contrast-with-pythonbrew-and-pythonz-pyenv-does-not

if you want isolation you also need to use a plugin of pyenv
pyenv-virtualenv
https://github.com/pyenv/pyenv-virtualenv


or is the windows version different somehow?

@viking1304
Copy link
Contributor Author

viking1304 commented Jul 29, 2024

if you want isolation you also need to use a plugin of pyenv pyenv-virtualenv https://github.com/pyenv/pyenv-virtualenv

Ouch! It is more than obvious that I do not use pyenv, and that I misunderstood some things. I used conda in the past, so I thought pyenv local does the same thing as conda create --prefix, but it is now clear that pyenv is basically just a Python equivalent of nvm.

Sorry for the confusion, and thanks for the clarification.

I suspect you don't know what pyenv and venv do

I have been using venv for years, but I misunderstood what pyenv does 🙈

@catboxanon catboxanon merged commit f31faf6 into AUTOMATIC1111:dev Oct 29, 2024
3 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.

3 participants