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

Update install_deps.sh #1187

Closed
wants to merge 1 commit into from
Closed

Update install_deps.sh #1187

wants to merge 1 commit into from

Conversation

matejsmycka
Copy link

No description provided.

@ethteck
Copy link
Member

ethteck commented May 28, 2024

I appreciate the intent behind this pr, but I don't think we should be automatically running pip install with the break system packages flag.

We have an open issue to rewrite this into a document that can be followed and doesn't have people mindlessly running sudo some script.sh. I think it'd be great to turn this into that document so we can explain things like why you might need to run pip install with this flag and what that means.

@matejsmycka
Copy link
Author

matejsmycka commented May 28, 2024

I understand. While this initiative is great, users should be able to follow the guide or run a random script; taking responsibility for the script, the user should decide which approach he prefers.

About pip.

You are inherently breaking system dependencies using pip. Either you are using a proper virtual environment or want it to "just work". The current approach is halfway. My approach is to "just work". Maybe a better way would be to warn the user if command x doesn't work, use --break-dependencies. Or add poetry or pyenv.... to the install script.

@Darxoon
Copy link

Darxoon commented May 28, 2024

If we are forcing everyone who wants to use the decomp to run a random shell script then we should really make sure it doesn't break people's systems. There's a reason break-system-packages is called the way it is

@matejsmycka
Copy link
Author

Yes, but the current state still breaks them.

@Darxoon
Copy link

Darxoon commented May 28, 2024

No? You will need to setup a venv before running install_deps.sh but it won't be able to harm the stability of your linux installation in general like break-system-packages has the potential to.

Remember that install_deps.sh is ran by a lot of people locally on their machines and using this flag carelessly like in your PR will inevitably make someone have to reinstall their OS because this flag has the potential to brick your entire linux installation if you're unfortunate.

@matejsmycka
Copy link
Author

matejsmycka commented May 28, 2024

This is implicit info not mentioned anywhere afaik, but my PR is not a good way, i will close it

@bates64
Copy link
Member

bates64 commented May 29, 2024

See #1059

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.

4 participants