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

feat+fix: adds compatibility to run.sh for pyenv and let pyenv install python 3.8 if it's not installed #2201

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

pato-pan
Copy link
Contributor

@pato-pan pato-pan commented Jul 18, 2024

Pull request checklist

  • The PR has a proper title. Use Semantic Commit Messages. (No more branch-name title please)

  • Make sure this is ready to be merged into the relevant branch. Please don't create a PR and let it hang for a few days.

  • Ensure you can run the codes you submitted succesfully. These submissions will be prioritized for review:

    Introduce improvements in program execution speed;

    Introduce improvements in synthesis quality;

    Fix existing bugs reported by user feedback (or you met);

    Introduce more convenient user operations.

PR type

  • Bug fix / new feature

Description

updates run.sh to add pyenv compatibility. Pyenv check was not working as intended, it should be inverted match (with -v). if pyenv exists in the system, it will now install python3.8 and set it as the default for the current folder

This should give support to Arch Linux users, among other distros who suggest users to use pyenv.

Review needed, 2 questions:

  1. Is it safe to set python3.8 as the default for the current folder? If you don't know and no one can verify, let's assume it is. I think it likely is, but I can't account for every possibility.
  2. Which version of python 3.8 do you prefer? If it's not 3.8.19, it should be changed to the one you wish to have installed (like 3.8.0)

Screenshot

  • Please include a screenshot if applicable

not applicable

pyenv compatibility. Pyenv check was not working as intended, it should be inverted match (with -v).
if pyenv exists in the system, it will now install python3.8 and set it as the default for the current folder

This should give support to Arch Linux users, among other distros.

Review needed, 2 questions:
1. Is it safe to set python3.8 as the default for the current folder? If you don't know and no one can verify, let's assume it is. I think it likely is, but I can't consider for every possibility.
2. Which version of python 3.8 do you prefer? If it's not 3.8.19, it should be changed to the one you wish to have installed (like 3.8.0)
@pato-pan
Copy link
Contributor Author

pato-pan commented Jul 18, 2024

I just realized

this needs to set pyenv local 3.8 and also alias python to python3.8 in the case where pyenv already has 3.8 installed. My commit currently only does it when 3.8 is not installed.

Currently, the script will try to run python3.8 and fail if python3.8 is already installed because pyenv hasn't set python3.8 as local to the folder and it uses python as a command (and not python3.8). There has to be some way to check if the user's method of running this program is pyenv, and in that case see if the necessary changes to make this program work had already been done.

pyenv only checks for python version in the current directory now. The working directory will now always be the one where the script is located (in case you run it outside the folder, example: `./RVC/run.sh`) this also makes it so pyenv will always only show the version that applies to the current folder.

I considered this alternative before coming up with this, "-qvE "3.8|`dirname -- $(readlink -fn -- "$0")`"", but this is longer and more complicated, and has the same requirements, so I discarded it. Sharing it in case someone else finds it useful
@pato-pan
Copy link
Contributor Author

pato-pan commented Jul 18, 2024

fixed it just now by making the script always set the working directory to the one where the script is located. I guess new feature, foolproof, if you accidentally run it outside the program's directory it might still work as intended!

@pato-pan pato-pan changed the title feat+fix: adds compatibility to pyenv and let pyenv install python 3.8 if it's not installed feat+fix: adds compatibility to run.sh for pyenv and let pyenv install python 3.8 if it's not installed Jul 19, 2024
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.

1 participant