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

solver: fix special case where a direct origin dependency without extras ... #5770

Merged

Conversation

radoering
Copy link
Member

… is requested by the project and the same dependency with extras is requested by another dependency

Pull Request Check List

Resolves: #5311

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

Copy link
Member

@neersighted neersighted left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather hold off merging this until we bump the core version -- could you rebase this onto poetry-core main in the mean time, and then once we bump core we can switch back?

# TODO: remove https://github.com/python-poetry/poetry-core/pull/375
dep._extras = frozenset(term.dependency.features)
positive_with_features = Term(dep, is_positive=True)
self._positive[term.dependency.complete_name] = positive_with_features
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this doesn't look like a method that I would expect to be making updates to self: is there some other moment at which this should have happened?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe, you're right. I am not completely satisfied with the solution myself. Will take another look at it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved it to _register. Looks better, doesn't it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this looks a much better place for it - thanks!

suggestion: we only need to look for old_positive_without_features if the assignment dependency has features: else - which would usually be the case - we're just looking up the same thing twice. So

         """
         name = assignment.dependency.complete_name
         old_positive = self._positive.get(name)
-        if old_positive is None:
+        if old_positive is None and assignment.dependency.features:
             old_positive_without_features = self._positive.get(
                 assignment.dependency.name
             )

?

@radoering radoering marked this pull request as draft June 5, 2022 13:39
@radoering radoering force-pushed the direct_origin_transitive_extras branch 2 times, most recently from 6b13028 to b3e637e Compare June 6, 2022 07:53
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@neersighted
Copy link
Member

#5782

I think you might have taken a wrong turn -- please elaborate if you think these issues are somehow related (and use a regular comment instead of a review comment, please).

@dimbleby dimbleby mentioned this pull request Jul 2, 2022
…ras is requested by the project and the same dependency with extras is requested by another dependency
@radoering radoering force-pushed the direct_origin_transitive_extras branch from b3e637e to 2ca6e08 Compare July 13, 2022 04:23
@radoering radoering marked this pull request as ready for review July 13, 2022 04:37
@neersighted neersighted merged commit bd9c5c8 into python-poetry:master Jul 13, 2022
@mkniewallner mkniewallner mentioned this pull request Jul 13, 2022
dimbleby added a commit to dimbleby/poetry that referenced this pull request Jul 24, 2022
dimbleby added a commit to dimbleby/poetry that referenced this pull request Jul 24, 2022
dimbleby added a commit to dimbleby/poetry that referenced this pull request Jul 24, 2022
dimbleby added a commit to dimbleby/poetry that referenced this pull request Jul 24, 2022
dimbleby added a commit to dimbleby/poetry that referenced this pull request Jul 30, 2022
radoering added a commit that referenced this pull request Jul 31, 2022
Co-authored-by: Randy Döring <30527984+radoering@users.noreply.github.com>
efugier pushed a commit to efugier/poetry that referenced this pull request Aug 4, 2022
Co-authored-by: Randy Döring <30527984+radoering@users.noreply.github.com>
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
@radoering radoering deleted the direct_origin_transitive_extras branch November 24, 2024 12:45
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.

Unable to poetry install twisted trunk and treq
3 participants