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

Poetry evaluates platform_python_implementation marker incorrectly #8996

Closed
4 tasks done
ioannis-balo opened this issue Feb 20, 2024 · 11 comments
Closed
4 tasks done
Labels
kind/bug Something isn't working as expected

Comments

@ioannis-balo
Copy link

ioannis-balo commented Feb 20, 2024

  • Poetry version: 1.7.1

  • Python version: 3.11.7

  • OS version and name: macOS 14.3

  • pyproject.toml: https://gist.github.com/ioannis-balo/54ef5e13090d4e0eecdcd3f9fee5e1f3

  • I am on the latest stable Poetry version, installed using a recommended method.

  • I have searched the issues of this repo and believe that this is not a duplicate.

  • I have consulted the FAQ and blog for any relevant entries or release notes.

  • If an exception occurs when executing a command, I executed it again in debug mode (-vvv option) and have included the output below.

Issue

In an empty project, I am trying to install vcrpy using poetry add vcrpy. It is correctly installing the latest version (6.0.1).
However, in the package's dependencies, urllib3 is constrained by the implementation marker:

[package.dependencies]
PyYAML = "*"
urllib3 = {version = "<2", markers = "platform_python_implementation == \"PyPy\""}
wrapt = "*"
yarl = "*"

and indeed, urllib3 version installed is <2:

[[package]]
name = "urllib3"
version = "1.26.18"

My python implementation is

>>> platform.python_implementation()
'CPython'

vcrpy setup.py https://github.com/kevin1024/vcrpy/blob/v6.0.1/setup.py#L45-L58

I would expect urllib3 to not be installed based on the markers and my environment.

@ioannis-balo ioannis-balo added kind/bug Something isn't working as expected status/triage This issue needs to be triaged labels Feb 20, 2024
@dimbleby
Copy link
Contributor

dimbleby commented Feb 20, 2024

does not reproduce:

$ poetry add --lock vcrpy
Using version ^6.0.1 for vcrpy

Updating dependencies
Resolving dependencies... (0.8s)

Writing lock file
$ poetry install --dry-run
Installing dependencies from lock file

Package operations: 6 installs, 0 updates, 0 removals

  • Installing idna (3.6)
  • Installing multidict (6.0.5)
  • Installing pyyaml (6.0.1)
  • Installing wrapt (1.16.0)
  • Installing yarl (1.9.4)
  • Installing vcrpy (6.0.1)

Installing the current project: foo (0.1.0)

please provide a way to reproduce, else close

@josumoreno-BP
Copy link

@dimbleby I have the same output as you have if I run the install (without dry-run), but the dependency is added to the .lock file anyway 🤷
You can run a poetry show urllib3 after the install to see it.

@dimbleby
Copy link
Contributor

poetry builds cross platform lockfiles, it is correct to write the dependency to the lockfile so that the lockfile will be valid if someone tries to install your project using PyPy.

what you said was

I would expect urllib3 to not be installed based on the markers and my environment.

which is exactly what happens

Please close.

@hartwork
Copy link

hartwork commented Mar 6, 2024

Hi! 👋

I've been helping out in VCR.py as a collaborator and I had a closer look now at the situation here after it was brought to my attention at kevin1024/vcrpy#826 . Here is what I found:

poetry builds cross platform lockfiles, it is correct to write the dependency to the lockfile so that the lockfile will be valid if someone tries to install your project using PyPy.

I would expect that Poetry would persist all markers rather than only a subset. The full set in VCR.py 6.0.1 is:

    "urllib3 <2; python_version <'3.10'",
    "urllib3 <2; platform_python_implementation =='PyPy'",

Am I missing something?

what you said was

I would expect urllib3 to not be installed based on the markers and my environment.

which is exactly what happens

Please close.

That's not what happens in practice. Poetry does install urllib3 in version 1.26.18 and then blocks the update to ulrlib2 >=2 saying that VCR.py would be the blocker (which VCR.py 6.0.1 is not asking for). Here's the full reproducer:

# cd "$(mktemp -d)"

# poetry --version
Poetry (version 1.8.2)

# python -c 'import platform; print(platform.python_implementation(), platform.python_version())'
CPython 3.10.13

# poetry init -n

# poetry add --lock vcrpy==6.0.1
Updating dependencies
Resolving dependencies... (0.7s)

Writing lock file

