poetry icon indicating copy to clipboard operation
poetry copied to clipboard

poetry should not prepend `sys.path` with `__vendor_site__`

Open mhils opened this issue 1 year ago • 7 comments

Problem Description

poetry-core currently prepends its _vendor directory (__vendor_site__) to sys.path here. This causes poetry plugins to pick up whatever dependency version poetry has vendored, which may not be compatible with what the plugin requires / states in its dependencies.

For example, here's what we get after trying to update our poetry plugin to pydantic 2.0, which imports TypeAliasType from typing_extensions. Poetry's 1.6.1 vendored copy does not include TypeAliasType, so every poetry command crashes immediately:

$ poetry help

cannot import name 'TypeAliasType' from 'typing_extensions' (/home/user/venv/lib/python3.11/site-packages/poetry/core/_vendor/typing_extensions.py)

This could be fixed by vendoring similar to how pip does it, i.e. without modifying sys.path but by explictly importing from poetry.core._vendor.

mhils avatar Jul 05 '23 11:07 mhils

That would not be enough: eg the reason for vendoring typing-extensions is not that poetry-core depends on it directly, but that the vendored jsonschema depends on it.

Arguably poetry-core ought to patch jsonschema to pick up the vendored typing-extensions, but this is certainly going to get tedious...

fwiw the main branch of poetry-core doesn't vendor typing-extensions any more, so this particular issue will go away.

dimbleby avatar Jul 05 '23 11:07 dimbleby

I agree this is not particularly fun to fix, but mangling sys.path the way it is done right now is practically guaranteed to break other poetry plugins in the future. :/

mhils avatar Jul 06 '23 10:07 mhils

I suspect plugin authors will find it more efficient in the rare cases where this happens to submit an MR that simply updates the packages that poetry-core vendors. Or: to sit and wait for someone else to do that!

I guess you will be the testcase for this theory...

dimbleby avatar Jul 06 '23 10:07 dimbleby

typing-extensions was already removed from _vendor when I started to look into this, so there was no opportunity for me to submit a PR that simply updates the package. As a plugin author, my feedback is that I would find it even more efficient if I would get the dependencies I actually declared on import.

My (hopefully constructive) feedback is that pip has a different approach^1, which seems to work reasonably well. I appreciate y'all's work on poetry, I just don't particularly appreciate poetry fiddling with my sys.path. :-)

mhils avatar Jul 06 '23 13:07 mhils

Not arguing with your feedback, rather trying to be realistic about what's likely to result from it. We agree that the solution you are suggesting is likely to be tedious / not fun to fix: therefore the only way it happens is if someone who is motivated to make it happen shows up.

At the moment you are the leading candidate, and you seem to be counting yourself out.

Agree that this is a legit report but - like most of the backlog - I wouldn't expect that simply reporting it is likely to lead to action.

dimbleby avatar Jul 06 '23 13:07 dimbleby

I wouldn't expect that simply reporting it is likely to lead to action.

Perfectly fine, no expectations. I'm doing enough FOSS myself. :)

We agree that the solution you are suggesting is likely to be tedious / not fun to fix:

More importantly, tedious to maintain. Doing a one-pass to fix it is releatively easy. But I can't commit to any long-term help, and it arguably makes all vendored dependency updates more time-consuming going forward. And I don't want to put that burden onto anyone without an explicit "yes, that's a tradeoff we want" from the maintainers.

mhils avatar Jul 06 '23 13:07 mhils

It took a good moment but I finally got to changing it. python-poetry/poetry-core#753

Secrus avatar Aug 30 '24 21:08 Secrus