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

bundled/tool/*.py incompatibilities with packages in venv with importStrategy=fromEnvironment #369

Open
edwardpeek-crown opened this issue Nov 7, 2023 · 7 comments
Assignees
Labels
triage-needed Issue is not triaged.

Comments

@edwardpeek-crown
Copy link

The fromEnvironment option for the extension setting black-formatter.importStrategy indicates that it controls where the black tool is used from, but the setting also affects the venv in which the bundled LSP server runs. If the venv contains old versions of dependencies of the LSP server (eg. typing_extensions==4.5.0) then the extension fails to load with errors as below:

2023-11-07 14:52:24.748 [info] Traceback (most recent call last):
  File "<redacted>/.vscode/extensions/ms-python.black-formatter-2023.6.0/bundled/tool/lsp_server.py", line 68, in <module>
    from pygls import server, uris, workspace
  File "<redacted>/.vscode/extensions/ms-python.black-formatter-2023.6.0/bundled/libs/pygls/server.py", line 37, in <module>
    from pygls.lsp import ConfigCallbackType, ShowDocumentCallbackType
  File "<redacted>/.vscode/extensions/ms-python.black-formatter-2023.6.0/bundled/libs/pygls/lsp/__init__.py", line 38, in <module>
    from typeguard import check_type
  File "<redacted>/.vscode/extensions/ms-python.black-formatter-2023.6.0/bundled/libs/typeguard/__init__.py", line 21, in <module>
    from ._importhook import ImportHookManager as ImportHookManager
  File "<redacted>/.vscode/extensions/ms-python.black-formatter-2023.6.0/bundled/libs/typeguard/_importhook.py", line 22, in <module>
    from typing_extensions import Buffer
ImportError: cannot import name 'Buffer' from 'typing_extensions' (<redacted>/venv/lib/python3.10/site-packages/typing_extensions.py)

This issue can currently be worked around by instead configuring black-formatter.path to invoke black from the desired venv.

A cursory suggestion would be for the LSP server process to be sandboxed to only use only the bundled packages, regardless of whether "fromEnvironment" is configured.

Note that this issue is also present in the other Microsoft LSP Python extensions.

@github-actions github-actions bot added the triage-needed Issue is not triaged. label Nov 7, 2023
@devrelm
Copy link

devrelm commented Nov 7, 2023

As a workaround, you could upgrade your local/venv version of typing_extensions to 4.7 or greater (current latest version is 4.8 as of me writing this.)

That said, it does appear that ms-python.black-formatter is bundling typing_extensions v4.8, so I'm unsure why it's not importing from bundled/libs. Perhaps because typeguard/_importhook.py imports typing_extensions conditionally? I'm relatively new to python though, so I'm not familiar with its module resolution rules.

@karthiknadig karthiknadig self-assigned this Nov 7, 2023
@edwardpeek-crown
Copy link
Author

I'm unsure why it's not importing from bundled/libs

From https://github.com/microsoft/vscode-black-formatter/blob/main/bundled/tool/lsp_runner.py#L28-L32 it looks like package lookup order will end up as:

  1. bundle/tool
  2. venv and interpreter site packages
  3. bundle/libs

So if a dependency exists in the venv it will be used instead of a bundled equivalent.

On top of that I expect https://github.com/microsoft/vscode-black-formatter/blob/main/bundled/tool/lsp_server.py#L372 will need to treat the fromEnvironment case more akin to how it spawns a new process in the custom interpreter/path cases rather than trying to run it in-process.

@karthiknadig
Copy link
Member

We chose to load black into the same process as the server even in fromEvironment case to reduce the time it takes to format. We saw that having black loaded, reduced it format time significantly. We can change it where it useBundled acts as is does today, and fromEnvironment will spawn a new process. That will reduce the efficiency gain we get from blacks API.

I think a better solution is to get rid if the dependency if possible on the server. see here openlawlibrary/pygls#409

@edwardpeek-crown
Copy link
Author

Can the run_over_json_rpc launch method be used to keep most of the the efficiency benefits using a cached interpreter process? Reducing dependencies reduces the likelihood of issues, but does not eliminate them entirely.

@scotgopal
Copy link

As a workaround, you could upgrade your local/venv version of typing_extensions to 4.7 or greater (current latest version is 4.8 as of me writing this.)

That said, it does appear that ms-python.black-formatter is bundling typing_extensions v4.8, so I'm unsure why it's not importing from bundled/libs. Perhaps because typeguard/_importhook.py imports typing_extensions conditionally? I'm relatively new to python though, so I'm not familiar with its module resolution rules.

I think 4.6 and above is good enough. Buffer was introduced in this version. Updating from 4.5.0 to 4.6.0 fixed the issue for me.

Ref: https://github.com/python/typing_extensions/blob/main/CHANGELOG.md#release-460-may-22-2023

Anyways, hopefully a better solution arises from all the discussions here. Thank you all. 😄

@Morikko
Copy link

Morikko commented Feb 6, 2024

Since 1.86.0, another use case that @side2k and me also faced (from #444 (comment)):

  File "/my/home/dir/.vscode/extensions/ms-python.black-formatter-2024.1.10361006/bundled/libs/cattrs/gen/__init__.py", line 355, in make_dict_structure_fn
    ian = a.alias
AttributeError: 'Attribute' object has no attribute 'alias'

The attrs library version from the environment is too old.

New in version 22.2.0: alias

https://www.attrs.org/en/stable/api.html#attrs.Attribute

By upgrading to 22.2, it works now. Also the wrong attrs was not taken from my venv workspace (where both cattrs and attrs are still outdated) but from the root interpreter environment (Pyenv version).

@robotoshi
Copy link

Since 1.86.0, another use case that @side2k and me also faced (from #444 (comment)):

  File "/my/home/dir/.vscode/extensions/ms-python.black-formatter-2024.1.10361006/bundled/libs/cattrs/gen/__init__.py", line 355, in make_dict_structure_fn
    ian = a.alias
AttributeError: 'Attribute' object has no attribute 'alias'

The attrs library version from the environment is too old.

New in version 22.2.0: alias

https://www.attrs.org/en/stable/api.html#attrs.Attribute

By upgrading to 22.2, it works now. Also the wrong attrs was not taken from my venv workspace (where both cattrs and attrs are still outdated) but from the root interpreter environment (Pyenv version).

I have been having this exact issue too, and it happens regardless of the importStrategy setting.
The workaround is to install the extension's dependencies in my project's virtual environment, but it's not a very elegant solution. The extension is not using the bundled versions at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage-needed Issue is not triaged.
Projects
None yet
Development

No branches or pull requests

6 participants