python-poetry / poetry

Python packaging and dependency management made easy
https://python-poetry.org
MIT License
31.17k stars 2.25k forks source link

Poetry exports incorrect requirements for transitive dependencies, breaks installation with hashes #4719

Closed mathrick closed 2 years ago

mathrick commented 2 years ago

Issue

This seems related to #3363, but I don't believe it's a duplicate, and I haven't seen this mentioned elsewhere.

When exporting requirements.txt, poetry will add incorrect implementation markers to the output, which in some cases can result in invalid requirement files when used with hashes:

Collecting cffi>=1.0.0
ERROR: In --require-hashes mode, all requirements must have their versions pinned with ==. These do not:
    cffi>=1.0.0 from https://devpi.cerebras.aws/root/pypi/%2Bf/57e/9ac9ccc3101fa/cffi-1.15.0-cp38-cp38-manylinux_2_12_x86_64.manylinux2010_x86_64.whl#sha256=57e9ac9ccc3101fac9d6014fba037473e4358ef4e89f8e181f8951a2c0162024 (from argon2-cffi==21.1.0->-r /tmp/reqs.txt (line 16))

This happens because in the generated requirements.txt, the section for cffi is as follows:

cffi==1.15.0; implementation_name == "pypy" and python_version >= "3.6" \
    --hash=sha256:c2502a1a03b6312837279c8c1bd3ebedf6c12c4228ddbad40912d671ccc8a962 \
    --hash=sha256:23cfe892bd5dd8941608f93348c0737e369e51c100d03718f108bf1add7bd6d0 \
[...]
    --hash=sha256:920f0d66a896c2d99f0adbb391f990a84091179542c205fa53ce5787aff87954

Notice the implementation_name == "pypy" marker, even though the implementation used is CPython.

dimbleby commented 2 years ago

recent master gives

cffi==1.15.0; implementation_name == "pypy" and python_version >= "3.7" and python_full_version >= "3.6.1" or python_version >= "3.6"

which seems satisfactory

mathrick commented 2 years ago

Any ETA for when that could make its way into a release?

DMRobertson commented 2 years ago

I think I might have come across this on poetry 1.1.12. With the following pyproject.toml:

[tool.poetry]
name = "foo"
version = "1.2.3"
description = "foo"
authors = ["foo"]

[tool.poetry.dependencies]
python = "^3.7"

signedjson = "1.1.1"
matrix-common = "1.1.0"

[tool.poetry.dev-dependencies]
flake8 = "4.0.1"

[build-system]
requires = ["poetry-core>=1.0.0"]
build-backend = "poetry.core.masonry.api"

If I run poetry export, the generated requirements stipulate that importlib-metadata is only required for Python 3.7:

$ poetry lock && poetry export | grep ^importlib-metadata -A 3
importlib-metadata==4.11.2; python_version < "3.8" and python_version >= "3.7" \
    --hash=sha256:d16e8c1deb60de41b8e8ed21c1a7b947b0bc62fab7e1d470bcdf331cea2e67\
    --hash=sha256:b36ffa925fe3139b2f6ff11d6925ffd4fa7bc47870165e3ac260ac7b4f91e6ac

However, signedjson 1.1.1 requires importlib_metadata unconditionally. Somehow, combining this with the other two requirements and exporting from the results makes this only required on Python 3.7.

Quoting from the lockfile:

[[package]]
  name = "flake8"
  version = "4.0.1"
  ...

  [package.dependencies]
  importlib-metadata = {version = "<4.3", markers = "python_version < \"3.8\""}
  ...

  [[package]]
  name = "matrix-common"
  version = "1.1.0"  ...

  [package.dependencies]
  importlib-metadata = {version = ">=1.4", markers = "python_version < \"3.8\""}
DMRobertson commented 2 years ago

Using the same pyproject.toml and poetry@b06658f9a730b4d28b4b517d7f5c982514e524df I now get

$ poetry lock && poetry export | \grep ^importlib-metadata -A 3
Updating dependencies
Resolving dependencies... (0.2s)
importlib-metadata==4.2.0 ; python_version >= "3.6" \
    --hash=sha256:057e92c15bc8d9e8109738a48db0ccb31b4d9d5cfbee5a8670879a30be66304b \
    --hash=sha256:b7e52a1f8dec14a75ea73e0891f3060099ca1d8e6a462a4dff11c3e119ea1b31
matrix-common==1.1.0 \

which seems sensible to me.

shaib commented 2 years ago

FWIW with 1.1.13, and this pyproject.toml:

[tool.poetry]
name = "test2"
version = "0.1.0"
description = ""
authors = ["Shai Berger <shai@platonix.com>"]

[tool.poetry.dependencies]
python = "^3.10"
ipython = "^8.3.0"
pytest = "^7.1.2"
pytest-watch = "^4.2.0"

[tool.poetry.dev-dependencies]

[build-system]
requires = ["poetry-core>=1.0.0"]
build-backend = "poetry.core.masonry.api"

we still get a similar failure on colorama -- it is required by ipython and pytest only on Windows, but pytest-watch requires it uncoditionally, and in export we get

colorama==0.4.4; python_version >= "3.8" and python_full_version < "3.0.0" and sys_platform == "win32" or sys_platform == "win32" and python_version >= "3.8" and python_full_version >= "3.5.0" \
    --hash=sha256:9f47eda37229f68eee03b24b9748937c7dc3868f906e8ba69fbcbdd3bc5dc3e2 \
    --hash=sha256:5941b2b48a20143d2267e95b1c2a7603ce057ee39fd88e7329b0c292aa16869b

This leads to failure when trying to pip-install the exported requirements on a non-Windows.

dimbleby commented 2 years ago

pytest-watch publishes only a source distribution, forcing poetry to inspect its setup.py and do its best.

This is always going to be error-prone, packages that publish wheels with metadata or (even better) populate the JSON API at (say) https://pypi.org/pypi/pytest-watch/3.8.0/json will always give more reliable answers.

ie don't bet the house on this being improved in later poetry releases; if you care, ask pytest-watch to improve their publishing process.

dimbleby commented 2 years ago

Having said which, this example actually exposes https://github.com/python-poetry/poetry/issues/5593, so thanks for that!

shaib commented 2 years ago

(was going to point out that all the relevant data is in poetry.lock by the time we export, but you seem to already have the root cause, which is great!)

dimbleby commented 2 years ago

anyway both the original issue and the new example are fixed on master, so probably this should be closed

github-actions[bot] commented 6 months ago

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.