# grep -B1 urllib3 poetry.lock 
[[package]]
name = "urllib3"
--
files = [
    {file = "urllib3-1.26.18-py2.py3-none-any.whl", hash = "sha256:34b97092d7e0a3a8cf7cd10e386f401b3737364026c45e622aa02903dffe0f07"},
    {file = "urllib3-1.26.18.tar.gz", hash = "sha256:f8ecc1bba5667413457c529ab955bf8c67b45db799d159066261719e328580a0"},
--
brotli = ["brotli (==1.0.9)", "brotli (>=1.0.9)", "brotlicffi (>=0.8.0)", "brotlipy (>=0.6.0)"]
secure = ["certifi", "cryptography (>=1.3.4)", "idna (>=2.0.0)", "ipaddress", "pyOpenSSL (>=0.14)", "urllib3-secure-extra"]
--
PyYAML = "*"
urllib3 = {version = "<2", markers = "platform_python_implementation == \"PyPy\""}
--
[package.extras]
tests = ["Werkzeug (==2.0.3)", "aiohttp", "boto3", "httplib2", "httpx", "pytest", "pytest-aiohttp", "pytest-asyncio", "pytest-cov", "pytest-httpbin", "requests (>=2.22.0)", "tornado", "urllib3"]

# poetry add --lock 'urllib3>=2'
Updating dependencies
Resolving dependencies... (0.0s)

Because tmp-hxcfg8nblm depends on vcrpy (6.0.1) which depends on urllib3 (<2), urllib3 is required.
So, because tmp-hxcfg8nblm depends on urllib3 (>=2), version solving failed.

So that does seem like a bug to me, PEP 508 environment markers work differently. Am I missing something? Please re-open 🙏

CC @benwah @dimbleby (alphabetical order)

@dimbleby
Copy link
Contributor

dimbleby commented Mar 6, 2024

Am I missing something?

yes, your pyproject.toml requires python at least 3.10, so the marker for python less than 3.10 is irrelevant

So that does seem like a bug to me

this is not a bug, if poetry allowed you unconditionally to lock urllib3 2.0 then your project would be broken when running on pypy

@josumoreno-BP
Copy link

@dimbleby is right in the sense that poetry tries to construct a .lock platform agnostic. The only problem is that for example on our use case we are not interested on pypy, but afaik there is no option to ignore it on poetry (the same way you tell poetry to respect a python version, you cannot select the platform implementation you are interested on supporting or excluding).

@hartwork you can ofc add to the platform_python_implementation == "PyPy" marker the python version too, but I feel that as an ad-hock change to make things work for people that use poetry like us (which in my case would solve the problem we have but I guess it should be better to fix it on the library or pypy 😅 ).

@hartwork
Copy link

hartwork commented Mar 6, 2024

Hi @dimbleby,

Am I missing something?

yes, your pyproject.toml requires python at least 3.10, so the marker for python less than 3.10 is irrelevant

I confirm, I get line…

urllib3 = {version = "<2", markers = "platform_python_implementation == \"PyPy\" or python_version < \"3.10\""}
                                                                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

…once I retry with CPython 3.9.

@hartwork
Copy link

hartwork commented Mar 6, 2024

@dimbleby is right in the sense that poetry tries to construct a .lock platform agnostic. The only problem is that for example on our use case we are not interested on pypy, but afaik there is no option to ignore it on poetry (the same way you tell poetry to respect a python version, you cannot select the platform implementation you are interested on supporting or excluding).

@josumoreno-BP I was thinking the same. Like an "ignore PyPy" switch or declaring "just CPython on Linux, break all else if you want".

@hartwork you can ofc add to the platform_python_implementation == "PyPy" marker the python version too, but I feel that as an ad-hock change to make things work for people that use poetry like us (which in my case would solve the problem we have but I guess it should be better to fix it on the library or pypy 😅 ).

@josumoreno-BP VCR.py is broken for all releases of PyPy (in GitHub Actions at least) — 3.8, 3.9, 3.10 — so I'm not sure what version to add for a workaround to make things work for Poetry. Am I missing something?

@josumoreno-BP
Copy link

@hartwork I did not had time to look at the issue itself. The only thing that you can quickly do to narrow it is to add the marker to PyPy + python versions: platform_python_implementation == "PyPy" and python_version >= "3.8" and python_version < "3.11". But as I said before, this feels like something ad-hock to poetry users. In my case I would benefit from this for sure because I'm using 3.11, but I can understand that you don't want to do it.

@hartwork
Copy link

hartwork commented Mar 6, 2024

@josumoreno-BP thanks for elaborating, I now understand the idea: It should get the marker dropped for projects with python >=3.11. I'm not sure I can justify a workaround like that. Thanks for understanding that it would be controversial to add.

Copy link

github-actions bot commented Apr 6, 2024

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/bug Something isn't working as expected
Projects
None yet
Development

No branches or pull requests

5 participants