-
-
Notifications
You must be signed in to change notification settings - Fork 271
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
Let python lsps and debuggers access installed packages #1584
Conversation
Sorry about the failing tests, hadn't expected there to be a test for exact arguments. I'll fix that now |
Currently, installing something like debugpy via Mason installs it to a virtual environment that does not have anything else installed. If you are using a second virtual environment, mason handles things to access installed packages in that environment. However, if you are using the system python, debugpy or lsps don't see any of your installed packages. For debugpy, this means you cannot run your program. This might be the case when you have packages that you almost always want installed, for example pandas. This change fixes this by transparently letting mason python packages see the system packages, assuming no other dependencies have been installed to the venv and overwritten them. I think this has no downsides for a purpose like mason - Although this is bad practise for regular python software projects, because it disguises required dependencies, in the case of Mason we don't care about required dependencies so long as they do exist.
@williamboman ready to re-run tests |
I think this is a duplicate of #1557. |
Thanks mmoya for bringing that to my attention. I agree that they are related, although I think they aren't duplicates because they solve the issues in different ways - this one applies the fix by default rather than requiring users to do it, whereas #1557 offers more flexibility. I think having these things work out of the box is worthwhile though, so either way I think this should be a default argument. Responding to William's comment on that PR: I think there are a lot of common use-cases for having system site packages available that people often want to access without setting up venvs.
Overall I think that justifies this PR even if for some people (using project-local venvs) it does nothing. We get more 'batteries included', more value out of the box without additional config. We sacrifice nothing, as I outlined in my first comment regarding the purpose of separating site and venv packages. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is fine! One thing though, this will be problematic when a dependency is already available globally. pip will currently skip installing it, introducing a dependency on global state which could get broken pretty easily (and unexpectedly).
I believe passing --ignore-installed
to pip install should however ensure all dependencies are always installed in the virtual environment, which should mitigate this.
Oh interesting. Perceptive insight, I'll add that one too unless you're happy to. |
* origin/main: tests: remove old spec (williamboman#1634) chore(main): release 1.10.0 (williamboman#1605) fix(pypi): fix variable shadowing (williamboman#1610) feat(pypi): attempt more python3 candidates (williamboman#1608) fix(golang): fix fetching package versions for packages containing subpath specifier (williamboman#1607) feat: don't use vim.g.python3_host_prog as a candidate for python (williamboman#1606) fix(ui): don't indent empty lines (williamboman#1597)
I pushed some commits. I just noticed that the changes were originally made to an old, deprecated, manager that is still lingering around in order to not break 3rd party usage. Would you be able to try out the latest changes and see if it behaves as you'd expect 🙏? |
* origin/main: tests(pypi): fix assertions (williamboman#1750) fix(pypi): allow access to system site packages by default (williamboman#1584)
Currently, installing something like debugpy via Mason installs it to a virtual environment that does not have anything else installed. If you are using a second virtual environment, mason handles things to access installed packages in that environment. However, if you are using the system python, debugpy or lsps don't see any of your installed packages. For debugpy, this means you cannot run your program. This might be the case when you have packages that you almost always want installed, for example pandas.
This change fixes this by transparently letting mason python packages see the system packages, assuming no other dependencies have been installed to the venv and overwritten them. I think this has no downsides for a purpose like mason - Although this is bad practise for regular python software projects, because it disguises required dependencies, in the case of Mason we don't care about required dependencies so long as they do exist.