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

group packages correctly when exporting #5154

Closed
wants to merge 1 commit into from

Conversation

dimbleby
Copy link
Contributor

@dimbleby dimbleby commented Feb 5, 2022

Pull Request Check List

Resolves: #issue-number-here

  • Added tests for changed code.
  • Updated documentation for changed code.

The attempt to groupby during exports can be foiled by cases where a dependency is present with more than one version: eg if we see [foo 1, foo 2, foo 1] then we end up with three groups rather than two.

This improves #5141, in particular the lines for click that we now get for the example given there are

click==7.1.2 ; python_version >= "3.6" and python_full_version < "3.0.0" and python_version < "3.7" or python_version >= "3.6" and python_version < "3.7" and python_full_version >= "3.5.0"
click==8.0.3 ; python_version >= "3.7"

which looks acceptable.

However, in the effort to simplify this to a testcase I ran into at least two bugs that I don't know how to fix, and these are exhibited by the testcase...


Bug 1

Here's what we get for click:

click==7.1.2 ; python_version >= "2.7" and python_full_version < "3.0.0" and python_version < "3.7" or python_full_version >= "3.5.0" and python_version < "3.7" or python_full_version >= "3.6.2" and python_full_version < "4.0.0" or python_version >= "2.7" and python_full_version < "3.0.0" or python_full_version >= "3.5.0"
click==8.0.3 ; python_version >= "3.6"

two of the 7.1.2 possibilities overlap the 8.0.3 marker, which is not OK.

The cause seems to be that when the export code comes to process the "top-level" entries for click-didyoumean and click-plugins it sees that these should be installed for a broad range of python versions, and decides that therefore so should their dependency click.

As best I can see the fundamental problem is that the lock file does not record which packages are actually top-level dependencies, and which are not. I don't see how to fix this without recording the actual top-level requirements as taken from pyproject.toml, which would allow the export code to reach click-didyoumean and click-plugins only in a path where it is taking account of parent markers.


Bug 2

The output markers for click-plugins and click-didyoumean (sort of) are also faulty in the test.

 click-didyoumean==0.0.3 ; python_version < "3.7"
 click-didyoumean==0.3.0 ; python_full_version >= "3.6.2" and python_full_version < "4.0.0"
 click-plugins==1.1.1 ; python_version < "3.7"
  • click-plugins should be installed unconditionally
  • click-didyoumean 0.0.3 is right, but only accidentally
  • click-didyoumean 0.3.0 is wrong, it should be >= "3.7" (per bug 1, it is getting the versions from the "top-level" which is wrong)

The new problem here is that the marker setter at https://github.com/python-poetry/poetry-core/blob/0113a5a172e57424dbf5b2031d8c77097c725b5e/src/poetry/core/packages/dependency.py#L161 does not update the python versions correctly:

  • start with a dependency whose marker is "< 3.7"
  • update to an Any marker
  • that setter does not see python_versions in the Any marker, and therefore does not clear "python < 3.7" from the _python_versions

@dimbleby
Copy link
Contributor Author

dimbleby commented Feb 5, 2022

The CI pipeline points out that bug 2 is new since poetry-core 1.1.0a6. I had been testing locally with master

@dimbleby
Copy link
Contributor Author

dimbleby commented Feb 5, 2022

The natural way to approach bug 1, I think, would be to introduce an entry in poetry.lock for the top-level package, recording its dependencies. (This is how Cargo.lock works in rust-world).

I don't have a sense of how breaking that would be...

@dimbleby dimbleby mentioned this pull request Feb 6, 2022
2 tasks
@dimbleby
Copy link
Contributor Author

superseded by #5156

@dimbleby dimbleby closed this Mar 13, 2022
@dimbleby dimbleby deleted the export-fix branch March 13, 2022 11:54
Copy link

This pull request 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 Feb 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant