python-lsp / python-lsp-server

Fork of the python-language-server project, maintained by the Spyder IDE team and the community
MIT License
1.91k stars 196 forks source link

Idea: pylsp should automatically use libraries that are installed by pre-commit #259

Open lieryan opened 2 years ago

lieryan commented 2 years ago

For example, I might have pyflakes (or pylint, or black, etc) already installed by .pre-commit-config.yaml but not in my virtualenv. python-lsp-server could detect this and use the library installed in the pre-commit environment.

Why? In the usually configuration, pre-commit install their checkers in their own environment, not in the user's virtualenv, so the version of checker setup by pre-commit may not match the checker installed in project's virtualenv. Currently, because it is possible to have conflicting versions of a checker in both pyproject.toml/requirements.txt and .pre-commit-config.yaml, pylsp and pre-commit may advise conflicting fixes.

Projects can try to synchronise the versions in pyproject.toml/requirements.txt and .pre-commit-config.yaml, or they can configure pre-commit to use a repo checkers that is installed in the virtualenv instead of installing its own, but these also come with their own set of problems.

Having a single set of libraries also have the small benefit of not having two copies of the checker libraries, so it's less disk space.

lieryan commented 2 years ago

As a sketch of how this could be implemented, maybe is that pylsp could read the pre-commit for well known checker names, find where they've been installed by pre-commit, and then add those paths to sys.path. I'm probably glossing over lots of nuances and complexities here, but it seems like it could work.

ccordoba12 commented 2 years ago

I don't think that would work because you're asking us to mix packages from two different Python envs (the precommit one and the other where pylsp is installed). I certainly wouldn't accept a proposal like that as a maintainer because it'd lead to odd problems and crashes.

Instead, I think it's way easier to simply install pylsp in your precommit env and point your editor/IDE to use it from there.

lieryan commented 2 years ago

From my understanding, pre-commit actually creates one environment for each tool, so there isn't a single environment you can install pylsp into.

Merging those environments should be possible with sys.path/PYTHONPATH hacks, and likely something that I could POC over a weekend just to see how badly it would work. But because it has a good chance of running with incompatible/conflicting versions of the tools' dependencies, and all sort of subtle issues, so yeah, I agree that it could never become an officially supported way of running pylsp.

A more careful implementation would have to run each tool in their own subprocess, each running with the interpreter that is defined by their respective environment. That's a much bigger change to how plugins are implemented, and is only really practical with plugins that already either just called into a subprocess to parse their output or is already running on a daemon like mypy.