search

dependabot / dependabot-core

🤖 Dependabot's core logic for creating update PRs.
https://docs.github.com/en/code-security/dependabot
MIT License
4.72k stars 1.02k forks source link

Poetry multiple-constraint dependencies broken by unrelated dependency update #5862

Closed greschd closed 2 years ago

greschd commented 2 years ago

Is there an existing issue for this?

Package ecosystem

pip

Package manager version

Poetry (version 1.2.1)

Language version

python = ">=3.7,<3.10"

Manifest location and content before the Dependabot update

pyproject.toml: https://github.com/greschd/dependabot-poetry-multiple-constraints/blob/4c051f7216cbb70decb1b206fe8de7ff060c188a/pyproject.toml poetry.lock: https://github.com/greschd/dependabot-poetry-multiple-constraints/blob/4c051f7216cbb70decb1b206fe8de7ff060c188a/poetry.lock

dependabot.yml content

https://github.com/greschd/dependabot-poetry-multiple-constraints/blob/4c051f7216cbb70decb1b206fe8de7ff060c188a/.github/dependabot.yml

Updated dependency

wheel updated from 0.37.0 to 0.37.1

What you expected to see, versus what you actually saw

Only the wheel related entries in poetry.lock should be updated. Instead, the multiple-constraint numpy dependency is partially removed.

It seems that dependabot doesn't understand the multiple-constraint dependency (different numpy version for different python versions)

numpy = [
    {version = ">=1.15,<1.22", python = ">=3.7,<3.8"},
    {version = ">=1.22", python = ">=3.8,<3.11"}
]

This is mentioned in https://github.com/dependabot/dependabot-core/issues/2715#issuecomment-777435053, but I couldn't find an issue specific to this incompatibility.

In the poetry.lock, the multiple-constraint dependency is added as multiple numpy entries (AFAICT, the last matching one is selected for a given Python version). Dependabot updates keep only the first entry, and discard the rest.

Native package manager behavior

PR created manually, using poetry lock: https://github.com/greschd/dependabot-poetry-multiple-constraints/pull/2

Images of the diff or a link to the PR, issue, or logs

https://github.com/greschd/dependabot-poetry-multiple-constraints/pull/1

Smallest manifest that reproduces the issue

Almost minimal repository: https://github.com/greschd/dependabot-poetry-multiple-constraints

jeffwidman commented 2 years ago

Thanks for the detailed issue. We have a few python improvements that we're currently working on, although looking at them I don't think any will fix this particular issues. So it might be a bit til one of us can get to it, in the meantime if you have any interest in opening a PR I'm more than happy to guide you.

greschd commented 2 years ago

if you have any interest in opening a PR I'm more than happy to guide you

Happy to give it a try 🙂

We have a few python improvements that we're currently working on

If these include PEP621 support, it might also be worth keeping an eye on https://github.com/python-poetry/roadmap/issues/3 -- at least for my use case, I wouldn't mind migrating the pyproject.toml as long as both dependabot and poetry support it.

I think there are two goals for this issue, with potentially quite different difficulty:

  1. making it such that dependabot simply doesn't touch the multiply-constrained dependency in poetry.lock
  2. "full" support of multiply-constrained dependencies, where they are correctly updated

For 2., it would be helpful to know how (if?) dependabot handles environment markers in the other supported Python formats (requirements.txt etc.).

deivid-rodriguez commented 2 years ago

Hei @greschd!

We're adding PEP621 support soon indeed and I'm already subscribed to that ticket in the poetry roadmap, thanks!

Regarding your question, no, I don't think we support updating dependencies with environment markers in any Python format as of now, so I think for now fixing 1 is the easiest, just make sure they are properly ignored.